* [PATCH 1 of 4 v2] libxl: Make a helper function write a BDF to a sysfs path
2012-05-11 13:31 [PATCH 0 of 4 v2] Add commands to automatically prep devices for pass-through George Dunlap
@ 2012-05-11 13:31 ` George Dunlap
2012-05-11 13:31 ` [PATCH 2 of 4 v2] libxl: Rename pci_list_assignable to pci_assignable_list George Dunlap
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: George Dunlap @ 2012-05-11 13:31 UTC (permalink / raw)
To: xen-devel; +Cc: george.dunlap
This functionality will be used several times in subsequent patches.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
diff -r ef0365894118 -r 6e2d2728620b tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Thu May 10 14:26:47 2012 +0100
+++ b/tools/libxl/libxl_pci.c Fri May 11 13:50:47 2012 +0100
@@ -327,6 +327,36 @@ static int is_pcidev_in_array(libxl_devi
return 0;
}
+/* Write the standard BDF into the sysfs path given by sysfs_path. */
+static int sysfs_write_bdf(libxl__gc *gc, const char * sysfs_path,
+ libxl_device_pci *pcidev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ int rc, fd;
+ char *buf;
+
+ fd = open(sysfs_path, O_WRONLY);
+ if (fd < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s",
+ sysfs_path);
+ return ERROR_FAIL;
+ }
+
+ buf = libxl__sprintf(gc, PCI_BDF, pcidev->domain, pcidev->bus,
+ pcidev->dev, pcidev->func);
+ rc = write(fd, buf, strlen(buf));
+ /* Annoying to have two if's, but we need the errno */
+ if (rc < 0)
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "write to %s returned %d", sysfs_path, rc);
+ close(fd);
+
+ if (rc < 0)
+ return ERROR_FAIL;
+
+ return 0;
+}
+
libxl_device_pci *libxl_device_pci_list_assignable(libxl_ctx *ctx, int *num)
{
GC_INIT(ctx);
@@ -571,27 +601,12 @@ static int do_pci_add(libxl__gc *gc, uin
/* Don't restrict writes to the PCI config space from this VM */
if (pcidev->permissive) {
- int fd;
- char *buf;
-
- sysfs_path = libxl__sprintf(gc, SYSFS_PCIBACK_DRIVER"/permissive");
- fd = open(sysfs_path, O_WRONLY);
- if (fd < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s",
- sysfs_path);
+ if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/permissive",
+ pcidev) < 0 ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Setting permissive for device");
return ERROR_FAIL;
}
-
- buf = libxl__sprintf(gc, PCI_BDF, pcidev->domain, pcidev->bus,
- pcidev->dev, pcidev->func);
- rc = write(fd, buf, strlen(buf));
- /* Annoying to have two if's, but we need the errno */
- if (rc < 0)
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
- "write to %s returned %d", sysfs_path, rc);
- close(fd);
- if (rc < 0)
- return ERROR_FAIL;
}
break;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 2 of 4 v2] libxl: Rename pci_list_assignable to pci_assignable_list
2012-05-11 13:31 [PATCH 0 of 4 v2] Add commands to automatically prep devices for pass-through George Dunlap
2012-05-11 13:31 ` [PATCH 1 of 4 v2] libxl: Make a helper function write a BDF to a sysfs path George Dunlap
@ 2012-05-11 13:31 ` George Dunlap
2012-05-11 13:31 ` [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove George Dunlap
2012-05-11 13:31 ` [PATCH 4 of 4 v2] xl: Add pci_assignable_add and remove commands George Dunlap
3 siblings, 0 replies; 10+ messages in thread
From: George Dunlap @ 2012-05-11 13:31 UTC (permalink / raw)
To: xen-devel; +Cc: george.dunlap
...to prepare for a consistent "pci_assignable_*" naming scheme.
Also move the man page entry into the PCI PASS-THROUGH section, rather
than the XEN HOST section.
No functional changes.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
diff -r 6e2d2728620b -r aa29f428a005 docs/man/xl.pod.1
--- a/docs/man/xl.pod.1 Fri May 11 13:50:47 2012 +0100
+++ b/docs/man/xl.pod.1 Fri May 11 13:51:12 2012 +0100
@@ -687,13 +687,6 @@ explanatory.
Prints the current uptime of the domains running.
-=item B<pci-list-assignable-devices>
-
-List all the assignable PCI devices.
-These are devices in the system which are configured to be
-available for passthrough and are bound to a suitable PCI
-backend driver in domain 0 rather than a real driver.
-
=back
=head1 SCHEDULER SUBCOMMANDS
@@ -1026,6 +1019,13 @@ List virtual network interfaces for a do
=over 4
+=item B<pci-assignable-list>
+
+List all the assignable PCI devices.
+These are devices in the system which are configured to be
+available for passthrough and are bound to a suitable PCI
+backend driver in domain 0 rather than a real driver.
+
=item B<pci-attach> I<domain-id> I<BDF>
Hot-plug a new pass-through pci device to the specified domain.
diff -r 6e2d2728620b -r aa29f428a005 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Fri May 11 13:50:47 2012 +0100
+++ b/tools/libxl/libxl.h Fri May 11 13:51:12 2012 +0100
@@ -662,7 +662,7 @@ libxl_device_pci *libxl_device_pci_list(
* could be assigned to a domain (i.e. are bound to the backend
* driver) but are not currently.
*/
-libxl_device_pci *libxl_device_pci_list_assignable(libxl_ctx *ctx, int *num);
+libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num);
/* CPUID handling */
int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str);
diff -r 6e2d2728620b -r aa29f428a005 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Fri May 11 13:50:47 2012 +0100
+++ b/tools/libxl/libxl_pci.c Fri May 11 13:51:12 2012 +0100
@@ -357,7 +357,7 @@ static int sysfs_write_bdf(libxl__gc *gc
return 0;
}
-libxl_device_pci *libxl_device_pci_list_assignable(libxl_ctx *ctx, int *num)
+libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
{
GC_INIT(ctx);
libxl_device_pci *pcidevs = NULL, *new, *assigned;
@@ -684,7 +684,7 @@ static int libxl_pcidev_assignable(libxl
libxl_device_pci *pcidevs;
int num, i;
- pcidevs = libxl_device_pci_list_assignable(ctx, &num);
+ pcidevs = libxl_device_pci_assignable_list(ctx, &num);
for (i = 0; i < num; i++) {
if (pcidevs[i].domain == pcidev->domain &&
pcidevs[i].bus == pcidev->bus &&
diff -r 6e2d2728620b -r aa29f428a005 tools/libxl/xl.h
--- a/tools/libxl/xl.h Fri May 11 13:50:47 2012 +0100
+++ b/tools/libxl/xl.h Fri May 11 13:51:12 2012 +0100
@@ -34,9 +34,9 @@ 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_pciassignable_list(int argc, char **argv);
int main_restore(int argc, char **argv);
int main_migrate_receive(int argc, char **argv);
int main_save(int argc, char **argv);
diff -r 6e2d2728620b -r aa29f428a005 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Fri May 11 13:50:47 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Fri May 11 13:51:12 2012 +0100
@@ -2223,34 +2223,6 @@ int main_vncviewer(int argc, char **argv
return 0;
}
-static void pcilist_assignable(void)
-{
- libxl_device_pci *pcidevs;
- int num, i;
-
- pcidevs = libxl_device_pci_list_assignable(ctx, &num);
-
- if ( pcidevs == NULL )
- 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);
- libxl_device_pci_dispose(&pcidevs[i]);
- }
- free(pcidevs);
-}
-
-int main_pcilist_assignable(int argc, char **argv)
-{
- int opt;
-
- if ((opt = def_getopt(argc, argv, "", "pci-list-assignable-devices", 0)) != -1)
- return opt;
-
- pcilist_assignable();
- return 0;
-}
-
static void pcilist(const char *dom)
{
libxl_device_pci *pcidevs;
@@ -2368,6 +2340,34 @@ int main_pciattach(int argc, char **argv
return 0;
}
+static void pciassignable_list(void)
+{
+ libxl_device_pci *pcidevs;
+ int num, i;
+
+ pcidevs = libxl_device_pci_assignable_list(ctx, &num);
+
+ if ( pcidevs == NULL )
+ 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);
+ libxl_device_pci_dispose(&pcidevs[i]);
+ }
+ free(pcidevs);
+}
+
+int main_pciassignable_list(int argc, char **argv)
+{
+ int opt;
+
+ if ((opt = def_getopt(argc, argv, "", "pci-assignable-list", 0)) != -1)
+ return opt;
+
+ pciassignable_list();
+ return 0;
+}
+
static void pause_domain(const char *p)
{
find_domain(p);
diff -r 6e2d2728620b -r aa29f428a005 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Fri May 11 13:50:47 2012 +0100
+++ b/tools/libxl/xl_cmdtable.c Fri May 11 13:51:12 2012 +0100
@@ -86,8 +86,8 @@ struct cmd_spec cmd_table[] = {
"List pass-through pci devices for a domain",
"<Domain>",
},
- { "pci-list-assignable-devices",
- &main_pcilist_assignable, 0,
+ { "pci-assignable-list",
+ &main_pciassignable_list, 0,
"List all the assignable pci devices",
"",
},
diff -r 6e2d2728620b -r aa29f428a005 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c Fri May 11 13:50:47 2012 +0100
+++ b/tools/python/xen/lowlevel/xl/xl.c Fri May 11 13:51:12 2012 +0100
@@ -566,13 +566,13 @@ static PyObject *pyxl_pci_parse(XlObject
return (PyObject *)pci;
}
-static PyObject *pyxl_pci_list_assignable(XlObject *self, PyObject *args)
+static PyObject *pyxl_pci_assignable_list(XlObject *self, PyObject *args)
{
libxl_device_pci *dev;
PyObject *list;
int nr_dev, i;
- dev = libxl_device_pci_list_assignable(self->ctx, &nr_dev);
+ dev = libxl_device_pci_assignable_list(self->ctx, &nr_dev);
if ( dev == NULL ) {
PyErr_SetString(xl_error_obj, "Cannot list assignable devices");
return NULL;
@@ -662,8 +662,8 @@ static PyMethodDef pyxl_methods[] = {
"Parse pass-through PCI device spec (BDF)"},
{"device_pci_list", (PyCFunction)pyxl_pci_list, METH_VARARGS,
"List PCI devices assigned to a domain"},
- {"device_pci_list_assignable",
- (PyCFunction)pyxl_pci_list_assignable, METH_NOARGS,
+ {"device_pci_assignable_list",
+ (PyCFunction)pyxl_pci_assignable_list, METH_NOARGS,
"List assignable PCI devices"},
{ NULL, NULL, 0, NULL }
};
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove
2012-05-11 13:31 [PATCH 0 of 4 v2] Add commands to automatically prep devices for pass-through George Dunlap
2012-05-11 13:31 ` [PATCH 1 of 4 v2] libxl: Make a helper function write a BDF to a sysfs path George Dunlap
2012-05-11 13:31 ` [PATCH 2 of 4 v2] libxl: Rename pci_list_assignable to pci_assignable_list George Dunlap
@ 2012-05-11 13:31 ` George Dunlap
2012-05-14 9:21 ` Ian Campbell
2012-05-11 13:31 ` [PATCH 4 of 4 v2] xl: Add pci_assignable_add and remove commands George Dunlap
3 siblings, 1 reply; 10+ messages in thread
From: George Dunlap @ 2012-05-11 13:31 UTC (permalink / raw)
To: xen-devel; +Cc: george.dunlap
Introduce libxl helper functions to prepare devices to be passed through to
guests. This is meant to replace of all the manual sysfs commands which
are currently required.
pci_assignable_add accepts a BDF for a device and will:
* Unbind a device from its current driver, if any
* If "rebind" is set, it will store the path of the driver from which we
unplugged it in /libxl/pciback/$BDF/driver_path
* If create a slot for it in pciback if one doesn't yet exist
* Bind the device to pciback
At this point it will show up in pci_assignable_list, and is ready to be passed
through to a guest.
pci_assignable_remove accepts a BDF for a device and will:
* Unbind the device from pciback
* Remove the slot from pciback
* If "rebind" is set, and /libx/pciback/$BDF/driver_path exists, it will
attempt to rebind the device to its original driver.
NB that "$BDF" in this case uses '-' instead of ':' and '.', because
':' and '.' are illegal characters in xenstore paths.
v2:
- sysfs_dev_unbind uses a local var for the path pointer, and sets only at
the end
- Actually read pci domain when looking at slots, instead of assuming 0000
- Call LOG_ERRNO after failed sysfs_write_bdf calls, rather than just LOG
- Removed stray FIXME
- Made xenstore reads and writes single operations, and removed transaction
infrastructure
- Wrapped a couple of lines that were above 80 characters
- Added a comment explaining the patch's treatment of slots
- Clarified patch description of when it creates slots
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
diff -r aa29f428a005 -r 5e21532dab5b tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Fri May 11 13:51:12 2012 +0100
+++ b/tools/libxl/libxl.h Fri May 11 13:55:04 2012 +0100
@@ -658,10 +658,25 @@ int libxl_device_pci_destroy(libxl_ctx *
libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num);
/*
- * Similar to libxl_device_pci_list but returns all devices which
- * could be assigned to a domain (i.e. are bound to the backend
- * driver) but are not currently.
+ * Functions related to making devices assignable -- that is, bound to
+ * the pciback driver, ready to be given to a guest via
+ * libxl_pci_device_add.
+ *
+ * - ..._add() will unbind the device from its current driver (if
+ * already bound) and re-bind it to pciback; at that point it will be
+ * ready to be assigned to a VM. If rebind is set, it will store the
+ * path to the old driver in xenstore so that it can be handed back to
+ * dom0 on restore.
+ *
+ * - ..._remove() will unbind the device from pciback, and if
+ * rebind is non-zero, attempt to assign it back to the driver
+ * from whence it came.
+ *
+ * - ..._list() will return a list of the PCI devices available to be
+ * assigned.
*/
+int libxl_device_pci_assignable_add(libxl_ctx *ctx, libxl_device_pci *pcidev, int rebind);
+int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_device_pci *pcidev, int rebind);
libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num);
/* CPUID handling */
diff -r aa29f428a005 -r 5e21532dab5b tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Fri May 11 13:51:12 2012 +0100
+++ b/tools/libxl/libxl_pci.c Fri May 11 13:55:04 2012 +0100
@@ -21,6 +21,7 @@
#define PCI_BDF "%04x:%02x:%02x.%01x"
#define PCI_BDF_SHORT "%02x:%02x.%01x"
#define PCI_BDF_VDEVFN "%04x:%02x:%02x.%01x@%02x"
+#define PCI_BDF_XSPATH "%04x-%02x-%02x-%01x"
static unsigned int pcidev_encode_bdf(libxl_device_pci *pcidev)
{
@@ -408,6 +409,334 @@ out:
return pcidevs;
}
+/* Unbind device from its current driver, if any. If driver_path is non-NULL,
+ * store the path to the original driver in it. */
+static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev,
+ char **driver_path)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char * spath, *dp = NULL;
+ struct stat st;
+
+ spath = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/driver",
+ pcidev->domain,
+ pcidev->bus,
+ pcidev->dev,
+ pcidev->func);
+ if ( !lstat(spath, &st) ) {
+ /* Find the canonical path to the driver. */
+ dp = libxl__zalloc(gc, PATH_MAX);
+ dp = realpath(spath, dp);
+ if ( !dp ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "realpath() failed");
+ return -1;
+ }
+
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Driver re-plug path: %s",
+ dp);
+
+ /* Unbind from the old driver */
+ spath = libxl__sprintf(gc, "%s/unbind", dp);
+ if ( sysfs_write_bdf(gc, spath, pcidev) < 0 ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't unbind device");
+ return -1;
+ }
+ }
+
+ if ( driver_path )
+ *driver_path = dp;
+
+ return 0;
+}
+
+/*
+ * A brief comment about slots. I don't know what slots are for; however,
+ * I have by experimentation determined:
+ * - Before a device can be bound to pciback, its BDF must first be listed
+ * in pciback/slots
+ * - The way to get the BDF listed there is to write BDF to
+ * pciback/new_slot
+ * - Writing the same BDF to pciback/new_slot is not idempotent; it results
+ * in two entries of the BDF in pciback/slots
+ * It's not clear whether having two entries in pciback/slots is a problem
+ * or not. Just to be safe, this code does the conservative thing, and
+ * first checks to see if there is a slot, adding one only if one does not
+ * already exist.
+ */
+
+/* Scan through /sys/.../pciback/slots looking for pcidev's BDF */
+static int pciback_dev_has_slot(libxl__gc *gc, libxl_device_pci *pcidev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ FILE *f;
+ int rc = 0;
+ unsigned dom, bus, dev, func;
+
+ f = fopen(SYSFS_PCIBACK_DRIVER"/slots", "r");
+
+ if (f == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s",
+ SYSFS_PCIBACK_DRIVER"/slots");
+ return ERROR_FAIL;
+ }
+
+ while(fscanf(f, "%x:%x:%x.%x\n", &dom, &bus, &dev, &func)==3) {
+ if(dom == pcidev->domain
+ && bus == pcidev->bus
+ && dev == pcidev->dev
+ && func == pcidev->func) {
+ rc = 1;
+ goto out;
+ }
+ }
+out:
+ fclose(f);
+ return rc;
+}
+
+static int pciback_dev_is_assigned(libxl__gc *gc, libxl_device_pci *pcidev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char * spath;
+ int rc;
+ struct stat st;
+
+ spath = libxl__sprintf(gc, SYSFS_PCIBACK_DRIVER"/"PCI_BDF,
+ pcidev->domain, pcidev->bus,
+ pcidev->dev, pcidev->func);
+ rc = lstat(spath, &st);
+
+ if( rc == 0 )
+ return 1;
+ if ( rc < 0 && errno == ENOENT )
+ return 0;
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Accessing %s", spath);
+ return -1;
+}
+
+static int pciback_dev_assign(libxl__gc *gc, libxl_device_pci *pcidev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ int rc;
+
+ if ( (rc=pciback_dev_has_slot(gc, pcidev)) < 0 ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Error checking for pciback slot");
+ return ERROR_FAIL;
+ } else if (rc == 0) {
+ if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/new_slot",
+ pcidev) < 0 ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Couldn't bind device to pciback!");
+ return ERROR_FAIL;
+ }
+ }
+
+ if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/bind", pcidev) < 0 ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Couldn't bind device to pciback!");
+ return ERROR_FAIL;
+ }
+ return 0;
+}
+
+static int pciback_dev_unassign(libxl__gc *gc, libxl_device_pci *pcidev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+
+ /* Remove from pciback */
+ if ( sysfs_dev_unbind(gc, pcidev, NULL) < 0 ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Couldn't unbind device!");
+ return ERROR_FAIL;
+ }
+
+ /* Remove slot if necessary */
+ if ( pciback_dev_has_slot(gc, pcidev) > 0 ) {
+ if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/remove_slot",
+ pcidev) < 0 ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Couldn't remove pciback slot");
+ return ERROR_FAIL;
+ }
+ }
+ return 0;
+}
+
+#define PCIBACK_INFO_PATH "/libxl/pciback"
+
+static void pci_assignable_driver_path_write(libxl__gc *gc,
+ libxl_device_pci *pcidev,
+ char *driver_path)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *path;
+
+ path = libxl__sprintf(gc, PCIBACK_INFO_PATH"/"PCI_BDF_XSPATH"/driver_path",
+ pcidev->domain,
+ pcidev->bus,
+ pcidev->dev,
+ pcidev->func);
+ if ( libxl__xs_write(gc, XBT_NULL, path, "%s", driver_path) < 0 ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_WARNING,
+ "Write of %s to node %s failed.",
+ driver_path, path);
+ }
+}
+
+static char * pci_assignable_driver_path_read(libxl__gc *gc,
+ libxl_device_pci *pcidev)
+{
+ return libxl__xs_read(gc, XBT_NULL,
+ libxl__sprintf(gc,
+ PCIBACK_INFO_PATH "/" PCI_BDF_XSPATH "/driver_path",
+ pcidev->domain,
+ pcidev->bus,
+ pcidev->dev,
+ pcidev->func));
+}
+
+static void pci_assignable_driver_path_remove(libxl__gc *gc,
+ libxl_device_pci *pcidev)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+
+ /* Remove the xenstore entry */
+ xs_rm(ctx->xsh, XBT_NULL,
+ libxl__sprintf(gc, PCIBACK_INFO_PATH "/" PCI_BDF_XSPATH,
+ pcidev->domain,
+ pcidev->bus,
+ pcidev->dev,
+ pcidev->func) );
+}
+
+static int libxl__device_pci_assignable_add(libxl__gc *gc,
+ libxl_device_pci *pcidev,
+ int rebind)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ unsigned dom, bus, dev, func;
+ char *spath, *driver_path = NULL;
+ struct stat st;
+
+ /* Local copy for convenience */
+ dom = pcidev->domain;
+ bus = pcidev->bus;
+ dev = pcidev->dev;
+ func = pcidev->func;
+
+ /* See if the device exists */
+ spath = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF, dom, bus, dev, func);
+ if ( lstat(spath, &st) ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't lstat %s", spath);
+ return ERROR_FAIL;
+ }
+
+ /* Check to see if it's already assigned to pciback */
+ if ( pciback_dev_is_assigned(gc, pcidev) ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, PCI_BDF" already assigned to pciback",
+ dom, bus, dev, func);
+ return 0;
+ }
+
+ /* Check to see if there's already a driver that we need to unbind from */
+ if ( sysfs_dev_unbind(gc, pcidev, &driver_path ) ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Couldn't unbind "PCI_BDF" from driver",
+ dom, bus, dev, func);
+ return ERROR_FAIL;
+ }
+
+ /* Store driver_path for rebinding to dom0 */
+ if ( rebind ) {
+ if ( driver_path ) {
+ pci_assignable_driver_path_write(gc, pcidev, driver_path);
+ } else {
+ LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
+ PCI_BDF" not bound to a driver, will not be rebound.",
+ dom, bus, dev, func);
+ }
+ }
+
+ if ( pciback_dev_assign(gc, pcidev) ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Couldn't bind device to pciback!");
+ return ERROR_FAIL;
+ }
+
+ return 0;
+}
+
+static int libxl__device_pci_assignable_remove(libxl__gc *gc,
+ libxl_device_pci *pcidev,
+ int rebind)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ int rc;
+ char *driver_path;
+
+ /* Unbind from pciback */
+ if ( (rc=pciback_dev_is_assigned(gc, pcidev)) < 0 ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Checking if pciback was assigned");
+ return ERROR_FAIL;
+ } else if ( rc ) {
+ pciback_dev_unassign(gc, pcidev);
+ } else {
+ LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
+ "Not bound to pciback");
+ }
+
+ /* Rebind if necessary */
+ driver_path = pci_assignable_driver_path_read(gc, pcidev);
+
+ if ( driver_path ) {
+ if ( rebind ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_INFO, "Rebinding to driver at %s",
+ driver_path);
+
+ if ( sysfs_write_bdf(gc,
+ libxl__sprintf(gc, "%s/bind", driver_path),
+ pcidev) < 0 ) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Couldn't bind device to %s", driver_path);
+ return -1;
+ }
+ }
+
+ pci_assignable_driver_path_remove(gc, pcidev);
+ } else {
+ if ( rebind ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
+ "Couldn't find path for original driver; not rebinding");
+ }
+ }
+
+ return 0;
+}
+
+int libxl_device_pci_assignable_add(libxl_ctx *ctx, libxl_device_pci *pcidev,
+ int rebind)
+{
+ GC_INIT(ctx);
+ int rc;
+
+ rc = libxl__device_pci_assignable_add(gc, pcidev, rebind);
+
+ GC_FREE;
+ return rc;
+}
+
+
+int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_device_pci *pcidev,
+ int rebind)
+{
+ GC_INIT(ctx);
+ int rc;
+
+ rc = libxl__device_pci_assignable_remove(gc, pcidev, rebind);
+
+ GC_FREE;
+ return rc;
+}
+
/*
* This function checks that all functions of a device are bound to pciback
* driver. It also initialises a bit-mask of which function numbers are present
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove
2012-05-11 13:31 ` [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove George Dunlap
@ 2012-05-14 9:21 ` Ian Campbell
2012-05-14 10:06 ` George Dunlap
2012-05-21 14:13 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 10+ messages in thread
From: Ian Campbell @ 2012-05-14 9:21 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk
> +
> +/* Scan through /sys/.../pciback/slots looking for pcidev's BDF */
> +static int pciback_dev_has_slot(libxl__gc *gc, libxl_device_pci *pcidev)
> +{
> + libxl_ctx *ctx = libxl__gc_owner(gc);
> + FILE *f;
> + int rc = 0;
> + unsigned dom, bus, dev, func;
> +
> + f = fopen(SYSFS_PCIBACK_DRIVER"/slots", "r");
> +
> + if (f == NULL) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s",
> + SYSFS_PCIBACK_DRIVER"/slots");
> + return ERROR_FAIL;
> + }
> +
> + while(fscanf(f, "%x:%x:%x.%x\n", &dom, &bus, &dev, &func)==3) {
Shouldn't this 3 be 4 now that you include dom?
Also ISTR some change to the precise formatting using by the kernel for
BDFs recently (A . became a : or vice versa?). CCing Konrad for input in
case it impacts this too.
The rest all looked fine.
> + if(dom == pcidev->domain
> + && bus == pcidev->bus
> + && dev == pcidev->dev
> + && func == pcidev->func) {
> + rc = 1;
> + goto out;
> + }
> + }
> +out:
> + fclose(f);
> + return rc;
> +}
> +
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove
2012-05-14 9:21 ` Ian Campbell
@ 2012-05-14 10:06 ` George Dunlap
2012-05-21 14:13 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 10+ messages in thread
From: George Dunlap @ 2012-05-14 10:06 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk
On 14/05/12 10:21, Ian Campbell wrote:
>> +
>> + if (f == NULL) {
>> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s",
>> + SYSFS_PCIBACK_DRIVER"/slots");
>> + return ERROR_FAIL;
>> + }
>> +
>> + while(fscanf(f, "%x:%x:%x.%x\n",&dom,&bus,&dev,&func)==3) {
> Shouldn't this 3 be 4 now that you include dom?
Ah, of course -- this is handling a case that doesn't happen normally
when you use the commands (i.e., having a slot in pciback but not being
bound), so it didn't get tested. I'll make sure to check that.
> Also ISTR some change to the precise formatting using by the kernel for
> BDFs recently (A . became a : or vice versa?). CCing Konrad for input in
> case it impacts this too.
That would be pretty unfortunate; but it seems like it would also be
pretty unlikely, as it would break an awful lot of user-mode programs,
yes? Linus seems to have a zero-tolerance policy towards that kind of
thing.
I'll fix this up, re-test, and wait for Konrad's comment before re-sending.
-George
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove
2012-05-14 9:21 ` Ian Campbell
2012-05-14 10:06 ` George Dunlap
@ 2012-05-21 14:13 ` Konrad Rzeszutek Wilk
2012-05-22 8:15 ` Ian Campbell
1 sibling, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-05-21 14:13 UTC (permalink / raw)
To: Ian Campbell; +Cc: George Dunlap, xen-devel@lists.xensource.com
On Mon, May 14, 2012 at 10:21:33AM +0100, Ian Campbell wrote:
> > +
> > +/* Scan through /sys/.../pciback/slots looking for pcidev's BDF */
> > +static int pciback_dev_has_slot(libxl__gc *gc, libxl_device_pci *pcidev)
> > +{
> > + libxl_ctx *ctx = libxl__gc_owner(gc);
> > + FILE *f;
> > + int rc = 0;
> > + unsigned dom, bus, dev, func;
> > +
> > + f = fopen(SYSFS_PCIBACK_DRIVER"/slots", "r");
> > +
> > + if (f == NULL) {
> > + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s",
> > + SYSFS_PCIBACK_DRIVER"/slots");
> > + return ERROR_FAIL;
> > + }
> > +
> > + while(fscanf(f, "%x:%x:%x.%x\n", &dom, &bus, &dev, &func)==3) {
And the last one should be %d.
>
> Shouldn't this 3 be 4 now that you include dom?
>
> Also ISTR some change to the precise formatting using by the kernel for
> BDFs recently (A . became a : or vice versa?). CCing Konrad for input in
> case it impacts this too.
>
> The rest all looked fine.
>
> > + if(dom == pcidev->domain
> > + && bus == pcidev->bus
> > + && dev == pcidev->dev
> > + && func == pcidev->func) {
> > + rc = 1;
> > + goto out;
> > + }
> > + }
> > +out:
> > + fclose(f);
> > + return rc;
> > +}
> > +
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove
2012-05-21 14:13 ` Konrad Rzeszutek Wilk
@ 2012-05-22 8:15 ` Ian Campbell
0 siblings, 0 replies; 10+ messages in thread
From: Ian Campbell @ 2012-05-22 8:15 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: George Dunlap, xen-devel@lists.xensource.com
On Mon, 2012-05-21 at 15:13 +0100, Konrad Rzeszutek Wilk wrote:
> On Mon, May 14, 2012 at 10:21:33AM +0100, Ian Campbell wrote:
> > > +
> > > +/* Scan through /sys/.../pciback/slots looking for pcidev's BDF */
> > > +static int pciback_dev_has_slot(libxl__gc *gc, libxl_device_pci *pcidev)
> > > +{
> > > + libxl_ctx *ctx = libxl__gc_owner(gc);
> > > + FILE *f;
> > > + int rc = 0;
> > > + unsigned dom, bus, dev, func;
> > > +
> > > + f = fopen(SYSFS_PCIBACK_DRIVER"/slots", "r");
> > > +
> > > + if (f == NULL) {
> > > + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s",
> > > + SYSFS_PCIBACK_DRIVER"/slots");
> > > + return ERROR_FAIL;
> > > + }
> > > +
> > > + while(fscanf(f, "%x:%x:%x.%x\n", &dom, &bus, &dev, &func)==3) {
>
> And the last one should be %d.
This patch went in already, can someone send an incremental fix
please?
Ian.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4 of 4 v2] xl: Add pci_assignable_add and remove commands
2012-05-11 13:31 [PATCH 0 of 4 v2] Add commands to automatically prep devices for pass-through George Dunlap
` (2 preceding siblings ...)
2012-05-11 13:31 ` [PATCH 3 of 4 v2] libxl: Introduce pci_assignable_add and pci_assignable_remove George Dunlap
@ 2012-05-11 13:31 ` George Dunlap
2012-05-14 9:24 ` Ian Campbell
3 siblings, 1 reply; 10+ messages in thread
From: George Dunlap @ 2012-05-11 13:31 UTC (permalink / raw)
To: xen-devel; +Cc: george.dunlap
pci-assignable-add will always store the driver rebind path, but
pci-assignable-remove will only actually rebind if asked to do so.
v2:
- Use libxl_device_pci_init() instead of memset()
- Call xlu_cfg_destroy() properly
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
diff -r 5e21532dab5b -r ba3d30d721ee docs/man/xl.pod.1
--- a/docs/man/xl.pod.1 Fri May 11 13:55:04 2012 +0100
+++ b/docs/man/xl.pod.1 Fri May 11 14:19:50 2012 +0100
@@ -1026,6 +1026,26 @@ These are devices in the system which ar
available for passthrough and are bound to a suitable PCI
backend driver in domain 0 rather than a real driver.
+=item B<pci-assignable-add> I<BDF>
+
+Make the device at PCI Bus/Device/Function BDF assignable to guests. This
+will bind the device to the pciback driver. If it is already bound to a
+driver, it will first be unbound, and the original driver stored so that it
+can be re-bound to the same driver later if desired.
+
+CAUTION: This will make the device unusable by Domain 0 until it is
+returned with pci-assignable-remove. Care should therefore be taken
+not to do this on a device critical to domain 0's operation, such as
+storage controllers, network interfaces, or GPUs that are currently
+being used.
+
+=item B<pci-assignable-remove> [I<-r>] I<BDF>
+
+Make the device at PCI Bus/Device/Function BDF assignable to guests. This
+will at least unbind the device from pciback. If the -r option is specified,
+it will also attempt to re-bind the device to its original driver, making it
+usable by Domain 0 again.
+
=item B<pci-attach> I<domain-id> I<BDF>
Hot-plug a new pass-through pci device to the specified domain.
diff -r 5e21532dab5b -r ba3d30d721ee tools/libxl/xl.h
--- a/tools/libxl/xl.h Fri May 11 13:55:04 2012 +0100
+++ b/tools/libxl/xl.h Fri May 11 14:19:50 2012 +0100
@@ -36,6 +36,8 @@ int main_vncviewer(int argc, char **argv
int main_pcilist(int argc, char **argv);
int main_pcidetach(int argc, char **argv);
int main_pciattach(int argc, char **argv);
+int main_pciassignable_add(int argc, char **argv);
+int main_pciassignable_remove(int argc, char **argv);
int main_pciassignable_list(int argc, char **argv);
int main_restore(int argc, char **argv);
int main_migrate_receive(int argc, char **argv);
diff -r 5e21532dab5b -r ba3d30d721ee tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Fri May 11 13:55:04 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Fri May 11 14:19:50 2012 +0100
@@ -2368,6 +2368,86 @@ int main_pciassignable_list(int argc, ch
return 0;
}
+static void pciassignable_add(const char *bdf, int rebind)
+{
+ libxl_device_pci pcidev;
+ XLU_Config *config;
+
+ libxl_device_pci_init(&pcidev);
+
+ config = xlu_cfg_init(stderr, "command line");
+ if (!config) { perror("xlu_cfg_inig"); exit(-1); }
+
+ if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
+ fprintf(stderr, "pci-assignable-add: malformed BDF specification \"%s\"\n", bdf);
+ exit(2);
+ }
+ libxl_device_pci_assignable_add(ctx, &pcidev, rebind);
+
+ libxl_device_pci_dispose(&pcidev);
+ xlu_cfg_destroy(config);
+}
+
+int main_pciassignable_add(int argc, char **argv)
+{
+ int opt;
+ const char *bdf = NULL;
+
+ while ((opt = def_getopt(argc, argv, "", "pci-assignable-add", 1)) != -1) {
+ switch (opt) {
+ case 0: case 2:
+ return opt;
+ }
+ }
+
+ bdf = argv[optind];
+
+ pciassignable_add(bdf, 1);
+ return 0;
+}
+
+static void pciassignable_remove(const char *bdf, int rebind)
+{
+ libxl_device_pci pcidev;
+ XLU_Config *config;
+
+ libxl_device_pci_init(&pcidev);
+
+ config = xlu_cfg_init(stderr, "command line");
+ if (!config) { perror("xlu_cfg_inig"); exit(-1); }
+
+ if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
+ fprintf(stderr, "pci-assignable-remove: malformed BDF specification \"%s\"\n", bdf);
+ exit(2);
+ }
+ libxl_device_pci_assignable_remove(ctx, &pcidev, rebind);
+
+ libxl_device_pci_dispose(&pcidev);
+ xlu_cfg_destroy(config);
+}
+
+int main_pciassignable_remove(int argc, char **argv)
+{
+ int opt;
+ const char *bdf = NULL;
+ int rebind = 0;
+
+ while ((opt = def_getopt(argc, argv, "r", "pci-assignable-remove", 1)) != -1) {
+ switch (opt) {
+ case 0: case 2:
+ return opt;
+ case 'r':
+ rebind=1;
+ break;
+ }
+ }
+
+ bdf = argv[optind];
+
+ pciassignable_remove(bdf, rebind);
+ return 0;
+}
+
static void pause_domain(const char *p)
{
find_domain(p);
diff -r 5e21532dab5b -r ba3d30d721ee tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Fri May 11 13:55:04 2012 +0100
+++ b/tools/libxl/xl_cmdtable.c Fri May 11 14:19:50 2012 +0100
@@ -86,6 +86,20 @@ struct cmd_spec cmd_table[] = {
"List pass-through pci devices for a domain",
"<Domain>",
},
+ { "pci-assignable-add",
+ &main_pciassignable_add, 0,
+ "Make a device assignable for pci-passthru",
+ "<BDF>",
+ "-h Print this help.\n"
+ },
+ { "pci-assignable-remove",
+ &main_pciassignable_remove, 0,
+ "Remove a device from being assignable",
+ "[options] <BDF>",
+ "-h Print this help.\n"
+ "-r Attempt to re-assign the device to the\n"
+ " original driver"
+ },
{ "pci-assignable-list",
&main_pciassignable_list, 0,
"List all the assignable pci devices",
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 4 of 4 v2] xl: Add pci_assignable_add and remove commands
2012-05-11 13:31 ` [PATCH 4 of 4 v2] xl: Add pci_assignable_add and remove commands George Dunlap
@ 2012-05-14 9:24 ` Ian Campbell
0 siblings, 0 replies; 10+ messages in thread
From: Ian Campbell @ 2012-05-14 9:24 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel@lists.xensource.com
On Fri, 2012-05-11 at 14:31 +0100, George Dunlap wrote:
> pci-assignable-add will always store the driver rebind path, but
> pci-assignable-remove will only actually rebind if asked to do so.
>
> v2:
> - Use libxl_device_pci_init() instead of memset()
> - Call xlu_cfg_destroy() properly
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(one typo below twice, if you don't end up reposting due to comments on
3/4 I'll fix that up as it goes in)
> +static void pciassignable_add(const char *bdf, int rebind)
> +{
> + libxl_device_pci pcidev;
> + XLU_Config *config;
> +
> + libxl_device_pci_init(&pcidev);
> +
> + config = xlu_cfg_init(stderr, "command line");
> + if (!config) { perror("xlu_cfg_inig"); exit(-1); }
init
[...]
> +static void pciassignable_remove(const char *bdf, int rebind)
> +{
> + libxl_device_pci pcidev;
> + XLU_Config *config;
> +
> + libxl_device_pci_init(&pcidev);
> +
> + config = xlu_cfg_init(stderr, "command line");
> + if (!config) { perror("xlu_cfg_inig"); exit(-1); }
And again.
Ian.
^ permalink raw reply [flat|nested] 10+ messages in thread