Re: [libvirt] [RFC PATCH v2 3/4] qemu: assign addresses for spapr vfio hostdevices and generate cli

On pseries, the vfio host devices attach to the spapr-pci-vfio domain instead of the default emulated domain. So, for a host device belonging to iommu group(say) 3, would need below host bridge. -device spapr-pci-vfio-host-bridge,iommu=3,id=vfiohostbridge3,index=1 The vfio device then needs to assign itself to the bus "vfiohostbridge3" as below : -device vfio-pci,host=0003:05:00.1,id=hostdev0,bus=vfiohostbridge3.0,\ addr=0x1 Since each host bridge controller adds a new domain, all the devices addressing would need to start from bus0:slot1:function0 in the new domain. The controller tag for spapr-pci-vfio-host-bridge has the domain and iommu number allocated during the parsing based on the hostdevs in the xml. Assign the pci addressses for the hostdevs from their respective domains. The domain id "vfiohostbridge<iommu>" is used for uniqueness in the controller alias. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; Signed-off-by: Pradipta Kumar Banerjee <bpradip(a)in.ibm.com&gt; Reviewed-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com&gt; --- src/conf/domain_addr.c | 8 + src/conf/domain_addr.h | 2 src/libvirt_private.syms | 1 src/qemu/qemu_command.c | 274 +++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_command.h | 17 +++ tests/qemuhotplugtest.c | 2 6 files changed, 282 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index fb4a76f..e6c96f8 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -110,11 +110,11 @@ virDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs, virReportError(errType, "%s", _("No PCI buses available")); return false; } - if (addr->domain != 0) { + if (addr->domain != addrs->pciDomainId) { virReportError(errType, _("Invalid PCI address %s. " - "Only PCI domain 0 is available"), - addrStr); + "Only PCI domain %d is available"), + addrStr, addrs->pciDomainId); return false; } if (addr->bus >= addrs->nbuses) { @@ -463,7 +463,7 @@ virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, /* default to starting the search for a free slot from * 0000:00:00.0 */ - virDevicePCIAddress a = { 0, 0, 0, 0, false }; + virDevicePCIAddress a = { addrs->pciDomainId, 0, 0, 0, false }; char *addrStr = NULL;

/* except if this search is for the exact same type of device as diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 2c3468e..df4d4e0 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -64,6 +64,8 @@ struct _virDomainPCIAddressSet { size_t nbuses; virDevicePCIAddress lastaddr; virDomainPCIConnectFlags lastFlags; + int pciDomainId; /* The PCI domain to which the devices should belong + to in the guest */ bool dryRun; /* on a dry run, new buses are auto-added and addresses aren't saved in device infos */ }; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e6ff977..797fa77 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -213,6 +213,7 @@ virDomainDeviceDefFree; virDomainDeviceDefParse; virDomainDeviceFindControllerModel; virDomainDeviceGetInfo; +virDomainDeviceInfoClear; virDomainDeviceInfoCopy; virDomainDeviceInfoIterate; virDomainDeviceTypeToString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f10e6d..5813332 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -70,6 +70,8 @@ VIR_LOG_INIT("qemu.qemu_command"); #define VIO_ADDR_SERIAL 0x30000000ul #define VIO_ADDR_NVRAM 0x3000ul +#define DEFAULT_PCI 0 + VIR_ENUM_DECL(virDomainDiskQEMUBus) VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, "ide", @@ -562,6 +564,12 @@ qemuNetworkPrepareDevices(virDomainDefPtr def) goto cleanup; if (virDomainDefMaybeAddHostdevSpaprPCIVfioControllers(def) < 0) goto cleanup; + /* The net device is originally assigned address in generic domain. + * Clear the original address for the new address to take effect. + */ + if (ARCH_IS_PPC64(def->os.arch) && def->os.machine && + STRPREFIX(def->os.machine, "pseries")) + virDomainDeviceInfoClear(&net->info); } } ret = 0; @@ -886,6 +894,9 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller) return virAsprintf(&controller->info.alias, "pcie.%d", controller->idx); else return virAsprintf(&controller->info.alias, "pci.%d", controller->idx); + } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO) { + return virAsprintf(&controller->info.alias, "vfiohostbridge%d.%d", + controller->opts.spaprvfio.iommuGroupNum, controller->idx);

} return virAsprintf(&controller->info.alias, "%s%d", prefix, controller->idx); @@ -1292,6 +1303,51 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, return ret; } +static int +qemuIsValidCurrentDomainDevice(virDomainDefPtr def, + virDomainDeviceDefPtr device, + int domain) +{ + size_t j; + int currentDomainHostdev = 0; + int actualType = -1; + virDomainHostdevDefPtr hostdev = NULL; + + if (device->type == VIR_DOMAIN_DEVICE_CONTROLLER && + domain != device->data.controller->domain) + return -1; /* Dont reserve controllers not belonging to the current pci + * domain */ + + if (device->type == VIR_DOMAIN_DEVICE_NET) { + actualType = virDomainNetGetActualType(device->data.net); + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) + hostdev = virDomainNetGetActualHostdev(device->data.net); + } else if (device->type == VIR_DOMAIN_DEVICE_HOSTDEV) { + hostdev = device->data.hostdev; + } + + if (domain == 0 && hostdev && (IS_PCI_VFIO_HOSTDEV(hostdev))) + return -1; /* Don't reserve vfio devices in emulated domain. */ + + if (domain != 0) { + if (device->type != VIR_DOMAIN_DEVICE_CONTROLLER && !hostdev) + return -1; /* Don't reserve non vfio devices and controllers + * in spapr-vfio pci domain */ + + if (hostdev && IS_PCI_VFIO_HOSTDEV(hostdev)) { + for (j = 0; j < def->ncontrollers; j++) { + if ((def->controllers[j]->type == VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO) && + (domain == def->controllers[j]->domain) && + (def->controllers[j]->opts.spaprvfio.iommuGroupNum == hostdev->source.subsys.u.pci.iommu)) + currentDomainHostdev = 1; + } + /* Dont reserve hostdevs which dont belong to current pci domain */ + if (!currentDomainHostdev) + return -1; + } + } + return 0; +} static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, @@ -1317,6 +1373,16 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } + /* For PPC64 the passthrough devices are assigned to non-emulated + * pci domain and wont be part of domain zero. + */ + if (ARCH_IS_PPC64(def->os.arch) && def->os.machine && + STRPREFIX(def->os.machine, "pseries")) + { + if (qemuIsValidCurrentDomainDevice(def, device, addrs->pciDomainId) < 0) + return 0; + } + /* Change flags according to differing requirements of different * devices. */ @@ -1324,6 +1390,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, case VIR_DOMAIN_DEVICE_CONTROLLER: switch (device->data.controller->type) { case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + case VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO: switch (device->data.controller->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: /* pci-bridge needs a PCI slot, but it isn't @@ -1461,24 +1528,35 @@ qemuDomainSupportsPCI(virDomainDefPtr def) int qemuDomainAssignPCIAddresses(virDomainDefPtr def, - virQEMUCapsPtr qemuCaps, - virDomainObjPtr obj) + virQEMUCapsPtr qemuCaps, + virDomainObjPtr obj, + int domain) { int ret = -1; + int iommu = -1; virDomainPCIAddressSetPtr addrs = NULL; qemuDomainObjPrivatePtr priv = NULL; + int controllerType = -1; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { int max_idx = -1; int nbuses = 0; + int ncontrollers = def->ncontrollers; size_t i; int rv; virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCI;