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 <sbhat@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Pradipta Kumar Banerjee <bpradip@xxxxxxxxxx>
> Reviewed-by: Prerna Saxena <prerna@xxxxxxxxxxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list