[libvirt] [PATCH v2 0/2] Expose PCI Express capabilities

diff to v1: -Matin's review suggestions worked in -I've found that (testing this on my new HW) that some PCI-Express devices doesn't have to necessarily export link info. That's why I'm inventing this virPCIDeviceHasPCIExpressLink() which tells us if that's the case. So for a PCI Express device without link info exposed, the XML will simply contain <pci-express/>. Michal Privoznik (2): virpci: Introduce virPCIDeviceIsPCIExpress and friends nodedev: Introduce <pci-express/> to PCI devices docs/formatnode.html.in | 19 +++ docs/schemas/nodedev.rng | 26 ++++ src/conf/node_device_conf.c | 138 ++++++++++++++++++++- src/conf/node_device_conf.h | 33 ++++- src/libvirt_private.syms | 3 + src/node_device/node_device_udev.c | 34 +++++ src/util/virpci.c | 96 +++++++++++++- src/util/virpci.h | 8 ++ .../pci_8086_0c0c_snd_hda_intel.xml | 16 +++ .../pci_8086_4238_pcie_wireless.xml | 26 ++++ tests/nodedevxml2xmltest.c | 2 + 11 files changed, 397 insertions(+), 4 deletions(-) create mode 100644 tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml create mode 100644 tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml -- 1.8.5.5

