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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]