WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Wed, 28 Jul 2010 20:40:51 +0100
Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Wed, 28 Jul 2010 12:44:26 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <patchbomb.1280346048@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1280346048@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mercurial-patchbomb/1.4.3
 tools/libxl/libxl.h      |   6 +---
 tools/libxl/libxl_pci.c  |  71 +++++++++++++++++++++++++++++++++++++----------
 tools/libxl/xl_cmdimpl.c |  46 ++++--------------------------
 3 files changed, 64 insertions(+), 59 deletions(-)


Introduce a new libxl call libxl_device_pci_parse_bdf() and use it
consistently.

This patch also fixes an infinite loop bug in xl create. If there is a
parse-error on any pci config file entry then xl will attempt to skip it, but
since num_pcidevs is used to index the config list and is not incremented when
skipping, this caused an infinite loop. Solve that problem by introducing a new
loop counter variable.

Note that virtual PCI slots are not parsed by the new code. However this is
a step towards support for virtual slots and multi-function device assignment
since only one location in the code will need to be changed now.

Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>

diff -r 099e4eb3803a -r e49597460a1a tools/libxl/libxl.h
--- a/tools/libxl/libxl.h       Wed Jul 28 20:31:59 2010 +0100
+++ b/tools/libxl/libxl.h       Wed Jul 28 20:35:38 2010 +0100
@@ -516,16 +516,12 @@ int libxl_device_vfb_add(struct libxl_ct
 int libxl_device_vfb_clean_shutdown(struct libxl_ctx *ctx, uint32_t domid);
 int libxl_device_vfb_hard_shutdown(struct libxl_ctx *ctx, uint32_t domid);
 
-#define PCI_BDF                "%04x:%02x:%02x.%01x"
-#define PCI_BDF_VDEVFN         "%04x:%02x:%02x.%01x@%02x"
 int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, 
libxl_device_pci *pcidev);
 int libxl_device_pci_remove(struct libxl_ctx *ctx, uint32_t domid, 
libxl_device_pci *pcidev);
 int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid);
 int libxl_device_pci_list_assigned(struct libxl_ctx *ctx, libxl_device_pci 
**list, uint32_t domid, int *num);
 int libxl_device_pci_list_assignable(struct libxl_ctx *ctx, libxl_device_pci 
**list, int *num);
-int libxl_device_pci_init(libxl_device_pci *pcidev, unsigned int domain,
-                          unsigned int bus, unsigned int dev,
-                          unsigned int func, unsigned int vdevfn);
+int libxl_device_pci_parse_bdf(libxl_device_pci *pcidev, const char *str);
 
 /*
  * Functions for allowing users of libxl to store private data
diff -r 099e4eb3803a -r e49597460a1a tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c   Wed Jul 28 20:31:59 2010 +0100
+++ b/tools/libxl/libxl_pci.c   Wed Jul 28 20:35:38 2010 +0100
@@ -36,6 +36,59 @@
 #include "libxl_internal.h"
 #include "flexarray.h"
 
+#define PCI_BDF                "%04x:%02x:%02x.%01x"
+#define PCI_BDF_SHORT          "%02x:%02x.%01x"
+#define PCI_BDF_VDEVFN         "%04x:%02x:%02x.%01x@%02x"
+
+static int pcidev_init(libxl_device_pci *pcidev, unsigned int domain,
+                          unsigned int bus, unsigned int dev,
+                          unsigned int func, unsigned int vdevfn)
+{
+    pcidev->domain = domain;
+    pcidev->bus = bus;
+    pcidev->dev = dev;
+    pcidev->func = func;
+    pcidev->vdevfn = vdevfn;
+    return 0;
+}
+
+int libxl_device_pci_parse_bdf(libxl_device_pci *pcidev, const char *str)
+{
+    unsigned dom, bus, dev, func;
+    char *p, *buf2;
+    int rc;
+
+    if ( NULL == (buf2 = strdup(str)) )
+        return ERROR_NOMEM;
+
+    p = strtok(buf2, ",");
+
+    if ( sscanf(str, PCI_BDF, &dom, &bus, &dev, &func) != 4 ) {
+        dom = 0;
+        if ( sscanf(str, PCI_BDF_SHORT, &bus, &dev, &func) != 3 ) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+    }
+
+    rc = pcidev_init(pcidev, dom, bus, dev, func, 0);
+
+    while ((p = strtok(NULL, ",=")) != NULL) {
+        while (*p == ' ')
+            p++;
+        if (!strcmp(p, "msitranslate")) {
+            p = strtok(NULL, ",=");
+            pcidev->msitranslate = atoi(p);
+        } else if (!strcmp(p, "power_mgmt")) {
+            p = strtok(NULL, ",=");
+            pcidev->power_mgmt = atoi(p);
+        }
+    }
+out:
+    free(buf2);
+    return rc;
+}
+
 static int libxl_create_pci_backend(struct libxl_ctx *ctx, uint32_t domid, 
libxl_device_pci *pcidev, int num)
 {
     flexarray_t *front;
@@ -288,7 +341,7 @@ int get_all_assigned_devices(struct libx
                     if ( sscanf(bdf, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
                         continue;
 
-                    libxl_device_pci_init(pcidevs + *num, dom, bus, dev, func, 
0);
+                    pcidev_init(pcidevs + *num, dom, bus, dev, func, 0);
                     (*num)++;
                 }
             }
@@ -581,7 +634,7 @@ static libxl_device_pci *scan_sys_pcidir
         new = pcidevs + *num;
 
         memset(new, 0, sizeof(*new));
-        libxl_device_pci_init(new, dom, bus, dev, func, 0);
+        pcidev_init(new, dom, bus, dev, func, 0);
         (*num)++;
     }
 
@@ -637,7 +690,7 @@ int libxl_device_pci_list_assigned(struc
         xsvdevfn = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, 
"%s/vdevfn-%d", be_path, i));
         if (xsvdevfn)
             vdevfn = strtol(xsvdevfn, (char **) NULL, 16);
-        libxl_device_pci_init(pcidevs + i, domain, bus, dev, func, vdevfn);
+        pcidev_init(pcidevs + i, domain, bus, dev, func, vdevfn);
         xsopts = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/opts-%d", 
be_path, i));
         if (xsopts) {
             char *saveptr;
@@ -676,18 +729,6 @@ int libxl_device_pci_shutdown(struct lib
     return 0;
 }
 
-int libxl_device_pci_init(libxl_device_pci *pcidev, unsigned int domain,
-                          unsigned int bus, unsigned int dev,
-                          unsigned int func, unsigned int vdevfn)
-{
-    pcidev->domain = domain;
-    pcidev->bus = bus;
-    pcidev->dev = dev;
-    pcidev->func = func;
-    pcidev->vdevfn = vdevfn;
-    return 0;
-}
-
 int libxl_device_pci_reset(struct libxl_ctx *ctx, unsigned int domain, 
unsigned int bus,
                          unsigned int dev, unsigned int func)
 {
diff -r 099e4eb3803a -r e49597460a1a tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Wed Jul 28 20:31:59 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Wed Jul 28 20:35:38 2010 +0100
@@ -485,7 +485,7 @@ static void printf_info(int domid,
     for (i = 0; i < d_config->num_pcidevs; i++) {
         printf("\t(device\n");
         printf("\t\t(pci\n");
-        printf("\t\t\t(pci dev "PCI_BDF_VDEVFN")\n",
+        printf("\t\t\t(pci dev %04x:%02x:%02x.%01x@%02x)\n",
                d_config->pcidevs[i].domain, d_config->pcidevs[i].bus,
                d_config->pcidevs[i].dev, d_config->pcidevs[i].func,
                d_config->pcidevs[i].vdevfn);
@@ -943,46 +943,20 @@ skip_vfb:
         pci_power_mgmt = l;
 
     if (!xlu_cfg_get_list (config, "pci", &pcis, 0)) {
+        int i;
         d_config->num_pcidevs = 0;
         d_config->pcidevs = NULL;
-        while ((buf = xlu_cfg_get_listitem (pcis, d_config->num_pcidevs)) != 
NULL) {
+        for(i = 0; (buf = xlu_cfg_get_listitem (pcis, i)) != NULL; i++) {
             libxl_device_pci *pcidev;
-            unsigned int domain = 0, bus = 0, dev = 0, func = 0, vdevfn = 0;
-            char *buf2 = strdup(buf);
-            char *p;
 
             d_config->pcidevs = (libxl_device_pci *) 
realloc(d_config->pcidevs, sizeof (libxl_device_pci) * (d_config->num_pcidevs + 
1));
             pcidev = d_config->pcidevs + d_config->num_pcidevs;
             memset(pcidev, 0x00, sizeof(libxl_device_pci));
 
-            p = strtok(buf2, ",");
-            if (!p)
-                goto skip_pci;
-            if (sscanf(p, PCI_BDF_VDEVFN, &domain, &bus, &dev, &func, &vdevfn) 
< 4) {
-                domain = 0;
-                if (sscanf(p, "%02x:%02x.%01x@%02x", &bus, &dev, &func, 
&vdevfn) < 3) {
-                    fprintf(stderr,"xl: Unable to parse pci bdf (%s)\n", p);
-                    goto skip_pci;
-                }
-            }
-
-            libxl_device_pci_init(pcidev, domain, bus, dev, func, vdevfn);
             pcidev->msitranslate = pci_msitranslate;
             pcidev->power_mgmt = pci_power_mgmt;
-            while ((p = strtok(NULL, ",=")) != NULL) {
-                while (*p == ' ')
-                    p++;
-                if (!strcmp(p, "msitranslate")) {
-                    p = strtok(NULL, ",=");
-                    pcidev->msitranslate = atoi(p);
-                } else if (!strcmp(p, "power_mgmt")) {
-                    p = strtok(NULL, ",=");
-                    pcidev->power_mgmt = atoi(p);
-                }
-            }
-            d_config->num_pcidevs++;
-skip_pci:
-            free(buf2);
+            if (!libxl_device_pci_parse_bdf(pcidev, buf))
+                d_config->num_pcidevs++;
         }
     }
 
@@ -1998,16 +1972,13 @@ int main_pcilist(int argc, char **argv)
 void pcidetach(char *dom, char *bdf)
 {
     libxl_device_pci pcidev;
-    unsigned int domain, bus, dev, func;
 
     find_domain(dom);
 
-    memset(&pcidev, 0x00, sizeof(pcidev));
-    if (sscanf(bdf, PCI_BDF, &domain, &bus, &dev, &func) != 4) {
+    if (libxl_device_pci_parse_bdf(&pcidev, bdf)) {
         fprintf(stderr, "pci-detach: malformed BDF specification \"%s\"\n", 
bdf);
         exit(2);
     }
-    libxl_device_pci_init(&pcidev, domain, bus, dev, func, 0);
     libxl_device_pci_remove(&ctx, domid, &pcidev);
 }
 
@@ -2040,16 +2011,13 @@ int main_pcidetach(int argc, char **argv
 void pciattach(char *dom, char *bdf, char *vs)
 {
     libxl_device_pci pcidev;
-    unsigned int domain, bus, dev, func;
 
     find_domain(dom);
 
-    memset(&pcidev, 0x00, sizeof(pcidev));
-    if (sscanf(bdf, PCI_BDF, &domain, &bus, &dev, &func) != 4) {
+    if (libxl_device_pci_parse_bdf(&pcidev, bdf)) {
         fprintf(stderr, "pci-attach: malformed BDF specification \"%s\"\n", 
bdf);
         exit(2);
     }
-    libxl_device_pci_init(&pcidev, domain, bus, dev, func, 0);
     libxl_device_pci_add(&ctx, domid, &pcidev);
 }
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>