These functions will handle PCIe devices and their link capabilities to query some info about it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 3 ++ src/util/virpci.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/virpci.h | 8 ++++ 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 122c572..29a9ed1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1697,6 +1697,7 @@ virPCIDeviceFree; virPCIDeviceGetDriverPathAndName; virPCIDeviceGetIOMMUGroupDev; virPCIDeviceGetIOMMUGroupList; +virPCIDeviceGetLinkCapSta; virPCIDeviceGetManaged; virPCIDeviceGetName; virPCIDeviceGetRemoveSlot; @@ -1704,7 +1705,9 @@ virPCIDeviceGetReprobe; virPCIDeviceGetStubDriver; virPCIDeviceGetUnbindFromStub; virPCIDeviceGetUsedBy; +virPCIDeviceHasPCIExpressLink; virPCIDeviceIsAssignable; +virPCIDeviceIsPCIExpress; virPCIDeviceListAdd; virPCIDeviceListAddCopy; virPCIDeviceListCount; diff --git a/src/util/virpci.c b/src/util/virpci.c index e0f2344..6f4f6af 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -120,6 +120,7 @@ struct _virPCIDeviceList { /* PCI30 6.7 Capabilities List */ #define PCI_CAPABILITY_LIST 0x34 /* Offset of first capability list entry */ +#define PCI_CAP_FLAGS 2 /* Capability defined flags (16 bits) */ /* PM12 3.2.1 Capability Identifier */ #define PCI_CAP_ID_PM 0x01 /* Power Management */ @@ -130,7 +131,13 @@ struct _virPCIDeviceList { /* PCIe20 7.8.3 Device Capabilities Register (Offset 04h) */ #define PCI_EXP_DEVCAP 0x4 /* Device capabilities */ -#define PCI_EXP_DEVCAP_FLR (1<<28) /* Function Level Reset */ +#define PCI_EXP_DEVCAP_FLR (1<<28) /* Function Level Reset */ +#define PCI_EXP_LNKCAP 0xc /* Link Capabilities */ +#define PCI_EXP_LNKCAP_SPEED 0x0000f /* Maximum Link Speed */ +#define PCI_EXP_LNKCAP_WIDTH 0x003f0 /* Maximum Link Width */ +#define PCI_EXP_LNKSTA 0x12 /* Link Status */ +#define PCI_EXP_LNKSTA_SPEED 0x000f /* Negotiated Link Speed */ +#define PCI_EXP_LNKSTA_WIDTH 0x03f0 /* Negotiated Link Width */ /* Header type 1 BR12 3.2 PCI-to-PCI Bridge Configuration Space Header Format */ #define PCI_PRIMARY_BUS 0x18 /* BR12 3.2.5.2 Primary bus number */ @@ -173,6 +180,9 @@ struct _virPCIDeviceList { PCI_EXT_CAP_ACS_CR | \ PCI_EXT_CAP_ACS_UF) +#define PCI_EXP_TYPE_ROOT_INT_EP 0x9 /* Root Complex Integrated Endpoint */ +#define PCI_EXP_TYPE_ROOT_EC 0xa /* Root Complex Event Collector */ + static virClassPtr virPCIDeviceListClass; static void virPCIDeviceListDispose(void *obj); @@ -2750,3 +2760,87 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path ATTRIBUTE_UNUSED, return -1; } #endif /* __linux__ */ + +int +virPCIDeviceIsPCIExpress(virPCIDevicePtr dev) +{ + int fd; + int ret = -1; + + if ((fd = virPCIDeviceConfigOpen(dev, true)) < 0) + return ret; + + if (virPCIDeviceInit(dev, fd) < 0) + goto cleanup; + + ret = dev->pcie_cap_pos != 0; + + cleanup: + virPCIDeviceConfigClose(dev, fd); + return ret; +} + +int +virPCIDeviceHasPCIExpressLink(virPCIDevicePtr dev) +{ + int fd; + int ret = -1; + uint16_t cap, type; + + if ((fd = virPCIDeviceConfigOpen(dev, true)) < 0) + return ret; + + if (virPCIDeviceInit(dev, fd) < 0) + goto cleanup; + + cap = virPCIDeviceRead16(dev, fd, dev->pcie_cap_pos + PCI_CAP_FLAGS); + type = (cap & PCI_EXP_FLAGS_TYPE) >> 4; + + ret = type != PCI_EXP_TYPE_ROOT_INT_EP && type != PCI_EXP_TYPE_ROOT_EC; + + cleanup: + virPCIDeviceConfigClose(dev, fd); + return ret; +} + +int +virPCIDeviceGetLinkCapSta(virPCIDevicePtr dev, + int *cap_port, + unsigned int *cap_speed, + unsigned int *cap_width, + unsigned int *sta_speed, + unsigned int *sta_width) +{ + uint32_t t; + int fd; + int ret = -1; + + if ((fd = virPCIDeviceConfigOpen(dev, true)) < 0) + return ret; + + if (virPCIDeviceInit(dev, fd) < 0) + goto cleanup; + + if (!dev->pcie_cap_pos) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("pci device %s is not a PCI-Express device"), + dev->name); + goto cleanup; + } + + t = virPCIDeviceRead32(dev, fd, dev->pcie_cap_pos + PCI_EXP_LNKCAP); + + *cap_port = t >> 24; + *cap_speed = t & PCI_EXP_LNKCAP_SPEED; + *cap_width = (t & PCI_EXP_LNKCAP_WIDTH) >> 4; + + t = virPCIDeviceRead16(dev, fd, dev->pcie_cap_pos + PCI_EXP_LNKSTA); + + *sta_speed = t & PCI_EXP_LNKSTA_SPEED; + *sta_width = (t & PCI_EXP_LNKSTA_WIDTH) >> 4; + ret = 0; + + cleanup: + virPCIDeviceConfigClose(dev, fd); + return ret; +} diff --git a/src/util/virpci.h b/src/util/virpci.h index 20ffe54..d64c3e2 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -176,4 +176,12 @@ int virPCIDeviceGetDriverPathAndName(virPCIDevicePtr dev, char **path, char **name); +int virPCIDeviceIsPCIExpress(virPCIDevicePtr dev); +int virPCIDeviceHasPCIExpressLink(virPCIDevicePtr dev); +int virPCIDeviceGetLinkCapSta(virPCIDevicePtr dev, + int *ca_port, + unsigned int *cap_speed, + unsigned int *cap_width, + unsigned int *sta_speed, + unsigned int *sta_width); #endif /* __VIR_PCI_H__ */ -- 1.8.5.5

On Thu, Jun 12, 2014 at 05:27:34PM +0200, Michal Privoznik wrote:
These functions will handle PCIe devices and their link capabilities to query some info about it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 3 ++ src/util/virpci.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/virpci.h | 8 ++++ 3 files changed, 106 insertions(+), 1 deletion(-)
[...]
diff --git a/src/util/virpci.c b/src/util/virpci.c index e0f2344..6f4f6af 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -120,6 +120,7 @@ struct _virPCIDeviceList {
/* PCI30 6.7 Capabilities List */ #define PCI_CAPABILITY_LIST 0x34 /* Offset of first capability list entry */ +#define PCI_CAP_FLAGS 2 /* Capability defined flags (16 bits) */
/* PM12 3.2.1 Capability Identifier */ #define PCI_CAP_ID_PM 0x01 /* Power Management */ @@ -130,7 +131,13 @@ struct _virPCIDeviceList {
/* PCIe20 7.8.3 Device Capabilities Register (Offset 04h) */ #define PCI_EXP_DEVCAP 0x4 /* Device capabilities */ -#define PCI_EXP_DEVCAP_FLR (1<<28) /* Function Level Reset */ +#define PCI_EXP_DEVCAP_FLR (1<<28) /* Function Level Reset */ +#define PCI_EXP_LNKCAP 0xc /* Link Capabilities */ +#define PCI_EXP_LNKCAP_SPEED 0x0000f /* Maximum Link Speed */ +#define PCI_EXP_LNKCAP_WIDTH 0x003f0 /* Maximum Link Width */ +#define PCI_EXP_LNKSTA 0x12 /* Link Status */ +#define PCI_EXP_LNKSTA_SPEED 0x000f /* Negotiated Link Speed */ +#define PCI_EXP_LNKSTA_WIDTH 0x03f0 /* Negotiated Link Width */
I'm still not convinced that this isn't just obfuscating the code, but as far as the usage goes, this work (and will work) as expected, I just blabbed about the looks of it. ACK, Martin

This new element is there to represent PCI-Express capabilities of a PCI devices, like link speed, number of lanes, etc. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatnode.html.in | 19 +++ docs/schemas/nodedev.rng | 26 ++++ src/conf/node_device_conf.c | 138 ++++++++++++++++++++- src/conf/node_device_conf.h | 33 ++++- src/node_device/node_device_udev.c | 34 +++++ .../pci_8086_0c0c_snd_hda_intel.xml | 16 +++ .../pci_8086_4238_pcie_wireless.xml | 26 ++++ tests/nodedevxml2xmltest.c | 2 + 8 files changed, 291 insertions(+), 3 deletions(-) create mode 100644 tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml create mode 100644 tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index d82bd3e..5932811 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -117,6 +117,21 @@ <code>node</code> attribute tells which NUMA node is the PCI device associated with. </dd> + <dt><code>pci-express</code></dt> + <dd> + This optional element contains information on PCI Express part of + the device. For example, it can contain a child element + <code>link</code> which addresses the PCI Express device's link. + While a device has it's own capabilities + (<code>validity='cap'</code>), the actual run time capabilities + are negotiated on the device initialization + (<code>validity='sta'</code>). The <code>link</code> element then + contains three attributes: <code>port</code> which says in which + port is the device plugged in, <code>speed</code> (in + GigaTransfers per second) and <code>width</code> for the number + of lanes used. Since the port can't be negotiated, it's not + exposed in <code>./pci-express/link/[@validity='sta']</code>. + </dd> </dl> </dd> <dt><code>usb_device</code></dt> @@ -305,6 +320,10 @@ <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> <address domain='0x0000' bus='0x02' slot='0x00' function='0x1'/> </iommuGroup> + <pci-express> + <link validity='cap' port='1' speed='2.5' width='1'/> + <link validity='sta' speed='2.5' width='1'/> + </pci-express> </capability> </device> </pre> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 505f913..0a8b7fa 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -168,6 +168,32 @@ </element> </optional> + <optional> + <element name='pci-express'> + <zeroOrMore> + <element name='link'> + <attribute name='validity'> + <choice> + <value>cap</value> + <value>sta</value> + </choice> + </attribute> + <optional> + <attribute name='port'> + <ref name='unsignedInt'/> + </attribute> + </optional> + <optional> + <attribute name='speed'> + </attribute> + </optional> + <attribute name='width'> + <ref name='unsignedInt'/> + </attribute> + </element> + </zeroOrMore> + </element> + </optional> </define> <define name='capusbdev'> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 12e40e6..cb85914 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -58,6 +58,9 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", "80211") +VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST, + "", "2.5", "5", "8") + static int virNodeDevCapsDefParseString(const char *xpath, xmlXPathContextPtr ctxt, @@ -218,6 +221,43 @@ void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs, } } +static void +virPCIELinkFormat(virBufferPtr buf, + virPCIELinkPtr lnk, + const char *attrib) +{ + if (!lnk) + return; + + virBufferAsprintf(buf, "<link validity='%s'", attrib); + if (lnk->port >= 0) + virBufferAsprintf(buf, " port='%d'", lnk->port); + if (lnk->speed) + virBufferAsprintf(buf, " speed='%s'", + virPCIELinkSpeedTypeToString(lnk->speed)); + virBufferAsprintf(buf, " width='%d'", lnk->width); + virBufferAddLit(buf, "/>\n"); +} + +static void +virPCIEDeviceInfoFormat(virBufferPtr buf, + virPCIEDeviceInfoPtr info) +{ + if (!info->link_cap && !info->link_sta) { + virBufferAddLit(buf, "<pci-express/>\n"); + return; + } + + virBufferAddLit(buf, "<pci-express>\n"); + virBufferAdjustIndent(buf, 2); + + virPCIELinkFormat(buf, info->link_cap, "cap"); + virPCIELinkFormat(buf, info->link_sta, "sta"); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</pci-express>\n"); +} + char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -349,6 +389,8 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) if (data->pci_dev.numa_node >= 0) virBufferAsprintf(&buf, "<numa node='%d'/>\n", data->pci_dev.numa_node); + if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCIE) + virPCIEDeviceInfoFormat(&buf, data->pci_dev.pci_express); break; case VIR_NODE_DEV_CAP_USB_DEV: virBufferAsprintf(&buf, "<bus>%d</bus>\n", data->usb_dev.bus); @@ -1086,6 +1128,86 @@ virNodeDevCapPCIDevIommuGroupParseXML(xmlXPathContextPtr ctxt, return ret; } +static int +virPCIEDeviceInfoLinkParseXML(xmlXPathContextPtr ctxt, + xmlNodePtr linkNode, + virPCIELinkPtr lnk) +{ + xmlNodePtr origNode = ctxt->node; + int ret = -1, speed; + char *speedStr = NULL, *portStr = NULL; + + ctxt->node = linkNode; + + if (virXPathUInt("number(./@width)", ctxt, &lnk->width) < 0) { + virReportError(VIR_ERR_XML_DETAIL, "%s", + _("mandatory attribute 'width' is missing or malformed")); + goto cleanup; + } + + if ((speedStr = virXPathString("string(./@speed)", ctxt))) { + if ((speed = virPCIELinkSpeedTypeFromString(speedStr)) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("malformed 'speed' attribute: %s"), + speedStr); + goto cleanup; + } + lnk->speed = speed; + } + + if ((portStr = virXPathString("string(./@port)", ctxt))) { + if (virStrToLong_i(portStr, NULL, 10, &lnk->port) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("malformed 'port' attribute: %s"), + portStr); + goto cleanup; + } + } else { + lnk->port = -1; + } + + ret = 0; + cleanup: + VIR_FREE(portStr); + VIR_FREE(speedStr); + ctxt->node = origNode; + return ret; +} + +static int +virPCIEDeviceInfoParseXML(xmlXPathContextPtr ctxt, + xmlNodePtr pciExpressNode, + virPCIEDeviceInfoPtr pci_express) +{ + xmlNodePtr lnk, origNode = ctxt->node; + int ret = -1; + + ctxt->node = pciExpressNode; + + if ((lnk = virXPathNode("./link[@validity='cap']", ctxt))) { + if (VIR_ALLOC(pci_express->link_cap) < 0) + goto cleanup; + + if (virPCIEDeviceInfoLinkParseXML(ctxt, lnk, + pci_express->link_cap) < 0) + goto cleanup; + } + + if ((lnk = virXPathNode("./link[@validity='sta']", ctxt))) { + if (VIR_ALLOC(pci_express->link_sta) < 0) + goto cleanup; + + if (virPCIEDeviceInfoLinkParseXML(ctxt, lnk, + pci_express->link_sta) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + ctxt->node = origNode; + return ret; +} + static int virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, @@ -1093,8 +1215,9 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, union _virNodeDevCapData *data) { - xmlNodePtr orignode, iommuGroupNode; + xmlNodePtr orignode, iommuGroupNode, pciExpress; int ret = -1; + virPCIEDeviceInfoPtr pci_express = NULL; orignode = ctxt->node; ctxt->node = node; @@ -1152,8 +1275,21 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, _("invalid NUMA node ID supplied for '%s'")) < 0) goto out; + if ((pciExpress = virXPathNode("./pci-express[1]", ctxt))) { + if (VIR_ALLOC(pci_express) < 0) + goto out; + + if (virPCIEDeviceInfoParseXML(ctxt, pciExpress, pci_express) < 0) + goto out; + + data->pci_dev.pci_express = pci_express; + pci_express = NULL; + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCIE; + } + ret = 0; out: + VIR_FREE(pci_express); ctxt->node = orignode; return ret; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index d1311dd..b95ccf1 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -76,10 +76,38 @@ typedef enum { } virNodeDevSCSIHostCapFlags; typedef enum { - VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0), - VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1), + VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0), + VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1), + VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2), } virNodeDevPCICapFlags; +typedef enum { + VIR_PCIE_LINK_SPEED_NA = 0, + VIR_PCIE_LINK_SPEED_25, + VIR_PCIE_LINK_SPEED_5, + VIR_PCIE_LINK_SPEED_8, + VIR_PCIE_LINK_SPEED_LAST +} virPCIELinkSpeed; + +VIR_ENUM_DECL(virPCIELinkSpeed) + +typedef struct _virPCIELink virPCIELink; +typedef virPCIELink *virPCIELinkPtr; +struct _virPCIELink { + int port; + virPCIELinkSpeed speed; + unsigned int width; +}; + +typedef struct _virPCIEDeviceInfo virPCIEDeviceInfo; +typedef virPCIEDeviceInfo *virPCIEDeviceInfoPtr; +struct _virPCIEDeviceInfo { + /* Not all PCI Express devices has link. For example this 'Root Complex + * Integrated Endpoint' and 'Root Complex Event Collector' doesn't have. */ + virPCIELink *link_cap; /* PCIe device link capabilities */ + virPCIELink *link_sta; /* Actually negotiated capabilities */ +}; + typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { @@ -117,6 +145,7 @@ struct _virNodeDevCapsDef { size_t nIommuGroupDevices; unsigned int iommuGroupNumber; int numa_node; + virPCIEDeviceInfoPtr pci_express; } 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 a8e74e4..bb6a0b9 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -426,6 +426,8 @@ static int udevProcessPCI(struct udev_device *device, const char *syspath = NULL; union _virNodeDevCapData *data = &def->caps->data; virPCIDeviceAddress addr; + virPCIEDeviceInfoPtr pci_express = NULL; + virPCIDevicePtr pciDev = NULL; int tmpGroup, ret = -1; char *p; int rc; @@ -535,9 +537,41 @@ static int udevProcessPCI(struct udev_device *device, data->pci_dev.iommuGroupNumber = tmpGroup; } + if (!(pciDev = virPCIDeviceNew(data->pci_dev.domain, + data->pci_dev.bus, + data->pci_dev.slot, + data->pci_dev.function))) + goto out; + + if (virPCIDeviceIsPCIExpress(pciDev) > 0) { + if (VIR_ALLOC(pci_express) < 0) + goto out; + + if (virPCIDeviceHasPCIExpressLink(pciDev) > 0) { + if (VIR_ALLOC(pci_express->link_cap) < 0 || + VIR_ALLOC(pci_express->link_sta) < 0) + goto out; + + if (virPCIDeviceGetLinkCapSta(pciDev, + &pci_express->link_cap->port, + &pci_express->link_cap->speed, + &pci_express->link_cap->width, + &pci_express->link_sta->speed, + &pci_express->link_sta->width) < 0) + goto out; + + pci_express->link_sta->port = -1; /* PCIe can't negotiate port. Yet :) */ + } + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCIE; + data->pci_dev.pci_express = pci_express; + pci_express = NULL; + } + ret = 0; out: + virPCIDeviceFree(pciDev); + VIR_FREE(pci_express); return ret; } diff --git a/tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml b/tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml new file mode 100644 index 0000000..8e89900 --- /dev/null +++ b/tests/nodedevschemadata/pci_8086_0c0c_snd_hda_intel.xml @@ -0,0 +1,16 @@ +<device> + <name>pci_0000_00_03_0</name> + <parent>computer</parent> + <capability type='pci'> + <domain>0</domain> + <bus>0</bus> + <slot>3</slot> + <function>0</function> + <product id='0x0c0c'>Xeon E3-1200 v3/4th Gen Core Processor HD Audio Controller</product> + <vendor id='0x8086'>Intel Corporation</vendor> + <iommuGroup number='2'> + <address domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </iommuGroup> + <pci-express/> + </capability> +</device> diff --git a/tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml b/tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml new file mode 100644 index 0000000..18172e9 --- /dev/null +++ b/tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml @@ -0,0 +1,26 @@ +<device> + <name>pci_0000_03_00_0</name> + <parent>pci_0000_00_1c_1</parent> + <capability type='pci'> + <domain>0</domain> + <bus>3</bus> + <slot>0</slot> + <function>0</function> + <product id='0x4238'>Centrino Ultimate-N 6300</product> + <vendor id='0x8086'>Intel Corporation</vendor> + <iommuGroup number='8'> + <address domain='0x0000' bus='0x00' slot='0x1c' function='0x0'/> + <address domain='0x0000' bus='0x00' slot='0x1c' function='0x1'/> + <address domain='0x0000' bus='0x00' slot='0x1c' function='0x3'/> + <address domain='0x0000' bus='0x00' slot='0x1c' function='0x4'/> + <address domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + <address domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/> + <address domain='0x0000' bus='0x0d' slot='0x00' function='0x1'/> + <address domain='0x0000' bus='0x0d' slot='0x00' function='0x3'/> + </iommuGroup> + <pci-express> + <link validity='cap' port='1' speed='2.5' width='1'/> + <link validity='sta' speed='2.5' width='1'/> + </pci-express> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index 9390bf5..fe1fd88 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -88,6 +88,8 @@ mymain(void) DO_TEST("storage_serial_SATA_HTS721010G9SA00_MPCZ12Y0GNGWSE"); DO_TEST("usb_device_1d6b_1_0000_00_1d_0_if0"); DO_TEST("usb_device_1d6b_1_0000_00_1d_0"); + DO_TEST("pci_8086_4238_pcie_wireless"); + DO_TEST("pci_8086_0c0c_snd_hda_intel"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.5.5

On Thu, Jun 12, 2014 at 05:27:35PM +0200, Michal Privoznik wrote:
This new element is there to represent PCI-Express capabilities of a PCI devices, like link speed, number of lanes, etc.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> [...] diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 505f913..0a8b7fa 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -168,6 +168,32 @@ </element> </optional>
+ <optional> + <element name='pci-express'> + <zeroOrMore> + <element name='link'> + <attribute name='validity'> + <choice> + <value>cap</value> + <value>sta</value> + </choice> + </attribute> + <optional> + <attribute name='port'> + <ref name='unsignedInt'/> + </attribute> + </optional> + <optional> + <attribute name='speed'>
List the values here or use the string with pattern from my suggestion in v1.
+ </attribute> + </optional> + <attribute name='width'> + <ref name='unsignedInt'/> + </attribute> + </element> + </zeroOrMore> + </element> + </optional> </define>
<define name='capusbdev'> [...] diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index d1311dd..b95ccf1 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -76,10 +76,38 @@ typedef enum { } virNodeDevSCSIHostCapFlags;
typedef enum { - VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0), - VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1), + VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0), + VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1), + VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2), } virNodeDevPCICapFlags;
+typedef enum { + VIR_PCIE_LINK_SPEED_NA = 0, + VIR_PCIE_LINK_SPEED_25, + VIR_PCIE_LINK_SPEED_5, + VIR_PCIE_LINK_SPEED_8, + VIR_PCIE_LINK_SPEED_LAST +} virPCIELinkSpeed; + +VIR_ENUM_DECL(virPCIELinkSpeed) + +typedef struct _virPCIELink virPCIELink; +typedef virPCIELink *virPCIELinkPtr; +struct _virPCIELink { + int port; + virPCIELinkSpeed speed; + unsigned int width; +}; + +typedef struct _virPCIEDeviceInfo virPCIEDeviceInfo; +typedef virPCIEDeviceInfo *virPCIEDeviceInfoPtr; +struct _virPCIEDeviceInfo { + /* Not all PCI Express devices has link. For example this 'Root Complex + * Integrated Endpoint' and 'Root Complex Event Collector' doesn't have. */
Actually, exactly only these two do not have it. [...]
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index a8e74e4..bb6a0b9 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -426,6 +426,8 @@ static int udevProcessPCI(struct udev_device *device, const char *syspath = NULL; union _virNodeDevCapData *data = &def->caps->data; virPCIDeviceAddress addr; + virPCIEDeviceInfoPtr pci_express = NULL; + virPCIDevicePtr pciDev = NULL; int tmpGroup, ret = -1; char *p; int rc; @@ -535,9 +537,41 @@ static int udevProcessPCI(struct udev_device *device, data->pci_dev.iommuGroupNumber = tmpGroup; }
+ if (!(pciDev = virPCIDeviceNew(data->pci_dev.domain, + data->pci_dev.bus, + data->pci_dev.slot, + data->pci_dev.function))) + goto out; + + if (virPCIDeviceIsPCIExpress(pciDev) > 0) { + if (VIR_ALLOC(pci_express) < 0) + goto out; + + if (virPCIDeviceHasPCIExpressLink(pciDev) > 0) { + if (VIR_ALLOC(pci_express->link_cap) < 0 || + VIR_ALLOC(pci_express->link_sta) < 0) + goto out; + + if (virPCIDeviceGetLinkCapSta(pciDev, + &pci_express->link_cap->port, + &pci_express->link_cap->speed, + &pci_express->link_cap->width, + &pci_express->link_sta->speed, + &pci_express->link_sta->width) < 0)
You wouldn't have to pass all the params if you defined the structs in virpci, but that's just a cosmetic thing. ACK with the RelaxNG schema fixed, Martin
participants (2)
-
Martin Kletzander
-
Michal Privoznik