On 11/17/2014 11:05 AM, Shivaprasad G Bhat wrote: > 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 <[email protected]> > Signed-off-by: Pradipta Kumar Banerjee <[email protected]> > Reviewed-by: Prerna Saxena <[email protected]> > --- > 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;
All the code adding support for non-zero domain to existing code should IMO be
in a separate commit.
>
> /* 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);
I'd rather use domain.index than iommuGroup.index here.
> }
>
> 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;
>
Do we need a new CONNECT_ flag that only allows hostdevs on the new controllers?
Jan
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
