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 @@
> <address>00:27:13:6a:fe:00</address>
> <capability type='80203'/>
> </capability>
> -</device></pre>
> +</device>
> +
> +<device>
> + <name>pci_0000_02_00_0</name>
> + <path>/sys/devices/pci0000:00/0000:00:04.0/0000:02:00.0</path>
> + <parent>pci_0000_00_04_0</parent>
> + <driver>
> + <name>igb</name>
> + </driver>
> + <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>
> + <capability type='virt_functions'>
> + <address domain='0x0000' bus='0x02' slot='0x10' function='0x0'/>
> + <address domain='0x0000' bus='0x02' slot='0x10' function='0x2'/>
> + <address domain='0x0000' bus='0x02' slot='0x10' function='0x4'/>
> + <address domain='0x0000' bus='0x02' slot='0x10' function='0x6'/>
> + <address domain='0x0000' bus='0x02' slot='0x11' function='0x0'/>
> + <address domain='0x0000' bus='0x02' slot='0x11' function='0x2'/>
> + <address domain='0x0000' bus='0x02' slot='0x11' function='0x4'/>
> + </capability>
> + <iommuGroup number='12'>
> + <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
> + <address domain='0x0000' bus='0x02' slot='0x00' function='0x1'/>
> + </iommuGroup>
> + </capability>
> +</device>
> + </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");
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list