Hi Andrea :
On Thu, Dec 14, 2023 at 02:08:47PM +0800, xianglai li wrote:
+++ b/src/qemu/qemu_domain.c @@ -5635,6 +5635,11 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont, cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI; else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; + } else if (ARCH_IS_LOONGARCH(def->os.arch)) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI; + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) + cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI; } I don't think you need to take into account the nec-xhci model for loongarch. aarch64 needs it because qemu-xhci didn't exist when that architecture was introduced, but that's not the case here so we can keep things simpler.
I'm surprised that this code doesn't have handling for riscv64. Not your problem, but likely an oversight that should be addressed.
Ok, I'll remove nec-xhci in the next version.
+static bool +qemuDomainMachineIsLoongson(const char *machine, + const virArch arch) The appropriate name for this function would be qemuDomainMachineIsLoongArchVirt, to match the existing Arm and RISC-V equivalents.
Ok, I'll correct that in the next version.
+bool +qemuDomainIsLoongson(const virDomainDef *def) +{ Same here.
Ok, I'll correct that in the next version.
+++ b/src/qemu/qemu_domain_address.c @@ -2093,6 +2143,11 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDef *def, return -1; }
+ if (qemuDomainIsLoongson(def) && + qemuDomainValidateDevicePCISlotsLoongson(def, addrs) < 0) { + return -1; + } The existing qemuDomainValidateDevicePCISlots* functions are intended to ensure that certain devices, that historically have been assigned to specific PCI slots by QEMU, always show up at those addresses.
We haven't needed anything like that for non-x86 architectures so far, and I believe that loongarch doesn't need it either.
Ok, I'll correct that in the next version.
+++ b/src/qemu/qemu_validate.c @@ -100,7 +100,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, switch ((virDomainFeature) i) { case VIR_DOMAIN_FEATURE_IOAPIC: if (def->features[i] != VIR_DOMAIN_IOAPIC_NONE) { - if (!ARCH_IS_X86(def->os.arch)) { + if (!ARCH_IS_X86(def->os.arch) && !ARCH_IS_LOONGARCH(def->os.arch)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The '%1$s' feature is not supported for architecture '%2$s' or machine type '%3$s'"), featureName, So does loongarch actually have ioapic support? Just making sure. I'm surprised because apparently no other non-x86 architecture supports it...
Yes, loongarch does have IOAPIC, but this feature has no effect on loongarch at this stage, I will cut it first to simplify the committed code. In addition, I have a question, if I understand correctly, the IOAPIC here should be the device interrupt controller, which is located in the bridge chip, it is called IOAPIC under x86, PCH_PIC under loongarch, and GIC under arm. The kernel_irqchip attribute of the machine parameter in qemu corresponding to the function VIR_DOMAIN_FEATURE_IOAPIC determines whether the device interrupt controller is simulated in qemu or kvm. So arm also has such a need, but why doesn't arm add? Thanks, Xianglai.