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.

> +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.

> +bool
> +qemuDomainIsLoongson(const virDomainDef *def)
> +{

Same here.

> +++ 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.

> +++ 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...

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

Reply via email to