[libvirt] [PATCH v2 2/2] nodedev: Introduce <pci-express/> to PCI devices

[Martin Kletzander]

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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140616/a5326cda/attachment-0001.sig>