Gianni Tedesco
2010-Jul-28 19:40 UTC
[Xen-devel] [PATCH 0 of 3] Introduce basic PCI safety checks in libxl and centralize BDF parsing
This patch series follows on from previous patch moving PCI code in to libxl_pci.c Changes from previous patch-series are thus: 1. Move pci FLR function in to libxl_pci.c - got it all in now. 2. Properly initialize pci device structs in new code 3. Incorporate suggestions from Stefano wrt. API and putting sysfs paths in macros. 4. Rename libxl_device_pci_list to libxl_device_pci_list_assigned due to change in parameters for consistency with the rest of libxl PCI API. 5. Also introduced a patch to centralise parsing of PCI BDF''s and allow omission of the PCI domain as a short-hand for both config files and hot-plug command parameters. This also fixes an infinite loop in xl create if there is a parse error in the pci config. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-28 19:40 UTC
[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(-) 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; +} _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-28 19:40 UTC
[Xen-devel] [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
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", _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-28 19:40 UTC
[Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF''s in xl
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); } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-29 14:36 UTC
[Xen-devel] Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
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", >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-29 14:42 UTC
[Xen-devel] Re: [PATCH 3 of 3] Centralize parsing of PCI BDF''s in xl
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 15:32 UTC
[Xen-devel] Re: [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
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", _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 15:33 UTC
[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@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); } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Jul-29 15:37 UTC
Re: [Xen-devel] [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 15:42 UTC
Re: [Xen-devel] [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 15:48 UTC
Re: [Xen-devel] [PATCH 1 of 3] Move libxl_device_pci_reset in to libxl_pci.c
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; +} _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 15:49 UTC
Re: [Xen-devel] [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
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", _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 15:50 UTC
Re: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF''s in xl
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); } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-29 16:00 UTC
Re: [Xen-devel] [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Jul-29 17:54 UTC
Re: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF''s in xl
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 17:56 UTC
Re: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF''s in xl
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-29 18:15 UTC
Re: [Xen-devel] [PATCH 3 of 3] Centralize parsing of PCI BDF''s in xl
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Jul-30 15:21 UTC
Re: [Xen-devel] [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-30 15:50 UTC
Re: [Xen-devel] [PATCH 2 of 3] Check whether a PCI device is assignable before assigning it do a domU
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel