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] Re: [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl

On Thu, 2010-07-29 at 15:42 +0100, Stefano Stabellini wrote:
> On Wed, 28 Jul 2010, Gianni Tedesco (3P) wrote:
> >  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>
> > 
> 
> 
> Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

In that case please apply the following, fixed an incorrect removal of
memset in pciattach/pcidetach.

diff -r bab9294a9b90 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h       Thu Jul 29 16:30:52 2010 +0100
+++ b/tools/libxl/libxl.h       Thu Jul 29 16:31:41 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 bab9294a9b90 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c   Thu Jul 29 16:30:52 2010 +0100
+++ b/tools/libxl/libxl_pci.c   Thu Jul 29 16:31:41 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 @@ static int get_all_assigned_devices(stru
                     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 bab9294a9b90 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Thu Jul 29 16:30:52 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Thu Jul 29 16:31:41 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,14 @@ 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 +2012,14 @@ 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>