Re: [libvirt PATCH V2 2/4] Config some capabilities for loongarch virt machine

On Wed, Jan 10, 2024 at 11:07:47AM +0800, Xianglai Li wrote: > Config some capabilities for loongarch virt machine > > Config some capabilities for loongarch virt machine such as > PCI multi bus. I would just say that you're implementing support for loongarch64 in the QEMU driver, because that's what's happening :)

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index f2339d6013..357183cdb5 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -1138,7 +1138,8 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps, > NULL, NULL, 0, NULL); > } > > - if ((ARCH_IS_X86(guestarch) || guestarch == VIR_ARCH_AARCH64)) > + if ((ARCH_IS_X86(guestarch) || guestarch == VIR_ARCH_AARCH64 || > + ARCH_IS_LOONGARCH(guestarch))) > virCapabilitiesAddGuestFeatureWithToggle(guest, VIR_CAPS_GUEST_FEATURE_TYPE_ACPI, > true, true); You can drop the unnecessary ()s around the condition. Doing so will also fix the alignment, which is slightly off right now.

> @@ -2080,6 +2081,11 @@ bool virQEMUCapsHasPCIMultiBus(const virDomainDef *def) > return true; > } > > + /* loongarch64 support PCI-multibus on all machine types > + * since forever */ > + if (ARCH_IS_LOONGARCH(def->os.arch)) > + return true; You can fold this into one of the existing architecture checks. Actually, I'll look into posting a patch that unifies a bunch of them, because the way the function is written today makes it way more complicated than it really needs to be :)

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 0cea0b323a..e07fc32630 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -5933,7 +5936,8 @@ qemuDomainDefaultVideoDevice(const virDomainDef *def, > return VIR_DOMAIN_VIDEO_TYPE_VGA; > if (qemuDomainIsARMVirt(def) || > qemuDomainIsRISCVVirt(def) || > - ARCH_IS_S390(def->os.arch)) { > + ARCH_IS_S390(def->os.arch) || > + qemuDomainIsLoongArchVirt(def)) { > return VIR_DOMAIN_VIDEO_TYPE_VIRTIO; > } You can add the condition between the RISC-V and s390 ones, thus making the diff smaller and the resulting code a tiny bit tidier.

You have missed a number of spots where the QEMU driver needs to be taught about the new architecture. Specifically, we need to ensure that virtio-net is used by default, that 16550a is recognized as a valid serial console model and used by default, and that we avoid using the legacy -usb command line option. You can squash in the diff below to take care of all of the above, but note that, just like virQEMUCapsHasPCIMultiBus(), qemuDomainDefaultNetModel() will likely see some cleaning up soon.

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 712feb7b81..2f73399e5d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2906,6 +2906,7 @@ qemuBuildDomainForbidLegacyUSBController(const virDomainDef *def) { if (qemuDomainIsQ35(def) || qemuDomainIsARMVirt(def) || + qemuDomainIsLoongArchVirt(def) || qemuDomainIsRISCVVirt(def)) return true; @@ -9218,9 +9219,11 @@ qemuChrIsPlatformDevice(const virDomainDef *def, } } - if (ARCH_IS_RISCV(def->os.arch)) { + if (ARCH_IS_RISCV(def->os.arch) || + ARCH_IS_LOONGARCH(def->os.arch)) { - /* 16550a (used by riscv/virt guests) is a platform device */ + /* 16550a (used by the virt machine type on some architectures) + * is a platform device */ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM && chr->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_16550A) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2f1b816628..48a16299eb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5410,8 +5415,9 @@ qemuDomainDefaultNetModel(const virDomainDef *def, return VIR_DOMAIN_NET_MODEL_LAN9118; } - /* virtio is a sensible default for RISC-V virt guests */ - if (qemuDomainIsRISCVVirt(def)) + /* virtio is a sensible default for the virt machine type */ + if (qemuDomainIsRISCVVirt(def) || + qemuDomainIsLoongArchVirt(def)) return VIR_DOMAIN_NET_MODEL_VIRTIO; /* In all other cases the model depends on the capabilities. If they were @@ -5734,7 +5740,9 @@ qemuDomainChrDefPostParse(virDomainChrDef *chr, chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; } else if (qemuDomainIsPSeries(def)) { chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SPAPR_VIO; - } else if (qemuDomainIsARMVirt(def) || qemuDomainIsRISCVVirt(def)) { + } else if (qemuDomainIsARMVirt(def) || + qemuDomainIsRISCVVirt(def) || + qemuDomainIsLoongArchVirt(def)) { chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM; } else if (ARCH_IS_S390(def->os.arch)) { chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SCLP; @@ -5760,7 +5768,8 @@ qemuDomainChrDefPostParse(virDomainChrDef *chr, case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_SYSTEM: if (qemuDomainIsARMVirt(def)) { chr->targetModel = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011; - } else if (qemuDomainIsRISCVVirt(def)) { + } else if (qemuDomainIsRISCVVirt(def) || + qemuDomainIsLoongArchVirt(def)) { chr->targetModel = VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_16550A; } break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b22d3618fe..2984bc5bb5 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2064,6 +2064,7 @@ qemuValidateDomainChrDef(const virDomainChrDef *dev, isCompatible = false; } if (dev->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_16550A && + !qemuDomainIsLoongArchVirt(def) && !qemuDomainIsRISCVVirt(def)) { isCompatible = false; }