Re: [RFC PATCH v2 2/4] qemu: parse spapr-vfio-pci controller from xml

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

 



This has a qemu: prefix, even though the parsing is done in conf:

It would look nicer split into several commits:
1) parsing the new controller type (without auto-adding them) + qemuxml2xml
test of the controllers
2) auto-adding the controllers
3) adding the 0 domain parameter to many functions (if needed)

On 11/17/2014 11:04 AM, Shivaprasad G Bhat wrote:
> This patch will get the iommu group for host devices by XML configuration
> of the vfio host bridge controller or from the sysfs.
> 
> On pseries, the iommu group can be shared by multiple host devices and
> they would share the same spapr-vfio-host-bus controller. A new
> controller is added for every new iommu group. Every spapr-vfio-host-bridge
> in the cli creates a new pci domain in the guest. For Example,
>       -device spapr-pci-vfio-host-bridge,iommu=1,id=SOMEDOMAIN,index=1
> The "SOMEDOMAIN" is the id for new pci domain inside guest.
> 
> spapr-pci-vfio-host-bridge is actually a PCI host bridge with VFIO support.
> It hosts pci-bridges, and has all the features/behaviours similar to the
> default emulated pci host bridge. The controller can host both pci and pcie
> devices. For convenience, this root controller uses model "pci-root".
> 
> The sample controller tags would look like below:
> <controller type='spapr-vfio-pci' index='0' model='pci-root'
>             iommuGroupNum='3' domain='1'/>
> <controller type='spapr-vfio-pci' index='1' model='pci-bridge'
>             iommuGroupNum='3' domain='1'>

Would it make sense to fill out the iommuGroup on domain startup, depending on
the devices that were assigned there?

So that libvirt user can do:
<controller type='spapr-vfio-pci' domain='1'/>
<controller type='spapr-vfio-pci' domain='2'/>

Then attach two hostdevs to these domains without knowing their IOMMU group?

(Although the user needs to know which devices are in the same group to attach
more than one to the same domain)

>       <address type='pci' domain='0x0001' bus='0x00' slot='0x02'
>                function='0x0'/>
> </controller>
> <controller type='spapr-vfio-pci' index='0' model='pci-root'
>             iommuGroupNum='13' domain='2'/>
> 
> Like other architectures, unassigned and unmanaged devices in the iommu group
> need to be detached manually before the guest is created .
> 
> The spapr-vfio-pci controllers are removed when there are no corresponding
> hostdevices in xml.
> 
> Signed-off-by: Shivaprasad G Bhat <sbhat@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Pradipta Kumar Banerjee <bpradip@xxxxxxxxxx>
> Reviewed-by: Prerna Saxena <prerna@xxxxxxxxxxxxxxxxxx>
> ---
>  docs/schemas/domaincommon.rng |   28 +++++++
>  src/bhyve/bhyve_domain.c      |    2 
>  src/conf/domain_conf.c        |  165 +++++++++++++++++++++++++++++++++++++++--
>  src/conf/domain_conf.h        |   19 +++++
>  src/libvirt_private.syms      |    1 
>  src/qemu/qemu_command.c       |    4 +
>  src/qemu/qemu_domain.c        |   12 +--
>  src/qemu/qemu_driver.c        |    6 +
>  8 files changed, 222 insertions(+), 15 deletions(-)
> 


> @@ -12113,6 +12165,95 @@ virDomainResourceDefParse(xmlNodePtr node,
>      return NULL;
>  }
>  
> +int
> +virDomainDefMaybeAddHostdevSpaprPCIVfioControllers(virDomainDefPtr def)
> +{
> +    size_t i, j;
> +    virDomainHostdevDefPtr hostdev;
> +    virDomainControllerDefPtr controller;
> +    int ret = -1;
> +    int maxDomainId = 0;
> +    int skip;
> +
> +    if (!(ARCH_IS_PPC64(def->os.arch)) ||
> +        !(def->os.machine && STRPREFIX(def->os.machine, "pseries")))
> +        return 0;
> +
> +    for (i = 0; i < def->nhostdevs; i++) {
> +        hostdev = def->hostdevs[i];
> +        if (IS_PCI_VFIO_HOSTDEV(hostdev))
> +            hostdev->source.subsys.u.pci.iommu = -1;
> +    }
> +    /* The hostdevs belonging to same iommu are
> +     * all part of same domain.
> +     */
> +    for (i = 0; i < def->ncontrollers; i++) {
> +        controller = def->controllers[i];
> +        if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO &&
> +            controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
> +            for (j = 0; j < def->nhostdevs; j++) {
> +                hostdev = def->hostdevs[j];
> +                if (IS_PCI_VFIO_HOSTDEV(hostdev))
> +                    if (hostdev->info->addr.pci.domain == controller->domain)
> +                        hostdev->source.subsys.u.pci.iommu = controller->opts.spaprvfio.iommuGroupNum;
> +            }
> +        if (controller->domain > maxDomainId)
> +            maxDomainId = controller->domain;
> +    }
> +    /* If the spapr-vfio controller doesnt exist for the hostdev
> +     * add a controller for that iommu group.
> +     */
> +    for (i = 0; i < def->nhostdevs; i++) {
> +        skip = 0;
> +        hostdev = def->hostdevs[i];
> +        if (IS_PCI_VFIO_HOSTDEV(hostdev)) {
> +            virPCIDeviceAddressPtr addr;
> +            int iommu = -1;
> +            if (hostdev->source.subsys.u.pci.iommu == -1) {
> +                addr = (virPCIDeviceAddressPtr)&hostdev->source.subsys.u.pci.addr;
> +                if ((iommu = virPCIDeviceAddressGetIOMMUGroupNum(addr)) < 0)
> +                    goto error;

I don't know if we should access /sysfs on XML parsing. Could we add the
controllers at startup instead?

> +                hostdev->source.subsys.u.pci.iommu = iommu;
> +
> +                for (j = 0; j < def->ncontrollers; j++) {
> +                    controller = def->controllers[j];
> +                    if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO &&
> +                        controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
> +                        if (iommu == controller->opts.spaprvfio.iommuGroupNum)
> +                            skip = 1;
> +                    }
> +                }
> +                if (skip)
> +                    continue;
> +                if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO,
> +                                                   ++maxDomainId, 0, VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
> +                    goto error;
> +                def->controllers[def->ncontrollers-1]->opts.spaprvfio.iommuGroupNum = iommu;
> +            }
> +        }
> +    }
> +
> +    /* Remove redundant controllers which dont serve any hostdevs. */

We don't delete unused controllers of other types, what if the user explicitly
requests them?

Jan

> +    for (i = 0; i < def->ncontrollers; i++) {
> +        controller = def->controllers[i];
> +        if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SPAPR_PCI_VFIO) {
> +           int usedController = 0;
> +           for (j = 0; j < def->nhostdevs; j++) {
> +               hostdev = def->hostdevs[j];
> +               if (IS_PCI_VFIO_HOSTDEV(hostdev))
> +                  if (hostdev->source.subsys.u.pci.iommu == controller->opts.spaprvfio.iommuGroupNum)
> +                     usedController = 1;
> +           }


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]