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

Re: [libvirt] [PATCHv2 05/12] nodedev: add iommuGroup to node device object



I just realized that I had only implemented this for the udev nodeDevice
driver, but not the HAL driver. I can easily add the same code into the
HAL driver, but don't have any system to test building it on.

Should I put that code in untested, or leave the HAL driver without this
functionality? (Does anyone still use HAL?)


On 06/24/2013 11:05 PM, Laine Stump wrote:
> This includes adding it to the nodedev parser and formatter, docs, and
> test.
> ---
>  docs/formatnode.html.in                            | 63 +++++++++++++++-
>  docs/schemas/nodedev.rng                           | 11 +++
>  src/conf/node_device_conf.c                        | 86 +++++++++++++++++++++-
>  src/conf/node_device_conf.h                        |  5 +-
>  src/node_device/node_device_udev.c                 | 21 +++++-
>  tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml | 16 ++++
>  tests/nodedevxml2xmltest.c                         |  1 +
>  7 files changed, 199 insertions(+), 4 deletions(-)
>  create mode 100644 tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml
>
> diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
> index 11654e5..ad05a70 100644
> --- a/docs/formatnode.html.in
> +++ b/docs/formatnode.html.in
> @@ -80,6 +80,36 @@
>                <dd>Vendor details from the device ROM, including an
>                  attribute <code>id</code> with the hexadecimal vendor
>                  id, and an optional text name of that vendor.</dd>
> +              <dt><code>iommuGroup</code></dt>
> +              <dd>
> +                This optional element describes the "IOMMU group" this
> +                device belongs to. If the element exists, it has a
> +                mandatory <code>number</code> attribute which tells
> +                the group number used for management of the group (all
> +                devices in group "n" will be found in
> +                "/sys/kernel/iommu_groups/n"). It will also have a
> +                list of <code>address</code> subelements, each
> +                containing the PCI address of a device in the same
> +                group. The toplevel device will itself be included in
> +                this list.
> +              </dd>
> +              <dt><code>capability</code></dt>
> +              <dd>
> +                This optional element can occur multiple times. If it
> +                exists, it has a mandatory <code>type</code> attribute
> +                which will be set to
> +                either <code>physical_function</code>
> +                or <code>virtual_functions</code>. If the type
> +                is <code>physical_function</code>, there will be a
> +                single <code>address</code> subelement which contains
> +                the PCI address of the SRIOV Physical Function (PF)
> +                that is the parent of this device (and this device is,
> +                by implication, an SRIOV Virtual Function (VF)). If
> +                the type is <code>virtual_functions</code>, then this
> +                device is an SRIOV PF, and the capability element will
> +                have a list of <code>address</code> subelements, one
> +                for each VF on this PF.
> +              </dd>
>              </dl>
>            </dd>
>            <dt><code>usb_device</code></dt>
> @@ -232,7 +262,38 @@
>      &lt;address&gt;00:27:13:6a:fe:00&lt;/address&gt;
>      &lt;capability type='80203'/&gt;
>    &lt;/capability&gt;
> -&lt;/device&gt;</pre>
> +&lt;/device&gt;
> +
> +&lt;device&gt;
> +  &lt;name&gt;pci_0000_02_00_0&lt;/name&gt;
> +  &lt;path&gt;/sys/devices/pci0000:00/0000:00:04.0/0000:02:00.0&lt;/path&gt;
> +  &lt;parent&gt;pci_0000_00_04_0&lt;/parent&gt;
> +  &lt;driver&gt;
> +    &lt;name&gt;igb&lt;/name&gt;
> +  &lt;/driver&gt;
> +  &lt;capability type='pci'&gt;
> +    &lt;domain&gt;0&lt;/domain&gt;
> +    &lt;bus&gt;2&lt;/bus&gt;
> +    &lt;slot&gt;0&lt;/slot&gt;
> +    &lt;function&gt;0&lt;/function&gt;
> +    &lt;product id='0x10c9'&gt;82576 Gigabit Network Connection&lt;/product&gt;
> +    &lt;vendor id='0x8086'&gt;Intel Corporation&lt;/vendor&gt;
> +    &lt;capability type='virt_functions'&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x10' function='0x0'/&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x10' function='0x2'/&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x10' function='0x4'/&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x10' function='0x6'/&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x11' function='0x0'/&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x11' function='0x2'/&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x11' function='0x4'/&gt;
> +    &lt;/capability&gt;
> +    &lt;iommuGroup number='12'&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/&gt;
> +      &lt;address domain='0x0000' bus='0x02' slot='0x00' function='0x1'/&gt;
> +    &lt;/iommuGroup&gt;
> +  &lt;/capability&gt;
> +&lt;/device&gt;
> +    </pre>
>  
>    </body>
>  </html>
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 1f5dcb9..d2bebff 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -144,6 +144,17 @@
>        </element>
>      </optional>
>  
> +    <optional>
> +      <element name='iommuGroup'>
> +        <attribute name='number'>
> +          <ref name='unsignedInt'/>
> +        </attribute>
> +        <oneOrMore>
> +          <ref name='address'/>
> +        </oneOrMore>
> +      </element>
> +    </optional>
> +
>    </define>
>  
>    <define name='capusbdev'>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 548cd8f..96742ef 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -1,6 +1,7 @@
>  /*
>   * node_device_conf.c: config handling for node devices
>   *
> + * Copyright (C) 2009-2013 Red Hat, Inc.
>   * Copyright (C) 2008 Virtual Iron Software, Inc.
>   * Copyright (C) 2008 David F. Lively
>   *
> @@ -31,6 +32,7 @@
>  #include "viralloc.h"
>  #include "virstring.h"
>  #include "node_device_conf.h"
> +#include "device_conf.h"
>  #include "virxml.h"
>  #include "virbuffer.h"
>  #include "viruuid.h"
> @@ -330,6 +332,20 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def)
>                  }
>                  virBufferAddLit(&buf, "    </capability>\n");
>              }
> +            if (data->pci_dev.nIommuGroupDevices) {
> +                virBufferAsprintf(&buf, "    <iommuGroup number='%d'>\n",
> +                                  data->pci_dev.iommuGroupNumber);
> +                for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++) {
> +                    virBufferAsprintf(&buf,
> +                                      "      <address domain='0x%.4x' bus='0x%.2x' "
> +                                      "slot='0x%.2x' function='0x%.1x'/>\n",
> +                                      data->pci_dev.iommuGroupDevices[i]->domain,
> +                                      data->pci_dev.iommuGroupDevices[i]->bus,
> +                                      data->pci_dev.iommuGroupDevices[i]->slot,
> +                                      data->pci_dev.iommuGroupDevices[i]->function);
> +                }
> +                virBufferAddLit(&buf, "    </iommuGroup>\n");
> +            }
>              break;
>          case VIR_NODE_DEV_CAP_USB_DEV:
>              virBufferAsprintf(&buf, "    <bus>%d</bus>\n", data->usb_dev.bus);
> @@ -966,12 +982,71 @@ out:
>  }
>  
>  static int
> +virNodeDevCapPciDevIommuGroupParseXML(xmlXPathContextPtr ctxt,
> +                                      xmlNodePtr iommuGroupNode,
> +                                      union _virNodeDevCapData *data)
> +{
> +    xmlNodePtr origNode = ctxt->node;
> +    xmlNodePtr *addrNodes = NULL;
> +    char *numberStr = NULL;
> +    int nAddrNodes, ii, ret = -1;
> +    virPCIDeviceAddressPtr pciAddr = NULL;
> +
> +    ctxt->node = iommuGroupNode;
> +
> +    numberStr = virXMLPropString(iommuGroupNode, "number");
> +    if (!numberStr) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       "%s", _("missing iommuGroup number attribute"));
> +        goto cleanup;
> +    }
> +    if (virStrToLong_ui(numberStr, NULL, 10,
> +                        &data->pci_dev.iommuGroupNumber) < 0) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("invalid iommuGroup number attribute '%s'"),
> +                       numberStr);
> +        goto cleanup;
> +    }
> +
> +    if ((nAddrNodes = virXPathNodeSet("./address", ctxt, &addrNodes)) < 0)
> +        goto cleanup;
> +
> +    for (ii = 0; ii < nAddrNodes; ii++) {
> +        virDevicePCIAddress addr = { 0, 0, 0, 0, 0 };
> +        if (virDevicePCIAddressParseXML(addrNodes[ii], &addr) < 0)
> +            goto cleanup;
> +        if (VIR_ALLOC(pciAddr) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +        pciAddr->domain = addr.domain;
> +        pciAddr->bus = addr.bus;
> +        pciAddr->slot = addr.slot;
> +        pciAddr->function = addr.function;
> +        if (VIR_APPEND_ELEMENT(data->pci_dev.iommuGroupDevices,
> +                               data->pci_dev.nIommuGroupDevices,
> +                               pciAddr) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +    }
> +
> +    ret = 0;
> +cleanup:
> +    ctxt->node = origNode;
> +    VIR_FREE(addrNodes);
> +    VIR_FREE(pciAddr);
> +    return ret;
> +}
> +
> +
> +static int
>  virNodeDevCapPciDevParseXML(xmlXPathContextPtr ctxt,
>                              virNodeDeviceDefPtr def,
>                              xmlNodePtr node,
>                              union _virNodeDevCapData *data)
>  {
> -    xmlNodePtr orignode;
> +    xmlNodePtr orignode, iommuGroupNode;
>      int ret = -1;
>  
>      orignode = ctxt->node;
> @@ -1016,6 +1091,12 @@ virNodeDevCapPciDevParseXML(xmlXPathContextPtr ctxt,
>      data->pci_dev.vendor_name  = virXPathString("string(./vendor[1])", ctxt);
>      data->pci_dev.product_name = virXPathString("string(./product[1])", ctxt);
>  
> +    if ((iommuGroupNode = virXPathNode("./iommuGroup[1]", ctxt))) {
> +        if (virNodeDevCapPciDevIommuGroupParseXML(ctxt, iommuGroupNode,
> +                                                  data) < 0) {
> +            goto out;
> +        }
> +    }
>      ret = 0;
>  out:
>      ctxt->node = orignode;
> @@ -1385,6 +1466,9 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
>          for (i = 0; i < data->pci_dev.num_virtual_functions; i++) {
>              VIR_FREE(data->pci_dev.virtual_functions[i]);
>          }
> +        for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++) {
> +            VIR_FREE(data->pci_dev.iommuGroupDevices[i]);
> +        }
>          break;
>      case VIR_NODE_DEV_CAP_USB_DEV:
>          VIR_FREE(data->usb_dev.product_name);
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index 17240be..ec35da2 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -1,7 +1,7 @@
>  /*
>   * node_device_conf.h: config handling for node devices
>   *
> - * Copyright (C) 2010-2011 Red Hat, Inc.
> + * Copyright (C) 2009-2013 Red Hat, Inc.
>   * Copyright (C) 2008 Virtual Iron Software, Inc.
>   * Copyright (C) 2008 David F. Lively
>   *
> @@ -112,6 +112,9 @@ struct _virNodeDevCapsDef {
>              virPCIDeviceAddressPtr *virtual_functions;
>              unsigned int num_virtual_functions;
>              unsigned int flags;
> +            virPCIDeviceAddressPtr *iommuGroupDevices;
> +            size_t nIommuGroupDevices;
> +            unsigned int iommuGroupNumber;
>          } pci_dev;
>          struct {
>              unsigned int bus;
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index b94be80..9bb4a17 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -421,7 +421,8 @@ static int udevProcessPCI(struct udev_device *device,
>  {
>      const char *syspath = NULL;
>      union _virNodeDevCapData *data = &def->caps->data;
> -    int ret = -1;
> +    virPCIDeviceAddress addr;
> +    int tmpGroup, ret = -1;
>      char *p;
>      int rc;
>  
> @@ -501,6 +502,24 @@ static int udevProcessPCI(struct udev_device *device,
>      else if (!rc && (data->pci_dev.num_virtual_functions > 0))
>          data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
>  
> +    /* iommu group */
> +    addr.domain = data->pci_dev.domain;
> +    addr.bus = data->pci_dev.bus;
> +    addr.slot = data->pci_dev.slot;
> +    addr.function = data->pci_dev.function;
> +    tmpGroup = virPCIGetIOMMUGroupNum(&addr);
> +    if (tmpGroup == -1) {
> +        /* error was already reported */
> +        goto out;
> +        /* -2 return means there is no iommu_group data */
> +    } else if (tmpGroup >= 0) {
> +        if (virPCIGetIOMMUGroupAddresses(&addr, &data->pci_dev.iommuGroupDevices,
> +                                         &data->pci_dev.nIommuGroupDevices) < 0) {
> +            goto out;
> +        }
> +        data->pci_dev.iommuGroupNumber = tmpGroup;
> +    }
> +
>      ret = 0;
>  
>  out:
> diff --git a/tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml b/tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml
> new file mode 100644
> index 0000000..eff8932
> --- /dev/null
> +++ b/tests/nodedevschemadata/pci_8086_10c9_sriov_pf.xml
> @@ -0,0 +1,16 @@
> +<device>
> +  <name>pci_0000_02_00_0</name>
> +  <parent>pci_0000_00_04_0</parent>
> +  <capability type='pci'>
> +    <domain>0</domain>
> +    <bus>2</bus>
> +    <slot>0</slot>
> +    <function>0</function>
> +    <product id='0x10c9'>82576 Gigabit Network Connection</product>
> +    <vendor id='0x8086'>Intel Corporation</vendor>
> +    <iommuGroup number='12'>
> +      <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
> +      <address domain='0x0000' bus='0x02' slot='0x00' function='0x1'/>
> +    </iommuGroup>
> +  </capability>
> +</device>
> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
> index e81c617..ed49857 100644
> --- a/tests/nodedevxml2xmltest.c
> +++ b/tests/nodedevxml2xmltest.c
> @@ -78,6 +78,7 @@ mymain(void)
>      DO_TEST("net_00_13_02_b9_f9_d3");
>      DO_TEST("net_00_15_58_2f_e9_55");
>      DO_TEST("pci_1002_71c4");
> +    DO_TEST("pci_8086_10c9_sriov_pf");
>      DO_TEST("pci_8086_27c5_scsi_host_0");
>      DO_TEST("pci_8086_27c5_scsi_host_scsi_device_lun0");
>      DO_TEST("pci_8086_27c5_scsi_host_scsi_host");


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