Gianni Tedesco
2010-Jul-27 16:24 UTC
[Xen-devel] [PATCH 0 of 3] xl: implement basic PCI passthrough sanity checks
Following patch series implements basic safety checks for PCI passthrough in libxl. Specifically it deals with the case where the same PCI device may be multiply assigned in to one or more domU''s. As a bonus, implement the pci-list-assignable-devices xl command. tools/libxl/Makefile | 2 +- tools/libxl/libxl.c | 501 ------------------------------------------ tools/libxl/libxl_pci.c | 538 ++++++++++++++++++++++++++++++++++++++++++++++ tools/libxl/libxl.h | 1 + tools/libxl/libxl_pci.c | 123 ++++++++++ tools/libxl/xl.h | 1 + tools/libxl/xl_cmdimpl.c | 34 ++ tools/libxl/xl_cmdtable.c | 5 + tools/libxl/libxl_pci.c | 11 +- 9 files changed, 713 insertions(+), 503 deletions(-) Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-27 16:24 UTC
[Xen-devel] [PATCH 1 of 3] Move PCI specific function definitions in to libxl_pci.c
tools/libxl/Makefile | 2 +- tools/libxl/libxl.c | 501 -------------------------------------------- tools/libxl/libxl_pci.c | 538 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 539 insertions(+), 502 deletions(-) diff -r ac7e4c6ec6c7 -r 20b2e15131c7 tools/libxl/Makefile --- a/tools/libxl/Makefile Fri Jul 23 19:23:49 2010 +0100 +++ b/tools/libxl/Makefile Tue Jul 27 17:13:35 2010 +0100 @@ -18,7 +18,7 @@ CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_ LIBS = $(LDFLAGS_libxenctrl) $(LDFLAGS_libxenguest) $(LDFLAGS_libxenstore) $(LDFLAGS_libblktapctl) -lutil LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o -LIBXL_OBJS = flexarray.o libxl.o libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o libxl_internal.o xenguest.o libxl_utils.o $(LIBXL_OBJS-y) +LIBXL_OBJS = flexarray.o libxl.o libxl_pci.o libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o libxl_internal.o xenguest.o libxl_utils.o $(LIBXL_OBJS-y) AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c diff -r ac7e4c6ec6c7 -r 20b2e15131c7 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Fri Jul 23 19:23:49 2010 +0100 +++ b/tools/libxl/libxl.c Tue Jul 27 17:13:35 2010 +0100 @@ -2327,507 +2327,6 @@ int libxl_device_vfb_hard_shutdown(struc /******************************************************************************/ -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; -} - -static int libxl_create_pci_backend(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev, int num) -{ - flexarray_t *front; - flexarray_t *back; - unsigned int boffset = 0; - unsigned int foffset = 0; - libxl_device device; - int i; - - front = flexarray_make(16, 1); - if (!front) - return ERROR_NOMEM; - back = flexarray_make(16, 1); - if (!back) - return ERROR_NOMEM; - - XL_LOG(ctx, XL_LOG_DEBUG, "Creating pci backend"); - - /* add pci device */ - device.backend_devid = 0; - device.backend_domid = 0; - device.backend_kind = DEVICE_PCI; - device.devid = 0; - device.domid = domid; - device.kind = DEVICE_PCI; - - flexarray_set(back, boffset++, "frontend-id"); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", domid)); - flexarray_set(back, boffset++, "online"); - flexarray_set(back, boffset++, "1"); - flexarray_set(back, boffset++, "state"); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1)); - flexarray_set(back, boffset++, "domain"); - flexarray_set(back, boffset++, libxl_domid_to_name(ctx, domid)); - for (i = 0; i < num; i++) { - flexarray_set(back, boffset++, libxl_sprintf(ctx, "key-%d", i)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "dev-%d", i)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); - if (pcidev->vdevfn) { - flexarray_set(back, boffset++, libxl_sprintf(ctx, "vdevfn-%d", i)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%x", pcidev->vdevfn)); - } - flexarray_set(back, boffset++, libxl_sprintf(ctx, "opts-%d", i)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "msitranslate=%d,power_mgmt=%d", pcidev->msitranslate, pcidev->power_mgmt)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "state-%d", i)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1)); - } - flexarray_set(back, boffset++, "num_devs"); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", num)); - - flexarray_set(front, foffset++, "backend-id"); - flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", 0)); - flexarray_set(front, foffset++, "state"); - flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", 1)); - - libxl_device_generic_add(ctx, &device, - libxl_xs_kvs_of_flexarray(ctx, back, boffset), - libxl_xs_kvs_of_flexarray(ctx, front, foffset)); - - flexarray_free(back); - flexarray_free(front); - return 0; -} - -static int libxl_device_pci_add_xenstore(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) -{ - flexarray_t *back; - char *num_devs, *be_path; - int num = 0; - unsigned int boffset = 0; - xs_transaction_t t; - - be_path = libxl_sprintf(ctx, "%s/backend/pci/%d/0", libxl_xs_get_dompath(ctx, 0), domid); - num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path)); - if (!num_devs) - return libxl_create_pci_backend(ctx, domid, pcidev, 1); - - if (!is_hvm(ctx, domid)) { - if (libxl_wait_for_backend(ctx, be_path, "4") < 0) - return ERROR_FAIL; - } - - back = flexarray_make(16, 1); - if (!back) - return ERROR_NOMEM; - - XL_LOG(ctx, XL_LOG_DEBUG, "Adding new pci device to xenstore"); - num = atoi(num_devs); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "key-%d", num)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "dev-%d", num)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); - if (pcidev->vdevfn) { - flexarray_set(back, boffset++, libxl_sprintf(ctx, "vdevfn-%d", num)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%x", pcidev->vdevfn)); - } - flexarray_set(back, boffset++, libxl_sprintf(ctx, "opts-%d", num)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "msitranslate=%d,power_mgmt=%d", pcidev->msitranslate, pcidev->power_mgmt)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "state-%d", num)); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1)); - flexarray_set(back, boffset++, "num_devs"); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", num + 1)); - flexarray_set(back, boffset++, "state"); - flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 7)); - -retry_transaction: - t = xs_transaction_start(ctx->xsh); - libxl_xs_writev(ctx, t, be_path, - libxl_xs_kvs_of_flexarray(ctx, back, boffset)); - if (!xs_transaction_end(ctx->xsh, t, 0)) - if (errno == EAGAIN) - goto retry_transaction; - - flexarray_free(back); - return 0; -} - -static int libxl_device_pci_remove_xenstore(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) -{ - char *be_path, *num_devs_path, *num_devs, *xsdev, *tmp, *tmppath; - int num, i, j; - xs_transaction_t t; - unsigned int domain = 0, bus = 0, dev = 0, func = 0; - - be_path = libxl_sprintf(ctx, "%s/backend/pci/%d/0", libxl_xs_get_dompath(ctx, 0), domid); - num_devs_path = libxl_sprintf(ctx, "%s/num_devs", be_path); - num_devs = libxl_xs_read(ctx, XBT_NULL, num_devs_path); - if (!num_devs) - return ERROR_INVAL; - num = atoi(num_devs); - - if (!is_hvm(ctx, domid)) { - if (libxl_wait_for_backend(ctx, be_path, "4") < 0) { - XL_LOG(ctx, XL_LOG_DEBUG, "pci backend at %s is not ready", be_path); - return ERROR_FAIL; - } - } - - for (i = 0; i < num; i++) { - xsdev = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/dev-%d", be_path, i)); - sscanf(xsdev, PCI_BDF, &domain, &bus, &dev, &func); - if (domain == pcidev->domain && bus == pcidev->bus && - pcidev->dev == dev && pcidev->func == func) { - break; - } - } - if (i == num) { - XL_LOG(ctx, XL_LOG_ERROR, "Couldn''t find the device on xenstore"); - return ERROR_INVAL; - } - -retry_transaction: - t = xs_transaction_start(ctx->xsh); - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/state-%d", be_path, i), "5", strlen("5")); - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/state", be_path), "7", strlen("7")); - if (!xs_transaction_end(ctx->xsh, t, 0)) - if (errno == EAGAIN) - goto retry_transaction; - - if (!is_hvm(ctx, domid)) { - if (libxl_wait_for_backend(ctx, be_path, "4") < 0) { - XL_LOG(ctx, XL_LOG_DEBUG, "pci backend at %s is not ready", be_path); - return ERROR_FAIL; - } - } - -retry_transaction2: - t = xs_transaction_start(ctx->xsh); - xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/state-%d", be_path, i)); - xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/key-%d", be_path, i)); - xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/dev-%d", be_path, i)); - xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdev-%d", be_path, i)); - xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/opts-%d", be_path, i)); - xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdevfn-%d", be_path, i)); - libxl_xs_write(ctx, t, num_devs_path, "%d", num - 1); - for (j = i + 1; j < num; j++) { - tmppath = libxl_sprintf(ctx, "%s/state-%d", be_path, j); - tmp = libxl_xs_read(ctx, t, tmppath); - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/state-%d", be_path, j - 1), tmp, strlen(tmp)); - xs_rm(ctx->xsh, t, tmppath); - tmppath = libxl_sprintf(ctx, "%s/dev-%d", be_path, j); - tmp = libxl_xs_read(ctx, t, tmppath); - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/dev-%d", be_path, j - 1), tmp, strlen(tmp)); - xs_rm(ctx->xsh, t, tmppath); - tmppath = libxl_sprintf(ctx, "%s/key-%d", be_path, j); - tmp = libxl_xs_read(ctx, t, tmppath); - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/key-%d", be_path, j - 1), tmp, strlen(tmp)); - xs_rm(ctx->xsh, t, tmppath); - tmppath = libxl_sprintf(ctx, "%s/vdev-%d", be_path, j); - tmp = libxl_xs_read(ctx, t, tmppath); - if (tmp) { - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdev-%d", be_path, j - 1), tmp, strlen(tmp)); - xs_rm(ctx->xsh, t, tmppath); - } - tmppath = libxl_sprintf(ctx, "%s/opts-%d", be_path, j); - tmp = libxl_xs_read(ctx, t, tmppath); - if (tmp) { - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/opts-%d", be_path, j - 1), tmp, strlen(tmp)); - xs_rm(ctx->xsh, t, tmppath); - } - tmppath = libxl_sprintf(ctx, "%s/vdevfn-%d", be_path, j); - tmp = libxl_xs_read(ctx, t, tmppath); - if (tmp) { - xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdevfn-%d", be_path, j - 1), tmp, strlen(tmp)); - xs_rm(ctx->xsh, t, tmppath); - } - } - if (!xs_transaction_end(ctx->xsh, t, 0)) - if (errno == EAGAIN) - goto retry_transaction2; - - if (num == 1) { - char *fe_path = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/frontend", be_path)); - libxl_device_destroy(ctx, be_path, 1); - xs_rm(ctx->xsh, XBT_NULL, be_path); - xs_rm(ctx->xsh, XBT_NULL, fe_path); - return 0; - } - - return 0; -} - -int libxl_device_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) { - if (libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL) < 0) { - return ERROR_FAIL; - } - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); - state = libxl_xs_read(ctx, XBT_NULL, path); - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/parameter", domid); - if (pcidev->vdevfn) - libxl_xs_write(ctx, XBT_NULL, path, PCI_BDF_VDEVFN, pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func, pcidev->vdevfn); - else - libxl_xs_write(ctx, XBT_NULL, path, PCI_BDF, pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func); - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid); - xs_write(ctx->xsh, XBT_NULL, path, "pci-ins", strlen("pci-ins")); - if (libxl_wait_for_device_model(ctx, domid, "pci-inserted", NULL, NULL) < 0) - XL_LOG(ctx, XL_LOG_ERROR, "Device Model didn''t respond in time"); - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/parameter", domid); - vdevfn = libxl_xs_read(ctx, XBT_NULL, path); - sscanf(vdevfn + 2, "%x", &pcidev->vdevfn); - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); - xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state)); - } else { - char *sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func); - FILE *f = fopen(sysfs_path, "r"); - unsigned long long start = 0, end = 0, flags = 0, size = 0; - int irq = 0; - int i; - - if (f == NULL) { - XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); - return ERROR_FAIL; - } - for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) { - if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3) - continue; - size = end - start + 1; - if (start) { - if (flags & PCI_BAR_IO) { - rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1); - if (rc < 0) { - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size); - fclose(f); - return ERROR_FAIL; - } - } else { - rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT, - (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1); - if (rc < 0) { - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size); - fclose(f); - return ERROR_FAIL; - } - } - } - } - fclose(f); - sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func); - f = fopen(sysfs_path, "r"); - if (f == NULL) { - XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); - goto out; - } - if ((fscanf(f, "%u", &irq) == 1) && irq) { - rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq); - if (rc < 0) { - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_physdev_map_pirq irq=%d", irq); - fclose(f); - return ERROR_FAIL; - } - rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1); - if (rc < 0) { - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_domain_irq_permission irq=%d", irq); - fclose(f); - return ERROR_FAIL; - } - } - fclose(f); - } -out: - if (!libxl_is_stubdom(ctx, domid, NULL)) { - rc = xc_assign_device(ctx->xch, domid, pcidev->value); - if (rc < 0) { - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_assign_device failed"); - return ERROR_FAIL; - } - } - - libxl_device_pci_add_xenstore(ctx, domid, pcidev); - return 0; -} - -int libxl_device_pci_remove(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) -{ - char *path; - char *state; - int hvm, rc; - int stubdomid = 0; - - /* TODO: check if the device can be detached */ - libxl_device_pci_remove_xenstore(ctx, domid, pcidev); - - hvm = is_hvm(ctx, domid); - if (hvm) { - if (libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL) < 0) { - return ERROR_FAIL; - } - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); - state = libxl_xs_read(ctx, XBT_NULL, path); - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/parameter", domid); - libxl_xs_write(ctx, XBT_NULL, path, PCI_BDF, pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func); - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid); - xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem")); - if (libxl_wait_for_device_model(ctx, domid, "pci-removed", NULL, NULL) < 0) { - XL_LOG(ctx, XL_LOG_ERROR, "Device Model didn''t respond in time"); - return ERROR_FAIL; - } - path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); - xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state)); - } else { - char *sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func); - FILE *f = fopen(sysfs_path, "r"); - unsigned int start = 0, end = 0, flags = 0, size = 0; - int irq = 0; - int i; - - if (f == NULL) { - XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); - goto skip1; - } - for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) { - if (fscanf(f, "0x%x 0x%x 0x%x\n", &start, &end, &flags) != 3) - continue; - size = end - start + 1; - if (start) { - if (flags & PCI_BAR_IO) { - rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 0); - if (rc < 0) - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_ioport_permission error 0x%x/0x%x", start, size); - } else { - rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT, - (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 0); - if (rc < 0) - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_iomem_permission error 0x%x/0x%x", start, size); - } - } - } - fclose(f); -skip1: - sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain, - pcidev->bus, pcidev->dev, pcidev->func); - f = fopen(sysfs_path, "r"); - if (f == NULL) { - XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); - goto out; - } - if ((fscanf(f, "%u", &irq) == 1) && irq) { - rc = xc_physdev_unmap_pirq(ctx->xch, domid, irq); - if (rc < 0) { - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_physdev_map_pirq irq=%d", irq); - } - rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); - if (rc < 0) { - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_irq_permission irq=%d", irq); - } - } - fclose(f); - } -out: - libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); - - if (!libxl_is_stubdom(ctx, domid, NULL)) { - rc = xc_deassign_device(ctx->xch, domid, pcidev->value); - if (rc < 0) - XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_deassign_device failed"); - } - - stubdomid = libxl_get_stubdom_id(ctx, domid); - if (stubdomid != 0) { - libxl_device_pci pcidev_s = *pcidev; - libxl_device_pci_remove(ctx, stubdomid, &pcidev_s); - } - - return 0; -} - -libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num) -{ - char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts; - int n, i; - unsigned int domain = 0, bus = 0, dev = 0, func = 0, vdevfn = 0; - libxl_device_pci *pcidevs; - - be_path = libxl_sprintf(ctx, "%s/backend/pci/%d/0", libxl_xs_get_dompath(ctx, 0), domid); - num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path)); - if (!num_devs) { - *num = 0; - return NULL; - } - n = atoi(num_devs); - pcidevs = calloc(n, sizeof(libxl_device_pci)); - *num = n; - - for (i = 0; i < n; i++) { - xsdev = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/dev-%d", be_path, i)); - sscanf(xsdev, PCI_BDF, &domain, &bus, &dev, &func); - 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); - xsopts = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/opts-%d", be_path, i)); - if (xsopts) { - char *saveptr; - char *p = strtok_r(xsopts, ",=", &saveptr); - do { - while (*p == '' '') - p++; - if (!strcmp(p, "msitranslate")) { - p = strtok_r(NULL, ",=", &saveptr); - pcidevs[i].msitranslate = atoi(p); - } else if (!strcmp(p, "power_mgmt")) { - p = strtok_r(NULL, ",=", &saveptr); - pcidevs[i].power_mgmt = atoi(p); - } - } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL); - } - } - return pcidevs; -} - -int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid) -{ - libxl_device_pci *pcidevs; - int num, i; - - pcidevs = libxl_device_pci_list(ctx, domid, &num); - for (i = 0; i < num; i++) { - if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0) - return ERROR_FAIL; - } - free(pcidevs); - return 0; -} - int libxl_domain_setmaxmem(struct libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb) { char *mem, *endptr; diff -r ac7e4c6ec6c7 -r 20b2e15131c7 tools/libxl/libxl_pci.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tools/libxl/libxl_pci.c Tue Jul 27 17:13:35 2010 +0100 @@ -0,0 +1,538 @@ +/* + * Copyright (C) 2009 Citrix Ltd. + * Author Vincent Hanquez <vincent.hanquez@eu.citrix.com> + * Author Stefano Stabellini <stefano.stabellini@eu.citrix.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + */ + +#include "libxl_osdeps.h" + +#include <stdio.h> +#include <string.h> +#include <stdlib.h> +#include <sys/types.h> +#include <fcntl.h> +#include <sys/select.h> +#include <sys/mman.h> +#include <sys/wait.h> +#include <signal.h> +#include <unistd.h> /* for write, unlink and close */ +#include <stdint.h> +#include <inttypes.h> +#include <assert.h> + +#include "libxl.h" +#include "libxl_utils.h" +#include "libxl_internal.h" +#include "flexarray.h" + +static int libxl_create_pci_backend(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev, int num) +{ + flexarray_t *front; + flexarray_t *back; + unsigned int boffset = 0; + unsigned int foffset = 0; + libxl_device device; + int i; + + front = flexarray_make(16, 1); + if (!front) + return ERROR_NOMEM; + back = flexarray_make(16, 1); + if (!back) + return ERROR_NOMEM; + + XL_LOG(ctx, XL_LOG_DEBUG, "Creating pci backend"); + + /* add pci device */ + device.backend_devid = 0; + device.backend_domid = 0; + device.backend_kind = DEVICE_PCI; + device.devid = 0; + device.domid = domid; + device.kind = DEVICE_PCI; + + flexarray_set(back, boffset++, "frontend-id"); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", domid)); + flexarray_set(back, boffset++, "online"); + flexarray_set(back, boffset++, "1"); + flexarray_set(back, boffset++, "state"); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1)); + flexarray_set(back, boffset++, "domain"); + flexarray_set(back, boffset++, libxl_domid_to_name(ctx, domid)); + for (i = 0; i < num; i++) { + flexarray_set(back, boffset++, libxl_sprintf(ctx, "key-%d", i)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "dev-%d", i)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); + if (pcidev->vdevfn) { + flexarray_set(back, boffset++, libxl_sprintf(ctx, "vdevfn-%d", i)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%x", pcidev->vdevfn)); + } + flexarray_set(back, boffset++, libxl_sprintf(ctx, "opts-%d", i)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "msitranslate=%d,power_mgmt=%d", pcidev->msitranslate, pcidev->power_mgmt)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "state-%d", i)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1)); + } + flexarray_set(back, boffset++, "num_devs"); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", num)); + + flexarray_set(front, foffset++, "backend-id"); + flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", 0)); + flexarray_set(front, foffset++, "state"); + flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", 1)); + + libxl_device_generic_add(ctx, &device, + libxl_xs_kvs_of_flexarray(ctx, back, boffset), + libxl_xs_kvs_of_flexarray(ctx, front, foffset)); + + flexarray_free(back); + flexarray_free(front); + return 0; +} + +static int libxl_device_pci_add_xenstore(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) +{ + flexarray_t *back; + char *num_devs, *be_path; + int num = 0; + unsigned int boffset = 0; + xs_transaction_t t; + + be_path = libxl_sprintf(ctx, "%s/backend/pci/%d/0", libxl_xs_get_dompath(ctx, 0), domid); + num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path)); + if (!num_devs) + return libxl_create_pci_backend(ctx, domid, pcidev, 1); + + if (!is_hvm(ctx, domid)) { + if (libxl_wait_for_backend(ctx, be_path, "4") < 0) + return ERROR_FAIL; + } + + back = flexarray_make(16, 1); + if (!back) + return ERROR_NOMEM; + + XL_LOG(ctx, XL_LOG_DEBUG, "Adding new pci device to xenstore"); + num = atoi(num_devs); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "key-%d", num)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "dev-%d", num)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, PCI_BDF, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func)); + if (pcidev->vdevfn) { + flexarray_set(back, boffset++, libxl_sprintf(ctx, "vdevfn-%d", num)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%x", pcidev->vdevfn)); + } + flexarray_set(back, boffset++, libxl_sprintf(ctx, "opts-%d", num)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "msitranslate=%d,power_mgmt=%d", pcidev->msitranslate, pcidev->power_mgmt)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "state-%d", num)); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 1)); + flexarray_set(back, boffset++, "num_devs"); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", num + 1)); + flexarray_set(back, boffset++, "state"); + flexarray_set(back, boffset++, libxl_sprintf(ctx, "%d", 7)); + +retry_transaction: + t = xs_transaction_start(ctx->xsh); + libxl_xs_writev(ctx, t, be_path, + libxl_xs_kvs_of_flexarray(ctx, back, boffset)); + if (!xs_transaction_end(ctx->xsh, t, 0)) + if (errno == EAGAIN) + goto retry_transaction; + + flexarray_free(back); + return 0; +} + +static int libxl_device_pci_remove_xenstore(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) +{ + char *be_path, *num_devs_path, *num_devs, *xsdev, *tmp, *tmppath; + int num, i, j; + xs_transaction_t t; + unsigned int domain = 0, bus = 0, dev = 0, func = 0; + + be_path = libxl_sprintf(ctx, "%s/backend/pci/%d/0", libxl_xs_get_dompath(ctx, 0), domid); + num_devs_path = libxl_sprintf(ctx, "%s/num_devs", be_path); + num_devs = libxl_xs_read(ctx, XBT_NULL, num_devs_path); + if (!num_devs) + return ERROR_INVAL; + num = atoi(num_devs); + + if (!is_hvm(ctx, domid)) { + if (libxl_wait_for_backend(ctx, be_path, "4") < 0) { + XL_LOG(ctx, XL_LOG_DEBUG, "pci backend at %s is not ready", be_path); + return ERROR_FAIL; + } + } + + for (i = 0; i < num; i++) { + xsdev = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/dev-%d", be_path, i)); + sscanf(xsdev, PCI_BDF, &domain, &bus, &dev, &func); + if (domain == pcidev->domain && bus == pcidev->bus && + pcidev->dev == dev && pcidev->func == func) { + break; + } + } + if (i == num) { + XL_LOG(ctx, XL_LOG_ERROR, "Couldn''t find the device on xenstore"); + return ERROR_INVAL; + } + +retry_transaction: + t = xs_transaction_start(ctx->xsh); + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/state-%d", be_path, i), "5", strlen("5")); + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/state", be_path), "7", strlen("7")); + if (!xs_transaction_end(ctx->xsh, t, 0)) + if (errno == EAGAIN) + goto retry_transaction; + + if (!is_hvm(ctx, domid)) { + if (libxl_wait_for_backend(ctx, be_path, "4") < 0) { + XL_LOG(ctx, XL_LOG_DEBUG, "pci backend at %s is not ready", be_path); + return ERROR_FAIL; + } + } + +retry_transaction2: + t = xs_transaction_start(ctx->xsh); + xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/state-%d", be_path, i)); + xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/key-%d", be_path, i)); + xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/dev-%d", be_path, i)); + xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdev-%d", be_path, i)); + xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/opts-%d", be_path, i)); + xs_rm(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdevfn-%d", be_path, i)); + libxl_xs_write(ctx, t, num_devs_path, "%d", num - 1); + for (j = i + 1; j < num; j++) { + tmppath = libxl_sprintf(ctx, "%s/state-%d", be_path, j); + tmp = libxl_xs_read(ctx, t, tmppath); + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/state-%d", be_path, j - 1), tmp, strlen(tmp)); + xs_rm(ctx->xsh, t, tmppath); + tmppath = libxl_sprintf(ctx, "%s/dev-%d", be_path, j); + tmp = libxl_xs_read(ctx, t, tmppath); + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/dev-%d", be_path, j - 1), tmp, strlen(tmp)); + xs_rm(ctx->xsh, t, tmppath); + tmppath = libxl_sprintf(ctx, "%s/key-%d", be_path, j); + tmp = libxl_xs_read(ctx, t, tmppath); + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/key-%d", be_path, j - 1), tmp, strlen(tmp)); + xs_rm(ctx->xsh, t, tmppath); + tmppath = libxl_sprintf(ctx, "%s/vdev-%d", be_path, j); + tmp = libxl_xs_read(ctx, t, tmppath); + if (tmp) { + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdev-%d", be_path, j - 1), tmp, strlen(tmp)); + xs_rm(ctx->xsh, t, tmppath); + } + tmppath = libxl_sprintf(ctx, "%s/opts-%d", be_path, j); + tmp = libxl_xs_read(ctx, t, tmppath); + if (tmp) { + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/opts-%d", be_path, j - 1), tmp, strlen(tmp)); + xs_rm(ctx->xsh, t, tmppath); + } + tmppath = libxl_sprintf(ctx, "%s/vdevfn-%d", be_path, j); + tmp = libxl_xs_read(ctx, t, tmppath); + if (tmp) { + xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/vdevfn-%d", be_path, j - 1), tmp, strlen(tmp)); + xs_rm(ctx->xsh, t, tmppath); + } + } + if (!xs_transaction_end(ctx->xsh, t, 0)) + if (errno == EAGAIN) + goto retry_transaction2; + + if (num == 1) { + char *fe_path = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/frontend", be_path)); + libxl_device_destroy(ctx, be_path, 1); + xs_rm(ctx->xsh, XBT_NULL, be_path); + xs_rm(ctx->xsh, XBT_NULL, fe_path); + return 0; + } + + return 0; +} + +int libxl_device_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) { + if (libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL) < 0) { + return ERROR_FAIL; + } + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); + state = libxl_xs_read(ctx, XBT_NULL, path); + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/parameter", domid); + if (pcidev->vdevfn) + libxl_xs_write(ctx, XBT_NULL, path, PCI_BDF_VDEVFN, pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func, pcidev->vdevfn); + else + libxl_xs_write(ctx, XBT_NULL, path, PCI_BDF, pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func); + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid); + xs_write(ctx->xsh, XBT_NULL, path, "pci-ins", strlen("pci-ins")); + if (libxl_wait_for_device_model(ctx, domid, "pci-inserted", NULL, NULL) < 0) + XL_LOG(ctx, XL_LOG_ERROR, "Device Model didn''t respond in time"); + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/parameter", domid); + vdevfn = libxl_xs_read(ctx, XBT_NULL, path); + sscanf(vdevfn + 2, "%x", &pcidev->vdevfn); + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); + xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state)); + } else { + char *sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func); + FILE *f = fopen(sysfs_path, "r"); + unsigned long long start = 0, end = 0, flags = 0, size = 0; + int irq = 0; + int i; + + if (f == NULL) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); + return ERROR_FAIL; + } + for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) { + if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3) + continue; + size = end - start + 1; + if (start) { + if (flags & PCI_BAR_IO) { + rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1); + if (rc < 0) { + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size); + fclose(f); + return ERROR_FAIL; + } + } else { + rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT, + (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1); + if (rc < 0) { + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size); + fclose(f); + return ERROR_FAIL; + } + } + } + } + fclose(f); + sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func); + f = fopen(sysfs_path, "r"); + if (f == NULL) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); + goto out; + } + if ((fscanf(f, "%u", &irq) == 1) && irq) { + rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq); + if (rc < 0) { + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_physdev_map_pirq irq=%d", irq); + fclose(f); + return ERROR_FAIL; + } + rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1); + if (rc < 0) { + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "Error: xc_domain_irq_permission irq=%d", irq); + fclose(f); + return ERROR_FAIL; + } + } + fclose(f); + } +out: + if (!libxl_is_stubdom(ctx, domid, NULL)) { + rc = xc_assign_device(ctx->xch, domid, pcidev->value); + if (rc < 0) { + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_assign_device failed"); + return ERROR_FAIL; + } + } + + libxl_device_pci_add_xenstore(ctx, domid, pcidev); + return 0; +} + +int libxl_device_pci_remove(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) +{ + char *path; + char *state; + int hvm, rc; + int stubdomid = 0; + + /* TODO: check if the device can be detached */ + libxl_device_pci_remove_xenstore(ctx, domid, pcidev); + + hvm = is_hvm(ctx, domid); + if (hvm) { + if (libxl_wait_for_device_model(ctx, domid, "running", NULL, NULL) < 0) { + return ERROR_FAIL; + } + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); + state = libxl_xs_read(ctx, XBT_NULL, path); + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/parameter", domid); + libxl_xs_write(ctx, XBT_NULL, path, PCI_BDF, pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func); + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command", domid); + xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem")); + if (libxl_wait_for_device_model(ctx, domid, "pci-removed", NULL, NULL) < 0) { + XL_LOG(ctx, XL_LOG_ERROR, "Device Model didn''t respond in time"); + return ERROR_FAIL; + } + path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state", domid); + xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state)); + } else { + char *sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func); + FILE *f = fopen(sysfs_path, "r"); + unsigned int start = 0, end = 0, flags = 0, size = 0; + int irq = 0; + int i; + + if (f == NULL) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); + goto skip1; + } + for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) { + if (fscanf(f, "0x%x 0x%x 0x%x\n", &start, &end, &flags) != 3) + continue; + size = end - start + 1; + if (start) { + if (flags & PCI_BAR_IO) { + rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 0); + if (rc < 0) + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_ioport_permission error 0x%x/0x%x", start, size); + } else { + rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT, + (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 0); + if (rc < 0) + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_iomem_permission error 0x%x/0x%x", start, size); + } + } + } + fclose(f); +skip1: + sysfs_path = libxl_sprintf(ctx, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain, + pcidev->bus, pcidev->dev, pcidev->func); + f = fopen(sysfs_path, "r"); + if (f == NULL) { + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn''t open %s", sysfs_path); + goto out; + } + if ((fscanf(f, "%u", &irq) == 1) && irq) { + rc = xc_physdev_unmap_pirq(ctx->xch, domid, irq); + if (rc < 0) { + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_physdev_map_pirq irq=%d", irq); + } + rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); + if (rc < 0) { + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_irq_permission irq=%d", irq); + } + } + fclose(f); + } +out: + libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); + + if (!libxl_is_stubdom(ctx, domid, NULL)) { + rc = xc_deassign_device(ctx->xch, domid, pcidev->value); + if (rc < 0) + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_deassign_device failed"); + } + + stubdomid = libxl_get_stubdom_id(ctx, domid); + if (stubdomid != 0) { + libxl_device_pci pcidev_s = *pcidev; + libxl_device_pci_remove(ctx, stubdomid, &pcidev_s); + } + + return 0; +} + +libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num) +{ + char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts; + int n, i; + unsigned int domain = 0, bus = 0, dev = 0, func = 0, vdevfn = 0; + libxl_device_pci *pcidevs; + + be_path = libxl_sprintf(ctx, "%s/backend/pci/%d/0", libxl_xs_get_dompath(ctx, 0), domid); + num_devs = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/num_devs", be_path)); + if (!num_devs) { + *num = 0; + return NULL; + } + n = atoi(num_devs); + pcidevs = calloc(n, sizeof(libxl_device_pci)); + *num = n; + + for (i = 0; i < n; i++) { + xsdev = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/dev-%d", be_path, i)); + sscanf(xsdev, PCI_BDF, &domain, &bus, &dev, &func); + 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); + xsopts = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/opts-%d", be_path, i)); + if (xsopts) { + char *saveptr; + char *p = strtok_r(xsopts, ",=", &saveptr); + do { + while (*p == '' '') + p++; + if (!strcmp(p, "msitranslate")) { + p = strtok_r(NULL, ",=", &saveptr); + pcidevs[i].msitranslate = atoi(p); + } else if (!strcmp(p, "power_mgmt")) { + p = strtok_r(NULL, ",=", &saveptr); + pcidevs[i].power_mgmt = atoi(p); + } + } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL); + } + } + return pcidevs; +} + +int libxl_device_pci_shutdown(struct libxl_ctx *ctx, uint32_t domid) +{ + libxl_device_pci *pcidevs; + int num, i; + + pcidevs = libxl_device_pci_list(ctx, domid, &num); + for (i = 0; i < num; i++) { + if (libxl_device_pci_remove(ctx, domid, pcidevs + i) < 0) + return ERROR_FAIL; + } + free(pcidevs); + 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; +} + _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-27 16:24 UTC
[Xen-devel] [PATCH 2 of 3] xl, implement pci-list-assignable-devices
tools/libxl/libxl.h | 1 + tools/libxl/libxl_pci.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++ tools/libxl/xl.h | 1 + tools/libxl/xl_cmdimpl.c | 34 ++++++++++++ tools/libxl/xl_cmdtable.c | 5 + 5 files changed, 164 insertions(+), 0 deletions(-) diff -r 20b2e15131c7 -r 214733749470 tools/libxl/libxl.h --- a/tools/libxl/libxl.h Tue Jul 27 17:13:35 2010 +0100 +++ b/tools/libxl/libxl.h Tue Jul 27 17:17:31 2010 +0100 @@ -496,6 +496,7 @@ int libxl_device_pci_add(struct libxl_ct 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); +libxl_device_pci *libxl_device_pci_list_assignable(struct libxl_ctx *ctx, 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 20b2e15131c7 -r 214733749470 tools/libxl/libxl_pci.c --- a/tools/libxl/libxl_pci.c Tue Jul 27 17:13:35 2010 +0100 +++ b/tools/libxl/libxl_pci.c Tue Jul 27 17:17:31 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,6 +259,75 @@ retry_transaction2: return 0; } +static libxl_device_pci *get_all_assigned_devices(struct libxl_ctx *ctx, int *num) +{ + libxl_device_pci *new, *pcidevs = NULL; + char **domlist; + unsigned int nd, i; + + *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; + + for(j = 0; 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; + + new = realloc(pcidevs, ((*num) + 1) * sizeof(*new)); + if ( NULL == new ) + continue; + + pcidevs = new; + new = pcidevs + *num; + + libxl_device_pci_init(new, dom, bus, dev, func, ~0); + (*num)++; + } + libxl_free(ctx, bdf); + libxl_free(ctx, devpath); + } + libxl_free(ctx, num_devs); + } + libxl_free(ctx, path); + } + libxl_free(ctx, domlist); + + return pcidevs; +} + +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; +} + int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) { char *path; @@ -466,6 +536,59 @@ out: return 0; } +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; + + libxl_device_pci_init(new, dom, bus, dev, func, ~0); + (*num)++; + } + + closedir(dir); + return pcidevs; +} + +libxl_device_pci *libxl_device_pci_list_assignable(struct libxl_ctx *ctx, int *num) +{ + libxl_device_pci *pcidevs = NULL; + libxl_device_pci *assigned; + int num_assigned; + + *num = 0; + + assigned = get_all_assigned_devices(ctx, &num_assigned); + + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, + "/sys/bus/pci/drivers/pciback", num); + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, + "/sys/bus/pci/drivers/pcistub", num); + + free(assigned); + return pcidevs; +} + libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num) { char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts; diff -r 20b2e15131c7 -r 214733749470 tools/libxl/xl.h --- a/tools/libxl/xl.h Tue Jul 27 17:13:35 2010 +0100 +++ b/tools/libxl/xl.h Tue Jul 27 17:17:31 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 20b2e15131c7 -r 214733749470 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Tue Jul 27 17:13:35 2010 +0100 +++ b/tools/libxl/xl_cmdimpl.c Tue Jul 27 17:17:31 2010 +0100 @@ -1706,6 +1706,40 @@ int main_vncviewer(int argc, char **argv exit(0); } +void pcilist_assignable(void) +{ + libxl_device_pci *pcidevs; + int num, i; + + pcidevs = libxl_device_pci_list_assignable(&ctx, &num); + if (!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; diff -r 20b2e15131c7 -r 214733749470 tools/libxl/xl_cmdtable.c --- a/tools/libxl/xl_cmdtable.c Tue Jul 27 17:13:35 2010 +0100 +++ b/tools/libxl/xl_cmdtable.c Tue Jul 27 17:17:31 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-27 16:24 UTC
[Xen-devel] [PATCH 3 of 3] xl, check a PCI device is assignable before adding it to a domU
tools/libxl/libxl_pci.c | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) This prevents PCI devices from being added multiply to the same domain or multiple different domains simultaneously. diff -r 214733749470 -r dbfc4f30efe2 tools/libxl/libxl_pci.c --- a/tools/libxl/libxl_pci.c Tue Jul 27 17:17:31 2010 +0100 +++ b/tools/libxl/libxl_pci.c Tue Jul 27 17:18:44 2010 +0100 @@ -334,8 +334,17 @@ int libxl_device_pci_add(struct libxl_ct char *state, *vdevfn; int rc, hvm; int stubdomid = 0; + libxl_device_pci *assigned; + int num_assigned; - /* TODO: check if the device can be assigned */ + assigned = get_all_assigned_devices(ctx, &num_assigned); + 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); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-28 17:20 UTC
Re: [Xen-devel] [PATCH 2 of 3] xl, implement pci-list-assignable-devices
On Tue, 2010-07-27 at 17:24 +0100, Gianni Tedesco wrote:> + 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; > + > + new = realloc(pcidevs, ((*num) + 1) * sizeof(*new)); > + if ( NULL == new ) > + continue; > + > + pcidevs = new; > + new = pcidevs + *num; > + > + libxl_device_pci_init(new, dom, bus, dev, func, ~0); > + (*num)++; > + }Please don''t apply patch 2 and therefore 3 from this set. This part is erroneous. Fixed in my tree and will submit the latest batch of updates soon. Thanks _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-28 17:39 UTC
[Xen-devel] Re: [PATCH 2 of 3] xl, implement pci-list-assignable-devices
On Tue, 27 Jul 2010, Gianni Tedesco (3P) wrote:> tools/libxl/libxl.h | 1 + > tools/libxl/libxl_pci.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++ > tools/libxl/xl.h | 1 + > tools/libxl/xl_cmdimpl.c | 34 ++++++++++++ > tools/libxl/xl_cmdtable.c | 5 + > 5 files changed, 164 insertions(+), 0 deletions(-) > > > diff -r 20b2e15131c7 -r 214733749470 tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Tue Jul 27 17:13:35 2010 +0100 > +++ b/tools/libxl/libxl.h Tue Jul 27 17:17:31 2010 +0100 > @@ -496,6 +496,7 @@ int libxl_device_pci_add(struct libxl_ct > 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); > +libxl_device_pci *libxl_device_pci_list_assignable(struct libxl_ctx *ctx, 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 20b2e15131c7 -r 214733749470 tools/libxl/libxl_pci.c > --- a/tools/libxl/libxl_pci.c Tue Jul 27 17:13:35 2010 +0100 > +++ b/tools/libxl/libxl_pci.c Tue Jul 27 17:17:31 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,6 +259,75 @@ retry_transaction2: > return 0; > } > > +static libxl_device_pci *get_all_assigned_devices(struct libxl_ctx *ctx, int *num) > +{ > + libxl_device_pci *new, *pcidevs = NULL; > + char **domlist; > + unsigned int nd, i; > + > + *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; > + > + for(j = 0; 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; > + > + new = realloc(pcidevs, ((*num) + 1) * sizeof(*new)); > + if ( NULL == new ) > + continue; > + > + pcidevs = new; > + new = pcidevs + *num; > + > + libxl_device_pci_init(new, dom, bus, dev, func, ~0); > + (*num)++; > + } > + libxl_free(ctx, bdf); > + libxl_free(ctx, devpath); > + } > + libxl_free(ctx, num_devs); > + } > + libxl_free(ctx, path); > + } > + libxl_free(ctx, domlist); > + > + return pcidevs; > +}You shouldn''t use libxl_free explicitly on variables allocated in the context. Actually I like explicit free''s but we have to keep the style consistent, so the memory management refactoring should come in as a separate patch, and here there shouldn''t be any libxl_free''s. The realloc is correct because it is used on a variable that is going to be returned to the caller. Also I would prefer this function to return an integer and take the libxl_device_pci array as a parameter.> + > +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; > +} > + > int libxl_device_pci_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev) > { > char *path; > @@ -466,6 +536,59 @@ out: > return 0; > } > > +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; > + > + libxl_device_pci_init(new, dom, bus, dev, func, ~0); > + (*num)++; > + } > + > + closedir(dir); > + return pcidevs; > +} > + > +libxl_device_pci *libxl_device_pci_list_assignable(struct libxl_ctx *ctx, int *num) > +{ > + libxl_device_pci *pcidevs = NULL; > + libxl_device_pci *assigned; > + int num_assigned; > + > + *num = 0; > + > + assigned = get_all_assigned_devices(ctx, &num_assigned); > + > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > + "/sys/bus/pci/drivers/pciback", num); > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > + "/sys/bus/pci/drivers/pcistub", num); > + > + free(assigned); > + return pcidevs; > +} > +Same comment about returning an integer, also I don''t like the way you are "assuming" scan_sys_pcidir is going to use realloc on pcidevs: that should be an implementation detail of scan_sys_pcidir that callers shouldn''t rely upon. Consider that devices bound to pcistub cannot be assigned to PV guests, so I would remove scan_sys_pcidir on pcistub from here. Please define "/sys/bus/pci/drivers/pciback" in a macro.> libxl_device_pci *libxl_device_pci_list(struct libxl_ctx *ctx, uint32_t domid, int *num) > { > char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts; > diff -r 20b2e15131c7 -r 214733749470 tools/libxl/xl.h > --- a/tools/libxl/xl.h Tue Jul 27 17:13:35 2010 +0100 > +++ b/tools/libxl/xl.h Tue Jul 27 17:17:31 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 20b2e15131c7 -r 214733749470 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Tue Jul 27 17:13:35 2010 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Tue Jul 27 17:17:31 2010 +0100 > @@ -1706,6 +1706,40 @@ int main_vncviewer(int argc, char **argv > exit(0); > } > > +void pcilist_assignable(void) > +{ > + libxl_device_pci *pcidevs; > + int num, i; > + > + pcidevs = libxl_device_pci_list_assignable(&ctx, &num); > + if (!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; > diff -r 20b2e15131c7 -r 214733749470 tools/libxl/xl_cmdtable.c > --- a/tools/libxl/xl_cmdtable.c Tue Jul 27 17:13:35 2010 +0100 > +++ b/tools/libxl/xl_cmdtable.c Tue Jul 27 17:17:31 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-28 17:41 UTC
[Xen-devel] Re: [PATCH 3 of 3] xl, check a PCI device is assignable before adding it to a domU
On Tue, 27 Jul 2010, Gianni Tedesco (3P) wrote:> tools/libxl/libxl_pci.c | 11 ++++++++++- > 1 files changed, 10 insertions(+), 1 deletions(-) > > > This prevents PCI devices from being added multiply to the same domain > or multiple different domains simultaneously.Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> > diff -r 214733749470 -r dbfc4f30efe2 tools/libxl/libxl_pci.c > --- a/tools/libxl/libxl_pci.c Tue Jul 27 17:17:31 2010 +0100 > +++ b/tools/libxl/libxl_pci.c Tue Jul 27 17:18:44 2010 +0100 > @@ -334,8 +334,17 @@ int libxl_device_pci_add(struct libxl_ct > char *state, *vdevfn; > int rc, hvm; > int stubdomid = 0; > + libxl_device_pci *assigned; > + int num_assigned; > > - /* TODO: check if the device can be assigned */ > + assigned = get_all_assigned_devices(ctx, &num_assigned); > + 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); > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-28 17:52 UTC
[Xen-devel] Re: [PATCH 2 of 3] xl, implement pci-list-assignable-devices
On Wed, 2010-07-28 at 18:39 +0100, Stefano Stabellini wrote:> > + } > > + libxl_free(ctx, bdf); > > + libxl_free(ctx, devpath); > > + } > > + libxl_free(ctx, num_devs); > > + } > > + libxl_free(ctx, path); > > + } > > + libxl_free(ctx, domlist); > > + > > + return pcidevs; > > +} > > You shouldn''t use libxl_free explicitly on variables allocated in the > context. Actually I like explicit free''s but we have to keep the style > consistent, so the memory management refactoring should come in as a > separate patch, and here there shouldn''t be any libxl_free''s. > The realloc is correct because it is used on a variable that is going > to be returned to the caller.OK, this was written before the idea for refactoring memory management and just done out of habit. Are you saying just remove the libxl_free''s and re-submit it like that?> Also I would prefer this function to return an integer and take the > libxl_device_pci array as a parameter.Hmm, OK.> > +libxl_device_pci *libxl_device_pci_list_assignable(struct libxl_ctx *ctx, int *num) > > +{ > > + libxl_device_pci *pcidevs = NULL; > > + libxl_device_pci *assigned; > > + int num_assigned; > > + > > + *num = 0; > > + > > + assigned = get_all_assigned_devices(ctx, &num_assigned); > > + > > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > > + "/sys/bus/pci/drivers/pciback", num); > > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > > + "/sys/bus/pci/drivers/pcistub", num); > > + > > + free(assigned); > > + return pcidevs; > > +} > > + > > Same comment about returning an integer, also I don''t like the way you > are "assuming" scan_sys_pcidir is going to use realloc on pcidevs: that > should be an implementation detail of scan_sys_pcidir that callers > shouldn''t rely upon.Actually it''s a static function with only one caller, this makes the implementation of list_assignable as simple as possible.> Consider that devices bound to pcistub cannot be assigned to PV guests, > so I would remove scan_sys_pcidir on pcistub from here. > Please define "/sys/bus/pci/drivers/pciback" in a macro.But they can be assigned to HVM guests? Nothing in the add-device path uses this list of devices anyway. What actually happens is a check whether a device appears in the get_all_assigned_devices() list. Even after this patch-set the user can always specify BDF''s which aren''t really assignable (eg. the Host''s PCI-to-ISA bridge or PCI root for maximum amusement) That is to be addressed in a future patch. Whether-or-not a specific device can be added is an issue for the add-device path. I think that the best approach there would then be just to stat the relevant sysfs paths to make sure the device is available. As for the macro, of course, I will change that. Thanks for the comments. Gianni _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Jul-28 18:07 UTC
[Xen-devel] Re: [PATCH 2 of 3] xl, implement pci-list-assignable-devices
On Wed, 28 Jul 2010, Gianni Tedesco (3P) wrote:> On Wed, 2010-07-28 at 18:39 +0100, Stefano Stabellini wrote: > > > + } > > > + libxl_free(ctx, bdf); > > > + libxl_free(ctx, devpath); > > > + } > > > + libxl_free(ctx, num_devs); > > > + } > > > + libxl_free(ctx, path); > > > + } > > > + libxl_free(ctx, domlist); > > > + > > > + return pcidevs; > > > +} > > > > You shouldn''t use libxl_free explicitly on variables allocated in the > > context. Actually I like explicit free''s but we have to keep the style > > consistent, so the memory management refactoring should come in as a > > separate patch, and here there shouldn''t be any libxl_free''s. > > The realloc is correct because it is used on a variable that is going > > to be returned to the caller. > > OK, this was written before the idea for refactoring memory management > and just done out of habit. Are you saying just remove the libxl_free''s > and re-submit it like that? >Yep> > > +libxl_device_pci *libxl_device_pci_list_assignable(struct libxl_ctx *ctx, int *num) > > > +{ > > > + libxl_device_pci *pcidevs = NULL; > > > + libxl_device_pci *assigned; > > > + int num_assigned; > > > + > > > + *num = 0; > > > + > > > + assigned = get_all_assigned_devices(ctx, &num_assigned); > > > + > > > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > > > + "/sys/bus/pci/drivers/pciback", num); > > > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > > > + "/sys/bus/pci/drivers/pcistub", num); > > > + > > > + free(assigned); > > > + return pcidevs; > > > +} > > > + > > > > Same comment about returning an integer, also I don''t like the way you > > are "assuming" scan_sys_pcidir is going to use realloc on pcidevs: that > > should be an implementation detail of scan_sys_pcidir that callers > > shouldn''t rely upon. > > Actually it''s a static function with only one caller, this makes the > implementation of list_assignable as simple as possible. > > > Consider that devices bound to pcistub cannot be assigned to PV guests, > > so I would remove scan_sys_pcidir on pcistub from here. > > Please define "/sys/bus/pci/drivers/pciback" in a macro. > > But they can be assigned to HVM guests? Nothing in the add-device path > uses this list of devices anyway. What actually happens is a check > whether a device appears in the get_all_assigned_devices() list. Even > after this patch-set the user can always specify BDF''s which aren''t > really assignable (eg. the Host''s PCI-to-ISA bridge or PCI root for > maximum amusement) That is to be addressed in a future patch. >Yes but libxl_device_pci_list_assignable is a public function, we don''t know what libxl users are going to do with it. In particular it seems reasonable to call libxl_device_pci_list_assignable before assigning a device even if xl doesn''t do it at the moment. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-28 18:18 UTC
[Xen-devel] Re: [PATCH 2 of 3] xl, implement pci-list-assignable-devices
On Wed, 2010-07-28 at 19:07 +0100, Stefano Stabellini wrote:> > > > +libxl_device_pci *libxl_device_pci_list_assignable(struct libxl_ctx *ctx, int *num) > > > > +{ > > > > + libxl_device_pci *pcidevs = NULL; > > > > + libxl_device_pci *assigned; > > > > + int num_assigned; > > > > + > > > > + *num = 0; > > > > + > > > > + assigned = get_all_assigned_devices(ctx, &num_assigned); > > > > + > > > > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > > > > + "/sys/bus/pci/drivers/pciback", num); > > > > + pcidevs = scan_sys_pcidir(pcidevs, assigned, num_assigned, > > > > + "/sys/bus/pci/drivers/pcistub", num); > > > > + > > > > + free(assigned); > > > > + return pcidevs; > > > > +} > > > > + > > > > > > Same comment about returning an integer, also I don''t like the way you > > > are "assuming" scan_sys_pcidir is going to use realloc on pcidevs: that > > > should be an implementation detail of scan_sys_pcidir that callers > > > shouldn''t rely upon. > > > > Actually it''s a static function with only one caller, this makes the > > implementation of list_assignable as simple as possible. > > > > > Consider that devices bound to pcistub cannot be assigned to PV guests, > > > so I would remove scan_sys_pcidir on pcistub from here. > > > Please define "/sys/bus/pci/drivers/pciback" in a macro. > > > > But they can be assigned to HVM guests? Nothing in the add-device path > > uses this list of devices anyway. What actually happens is a check > > whether a device appears in the get_all_assigned_devices() list. Even > > after this patch-set the user can always specify BDF''s which aren''t > > really assignable (eg. the Host''s PCI-to-ISA bridge or PCI root for > > maximum amusement) That is to be addressed in a future patch. > > > > Yes but libxl_device_pci_list_assignable is a public function, we don''t > know what libxl users are going to do with it. > In particular it seems reasonable to call > libxl_device_pci_list_assignable before assigning a device even if xl > doesn''t do it at the moment.Yes but the realloc semantics don''t apply to libxl_device_pci_list_assignable only scan_sys_pcidir. Callers of the former are free to do what they like with what is returned. My argument is that implementation of the latter is wholly appropriate to it''s only caller. In other words, scan_sys_pcidir is not going to have any new callers ever, as far as I can see anyway. As for whether a device is assigned to picback or pcistub, I think the semantics of list_assignable() ought to be that it returns anything which may potentially be assignable at the time of calling. Unless we want to implement some specific way of signalling which devices are bound to which backend drivers. Personally I think these additional details should be handled by letting an add-device fail with an ERROR_INVAL so that callers don''t need to know about backend-specific details. In other words I''d like to keep the code here as it is (modulo putting sysfs paths in macros) Unless there''s something deeper to this? Gianni _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel