* [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c
2010-07-28 19:40 [PATCH 0 of 3] Introduce basic PCI safety checks in libxl and centralize BDF parsing Gianni Tedesco
@ 2010-07-28 19:40 ` Gianni Tedesco
2010-07-29 15:37 ` Ian Jackson
2010-07-29 15:48 ` Gianni Tedesco
2010-07-28 19:40 ` [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU Gianni Tedesco
2010-07-28 19:40 ` [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl Gianni Tedesco
2 siblings, 2 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-28 19:40 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Stefano Stabellini
tools/libxl/libxl_device.c | 34 ----------------------------------
tools/libxl/libxl_pci.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 34 deletions(-)
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r ebede381efe8 -r e2520c8b96b7 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c Tue Jul 27 18:09:22 2010 +0100
+++ b/tools/libxl/libxl_device.c Wed Jul 28 20:29:08 2010 +0100
@@ -377,40 +377,6 @@ int libxl_device_del(struct libxl_ctx *c
return 0;
}
-int libxl_device_pci_reset(struct libxl_ctx *ctx, unsigned int domain, unsigned int bus,
- unsigned int dev, unsigned int func)
-{
- char *reset = "/sys/bus/pci/drivers/pciback/do_flr";
- int fd, rc;
-
- fd = open(reset, O_WRONLY);
- if (fd > 0) {
- char *buf = libxl_sprintf(ctx, PCI_BDF, domain, bus, dev, func);
- rc = write(fd, buf, strlen(buf));
- if (rc < 0)
- XL_LOG(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
- close(fd);
- return rc < 0 ? rc : 0;
- }
- if (errno != ENOENT)
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access pciback path %s", reset);
- reset = libxl_sprintf(ctx, "/sys/bus/pci/devices/"PCI_BDF"/reset", domain, bus, dev, func);
- fd = open(reset, O_WRONLY);
- if (fd > 0) {
- rc = write(fd, "1", 1);
- if (rc < 0)
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
- close(fd);
- return rc < 0 ? rc : 0;
- }
- if (errno == ENOENT) {
- XL_LOG(ctx, XL_LOG_ERROR, "The kernel doesn't support PCI device reset from sysfs");
- } else {
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access reset path %s", reset);
- }
- return -1;
-}
-
int libxl_wait_for_device_model(struct libxl_ctx *ctx,
uint32_t domid, char *state,
int (*check_callback)(struct libxl_ctx *ctx,
diff -r ebede381efe8 -r e2520c8b96b7 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Tue Jul 27 18:09:22 2010 +0100
+++ b/tools/libxl/libxl_pci.c Wed Jul 28 20:29:08 2010 +0100
@@ -536,3 +536,36 @@ int libxl_device_pci_init(libxl_device_p
return 0;
}
+int libxl_device_pci_reset(struct libxl_ctx *ctx, unsigned int domain, unsigned int bus,
+ unsigned int dev, unsigned int func)
+{
+ char *reset = "/sys/bus/pci/drivers/pciback/do_flr";
+ int fd, rc;
+
+ fd = open(reset, O_WRONLY);
+ if (fd > 0) {
+ char *buf = libxl_sprintf(ctx, PCI_BDF, domain, bus, dev, func);
+ rc = write(fd, buf, strlen(buf));
+ if (rc < 0)
+ XL_LOG(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
+ close(fd);
+ return rc < 0 ? rc : 0;
+ }
+ if (errno != ENOENT)
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access pciback path %s", reset);
+ reset = libxl_sprintf(ctx, "/sys/bus/pci/devices/"PCI_BDF"/reset", domain, bus, dev, func);
+ fd = open(reset, O_WRONLY);
+ if (fd > 0) {
+ rc = write(fd, "1", 1);
+ if (rc < 0)
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
+ close(fd);
+ return rc < 0 ? rc : 0;
+ }
+ if (errno == ENOENT) {
+ XL_LOG(ctx, XL_LOG_ERROR, "The kernel doesn't support PCI device reset from sysfs");
+ } else {
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access reset path %s", reset);
+ }
+ return -1;
+}
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c
2010-07-28 19:40 ` [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c Gianni Tedesco
@ 2010-07-29 15:37 ` Ian Jackson
2010-07-29 15:42 ` Gianni Tedesco
2010-07-29 15:48 ` Gianni Tedesco
1 sibling, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2010-07-29 15:37 UTC (permalink / raw)
To: Gianni Tedesco; +Cc: xen-devel, Stefano Stabellini
Gianni Tedesco writes ("[Xen-devel] [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c"):
> tools/libxl/libxl_device.c | 34 ----------------------------------
> tools/libxl/libxl_pci.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+), 34 deletions(-)
This patch does not apply to tip. Have you pulled recently ? I guess
Ian Campbell's struct typedef consistency patch is causing the
conflicts.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c
2010-07-29 15:37 ` Ian Jackson
@ 2010-07-29 15:42 ` Gianni Tedesco
0 siblings, 0 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 15:42 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com, Stabellini
On Thu, 2010-07-29 at 16:37 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("[Xen-devel] [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c"):
> > tools/libxl/libxl_device.c | 34 ----------------------------------
> > tools/libxl/libxl_pci.c | 33 +++++++++++++++++++++++++++++++++
> > 2 files changed, 33 insertions(+), 34 deletions(-)
>
> This patch does not apply to tip. Have you pulled recently ? I guess
> Ian Campbell's struct typedef consistency patch is causing the
> conflicts.
Damnit, pulled (cloned afresh infact) right before putting together the
patch set. Things are just moving to fast to keep up :)
Will re-base and re-send.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c
2010-07-28 19:40 ` [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c Gianni Tedesco
2010-07-29 15:37 ` Ian Jackson
@ 2010-07-29 15:48 ` Gianni Tedesco
1 sibling, 0 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 15:48 UTC (permalink / raw)
To: xen-devel@lists.xensource.com; +Cc: Ian Jackson, Stefano Stabellini
On Wed, 2010-07-28 at 20:40 +0100, Gianni Tedesco wrote:
> tools/libxl/libxl_device.c | 34 ----------------------------------
> tools/libxl/libxl_pci.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+), 34 deletions(-)
>
>
> Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r ebede381efe8 -r 6d858d8546d4 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c Tue Jul 27 18:09:22 2010 +0100
+++ b/tools/libxl/libxl_device.c Wed Jul 28 18:37:39 2010 +0100
@@ -377,40 +377,6 @@ int libxl_device_del(libxl_ctx *c
return 0;
}
-int libxl_device_pci_reset(libxl_ctx *ctx, unsigned int domain, unsigned int bus,
- unsigned int dev, unsigned int func)
-{
- char *reset = "/sys/bus/pci/drivers/pciback/do_flr";
- int fd, rc;
-
- fd = open(reset, O_WRONLY);
- if (fd > 0) {
- char *buf = libxl_sprintf(ctx, PCI_BDF, domain, bus, dev, func);
- rc = write(fd, buf, strlen(buf));
- if (rc < 0)
- XL_LOG(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
- close(fd);
- return rc < 0 ? rc : 0;
- }
- if (errno != ENOENT)
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access pciback path %s", reset);
- reset = libxl_sprintf(ctx, "/sys/bus/pci/devices/"PCI_BDF"/reset", domain, bus, dev, func);
- fd = open(reset, O_WRONLY);
- if (fd > 0) {
- rc = write(fd, "1", 1);
- if (rc < 0)
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
- close(fd);
- return rc < 0 ? rc : 0;
- }
- if (errno == ENOENT) {
- XL_LOG(ctx, XL_LOG_ERROR, "The kernel doesn't support PCI device reset from sysfs");
- } else {
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access reset path %s", reset);
- }
- return -1;
-}
-
int libxl_wait_for_device_model(libxl_ctx *ctx,
uint32_t domid, char *state,
int (*check_callback)(libxl_ctx *ctx,
diff -r ebede381efe8 -r 6d858d8546d4 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Tue Jul 27 18:09:22 2010 +0100
+++ b/tools/libxl/libxl_pci.c Wed Jul 28 18:37:39 2010 +0100
@@ -536,3 +536,36 @@ int libxl_device_pci_init(libxl_device_p
return 0;
}
+int libxl_device_pci_reset(libxl_ctx *ctx, unsigned int domain, unsigned int bus,
+ unsigned int dev, unsigned int func)
+{
+ char *reset = "/sys/bus/pci/drivers/pciback/do_flr";
+ int fd, rc;
+
+ fd = open(reset, O_WRONLY);
+ if (fd > 0) {
+ char *buf = libxl_sprintf(ctx, PCI_BDF, domain, bus, dev, func);
+ rc = write(fd, buf, strlen(buf));
+ if (rc < 0)
+ XL_LOG(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
+ close(fd);
+ return rc < 0 ? rc : 0;
+ }
+ if (errno != ENOENT)
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access pciback path %s", reset);
+ reset = libxl_sprintf(ctx, "/sys/bus/pci/devices/"PCI_BDF"/reset", domain, bus, dev, func);
+ fd = open(reset, O_WRONLY);
+ if (fd > 0) {
+ rc = write(fd, "1", 1);
+ if (rc < 0)
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "write to %s returned %d", reset, rc);
+ close(fd);
+ return rc < 0 ? rc : 0;
+ }
+ if (errno == ENOENT) {
+ XL_LOG(ctx, XL_LOG_ERROR, "The kernel doesn't support PCI device reset from sysfs");
+ } else {
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Failed to access reset path %s", reset);
+ }
+ return -1;
+}
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
2010-07-28 19:40 [PATCH 0 of 3] Introduce basic PCI safety checks in libxl and centralize BDF parsing Gianni Tedesco
2010-07-28 19:40 ` [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c Gianni Tedesco
@ 2010-07-28 19:40 ` Gianni Tedesco
2010-07-29 14:36 ` Stefano Stabellini
` (2 more replies)
2010-07-28 19:40 ` [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl Gianni Tedesco
2 siblings, 3 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-28 19:40 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Stefano Stabellini
tools/libxl/libxl.h | 3 +-
tools/libxl/libxl_internal.h | 3 +
tools/libxl/libxl_pci.c | 186 +++++++++++++++++++++++++++++++++++++++---
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 36 +++++++-
tools/libxl/xl_cmdtable.c | 5 +
6 files changed, 214 insertions(+), 20 deletions(-)
Implement a new libxl function libxl_device_pci_list_assignable. This is
used to implement the xl list-assignable-pci-devices command and part of
the implementation is used to make sure that PCI devices are not multiply
assigned to one or more domU's before doing the passthrough assignment.
The function libxl_device_pci_list changes to libxl_device_pci_list_assigned
due to a parameter change for consistency with pci_list_assignable.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Wed Jul 28 20:29:08 2010 +0100
+++ b/tools/libxl/libxl.h Wed Jul 28 20:31:59 2010 +0100
@@ -521,7 +521,8 @@ int libxl_device_vfb_hard_shutdown(struc
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);
-libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num);
+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);
diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Wed Jul 28 20:29:08 2010 +0100
+++ b/tools/libxl/libxl_internal.h Wed Jul 28 20:31:59 2010 +0100
@@ -91,6 +91,9 @@ typedef struct {
#define XC_PCI_BDF "0x%x, 0x%x, 0x%x, 0x%x"
#define AUTO_PHP_SLOT 0x100
#define SYSFS_PCI_DEV "/sys/bus/pci/devices"
+#define SYSFS_PCIBACK_DRIVER "/sys/bus/pci/drivers/pciback"
+#define SYSFS_PCISTUB_DRIVER "/sys/bus/pci/drivers/pcistub"
+
#define PROC_PCI_NUM_RESOURCES 7
#define PCI_BAR_IO 0x01
diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Wed Jul 28 20:29:08 2010 +0100
+++ b/tools/libxl/libxl_pci.c Wed Jul 28 20:31:59 2010 +0100
@@ -28,6 +28,7 @@
#include <unistd.h> /* for write, unlink and close */
#include <stdint.h>
#include <inttypes.h>
+#include <dirent.h>
#include <assert.h>
#include "libxl.h"
@@ -258,22 +259,77 @@ retry_transaction2:
return 0;
}
-int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
+int get_all_assigned_devices(struct libxl_ctx *ctx, libxl_device_pci **list, int *num)
+{
+ libxl_device_pci *pcidevs = NULL;
+ char **domlist;
+ unsigned int nd = 0, i;
+
+ *list = NULL;
+ *num = 0;
+
+ domlist = libxl_xs_directory(ctx, XBT_NULL, "/local/domain", &nd);
+ for(i = 0; i < nd; i++) {
+ char *path, *num_devs;
+
+ path = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/num_devs", domlist[i]);
+ num_devs = libxl_xs_read(ctx, XBT_NULL, path);
+ if ( num_devs ) {
+ int ndev = atoi(num_devs), j;
+ char *devpath, *bdf;
+
+ pcidevs = calloc(sizeof(*pcidevs), ndev);
+ for(j = (pcidevs) ? 0 : ndev; j < ndev; j++) {
+ devpath = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/dev-%u",
+ domlist[i], j);
+ bdf = libxl_xs_read(ctx, XBT_NULL, devpath);
+ if ( bdf ) {
+ unsigned dom, bus, dev, func;
+ if ( sscanf(bdf, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
+ continue;
+
+ libxl_device_pci_init(pcidevs + *num, dom, bus, dev, func, 0);
+ (*num)++;
+ }
+ }
+ }
+ }
+
+ if ( 0 == *num ) {
+ free(pcidevs);
+ pcidevs = NULL;
+ }else{
+ *list = pcidevs;
+ }
+
+ return 0;
+}
+
+static int is_assigned(libxl_device_pci *assigned, int num_assigned,
+ int dom, int bus, int dev, int func)
+{
+ int i;
+
+ for(i = 0; i < num_assigned; i++) {
+ if ( assigned[i].domain != dom )
+ continue;
+ if ( assigned[i].bus != bus )
+ continue;
+ if ( assigned[i].dev != dev )
+ continue;
+ if ( assigned[i].func != func )
+ continue;
+ return 1;
+ }
+
+ return 0;
+}
+
+static int do_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
{
char *path;
char *state, *vdevfn;
int rc, hvm;
- int stubdomid = 0;
-
- /* TODO: check if the device can be assigned */
-
- libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
-
- stubdomid = libxl_get_stubdom_id(ctx, domid);
- if (stubdomid != 0) {
- libxl_device_pci pcidev_s = *pcidev;
- libxl_device_pci_add(ctx, stubdomid, &pcidev_s);
- }
hvm = is_hvm(ctx, domid);
if (hvm) {
@@ -370,6 +426,38 @@ out:
return 0;
}
+int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
+{
+ libxl_device_pci *assigned;
+ int num_assigned, rc;
+ int stubdomid = 0;
+
+ rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
+ if ( rc ) {
+ XL_LOG(ctx, XL_LOG_ERROR, "cannot determine if device is assigned, refusing to continue");
+ return ERROR_FAIL;
+ }
+ if ( is_assigned(assigned, num_assigned, pcidev->domain,
+ pcidev->bus, pcidev->dev, pcidev->func) ) {
+ XL_LOG(ctx, XL_LOG_ERROR, "PCI device already attached to a domain");
+ free(assigned);
+ return ERROR_FAIL;
+ }
+ free(assigned);
+
+ libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+
+ stubdomid = libxl_get_stubdom_id(ctx, domid);
+ if (stubdomid != 0) {
+ libxl_device_pci pcidev_s = *pcidev;
+ rc = do_pci_add(ctx, stubdomid, &pcidev_s);
+ if ( rc )
+ return rc;
+ }
+
+ return do_pci_add(ctx, domid, pcidev);
+}
+
int libxl_device_pci_remove(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
{
char *path;
@@ -466,7 +554,66 @@ out:
return 0;
}
-libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num)
+static libxl_device_pci *scan_sys_pcidir(libxl_device_pci *pcidevs, libxl_device_pci *assigned,
+ int num_assigned, const char *path, int *num)
+{
+ libxl_device_pci *new;
+ struct dirent *de;
+ DIR *dir;
+
+ dir = opendir(path);
+ if ( NULL == dir )
+ return pcidevs;
+
+ while( (de = readdir(dir)) ) {
+ unsigned dom, bus, dev, func;
+ if ( sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
+ continue;
+
+ if ( is_assigned(assigned, num_assigned, dom, bus, dev, func) )
+ continue;
+
+ new = realloc(pcidevs, ((*num) + 1) * sizeof(*new));
+ if ( NULL == new )
+ continue;
+
+ pcidevs = new;
+ new = pcidevs + *num;
+
+ memset(new, 0, sizeof(*new));
+ libxl_device_pci_init(new, dom, bus, dev, func, 0);
+ (*num)++;
+ }
+
+ closedir(dir);
+ return pcidevs;
+}
+
+int libxl_device_pci_list_assignable(struct libxl_ctx *ctx, libxl_device_pci **list, int *num)
+{
+ libxl_device_pci *pcidevs = NULL;
+ libxl_device_pci *assigned;
+ int num_assigned, rc;
+
+ *num = 0;
+ *list = NULL;
+
+ rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
+ if ( rc )
+ return rc;
+
+ pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned,
+ SYSFS_PCIBACK_DRIVER, num);
+ pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned,
+ SYSFS_PCISTUB_DRIVER, num);
+
+ free(assigned);
+ if ( *num )
+ *list = pcidevs;
+ return 0;
+}
+
+int libxl_device_pci_list_assigned(struct libxl_ctx *ctx, libxl_device_pci **list, uint32_t domid, int *num)
{
char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts;
int n, i;
@@ -477,7 +624,8 @@ libxl_device_pci *libxl_device_pci_list(
num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path));
if (!num_devs) {
*num = 0;
- return NULL;
+ *list = NULL;
+ return ERROR_FAIL;
}
n = atoi(num_devs);
pcidevs = calloc(n, sizeof(libxl_device_pci));
@@ -507,15 +655,19 @@ libxl_device_pci *libxl_device_pci_list(
} while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL);
}
}
- return pcidevs;
+ if ( *num )
+ *list = pcidevs;
+ return 0;
}
int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid)
{
libxl_device_pci *pcidevs;
- int num, i;
+ int num, i, rc;
- pcidevs = libxl_device_pci_list(ctx, domid, &num);
+ rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num);
+ if ( rc )
+ return rc;
for (i = 0; i < num; i++) {
if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0)
return ERROR_FAIL;
diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/xl.h
--- a/tools/libxl/xl.h Wed Jul 28 20:29:08 2010 +0100
+++ b/tools/libxl/xl.h Wed Jul 28 20:31:59 2010 +0100
@@ -32,6 +32,7 @@ int main_cd_insert(int argc, char **argv
int main_console(int argc, char **argv);
int main_vncviewer(int argc, char **argv);
int main_pcilist(int argc, char **argv);
+int main_pcilist_assignable(int argc, char **argv);
int main_pcidetach(int argc, char **argv);
int main_pciattach(int argc, char **argv);
int main_restore(int argc, char **argv);
diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed Jul 28 20:29:08 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed Jul 28 20:31:59 2010 +0100
@@ -1920,6 +1920,39 @@ int main_vncviewer(int argc, char **argv
exit(0);
}
+void pcilist_assignable(void)
+{
+ libxl_device_pci *pcidevs;
+ int num, i;
+
+ if ( libxl_device_pci_list_assignable(&ctx, &pcidevs, &num) )
+ return;
+ for (i = 0; i < num; i++) {
+ printf("%04x:%02x:%02x:%01x\n",
+ pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
+ }
+ free(pcidevs);
+}
+
+int main_pcilist_assignable(int argc, char **argv)
+{
+ int opt;
+
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("pci-list-assignable-devices");
+ exit(0);
+ default:
+ fprintf(stderr, "option not supported\n");
+ break;
+ }
+ }
+
+ pcilist_assignable();
+ exit(0);
+}
+
void pcilist(char *dom)
{
libxl_device_pci *pcidevs;
@@ -1927,8 +1960,7 @@ void pcilist(char *dom)
find_domain(dom);
- pcidevs = libxl_device_pci_list(&ctx, domid, &num);
- if (!num)
+ if (libxl_device_pci_list_assigned(&ctx, &pcidevs, domid, &num))
return;
printf("VFn domain bus slot func\n");
for (i = 0; i < num; i++) {
diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Wed Jul 28 20:29:08 2010 +0100
+++ b/tools/libxl/xl_cmdtable.c Wed Jul 28 20:31:59 2010 +0100
@@ -68,6 +68,11 @@ struct cmd_spec cmd_table[] = {
"List pass-through pci devices for a domain",
"<Domain>",
},
+ { "pci-list-assignable-devices",
+ &main_pcilist_assignable,
+ "List all the assignable pci devices",
+ "",
+ },
{ "pause",
&main_pause,
"Pause execution of a domain",
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
2010-07-28 19:40 ` [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU Gianni Tedesco
@ 2010-07-29 14:36 ` Stefano Stabellini
2010-07-29 15:32 ` Gianni Tedesco
2010-07-29 15:49 ` Gianni Tedesco
2010-07-30 15:21 ` Ian Campbell
2 siblings, 1 reply; 19+ messages in thread
From: Stefano Stabellini @ 2010-07-29 14:36 UTC (permalink / raw)
To: Gianni Tedesco (3P)
Cc: Ian Jackson, xen-devel@lists.xensource.com, Stabellini
On Wed, 28 Jul 2010, Gianni Tedesco (3P) wrote:
> tools/libxl/libxl.h | 3 +-
> tools/libxl/libxl_internal.h | 3 +
> tools/libxl/libxl_pci.c | 186 +++++++++++++++++++++++++++++++++++++++---
> tools/libxl/xl.h | 1 +
> tools/libxl/xl_cmdimpl.c | 36 +++++++-
> tools/libxl/xl_cmdtable.c | 5 +
> 6 files changed, 214 insertions(+), 20 deletions(-)
>
>
> Implement a new libxl function libxl_device_pci_list_assignable. This is
> used to implement the xl list-assignable-pci-devices command and part of
> the implementation is used to make sure that PCI devices are not multiply
> assigned to one or more domU's before doing the passthrough assignment.
>
> The function libxl_device_pci_list changes to libxl_device_pci_list_assigned
> due to a parameter change for consistency with pci_list_assignable.
>
> Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
>
> diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h Wed Jul 28 20:29:08 2010 +0100
> +++ b/tools/libxl/libxl.h Wed Jul 28 20:31:59 2010 +0100
> @@ -521,7 +521,8 @@ int libxl_device_vfb_hard_shutdown(struc
> 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);
> -libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num);
> +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);
> diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h Wed Jul 28 20:29:08 2010 +0100
> +++ b/tools/libxl/libxl_internal.h Wed Jul 28 20:31:59 2010 +0100
> @@ -91,6 +91,9 @@ typedef struct {
> #define XC_PCI_BDF "0x%x, 0x%x, 0x%x, 0x%x"
> #define AUTO_PHP_SLOT 0x100
> #define SYSFS_PCI_DEV "/sys/bus/pci/devices"
> +#define SYSFS_PCIBACK_DRIVER "/sys/bus/pci/drivers/pciback"
> +#define SYSFS_PCISTUB_DRIVER "/sys/bus/pci/drivers/pcistub"
> +
> #define PROC_PCI_NUM_RESOURCES 7
> #define PCI_BAR_IO 0x01
>
> diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c Wed Jul 28 20:29:08 2010 +0100
> +++ b/tools/libxl/libxl_pci.c Wed Jul 28 20:31:59 2010 +0100
> @@ -28,6 +28,7 @@
> #include <unistd.h> /* for write, unlink and close */
> #include <stdint.h>
> #include <inttypes.h>
> +#include <dirent.h>
> #include <assert.h>
>
> #include "libxl.h"
> @@ -258,22 +259,77 @@ retry_transaction2:
> return 0;
> }
>
> -int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
> +int get_all_assigned_devices(struct libxl_ctx *ctx, libxl_device_pci **list, int *num)
shouldn't this be static?
> +{
> + libxl_device_pci *pcidevs = NULL;
> + char **domlist;
> + unsigned int nd = 0, i;
> +
> + *list = NULL;
> + *num = 0;
> +
> + domlist = libxl_xs_directory(ctx, XBT_NULL, "/local/domain", &nd);
> + for(i = 0; i < nd; i++) {
> + char *path, *num_devs;
> +
> + path = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/num_devs", domlist[i]);
> + num_devs = libxl_xs_read(ctx, XBT_NULL, path);
> + if ( num_devs ) {
> + int ndev = atoi(num_devs), j;
> + char *devpath, *bdf;
> +
> + pcidevs = calloc(sizeof(*pcidevs), ndev);
> + for(j = (pcidevs) ? 0 : ndev; j < ndev; j++) {
> + devpath = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/dev-%u",
> + domlist[i], j);
> + bdf = libxl_xs_read(ctx, XBT_NULL, devpath);
> + if ( bdf ) {
> + unsigned dom, bus, dev, func;
> + if ( sscanf(bdf, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
> + continue;
> +
> + libxl_device_pci_init(pcidevs + *num, dom, bus, dev, func, 0);
> + (*num)++;
> + }
> + }
> + }
> + }
> +
> + if ( 0 == *num ) {
> + free(pcidevs);
> + pcidevs = NULL;
> + }else{
> + *list = pcidevs;
> + }
> +
> + return 0;
> +}
> +
> +static int is_assigned(libxl_device_pci *assigned, int num_assigned,
> + int dom, int bus, int dev, int func)
> +{
> + int i;
> +
> + for(i = 0; i < num_assigned; i++) {
> + if ( assigned[i].domain != dom )
> + continue;
> + if ( assigned[i].bus != bus )
> + continue;
> + if ( assigned[i].dev != dev )
> + continue;
> + if ( assigned[i].func != func )
> + continue;
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static int do_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
> {
> char *path;
> char *state, *vdevfn;
> int rc, hvm;
> - int stubdomid = 0;
> -
> - /* TODO: check if the device can be assigned */
> -
> - libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> -
> - stubdomid = libxl_get_stubdom_id(ctx, domid);
> - if (stubdomid != 0) {
> - libxl_device_pci pcidev_s = *pcidev;
> - libxl_device_pci_add(ctx, stubdomid, &pcidev_s);
> - }
>
> hvm = is_hvm(ctx, domid);
> if (hvm) {
> @@ -370,6 +426,38 @@ out:
> return 0;
> }
>
> +int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
> +{
> + libxl_device_pci *assigned;
> + int num_assigned, rc;
> + int stubdomid = 0;
> +
> + rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
> + if ( rc ) {
> + XL_LOG(ctx, XL_LOG_ERROR, "cannot determine if device is assigned, refusing to continue");
> + return ERROR_FAIL;
> + }
> + if ( is_assigned(assigned, num_assigned, pcidev->domain,
> + pcidev->bus, pcidev->dev, pcidev->func) ) {
> + XL_LOG(ctx, XL_LOG_ERROR, "PCI device already attached to a domain");
> + free(assigned);
> + return ERROR_FAIL;
> + }
> + free(assigned);
> +
> + libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> +
> + stubdomid = libxl_get_stubdom_id(ctx, domid);
> + if (stubdomid != 0) {
> + libxl_device_pci pcidev_s = *pcidev;
> + rc = do_pci_add(ctx, stubdomid, &pcidev_s);
> + if ( rc )
> + return rc;
> + }
> +
> + return do_pci_add(ctx, domid, pcidev);
> +}
> +
> int libxl_device_pci_remove(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
> {
> char *path;
> @@ -466,7 +554,66 @@ out:
> return 0;
> }
>
> -libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num)
> +static libxl_device_pci *scan_sys_pcidir(libxl_device_pci *pcidevs, libxl_device_pci *assigned,
> + int num_assigned, const char *path, int *num)
> +{
> + libxl_device_pci *new;
> + struct dirent *de;
> + DIR *dir;
> +
> + dir = opendir(path);
> + if ( NULL == dir )
> + return pcidevs;
> +
> + while( (de = readdir(dir)) ) {
> + unsigned dom, bus, dev, func;
> + if ( sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
> + continue;
> +
> + if ( is_assigned(assigned, num_assigned, dom, bus, dev, func) )
> + continue;
> +
> + new = realloc(pcidevs, ((*num) + 1) * sizeof(*new));
> + if ( NULL == new )
> + continue;
> +
> + pcidevs = new;
> + new = pcidevs + *num;
> +
> + memset(new, 0, sizeof(*new));
> + libxl_device_pci_init(new, dom, bus, dev, func, 0);
> + (*num)++;
> + }
> +
> + closedir(dir);
> + return pcidevs;
> +}
> +
> +int libxl_device_pci_list_assignable(struct libxl_ctx *ctx, libxl_device_pci **list, int *num)
> +{
> + libxl_device_pci *pcidevs = NULL;
> + libxl_device_pci *assigned;
> + int num_assigned, rc;
> +
> + *num = 0;
> + *list = NULL;
> +
> + rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
> + if ( rc )
> + return rc;
> +
> + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned,
> + SYSFS_PCIBACK_DRIVER, num);
> + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned,
> + SYSFS_PCISTUB_DRIVER, num);
> +
I still think we should remove pcistub scanning from here: when running
on xen people should use pciback anyway.
> + free(assigned);
> + if ( *num )
> + *list = pcidevs;
> + return 0;
> +}
> +
> +int libxl_device_pci_list_assigned(struct libxl_ctx *ctx, libxl_device_pci **list, uint32_t domid, int *num)
> {
> char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts;
> int n, i;
> @@ -477,7 +624,8 @@ libxl_device_pci *libxl_device_pci_list(
> num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path));
> if (!num_devs) {
> *num = 0;
> - return NULL;
> + *list = NULL;
> + return ERROR_FAIL;
> }
> n = atoi(num_devs);
> pcidevs = calloc(n, sizeof(libxl_device_pci));
> @@ -507,15 +655,19 @@ libxl_device_pci *libxl_device_pci_list(
> } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL);
> }
> }
> - return pcidevs;
> + if ( *num )
> + *list = pcidevs;
> + return 0;
> }
>
> int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid)
> {
> libxl_device_pci *pcidevs;
> - int num, i;
> + int num, i, rc;
>
> - pcidevs = libxl_device_pci_list(ctx, domid, &num);
> + rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num);
> + if ( rc )
> + return rc;
> for (i = 0; i < num; i++) {
> if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0)
> return ERROR_FAIL;
> diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/xl.h
> --- a/tools/libxl/xl.h Wed Jul 28 20:29:08 2010 +0100
> +++ b/tools/libxl/xl.h Wed Jul 28 20:31:59 2010 +0100
> @@ -32,6 +32,7 @@ int main_cd_insert(int argc, char **argv
> int main_console(int argc, char **argv);
> int main_vncviewer(int argc, char **argv);
> int main_pcilist(int argc, char **argv);
> +int main_pcilist_assignable(int argc, char **argv);
> int main_pcidetach(int argc, char **argv);
> int main_pciattach(int argc, char **argv);
> int main_restore(int argc, char **argv);
> diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Wed Jul 28 20:29:08 2010 +0100
> +++ b/tools/libxl/xl_cmdimpl.c Wed Jul 28 20:31:59 2010 +0100
> @@ -1920,6 +1920,39 @@ int main_vncviewer(int argc, char **argv
> exit(0);
> }
>
> +void pcilist_assignable(void)
> +{
> + libxl_device_pci *pcidevs;
> + int num, i;
> +
> + if ( libxl_device_pci_list_assignable(&ctx, &pcidevs, &num) )
> + return;
> + for (i = 0; i < num; i++) {
> + printf("%04x:%02x:%02x:%01x\n",
> + pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
> + }
> + free(pcidevs);
> +}
> +
> +int main_pcilist_assignable(int argc, char **argv)
> +{
> + int opt;
> +
> + while ((opt = getopt(argc, argv, "h")) != -1) {
> + switch (opt) {
> + case 'h':
> + help("pci-list-assignable-devices");
> + exit(0);
> + default:
> + fprintf(stderr, "option not supported\n");
> + break;
> + }
> + }
> +
> + pcilist_assignable();
> + exit(0);
> +}
> +
> void pcilist(char *dom)
> {
> libxl_device_pci *pcidevs;
> @@ -1927,8 +1960,7 @@ void pcilist(char *dom)
>
> find_domain(dom);
>
> - pcidevs = libxl_device_pci_list(&ctx, domid, &num);
> - if (!num)
> + if (libxl_device_pci_list_assigned(&ctx, &pcidevs, domid, &num))
> return;
> printf("VFn domain bus slot func\n");
> for (i = 0; i < num; i++) {
> diff -r e2520c8b96b7 -r 099e4eb3803a tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c Wed Jul 28 20:29:08 2010 +0100
> +++ b/tools/libxl/xl_cmdtable.c Wed Jul 28 20:31:59 2010 +0100
> @@ -68,6 +68,11 @@ struct cmd_spec cmd_table[] = {
> "List pass-through pci devices for a domain",
> "<Domain>",
> },
> + { "pci-list-assignable-devices",
> + &main_pcilist_assignable,
> + "List all the assignable pci devices",
> + "",
> + },
> { "pause",
> &main_pause,
> "Pause execution of a domain",
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
2010-07-29 14:36 ` Stefano Stabellini
@ 2010-07-29 15:32 ` Gianni Tedesco
0 siblings, 0 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 15:32 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Ian, xen-devel@lists.xensource.com, Jackson
On Thu, 2010-07-29 at 15:36 +0100, Stefano Stabellini wrote:
> > -int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
> > +int get_all_assigned_devices(struct libxl_ctx *ctx, libxl_device_pci **list, int *num)
>
> shouldn't this be static?
Indeed yes:
8<----------------
Implement a new libxl function libxl_device_pci_list_assignable. This is
used to implement the xl list-assignable-pci-devices command and part of
the implementation is used to make sure that PCI devices are not
multiply assigned to one or more domU's before doing the passthrough
assignment.
The function libxl_device_pci_list changes to
libxl_device_pci_list_assigned due to a parameter change for consistency
with pci_list_assignable.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r 5345595da04b tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Wed Jul 28 20:10:49 2010 +0100
+++ b/tools/libxl/libxl.h Wed Jul 28 20:15:47 2010 +0100
@@ -521,7 +521,8 @@ int libxl_device_vfb_hard_shutdown(struc
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);
-libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num);
+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);
diff -r 5345595da04b tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Wed Jul 28 20:10:49 2010 +0100
+++ b/tools/libxl/libxl_internal.h Wed Jul 28 20:15:47 2010 +0100
@@ -91,6 +91,9 @@ typedef struct {
#define XC_PCI_BDF "0x%x, 0x%x, 0x%x, 0x%x"
#define AUTO_PHP_SLOT 0x100
#define SYSFS_PCI_DEV "/sys/bus/pci/devices"
+#define SYSFS_PCIBACK_DRIVER "/sys/bus/pci/drivers/pciback"
+#define SYSFS_PCISTUB_DRIVER "/sys/bus/pci/drivers/pcistub"
+
#define PROC_PCI_NUM_RESOURCES 7
#define PCI_BAR_IO 0x01
diff -r 5345595da04b tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Wed Jul 28 20:10:49 2010 +0100
+++ b/tools/libxl/libxl_pci.c Wed Jul 28 20:15:47 2010 +0100
@@ -28,6 +28,7 @@
#include <unistd.h> /* for write, unlink and close */
#include <stdint.h>
#include <inttypes.h>
+#include <dirent.h>
#include <assert.h>
#include "libxl.h"
@@ -258,22 +259,77 @@ retry_transaction2:
return 0;
}
-int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
+static int get_all_assigned_devices(struct libxl_ctx *ctx, libxl_device_pci **list, int *num)
+{
+ libxl_device_pci *pcidevs = NULL;
+ char **domlist;
+ unsigned int nd = 0, i;
+
+ *list = NULL;
+ *num = 0;
+
+ domlist = libxl_xs_directory(ctx, XBT_NULL, "/local/domain", &nd);
+ for(i = 0; i < nd; i++) {
+ char *path, *num_devs;
+
+ path = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/num_devs", domlist[i]);
+ num_devs = libxl_xs_read(ctx, XBT_NULL, path);
+ if ( num_devs ) {
+ int ndev = atoi(num_devs), j;
+ char *devpath, *bdf;
+
+ pcidevs = calloc(sizeof(*pcidevs), ndev);
+ for(j = (pcidevs) ? 0 : ndev; j < ndev; j++) {
+ devpath = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/dev-%u",
+ domlist[i], j);
+ bdf = libxl_xs_read(ctx, XBT_NULL, devpath);
+ if ( bdf ) {
+ unsigned dom, bus, dev, func;
+ if ( sscanf(bdf, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
+ continue;
+
+ libxl_device_pci_init(pcidevs + *num, dom, bus, dev, func, 0);
+ (*num)++;
+ }
+ }
+ }
+ }
+
+ if ( 0 == *num ) {
+ free(pcidevs);
+ pcidevs = NULL;
+ }else{
+ *list = pcidevs;
+ }
+
+ return 0;
+}
+
+static int is_assigned(libxl_device_pci *assigned, int num_assigned,
+ int dom, int bus, int dev, int func)
+{
+ int i;
+
+ for(i = 0; i < num_assigned; i++) {
+ if ( assigned[i].domain != dom )
+ continue;
+ if ( assigned[i].bus != bus )
+ continue;
+ if ( assigned[i].dev != dev )
+ continue;
+ if ( assigned[i].func != func )
+ continue;
+ return 1;
+ }
+
+ return 0;
+}
+
+static int do_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
{
char *path;
char *state, *vdevfn;
int rc, hvm;
- int stubdomid = 0;
-
- /* TODO: check if the device can be assigned */
-
- libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
-
- stubdomid = libxl_get_stubdom_id(ctx, domid);
- if (stubdomid != 0) {
- libxl_device_pci pcidev_s = *pcidev;
- libxl_device_pci_add(ctx, stubdomid, &pcidev_s);
- }
hvm = is_hvm(ctx, domid);
if (hvm) {
@@ -370,6 +426,38 @@ out:
return 0;
}
+int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
+{
+ libxl_device_pci *assigned;
+ int num_assigned, rc;
+ int stubdomid = 0;
+
+ rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
+ if ( rc ) {
+ XL_LOG(ctx, XL_LOG_ERROR, "cannot determine if device is assigned, refusing to continue");
+ return ERROR_FAIL;
+ }
+ if ( is_assigned(assigned, num_assigned, pcidev->domain,
+ pcidev->bus, pcidev->dev, pcidev->func) ) {
+ XL_LOG(ctx, XL_LOG_ERROR, "PCI device already attached to a domain");
+ free(assigned);
+ return ERROR_FAIL;
+ }
+ free(assigned);
+
+ libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+
+ stubdomid = libxl_get_stubdom_id(ctx, domid);
+ if (stubdomid != 0) {
+ libxl_device_pci pcidev_s = *pcidev;
+ rc = do_pci_add(ctx, stubdomid, &pcidev_s);
+ if ( rc )
+ return rc;
+ }
+
+ return do_pci_add(ctx, domid, pcidev);
+}
+
int libxl_device_pci_remove(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
{
char *path;
@@ -466,7 +554,66 @@ out:
return 0;
}
-libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num)
+static libxl_device_pci *scan_sys_pcidir(libxl_device_pci *pcidevs, libxl_device_pci *assigned,
+ int num_assigned, const char *path, int *num)
+{
+ libxl_device_pci *new;
+ struct dirent *de;
+ DIR *dir;
+
+ dir = opendir(path);
+ if ( NULL == dir )
+ return pcidevs;
+
+ while( (de = readdir(dir)) ) {
+ unsigned dom, bus, dev, func;
+ if ( sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
+ continue;
+
+ if ( is_assigned(assigned, num_assigned, dom, bus, dev, func) )
+ continue;
+
+ new = realloc(pcidevs, ((*num) + 1) * sizeof(*new));
+ if ( NULL == new )
+ continue;
+
+ pcidevs = new;
+ new = pcidevs + *num;
+
+ memset(new, 0, sizeof(*new));
+ libxl_device_pci_init(new, dom, bus, dev, func, 0);
+ (*num)++;
+ }
+
+ closedir(dir);
+ return pcidevs;
+}
+
+int libxl_device_pci_list_assignable(struct libxl_ctx *ctx, libxl_device_pci **list, int *num)
+{
+ libxl_device_pci *pcidevs = NULL;
+ libxl_device_pci *assigned;
+ int num_assigned, rc;
+
+ *num = 0;
+ *list = NULL;
+
+ rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
+ if ( rc )
+ return rc;
+
+ pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned,
+ SYSFS_PCIBACK_DRIVER, num);
+ pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned,
+ SYSFS_PCISTUB_DRIVER, num);
+
+ free(assigned);
+ if ( *num )
+ *list = pcidevs;
+ return 0;
+}
+
+int libxl_device_pci_list_assigned(struct libxl_ctx *ctx, libxl_device_pci **list, uint32_t domid, int *num)
{
char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts;
int n, i;
@@ -477,7 +624,8 @@ libxl_device_pci *libxl_device_pci_list(
num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path));
if (!num_devs) {
*num = 0;
- return NULL;
+ *list = NULL;
+ return ERROR_FAIL;
}
n = atoi(num_devs);
pcidevs = calloc(n, sizeof(libxl_device_pci));
@@ -507,15 +655,19 @@ libxl_device_pci *libxl_device_pci_list(
} while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL);
}
}
- return pcidevs;
+ if ( *num )
+ *list = pcidevs;
+ return 0;
}
int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid)
{
libxl_device_pci *pcidevs;
- int num, i;
+ int num, i, rc;
- pcidevs = libxl_device_pci_list(ctx, domid, &num);
+ rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num);
+ if ( rc )
+ return rc;
for (i = 0; i < num; i++) {
if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0)
return ERROR_FAIL;
diff -r 5345595da04b tools/libxl/xl.h
--- a/tools/libxl/xl.h Wed Jul 28 20:10:49 2010 +0100
+++ b/tools/libxl/xl.h Wed Jul 28 20:15:47 2010 +0100
@@ -32,6 +32,7 @@ int main_cd_insert(int argc, char **argv
int main_console(int argc, char **argv);
int main_vncviewer(int argc, char **argv);
int main_pcilist(int argc, char **argv);
+int main_pcilist_assignable(int argc, char **argv);
int main_pcidetach(int argc, char **argv);
int main_pciattach(int argc, char **argv);
int main_restore(int argc, char **argv);
diff -r 5345595da04b tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed Jul 28 20:10:49 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed Jul 28 20:15:47 2010 +0100
@@ -1920,6 +1920,39 @@ int main_vncviewer(int argc, char **argv
exit(0);
}
+void pcilist_assignable(void)
+{
+ libxl_device_pci *pcidevs;
+ int num, i;
+
+ if ( libxl_device_pci_list_assignable(&ctx, &pcidevs, &num) )
+ return;
+ for (i = 0; i < num; i++) {
+ printf("%04x:%02x:%02x:%01x\n",
+ pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
+ }
+ free(pcidevs);
+}
+
+int main_pcilist_assignable(int argc, char **argv)
+{
+ int opt;
+
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("pci-list-assignable-devices");
+ exit(0);
+ default:
+ fprintf(stderr, "option not supported\n");
+ break;
+ }
+ }
+
+ pcilist_assignable();
+ exit(0);
+}
+
void pcilist(char *dom)
{
libxl_device_pci *pcidevs;
@@ -1927,8 +1960,7 @@ void pcilist(char *dom)
find_domain(dom);
- pcidevs = libxl_device_pci_list(&ctx, domid, &num);
- if (!num)
+ if (libxl_device_pci_list_assigned(&ctx, &pcidevs, domid, &num))
return;
printf("VFn domain bus slot func\n");
for (i = 0; i < num; i++) {
diff -r 5345595da04b tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Wed Jul 28 20:10:49 2010 +0100
+++ b/tools/libxl/xl_cmdtable.c Wed Jul 28 20:15:47 2010 +0100
@@ -68,6 +68,11 @@ struct cmd_spec cmd_table[] = {
"List pass-through pci devices for a domain",
"<Domain>",
},
+ { "pci-list-assignable-devices",
+ &main_pcilist_assignable,
+ "List all the assignable pci devices",
+ "",
+ },
{ "pause",
&main_pause,
"Pause execution of a domain",
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
2010-07-28 19:40 ` [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU Gianni Tedesco
2010-07-29 14:36 ` Stefano Stabellini
@ 2010-07-29 15:49 ` Gianni Tedesco
2010-07-29 16:00 ` Stefano Stabellini
2010-07-30 15:21 ` Ian Campbell
2 siblings, 1 reply; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 15:49 UTC (permalink / raw)
To: xen-devel@lists.xensource.com; +Cc: Ian Jackson, Stefano Stabellini
Implement both of Stefanos suggestions this time:
On Wed, 2010-07-28 at 20:40 +0100, Gianni Tedesco wrote:
> tools/libxl/libxl.h | 3 +-
> tools/libxl/libxl_internal.h | 3 +
> tools/libxl/libxl_pci.c | 186 +++++++++++++++++++++++++++++++++++++++---
> tools/libxl/xl.h | 1 +
> tools/libxl/xl_cmdimpl.c | 36 +++++++-
> tools/libxl/xl_cmdtable.c | 5 +
> 6 files changed, 214 insertions(+), 20 deletions(-)
>
>
> Implement a new libxl function libxl_device_pci_list_assignable. This is
> used to implement the xl list-assignable-pci-devices command and part of
> the implementation is used to make sure that PCI devices are not multiply
> assigned to one or more domU's before doing the passthrough assignment.
>
> The function libxl_device_pci_list changes to libxl_device_pci_list_assigned
> due to a parameter change for consistency with pci_list_assignable.
>
> Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r 43150fdd0106 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Thu Jul 29 16:47:16 2010 +0100
+++ b/tools/libxl/libxl.h Thu Jul 29 16:48:48 2010 +0100
@@ -521,7 +521,8 @@ int libxl_device_vfb_hard_shutdown(libxl
int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
int libxl_device_pci_shutdown(libxl_ctx *ctx, uint32_t domid);
-libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num);
+int libxl_device_pci_list_assigned(libxl_ctx *ctx, libxl_device_pci **list, uint32_t domid, int *num);
+int libxl_device_pci_list_assignable(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);
diff -r 43150fdd0106 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Thu Jul 29 16:47:16 2010 +0100
+++ b/tools/libxl/libxl_internal.h Thu Jul 29 16:48:48 2010 +0100
@@ -91,6 +91,8 @@ typedef struct {
#define XC_PCI_BDF "0x%x, 0x%x, 0x%x, 0x%x"
#define AUTO_PHP_SLOT 0x100
#define SYSFS_PCI_DEV "/sys/bus/pci/devices"
+#define SYSFS_PCIBACK_DRIVER "/sys/bus/pci/drivers/pciback"
+
#define PROC_PCI_NUM_RESOURCES 7
#define PCI_BAR_IO 0x01
diff -r 43150fdd0106 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Thu Jul 29 16:47:16 2010 +0100
+++ b/tools/libxl/libxl_pci.c Thu Jul 29 16:48:48 2010 +0100
@@ -28,6 +28,7 @@
#include <unistd.h> /* for write, unlink and close */
#include <stdint.h>
#include <inttypes.h>
+#include <dirent.h>
#include <assert.h>
#include "libxl.h"
@@ -258,22 +259,77 @@ retry_transaction2:
return 0;
}
-int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
+static int get_all_assigned_devices(libxl_ctx *ctx, libxl_device_pci **list, int *num)
+{
+ libxl_device_pci *pcidevs = NULL;
+ char **domlist;
+ unsigned int nd = 0, i;
+
+ *list = NULL;
+ *num = 0;
+
+ domlist = libxl_xs_directory(ctx, XBT_NULL, "/local/domain", &nd);
+ for(i = 0; i < nd; i++) {
+ char *path, *num_devs;
+
+ path = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/num_devs", domlist[i]);
+ num_devs = libxl_xs_read(ctx, XBT_NULL, path);
+ if ( num_devs ) {
+ int ndev = atoi(num_devs), j;
+ char *devpath, *bdf;
+
+ pcidevs = calloc(sizeof(*pcidevs), ndev);
+ for(j = (pcidevs) ? 0 : ndev; j < ndev; j++) {
+ devpath = libxl_sprintf(ctx, "/local/domain/0/backend/pci/%s/0/dev-%u",
+ domlist[i], j);
+ bdf = libxl_xs_read(ctx, XBT_NULL, devpath);
+ if ( bdf ) {
+ unsigned dom, bus, dev, func;
+ if ( sscanf(bdf, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
+ continue;
+
+ libxl_device_pci_init(pcidevs + *num, dom, bus, dev, func, 0);
+ (*num)++;
+ }
+ }
+ }
+ }
+
+ if ( 0 == *num ) {
+ free(pcidevs);
+ pcidevs = NULL;
+ }else{
+ *list = pcidevs;
+ }
+
+ return 0;
+}
+
+static int is_assigned(libxl_device_pci *assigned, int num_assigned,
+ int dom, int bus, int dev, int func)
+{
+ int i;
+
+ for(i = 0; i < num_assigned; i++) {
+ if ( assigned[i].domain != dom )
+ continue;
+ if ( assigned[i].bus != bus )
+ continue;
+ if ( assigned[i].dev != dev )
+ continue;
+ if ( assigned[i].func != func )
+ continue;
+ return 1;
+ }
+
+ return 0;
+}
+
+static int do_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
{
char *path;
char *state, *vdevfn;
int rc, hvm;
- int stubdomid = 0;
-
- /* TODO: check if the device can be assigned */
-
- libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
-
- stubdomid = libxl_get_stubdom_id(ctx, domid);
- if (stubdomid != 0) {
- libxl_device_pci pcidev_s = *pcidev;
- libxl_device_pci_add(ctx, stubdomid, &pcidev_s);
- }
hvm = is_hvm(ctx, domid);
if (hvm) {
@@ -370,6 +426,38 @@ out:
return 0;
}
+int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
+{
+ libxl_device_pci *assigned;
+ int num_assigned, rc;
+ int stubdomid = 0;
+
+ rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
+ if ( rc ) {
+ XL_LOG(ctx, XL_LOG_ERROR, "cannot determine if device is assigned, refusing to continue");
+ return ERROR_FAIL;
+ }
+ if ( is_assigned(assigned, num_assigned, pcidev->domain,
+ pcidev->bus, pcidev->dev, pcidev->func) ) {
+ XL_LOG(ctx, XL_LOG_ERROR, "PCI device already attached to a domain");
+ free(assigned);
+ return ERROR_FAIL;
+ }
+ free(assigned);
+
+ libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+
+ stubdomid = libxl_get_stubdom_id(ctx, domid);
+ if (stubdomid != 0) {
+ libxl_device_pci pcidev_s = *pcidev;
+ rc = do_pci_add(ctx, stubdomid, &pcidev_s);
+ if ( rc )
+ return rc;
+ }
+
+ return do_pci_add(ctx, domid, pcidev);
+}
+
int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev)
{
char *path;
@@ -466,7 +554,64 @@ out:
return 0;
}
-libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num)
+static libxl_device_pci *scan_sys_pcidir(libxl_device_pci *assigned,
+ int num_assigned, const char *path, int *num)
+{
+ libxl_device_pci *pcidevs = NULL, *new;
+ struct dirent *de;
+ DIR *dir;
+
+ dir = opendir(path);
+ if ( NULL == dir )
+ return pcidevs;
+
+ while( (de = readdir(dir)) ) {
+ unsigned dom, bus, dev, func;
+ if ( sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
+ continue;
+
+ if ( is_assigned(assigned, num_assigned, dom, bus, dev, func) )
+ continue;
+
+ new = realloc(pcidevs, ((*num) + 1) * sizeof(*new));
+ if ( NULL == new )
+ continue;
+
+ pcidevs = new;
+ new = pcidevs + *num;
+
+ memset(new, 0, sizeof(*new));
+ libxl_device_pci_init(new, dom, bus, dev, func, 0);
+ (*num)++;
+ }
+
+ closedir(dir);
+ return pcidevs;
+}
+
+int libxl_device_pci_list_assignable(libxl_ctx *ctx, libxl_device_pci **list, int *num)
+{
+ libxl_device_pci *pcidevs = NULL;
+ libxl_device_pci *assigned;
+ int num_assigned, rc;
+
+ *num = 0;
+ *list = NULL;
+
+ rc = get_all_assigned_devices(ctx, &assigned, &num_assigned);
+ if ( rc )
+ return rc;
+
+ pcidevs = scan_sys_pcidir(assigned, num_assigned,
+ SYSFS_PCIBACK_DRIVER, num);
+
+ free(assigned);
+ if ( *num )
+ *list = pcidevs;
+ return 0;
+}
+
+int libxl_device_pci_list_assigned(libxl_ctx *ctx, libxl_device_pci **list, uint32_t domid, int *num)
{
char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts;
int n, i;
@@ -477,7 +622,8 @@ libxl_device_pci *libxl_device_pci_list(
num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path));
if (!num_devs) {
*num = 0;
- return NULL;
+ *list = NULL;
+ return ERROR_FAIL;
}
n = atoi(num_devs);
pcidevs = calloc(n, sizeof(libxl_device_pci));
@@ -507,15 +653,19 @@ libxl_device_pci *libxl_device_pci_list(
} while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL);
}
}
- return pcidevs;
+ if ( *num )
+ *list = pcidevs;
+ return 0;
}
int libxl_device_pci_shutdown(libxl_ctx *ctx, uint32_t domid)
{
libxl_device_pci *pcidevs;
- int num, i;
+ int num, i, rc;
- pcidevs = libxl_device_pci_list(ctx, domid, &num);
+ rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num);
+ if ( rc )
+ return rc;
for (i = 0; i < num; i++) {
if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0)
return ERROR_FAIL;
diff -r 43150fdd0106 tools/libxl/xl.h
--- a/tools/libxl/xl.h Thu Jul 29 16:47:16 2010 +0100
+++ b/tools/libxl/xl.h Thu Jul 29 16:48:48 2010 +0100
@@ -32,6 +32,7 @@ int main_cd_insert(int argc, char **argv
int main_console(int argc, char **argv);
int main_vncviewer(int argc, char **argv);
int main_pcilist(int argc, char **argv);
+int main_pcilist_assignable(int argc, char **argv);
int main_pcidetach(int argc, char **argv);
int main_pciattach(int argc, char **argv);
int main_restore(int argc, char **argv);
diff -r 43150fdd0106 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Thu Jul 29 16:47:16 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Thu Jul 29 16:48:48 2010 +0100
@@ -1920,6 +1920,39 @@ int main_vncviewer(int argc, char **argv
exit(0);
}
+void pcilist_assignable(void)
+{
+ libxl_device_pci *pcidevs;
+ int num, i;
+
+ if ( libxl_device_pci_list_assignable(&ctx, &pcidevs, &num) )
+ return;
+ for (i = 0; i < num; i++) {
+ printf("%04x:%02x:%02x:%01x\n",
+ pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
+ }
+ free(pcidevs);
+}
+
+int main_pcilist_assignable(int argc, char **argv)
+{
+ int opt;
+
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("pci-list-assignable-devices");
+ exit(0);
+ default:
+ fprintf(stderr, "option not supported\n");
+ break;
+ }
+ }
+
+ pcilist_assignable();
+ exit(0);
+}
+
void pcilist(char *dom)
{
libxl_device_pci *pcidevs;
@@ -1927,8 +1960,7 @@ void pcilist(char *dom)
find_domain(dom);
- pcidevs = libxl_device_pci_list(&ctx, domid, &num);
- if (!num)
+ if (libxl_device_pci_list_assigned(&ctx, &pcidevs, domid, &num))
return;
printf("VFn domain bus slot func\n");
for (i = 0; i < num; i++) {
diff -r 43150fdd0106 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Thu Jul 29 16:47:16 2010 +0100
+++ b/tools/libxl/xl_cmdtable.c Thu Jul 29 16:48:48 2010 +0100
@@ -69,6 +69,11 @@ struct cmd_spec cmd_table[] = {
"List pass-through pci devices for a domain",
"<Domain>",
},
+ { "pci-list-assignable-devices",
+ &main_pcilist_assignable,
+ "List all the assignable pci devices",
+ "",
+ },
{ "pause",
&main_pause,
"Pause execution of a domain",
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
2010-07-29 15:49 ` Gianni Tedesco
@ 2010-07-29 16:00 ` Stefano Stabellini
0 siblings, 0 replies; 19+ messages in thread
From: Stefano Stabellini @ 2010-07-29 16:00 UTC (permalink / raw)
To: Gianni Tedesco (3P)
Cc: Ian, xen-devel@lists.xensource.com, Jackson, Stefano Stabellini
On Thu, 29 Jul 2010, Gianni Tedesco (3P) wrote:
> Implement both of Stefanos suggestions this time:
>
> On Wed, 2010-07-28 at 20:40 +0100, Gianni Tedesco wrote:
> > tools/libxl/libxl.h | 3 +-
> > tools/libxl/libxl_internal.h | 3 +
> > tools/libxl/libxl_pci.c | 186 +++++++++++++++++++++++++++++++++++++++---
> > tools/libxl/xl.h | 1 +
> > tools/libxl/xl_cmdimpl.c | 36 +++++++-
> > tools/libxl/xl_cmdtable.c | 5 +
> > 6 files changed, 214 insertions(+), 20 deletions(-)
> >
> >
> > Implement a new libxl function libxl_device_pci_list_assignable. This is
> > used to implement the xl list-assignable-pci-devices command and part of
> > the implementation is used to make sure that PCI devices are not multiply
> > assigned to one or more domU's before doing the passthrough assignment.
> >
> > The function libxl_device_pci_list changes to libxl_device_pci_list_assigned
> > due to a parameter change for consistency with pci_list_assignable.
> >
> > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
2010-07-28 19:40 ` [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU Gianni Tedesco
2010-07-29 14:36 ` Stefano Stabellini
2010-07-29 15:49 ` Gianni Tedesco
@ 2010-07-30 15:21 ` Ian Campbell
2010-07-30 15:50 ` Gianni Tedesco
2 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2010-07-30 15:21 UTC (permalink / raw)
To: Gianni Tedesco
Cc: xen-devel@lists.xensource.com, Ian Jackson, Stefano Stabellini
On Wed, 2010-07-28 at 20:40 +0100, Gianni Tedesco wrote:
> int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid)
> {
> libxl_device_pci *pcidevs;
> - int num, i;
> + int num, i, rc;
>
> - pcidevs = libxl_device_pci_list(ctx, domid, &num);
> + rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num);
> + if ( rc )
> + return rc;
> for (i = 0; i < num; i++) {
> if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0)
> return ERROR_FAIL;
I have a feeling this is responsible for my getting this message:
libxl: error: libxl.c:851:libxl_domain_destroy pci shutdown failed for domid 2
when shutting down a PV domain which has no PCI devices assigned to it.
Presumably this is because libxl_device_pci_list_assigned() returns
error if "backend/pci/%d/0/num_devs" does not exist in xenstore, which
it won't for a domain with no devices passed through.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
2010-07-30 15:21 ` Ian Campbell
@ 2010-07-30 15:50 ` Gianni Tedesco
0 siblings, 0 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-30 15:50 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel@lists.xensource.com, Ian Jackson, Stefano Stabellini
On Fri, 2010-07-30 at 16:21 +0100, Ian Campbell wrote:
> On Wed, 2010-07-28 at 20:40 +0100, Gianni Tedesco wrote:
> > int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid)
> > {
> > libxl_device_pci *pcidevs;
> > - int num, i;
> > + int num, i, rc;
> >
> > - pcidevs = libxl_device_pci_list(ctx, domid, &num);
> > + rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num);
> > + if ( rc )
> > + return rc;
> > for (i = 0; i < num; i++) {
> > if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0)
> > return ERROR_FAIL;
>
> I have a feeling this is responsible for my getting this message:
> libxl: error: libxl.c:851:libxl_domain_destroy pci shutdown failed for domid 2
> when shutting down a PV domain which has no PCI devices assigned to it.
>
> Presumably this is because libxl_device_pci_list_assigned() returns
> error if "backend/pci/%d/0/num_devs" does not exist in xenstore, which
> it won't for a domain with no devices passed through.
>
> Ian.
Thanks, spotted this late last night and fixed in my tree. Will be
posting an updated patchbomb early next week.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
2010-07-28 19:40 [PATCH 0 of 3] Introduce basic PCI safety checks in libxl and centralize BDF parsing Gianni Tedesco
2010-07-28 19:40 ` [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c Gianni Tedesco
2010-07-28 19:40 ` [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU Gianni Tedesco
@ 2010-07-28 19:40 ` Gianni Tedesco
2010-07-29 14:42 ` Stefano Stabellini
2010-07-29 15:50 ` Gianni Tedesco
2 siblings, 2 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-28 19:40 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Stefano Stabellini
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@citrix.com>
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);
}
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
2010-07-28 19:40 ` [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl Gianni Tedesco
@ 2010-07-29 14:42 ` Stefano Stabellini
2010-07-29 15:33 ` Gianni Tedesco
2010-07-29 15:50 ` Gianni Tedesco
1 sibling, 1 reply; 19+ messages in thread
From: Stefano Stabellini @ 2010-07-29 14:42 UTC (permalink / raw)
To: Gianni Tedesco (3P)
Cc: Ian Jackson, xen-devel@lists.xensource.com, Stabellini
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@citrix.com>
>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
2010-07-29 14:42 ` Stefano Stabellini
@ 2010-07-29 15:33 ` Gianni Tedesco
0 siblings, 0 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 15:33 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Ian, xen-devel@lists.xensource.com, Jackson
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@citrix.com>
> >
>
>
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
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);
}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
2010-07-28 19:40 ` [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl Gianni Tedesco
2010-07-29 14:42 ` Stefano Stabellini
@ 2010-07-29 15:50 ` Gianni Tedesco
2010-07-29 17:54 ` Ian Jackson
1 sibling, 1 reply; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 15:50 UTC (permalink / raw)
To: xen-devel@lists.xensource.com; +Cc: Ian Jackson, Stefano Stabellini
On Wed, 2010-07-28 at 20:40 +0100, Gianni Tedesco 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@citrix.com>
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(libxl_ctx *ctx, uint32_t domid);
int libxl_device_vfb_hard_shutdown(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(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
int libxl_device_pci_shutdown(libxl_ctx *ctx, uint32_t domid);
int libxl_device_pci_list_assigned(libxl_ctx *ctx, libxl_device_pci **list, uint32_t domid, int *num);
int libxl_device_pci_list_assignable(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(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(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);
}
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
2010-07-29 15:50 ` Gianni Tedesco
@ 2010-07-29 17:54 ` Ian Jackson
2010-07-29 17:56 ` Gianni Tedesco
2010-07-29 18:15 ` Gianni Tedesco
0 siblings, 2 replies; 19+ messages in thread
From: Ian Jackson @ 2010-07-29 17:54 UTC (permalink / raw)
To: Gianni Tedesco; +Cc: xen-devel@lists.xensource.com, Stabellini
Gianni Tedesco writes ("Re: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl"):
> - printf("\t\t\t(pci dev "PCI_BDF_VDEVFN")\n",
> + printf("\t\t\t(pci dev %04x:%02x:%02x.%01x@%02x)\n",
I like this whole patch apart from this one line :-). Perhaps
PCI_BDF_VDEVFN should be LIBXL_PCI_BDF_VDEVFN and be exported in
libxl.h ?
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
2010-07-29 17:54 ` Ian Jackson
@ 2010-07-29 17:56 ` Gianni Tedesco
2010-07-29 18:15 ` Gianni Tedesco
1 sibling, 0 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 17:56 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com, Stabellini
On Thu, 2010-07-29 at 18:54 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("Re: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl"):
> > - printf("\t\t\t(pci dev "PCI_BDF_VDEVFN")\n",
> > + printf("\t\t\t(pci dev %04x:%02x:%02x.%01x@%02x)\n",
>
> I like this whole patch apart from this one line :-). Perhaps
> PCI_BDF_VDEVFN should be LIBXL_PCI_BDF_VDEVFN and be exported in
> libxl.h ?
>
> Ian.
Yeah makes sense, although I am not so keen on exporting bits of format
strings because it smells too much like policy. Perhaps xl could do it's
own macro for that.
It's your call.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl
2010-07-29 17:54 ` Ian Jackson
2010-07-29 17:56 ` Gianni Tedesco
@ 2010-07-29 18:15 ` Gianni Tedesco
1 sibling, 0 replies; 19+ messages in thread
From: Gianni Tedesco @ 2010-07-29 18:15 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com, Stabellini
On Thu, 2010-07-29 at 18:54 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("Re: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF's in xl"):
> > - printf("\t\t\t(pci dev "PCI_BDF_VDEVFN")\n",
> > + printf("\t\t\t(pci dev %04x:%02x:%02x.%01x@%02x)\n",
>
> I like this whole patch apart from this one line :-). Perhaps
> PCI_BDF_VDEVFN should be LIBXL_PCI_BDF_VDEVFN and be exported in
> libxl.h ?
>
> Ian.
Actually thinking about this the format is confusing anyway since the
actual notation would be:
printf("%04x:%02x:%02x.%01x=%01x@%02x", domain, bus, dev, func, vdevfn & 0x7, vdevfn >> 3);
Whatever you do with it shall be getting cleaned up anyway in my
forthcoming patch-series which actually parses and implements virtual
slots and multi-function devices with function remapping.
^ permalink raw reply [flat|nested] 19+ messages in thread