* [PATCH v7 1/7] docs/man: modify xl(1) in preparation for naming of assignable devices
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
@ 2021-01-05 17:46 ` Paul Durrant
2021-01-05 17:46 ` [PATCH v7 2/7] libxlu: introduce xlu_pci_parse_spec_string() Paul Durrant
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2021-01-05 17:46 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Ian Jackson
From: Paul Durrant <pdurrant@amazon.com>
A subsequent patch will introduce code to allow a name to be specified to
'xl pci-assignable-add' such that the assignable device may be referred to
by than name in subsequent operations.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
---
Cc: Ian Jackson <iwj@xenproject.org>
---
docs/man/xl.1.pod.in | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index af31d2b572..f4779d8fd6 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -1595,19 +1595,23 @@ List virtual network interfaces for a domain.
=over 4
-=item B<pci-assignable-list>
+=item B<pci-assignable-list> [I<-n>]
List all the B<BDF> of assignable PCI devices. See
-L<xl-pci-configuration(5)> for more information.
+L<xl-pci-configuration(5)> for more information. If the -n option is
+specified then any name supplied when the device was made assignable
+will also be displayed.
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-assignable-add> I<BDF>
+=item B<pci-assignable-add> [I<-n NAME>] I<BDF>
Make the device at B<BDF> assignable to guests. See
-L<xl-pci-configuration(5)> for more information.
+L<xl-pci-configuration(5)> for more information. If the -n option is
+supplied then the assignable device entry will the named with the
+given B<NAME>.
This will bind the device to the pciback driver and assign it to the
"quarantine domain". If it is already bound to a driver, it will
@@ -1622,10 +1626,11 @@ 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>
+=item B<pci-assignable-remove> [I<-r>] I<BDF>|I<NAME>
-Make the device at B<BDF> not assignable to guests. See
-L<xl-pci-configuration(5)> for more information.
+Make a device non-assignable to guests. The device may be identified
+either by its B<BDF> or the B<NAME> supplied when the device was made
+assignable. See L<xl-pci-configuration(5)> for more information.
This will at least unbind the device from pciback, and
re-assign it from the "quarantine domain" back to domain 0. If the -r
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v7 2/7] libxlu: introduce xlu_pci_parse_spec_string()
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
2021-01-05 17:46 ` [PATCH v7 1/7] docs/man: modify xl(1) in preparation for naming of assignable devices Paul Durrant
@ 2021-01-05 17:46 ` Paul Durrant
2021-01-05 17:46 ` [PATCH v7 3/7] libxl: stop setting 'vdevfn' in pci_struct_fill() Paul Durrant
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2021-01-05 17:46 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Ian Jackson, Anthony PERARD
From: Paul Durrant <pdurrant@amazon.com>
This patch largely re-writes the code to parse a PCI_SPEC_STRING and enters
it via the newly introduced function. The new parser also deals with 'bdf'
and 'vslot' as non-positional paramaters, as per the documentation in
xl-pci-configuration(5).
The existing xlu_pci_parse_bdf() function remains, but now strictly parses
BDF values. Some existing callers of xlu_pci_parse_bdf() are
modified to call xlu_pci_parse_spec_string() as per the documentation in xl(1).
NOTE: Usage text in xl_cmdtable.c and error messages are also modified
appropriately.
As a side-effect this patch also fixes a bug where using '*' to specify
all functions would lead to an assertion failure at the end of
xlu_pci_parse_bdf().
Fixes: d25cc3ec93eb ("libxl: workaround gcc 10.2 maybe-uninitialized warning")
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
---
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
v7:
- Minor modifications now that the 'libxl_pci_bdf' type no longer exists
- Modifications are largely mechanical so Wei's A-b is retained
---
tools/include/libxlutil.h | 8 +-
tools/libs/util/libxlu_pci.c | 348 +++++++++++++++++++++++--------------------
tools/xl/xl_cmdtable.c | 4 +-
tools/xl/xl_parse.c | 4 +-
tools/xl/xl_pci.c | 33 ++--
5 files changed, 213 insertions(+), 184 deletions(-)
diff --git a/tools/include/libxlutil.h b/tools/include/libxlutil.h
index 92e35c5462..4dd3c5e92b 100644
--- a/tools/include/libxlutil.h
+++ b/tools/include/libxlutil.h
@@ -109,9 +109,15 @@ int xlu_disk_parse(XLU_Config *cfg, int nspecs, const char *const *specs,
*/
/*
+ * PCI BDF
+ */
+int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pci, const char *str);
+
+/*
* PCI specification parsing
*/
-int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str);
+int xlu_pci_parse_spec_string(XLU_Config *cfg, libxl_device_pci *pci,
+ const char *str);
/*
* RDM parsing
diff --git a/tools/libs/util/libxlu_pci.c b/tools/libs/util/libxlu_pci.c
index 1d38fffce3..05472a0bd1 100644
--- a/tools/libs/util/libxlu_pci.c
+++ b/tools/libs/util/libxlu_pci.c
@@ -1,5 +1,7 @@
#define _GNU_SOURCE
+#include <ctype.h>
+
#include "libxlu_internal.h"
#include "libxlu_disk_l.h"
#include "libxlu_disk_i.h"
@@ -9,185 +11,209 @@
#define XLU__PCI_ERR(_c, _x, _a...) \
if((_c) && (_c)->report) fprintf((_c)->report, _x, ##_a)
-static int hex_convert(const char *str, unsigned int *val, unsigned int mask)
+static int parse_bdf(libxl_device_pci *pci, const char *str, const char **endp)
{
- unsigned long ret;
- char *end;
-
- ret = strtoul(str, &end, 16);
- if ( end == str || *end != '\0' )
- return -1;
- if ( ret & ~mask )
- return -1;
- *val = (unsigned int)ret & mask;
- return 0;
-}
+ const char *ptr = str;
+ unsigned int colons = 0;
+ unsigned int domain, bus, dev, func;
+ int n;
+
+ /* Count occurrences of ':' to detrmine presence/absence of the 'domain' */
+ while (isxdigit(*ptr) || *ptr == ':') {
+ if (*ptr == ':')
+ colons++;
+ ptr++;
+ }
+
+ ptr = str;
+ switch (colons) {
+ case 1:
+ domain = 0;
+ if (sscanf(ptr, "%x:%x.%n", &bus, &dev, &n) != 2)
+ return ERROR_INVAL;
+ break;
+ case 2:
+ if (sscanf(ptr, "%x:%x:%x.%n", &domain, &bus, &dev, &n) != 3)
+ return ERROR_INVAL;
+ break;
+ default:
+ return ERROR_INVAL;
+ }
+
+ if (domain > 0xffff || bus > 0xff || dev > 0x1f)
+ return ERROR_INVAL;
+
+ ptr += n;
+ if (*ptr == '*') {
+ pci->vfunc_mask = LIBXL_PCI_FUNC_ALL;
+ func = 0;
+ ptr++;
+ } else {
+ if (sscanf(ptr, "%x%n", &func, &n) != 1)
+ return ERROR_INVAL;
+ if (func > 7)
+ return ERROR_INVAL;
+ pci->vfunc_mask = 1;
+ ptr += n;
+ }
-static int pci_struct_fill(libxl_device_pci *pci, unsigned int domain,
- unsigned int bus, unsigned int dev,
- unsigned int func, unsigned int vdevfn)
-{
pci->domain = domain;
pci->bus = bus;
pci->dev = dev;
pci->func = func;
- pci->vdevfn = vdevfn;
+
+ if (endp)
+ *endp = ptr;
+
return 0;
}
-#define STATE_DOMAIN 0
-#define STATE_BUS 1
-#define STATE_DEV 2
-#define STATE_FUNC 3
-#define STATE_VSLOT 4
-#define STATE_OPTIONS_K 6
-#define STATE_OPTIONS_V 7
-#define STATE_TERMINAL 8
-#define STATE_TYPE 9
-#define STATE_RDM_STRATEGY 10
-#define STATE_RESERVE_POLICY 11
-#define INVALID 0xffffffff
-int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pci, const char *str)
+static int parse_vslot(uint32_t *vdevfnp, const char *str, const char **endp)
{
- unsigned state = STATE_DOMAIN;
- unsigned dom = INVALID, bus = INVALID, dev = INVALID, func = INVALID, vslot = 0;
- char *buf2, *tok, *ptr, *end, *optkey = NULL;
+ const char *ptr = str;
+ unsigned int val;
+ int n;
+
+ if (sscanf(ptr, "%x%n", &val, &n) != 1)
+ return ERROR_INVAL;
+
+ if (val > 0x1f)
+ return ERROR_INVAL;
+
+ ptr += n;
+
+ *vdevfnp = val << 3;
+
+ if (endp)
+ *endp = ptr;
+
+ return 0;
+}
+
+static int parse_key_val(char **keyp, char**valp, const char *str,
+ const char **endp)
+{
+ const char *ptr = str;
+ char *key, *val;
+
+ while (*ptr != '=' && *ptr != '\0')
+ ptr++;
+
+ if (*ptr == '\0')
+ return ERROR_INVAL;
- if ( NULL == (buf2 = ptr = strdup(str)) )
+ key = strndup(str, ptr - str);
+ if (!key)
return ERROR_NOMEM;
- for(tok = ptr, end = ptr + strlen(ptr) + 1; ptr < end; ptr++) {
- switch(state) {
- case STATE_DOMAIN:
- if ( *ptr == ':' ) {
- state = STATE_BUS;
- *ptr = '\0';
- if ( hex_convert(tok, &dom, 0xffff) )
- goto parse_error;
- tok = ptr + 1;
- }
- break;
- case STATE_BUS:
- if ( *ptr == ':' ) {
- state = STATE_DEV;
- *ptr = '\0';
- if ( hex_convert(tok, &bus, 0xff) )
- goto parse_error;
- tok = ptr + 1;
- }else if ( *ptr == '.' ) {
- state = STATE_FUNC;
- *ptr = '\0';
- if ( dom & ~0xff )
- goto parse_error;
- bus = dom;
- dom = 0;
- if ( hex_convert(tok, &dev, 0xff) )
- goto parse_error;
- tok = ptr + 1;
- }
- break;
- case STATE_DEV:
- if ( *ptr == '.' ) {
- state = STATE_FUNC;
- *ptr = '\0';
- if ( hex_convert(tok, &dev, 0xff) )
- goto parse_error;
- tok = ptr + 1;
- }
- break;
- case STATE_FUNC:
- if ( *ptr == '\0' || *ptr == '@' || *ptr == ',' ) {
- switch( *ptr ) {
- case '\0':
- state = STATE_TERMINAL;
- break;
- case '@':
- state = STATE_VSLOT;
- break;
- case ',':
- state = STATE_OPTIONS_K;
- break;
- }
- *ptr = '\0';
- if ( !strcmp(tok, "*") ) {
- pci->vfunc_mask = LIBXL_PCI_FUNC_ALL;
- }else{
- if ( hex_convert(tok, &func, 0x7) )
- goto parse_error;
- pci->vfunc_mask = (1 << 0);
- }
- tok = ptr + 1;
- }
- break;
- case STATE_VSLOT:
- if ( *ptr == '\0' || *ptr == ',' ) {
- state = ( *ptr == ',' ) ? STATE_OPTIONS_K : STATE_TERMINAL;
- *ptr = '\0';
- if ( hex_convert(tok, &vslot, 0xff) )
- goto parse_error;
- tok = ptr + 1;
- }
- break;
- case STATE_OPTIONS_K:
- if ( *ptr == '=' ) {
- state = STATE_OPTIONS_V;
- *ptr = '\0';
- optkey = tok;
- tok = ptr + 1;
- }
- break;
- case STATE_OPTIONS_V:
- if ( *ptr == ',' || *ptr == '\0' ) {
- state = (*ptr == ',') ? STATE_OPTIONS_K : STATE_TERMINAL;
- *ptr = '\0';
- if ( !strcmp(optkey, "msitranslate") ) {
- pci->msitranslate = atoi(tok);
- }else if ( !strcmp(optkey, "power_mgmt") ) {
- pci->power_mgmt = atoi(tok);
- }else if ( !strcmp(optkey, "permissive") ) {
- pci->permissive = atoi(tok);
- }else if ( !strcmp(optkey, "seize") ) {
- pci->seize = atoi(tok);
- } else if (!strcmp(optkey, "rdm_policy")) {
- if (!strcmp(tok, "strict")) {
- pci->rdm_policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
- } else if (!strcmp(tok, "relaxed")) {
- pci->rdm_policy = LIBXL_RDM_RESERVE_POLICY_RELAXED;
- } else {
- XLU__PCI_ERR(cfg, "%s is not an valid PCI RDM property"
- " policy: 'strict' or 'relaxed'.",
- tok);
- goto parse_error;
- }
- } else {
- XLU__PCI_ERR(cfg, "Unknown PCI BDF option: %s", optkey);
- }
- tok = ptr + 1;
- }
- default:
- break;
+ str = ++ptr; /* skip '=' */
+ while (*ptr != ',' && *ptr != '\0')
+ ptr++;
+
+ val = strndup(str, ptr - str);
+ if (!val) {
+ free(key);
+ return ERROR_NOMEM;
+ }
+
+ if (*ptr == ',')
+ ptr++;
+
+ *keyp = key;
+ *valp = val;
+ *endp = ptr;
+
+ return 0;
+}
+
+static int parse_rdm_policy(XLU_Config *cfg, libxl_rdm_reserve_policy *policy,
+ const char *str)
+{
+ int ret = libxl_rdm_reserve_policy_from_string(str, policy);
+
+ if (ret)
+ XLU__PCI_ERR(cfg, "Unknown RDM policy: %s", str);
+
+ return ret;
+}
+
+int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pci, const char *str)
+{
+ return parse_bdf(pci, str, NULL);
+}
+
+int xlu_pci_parse_spec_string(XLU_Config *cfg, libxl_device_pci *pci,
+ const char *str)
+{
+ const char *ptr = str;
+ bool bdf_present = false;
+ int ret;
+
+ /* Attempt to parse 'bdf' as positional parameter */
+ ret = parse_bdf(pci, ptr, &ptr);
+ if (!ret) {
+ bdf_present = true;
+
+ /* Check whether 'vslot' if present */
+ if (*ptr == '@') {
+ ret = parse_vslot(&pci->vdevfn, ++ptr, &ptr);
+ if (ret)
+ return ret;
}
+ if (*ptr == ',')
+ ptr++;
+ else if (*ptr != '\0')
+ return ERROR_INVAL;
}
- if ( tok != ptr || state != STATE_TERMINAL )
- goto parse_error;
+ /* Parse the rest as 'key=val' pairs */
+ while (*ptr != '\0') {
+ char *key, *val;
- assert(dom != INVALID && bus != INVALID && dev != INVALID && func != INVALID);
+ ret = parse_key_val(&key, &val, ptr, &ptr);
+ if (ret)
+ return ret;
- /* Just a pretty way to fill in the values */
- pci_struct_fill(pci, dom, bus, dev, func, vslot << 3);
+ if (!strcmp(key, "bdf")) {
+ ret = parse_bdf(pci, val, NULL);
+ if (!ret) bdf_present = true;
+ } else if (!strcmp(key, "vslot")) {
+ ret = parse_vslot(&pci->vdevfn, val, NULL);
+ } else if (!strcmp(key, "permissive")) {
+ pci->permissive = atoi(val);
+ } else if (!strcmp(key, "msitranslate")) {
+ pci->msitranslate = atoi(val);
+ } else if (!strcmp(key, "seize")) {
+ pci->seize= atoi(val);
+ } else if (!strcmp(key, "power_mgmt")) {
+ pci->power_mgmt = atoi(val);
+ } else if (!strcmp(key, "rdm_policy")) {
+ ret = parse_rdm_policy(cfg, &pci->rdm_policy, val);
+ } else {
+ XLU__PCI_ERR(cfg, "Unknown PCI_SPEC_STRING option: %s", key);
+ ret = ERROR_INVAL;
+ }
- free(buf2);
+ free(key);
+ free(val);
- return 0;
+ if (ret)
+ return ret;
+ }
-parse_error:
- free(buf2);
- return ERROR_INVAL;
+ if (!bdf_present)
+ return ERROR_INVAL;
+
+ return 0;
}
int xlu_rdm_parse(XLU_Config *cfg, libxl_rdm_reserve *rdm, const char *str)
{
+#define STATE_TYPE 0
+#define STATE_RDM_STRATEGY 1
+#define STATE_RESERVE_POLICY 2
+#define STATE_TERMINAL 3
+
unsigned state = STATE_TYPE;
char *buf2, *tok, *ptr, *end;
@@ -227,15 +253,8 @@ int xlu_rdm_parse(XLU_Config *cfg, libxl_rdm_reserve *rdm, const char *str)
if (*ptr == ',' || *ptr == '\0') {
state = *ptr == ',' ? STATE_TYPE : STATE_TERMINAL;
*ptr = '\0';
- if (!strcmp(tok, "strict")) {
- rdm->policy = LIBXL_RDM_RESERVE_POLICY_STRICT;
- } else if (!strcmp(tok, "relaxed")) {
- rdm->policy = LIBXL_RDM_RESERVE_POLICY_RELAXED;
- } else {
- XLU__PCI_ERR(cfg, "Unknown RDM property policy value: %s",
- tok);
+ if (!parse_rdm_policy(cfg, &rdm->policy, tok))
goto parse_error;
- }
tok = ptr + 1;
}
default:
@@ -253,6 +272,11 @@ int xlu_rdm_parse(XLU_Config *cfg, libxl_rdm_reserve *rdm, const char *str)
parse_error:
free(buf2);
return ERROR_INVAL;
+
+#undef STATE_TYPE
+#undef STATE_RDM_STRATEGY
+#undef STATE_RESERVE_POLICY
+#undef STATE_TERMINAL
}
/*
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 6ab5e47da3..30e17a2848 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -90,12 +90,12 @@ struct cmd_spec cmd_table[] = {
{ "pci-attach",
&main_pciattach, 0, 1,
"Insert a new pass-through pci device",
- "<Domain> <BDF> [Virtual Slot]",
+ "<Domain> <PCI_SPEC_STRING>",
},
{ "pci-detach",
&main_pcidetach, 0, 1,
"Remove a domain's pass-through pci device",
- "<Domain> <BDF>",
+ "<Domain> <PCI_SPEC_STRING>",
},
{ "pci-list",
&main_pcilist, 0, 0,
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 4ebf39620a..867e4d068a 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1487,10 +1487,10 @@ void parse_config_data(const char *config_source,
* the global policy by default.
*/
pci->rdm_policy = b_info->u.hvm.rdm.policy;
- e = xlu_pci_parse_bdf(config, pci, buf);
+ e = xlu_pci_parse_spec_string(config, pci, buf);
if (e) {
fprintf(stderr,
- "unable to parse PCI BDF `%s' for passthrough\n",
+ "unable to parse PCI_SPEC_STRING `%s' for passthrough\n",
buf);
exit(-e);
}
diff --git a/tools/xl/xl_pci.c b/tools/xl/xl_pci.c
index f71498cbb5..9a66494bb5 100644
--- a/tools/xl/xl_pci.c
+++ b/tools/xl/xl_pci.c
@@ -54,7 +54,7 @@ int main_pcilist(int argc, char **argv)
return 0;
}
-static int pcidetach(uint32_t domid, const char *bdf, int force)
+static int pcidetach(uint32_t domid, const char *spec_string, int force)
{
libxl_device_pci pci;
XLU_Config *config;
@@ -65,8 +65,9 @@ static int pcidetach(uint32_t domid, const char *bdf, int force)
config = xlu_cfg_init(stderr, "command line");
if (!config) { perror("xlu_cfg_inig"); exit(-1); }
- if (xlu_pci_parse_bdf(config, &pci, bdf)) {
- fprintf(stderr, "pci-detach: malformed BDF specification \"%s\"\n", bdf);
+ if (xlu_pci_parse_spec_string(config, &pci, spec_string)) {
+ fprintf(stderr, "pci-detach: malformed PCI_SPEC_STRING \"%s\"\n",
+ spec_string);
exit(2);
}
if (force) {
@@ -88,7 +89,7 @@ int main_pcidetach(int argc, char **argv)
uint32_t domid;
int opt;
int force = 0;
- const char *bdf = NULL;
+ const char *spec_string = NULL;
SWITCH_FOREACH_OPT(opt, "f", NULL, "pci-detach", 2) {
case 'f':
@@ -97,15 +98,15 @@ int main_pcidetach(int argc, char **argv)
}
domid = find_domain(argv[optind]);
- bdf = argv[optind + 1];
+ spec_string = argv[optind + 1];
- if (pcidetach(domid, bdf, force))
+ if (pcidetach(domid, spec_string, force))
return EXIT_FAILURE;
return EXIT_SUCCESS;
}
-static int pciattach(uint32_t domid, const char *bdf, const char *vs)
+static int pciattach(uint32_t domid, const char *spec_string)
{
libxl_device_pci pci;
XLU_Config *config;
@@ -116,8 +117,9 @@ static int pciattach(uint32_t domid, const char *bdf, const char *vs)
config = xlu_cfg_init(stderr, "command line");
if (!config) { perror("xlu_cfg_inig"); exit(-1); }
- if (xlu_pci_parse_bdf(config, &pci, bdf)) {
- fprintf(stderr, "pci-attach: malformed BDF specification \"%s\"\n", bdf);
+ if (xlu_pci_parse_spec_string(config, &pci, spec_string)) {
+ fprintf(stderr, "pci-attach: malformed PCI_SPEC_STRING \"%s\"\n",
+ spec_string);
exit(2);
}
@@ -134,19 +136,16 @@ int main_pciattach(int argc, char **argv)
{
uint32_t domid;
int opt;
- const char *bdf = NULL, *vs = NULL;
+ const char *spec_string = NULL;
SWITCH_FOREACH_OPT(opt, "", NULL, "pci-attach", 2) {
/* No options */
}
domid = find_domain(argv[optind]);
- bdf = argv[optind + 1];
-
- if (optind + 1 < argc)
- vs = argv[optind + 2];
+ spec_string = argv[optind + 1];
- if (pciattach(domid, bdf, vs))
+ if (pciattach(domid, spec_string))
return EXIT_FAILURE;
return EXIT_SUCCESS;
@@ -192,7 +191,7 @@ static int pciassignable_add(const char *bdf, int rebind)
if (!config) { perror("xlu_cfg_init"); exit(-1); }
if (xlu_pci_parse_bdf(config, &pci, bdf)) {
- fprintf(stderr, "pci-assignable-add: malformed BDF specification \"%s\"\n", bdf);
+ fprintf(stderr, "pci-assignable-add: malformed BDF \"%s\"\n", bdf);
exit(2);
}
@@ -234,7 +233,7 @@ static int pciassignable_remove(const char *bdf, int rebind)
if (!config) { perror("xlu_cfg_init"); exit(-1); }
if (xlu_pci_parse_bdf(config, &pci, bdf)) {
- fprintf(stderr, "pci-assignable-remove: malformed BDF specification \"%s\"\n", bdf);
+ fprintf(stderr, "pci-assignable-remove: malformed BDF \"%s\"\n", bdf);
exit(2);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v7 3/7] libxl: stop setting 'vdevfn' in pci_struct_fill()
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
2021-01-05 17:46 ` [PATCH v7 1/7] docs/man: modify xl(1) in preparation for naming of assignable devices Paul Durrant
2021-01-05 17:46 ` [PATCH v7 2/7] libxlu: introduce xlu_pci_parse_spec_string() Paul Durrant
@ 2021-01-05 17:46 ` Paul Durrant
2021-01-21 14:42 ` Wei Liu
2021-01-05 17:46 ` [PATCH v7 4/7] libxl: add 'name' field to 'libxl_device_pci' in the IDL Paul Durrant
` (4 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Paul Durrant @ 2021-01-05 17:46 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Ian Jackson, Wei Liu, Anthony PERARD
From: Paul Durrant <pdurrant@amazon.com>
There are only two call-sites. One always sets it to 0 (which is unnecessary
as the structure is already initialized to zero) and the other can simply set
the 'vdevfn' field directly (after proper structure initialization), avoiding
the need for a local variable.
A subsequent patch will also make use of pci_struct_fill() in a context
where 'vdevfn' may already have been set.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
v7:
- New in v7
---
tools/libs/light/libxl_pci.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 74c2196ae3..6feedadc62 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -39,13 +39,12 @@ static unsigned int pci_encode_bdf(libxl_device_pci *pci)
static void pci_struct_fill(libxl_device_pci *pci, unsigned int domain,
unsigned int bus, unsigned int dev,
- unsigned int func, unsigned int vdevfn)
+ unsigned int func)
{
pci->domain = domain;
pci->bus = bus;
pci->dev = dev;
pci->func = func;
- pci->vdevfn = vdevfn;
}
static void libxl_create_pci_backend_device(libxl__gc *gc,
@@ -451,7 +450,7 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
new = pcis + *num;
libxl_device_pci_init(new);
- pci_struct_fill(new, dom, bus, dev, func, 0);
+ pci_struct_fill(new, dom, bus, dev, func);
if (pci_info_xs_read(gc, new, "domid")) /* already assigned */
continue;
@@ -2288,17 +2287,19 @@ static int libxl__device_pci_from_xs_be(libxl__gc *gc,
libxl_devid nr, void *data)
{
char *s;
- unsigned int domain = 0, bus = 0, dev = 0, func = 0, vdevfn = 0;
+ unsigned int domain = 0, bus = 0, dev = 0, func = 0;
libxl_device_pci *pci = data;
+ libxl_device_pci_init(pci);
+
s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/dev-%d", be_path, nr));
sscanf(s, PCI_BDF, &domain, &bus, &dev, &func);
+ pci_struct_fill(pci, domain, bus, dev, func);
+
s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/vdevfn-%d", be_path, nr));
if (s)
- vdevfn = strtol(s, (char **) NULL, 16);
-
- pci_struct_fill(pci, domain, bus, dev, func, vdevfn);
+ pci->vdevfn = strtol(s, (char **) NULL, 16);
s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/opts-%d", be_path, nr));
if (s) {
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 3/7] libxl: stop setting 'vdevfn' in pci_struct_fill()
2021-01-05 17:46 ` [PATCH v7 3/7] libxl: stop setting 'vdevfn' in pci_struct_fill() Paul Durrant
@ 2021-01-21 14:42 ` Wei Liu
0 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2021-01-21 14:42 UTC (permalink / raw)
To: Paul Durrant
Cc: xen-devel, Paul Durrant, Ian Jackson, Wei Liu, Anthony PERARD
On Tue, Jan 05, 2021 at 05:46:38PM +0000, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
>
> There are only two call-sites. One always sets it to 0 (which is unnecessary
> as the structure is already initialized to zero) and the other can simply set
> the 'vdevfn' field directly (after proper structure initialization), avoiding
> the need for a local variable.
>
> A subsequent patch will also make use of pci_struct_fill() in a context
> where 'vdevfn' may already have been set.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 4/7] libxl: add 'name' field to 'libxl_device_pci' in the IDL...
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
` (2 preceding siblings ...)
2021-01-05 17:46 ` [PATCH v7 3/7] libxl: stop setting 'vdevfn' in pci_struct_fill() Paul Durrant
@ 2021-01-05 17:46 ` Paul Durrant
2021-01-21 14:45 ` Wei Liu
2021-01-05 17:46 ` [PATCH v7 5/7] xl: support naming of assignable devices Paul Durrant
` (3 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Paul Durrant @ 2021-01-05 17:46 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Ian Jackson, Wei Liu, Anthony PERARD
From: Paul Durrant <pdurrant@amazon.com>
... and modify libxl_pci_bdf_assignable_add/remove/list() to make use of it.
libxl_pci_bdf_assignable_add() will store the name of the device in xenstore
if the field is specified (i.e. non-NULL) and libxl_pci_bdf_assignable_remove()
will remove devices specified only by name, looking up the BDF as necessary.
libxl_pci_bdf_assignable_list() will also populate the 'name' field if a name
was stored by libxl_pci_bdf_assignable_add().
NOTE: This patch also fixes whitespace in the declaration of 'libxl_device_pci'
in the IDL.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
v7:
- New in v7
---
tools/include/libxl.h | 6 ++++
tools/libs/light/libxl_pci.c | 77 ++++++++++++++++++++++++++++++++++++++--
tools/libs/light/libxl_types.idl | 13 +++----
3 files changed, 87 insertions(+), 9 deletions(-)
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 3433c950f9..3488fbf56f 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -464,6 +464,12 @@
#define LIBXL_HAVE_DEVICE_PCI_ASSIGNABLE_LIST_FREE 1
/*
+ * LIBXL_HAVE_DEVICE_PCI_NAME indicates that the libxl_device_pci type
+ * has a name field.
+ */
+#define LIBXL_HAVE_DEVICE_PCI_NAME 1
+
+/*
* libxl ABI compatibility
*
* The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 6feedadc62..9e3a90dcda 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -438,7 +438,9 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
}
while((de = readdir(dir))) {
- unsigned dom, bus, dev, func;
+ unsigned int dom, bus, dev, func;
+ char *name;
+
if (sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4)
continue;
@@ -455,6 +457,9 @@ libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
if (pci_info_xs_read(gc, new, "domid")) /* already assigned */
continue;
+ name = pci_info_xs_read(gc, new, "name");
+ if (name) new->name = strdup(name);
+
(*num)++;
}
@@ -742,6 +747,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned dom, bus, dev, func;
char *spath, *driver_path = NULL;
+ const char *name;
int rc;
struct stat st;
@@ -750,6 +756,24 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
bus = pci->bus;
dev = pci->dev;
func = pci->func;
+ name = pci->name;
+
+ /* Sanitise any name that is set */
+ if (name) {
+ unsigned int i, n = strlen(name);
+
+ if (n > 64) { /* Reasonable upper bound on name length */
+ LOG(ERROR, "Name too long");
+ return ERROR_FAIL;
+ }
+
+ for (i = 0; i < n; i++) {
+ if (!isgraph(name[i])) {
+ LOG(ERROR, "Names may only include printable characters");
+ return ERROR_FAIL;
+ }
+ }
+ }
/* See if the device exists */
spath = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF, dom, bus, dev, func);
@@ -765,7 +789,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
}
if ( rc ) {
LOG(WARN, PCI_BDF" already assigned to pciback", dom, bus, dev, func);
- goto quarantine;
+ goto name;
}
/* Check to see if there's already a driver that we need to unbind from */
@@ -796,7 +820,12 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
return ERROR_FAIL;
}
-quarantine:
+name:
+ if (name)
+ pci_info_xs_write(gc, pci, "name", name);
+ else
+ pci_info_xs_remove(gc, pci, "name");
+
/*
* DOMID_IO is just a sentinel domain, without any actual mappings,
* so always pass XEN_DOMCTL_DEV_RDM_RELAXED to avoid assignment being
@@ -812,6 +841,40 @@ quarantine:
return 0;
}
+static int name2bdf(libxl__gc *gc, libxl_device_pci *pci)
+{
+ char **bdfs;
+ unsigned int i, n;
+ int rc = ERROR_NOTFOUND;
+
+ bdfs = libxl__xs_directory(gc, XBT_NULL, PCI_INFO_PATH, &n);
+ if (!n)
+ goto out;
+
+ for (i = 0; i < n; i++) {
+ unsigned dom, bus, dev, func;
+ char *name;
+
+ if (sscanf(bdfs[i], PCI_BDF_XSPATH, &dom, &bus, &dev, &func) != 4)
+ continue;
+
+ pci_struct_fill(pci, dom, bus, dev, func);
+
+ name = pci_info_xs_read(gc, pci, "name");
+ if (name && !strcmp(name, pci->name)) {
+ rc = 0;
+ break;
+ }
+ }
+
+out:
+ if (!rc)
+ LOG(DETAIL, "'%s' -> " PCI_BDF, pci->name, pci->domain,
+ pci->bus, pci->dev, pci->func);
+
+ return rc;
+}
+
static int libxl__device_pci_assignable_remove(libxl__gc *gc,
libxl_device_pci *pci,
int rebind)
@@ -820,6 +883,12 @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
int rc;
char *driver_path;
+ /* If the device is named then we need to look up the BDF */
+ if (pci->name) {
+ rc = name2bdf(gc, pci);
+ if (rc) return rc;
+ }
+
/* De-quarantine */
rc = xc_deassign_device(ctx->xch, DOMID_IO, pci_encode_bdf(pci));
if ( rc < 0 ) {
@@ -860,6 +929,8 @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
}
}
+ pci_info_xs_remove(gc, pci, "name");
+
return 0;
}
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 05324736b7..b4a9076b85 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -771,17 +771,18 @@ libxl_device_nic = Struct("device_nic", [
])
libxl_device_pci = Struct("device_pci", [
- ("func", uint8),
- ("dev", uint8),
- ("bus", uint8),
- ("domain", integer),
- ("vdevfn", uint32),
+ ("func", uint8),
+ ("dev", uint8),
+ ("bus", uint8),
+ ("domain", integer),
+ ("vdevfn", uint32),
("vfunc_mask", uint32),
("msitranslate", bool),
("power_mgmt", bool),
("permissive", bool),
("seize", bool),
- ("rdm_policy", libxl_rdm_reserve_policy),
+ ("rdm_policy", libxl_rdm_reserve_policy),
+ ("name", string),
])
libxl_device_rdm = Struct("device_rdm", [
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 4/7] libxl: add 'name' field to 'libxl_device_pci' in the IDL...
2021-01-05 17:46 ` [PATCH v7 4/7] libxl: add 'name' field to 'libxl_device_pci' in the IDL Paul Durrant
@ 2021-01-21 14:45 ` Wei Liu
0 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2021-01-21 14:45 UTC (permalink / raw)
To: Paul Durrant
Cc: xen-devel, Paul Durrant, Ian Jackson, Wei Liu, Anthony PERARD
On Tue, Jan 05, 2021 at 05:46:39PM +0000, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
>
> ... and modify libxl_pci_bdf_assignable_add/remove/list() to make use of it.
>
> libxl_pci_bdf_assignable_add() will store the name of the device in xenstore
> if the field is specified (i.e. non-NULL) and libxl_pci_bdf_assignable_remove()
> will remove devices specified only by name, looking up the BDF as necessary.
>
> libxl_pci_bdf_assignable_list() will also populate the 'name' field if a name
> was stored by libxl_pci_bdf_assignable_add().
>
> NOTE: This patch also fixes whitespace in the declaration of 'libxl_device_pci'
> in the IDL.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 5/7] xl: support naming of assignable devices
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
` (3 preceding siblings ...)
2021-01-05 17:46 ` [PATCH v7 4/7] libxl: add 'name' field to 'libxl_device_pci' in the IDL Paul Durrant
@ 2021-01-05 17:46 ` Paul Durrant
2021-01-21 14:49 ` Wei Liu
2021-01-05 17:46 ` [PATCH v7 6/7] docs/man: modify xl-pci-configuration(5) to add 'name' field to PCI_SPEC_STRING Paul Durrant
` (2 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Paul Durrant @ 2021-01-05 17:46 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Ian Jackson, Wei Liu, Anthony PERARD
From: Paul Durrant <pdurrant@amazon.com>
With this patch applied 'xl pci-assignable-add' will take an optional '--name'
parameter, 'xl pci-assignable-remove' can be passed either a BDF or a name and
'xl pci-assignable-list' will take a optional '--show-names' flag which
determines whether names are displayed in its output.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
v7:
- Heavily re-worked, bearing only some resemblance to similarly named v6 patch
---
tools/xl/xl_cmdtable.c | 12 ++++---
tools/xl/xl_pci.c | 89 ++++++++++++++++++++++++++++++++++++--------------
2 files changed, 73 insertions(+), 28 deletions(-)
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 30e17a2848..bd8af12ff3 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -105,21 +105,25 @@ struct cmd_spec cmd_table[] = {
{ "pci-assignable-add",
&main_pciassignable_add, 0, 1,
"Make a device assignable for pci-passthru",
- "<BDF>",
+ "[options] <BDF>",
+ "-n NAME, --name=NAME Name the assignable device.\n"
"-h Print this help.\n"
},
{ "pci-assignable-remove",
&main_pciassignable_remove, 0, 1,
"Remove a device from being assignable",
- "[options] <BDF>",
+ "[options] <BDF>|NAME",
"-h Print this help.\n"
"-r Attempt to re-assign the device to the\n"
- " original driver"
+ " original driver."
},
{ "pci-assignable-list",
&main_pciassignable_list, 0, 0,
"List all the assignable pci devices",
- "",
+ "[options]",
+ "-h Print this help.\n"
+ "-n, --show-names Display assignable device names where\n"
+ " supplied.\n"
},
{ "pause",
&main_pause, 0, 1,
diff --git a/tools/xl/xl_pci.c b/tools/xl/xl_pci.c
index 9a66494bb5..b1c3ae2a72 100644
--- a/tools/xl/xl_pci.c
+++ b/tools/xl/xl_pci.c
@@ -151,7 +151,7 @@ int main_pciattach(int argc, char **argv)
return EXIT_SUCCESS;
}
-static void pciassignable_list(void)
+static void pciassignable_list(bool show_names)
{
libxl_device_pci *pcis;
int num, i;
@@ -161,8 +161,12 @@ static void pciassignable_list(void)
if ( pcis == NULL )
return;
for (i = 0; i < num; i++) {
- printf("%04x:%02x:%02x.%01x\n",
- pcis[i].domain, pcis[i].bus, pcis[i].dev, pcis[i].func);
+ libxl_device_pci *pci = &pcis[i];
+ char *name = show_names ? pci->name : NULL;
+
+ printf("%04x:%02x:%02x.%01x %s\n",
+ pci->domain, pci->bus, pci->dev, pci->func,
+ name ?: "");
}
libxl_device_pci_assignable_list_free(pcis, num);
}
@@ -170,20 +174,27 @@ static void pciassignable_list(void)
int main_pciassignable_list(int argc, char **argv)
{
int opt;
-
- SWITCH_FOREACH_OPT(opt, "", NULL, "pci-assignable-list", 0) {
- /* No options */
+ static struct option opts[] = {
+ {"show-names", 0, 0, 'n'},
+ COMMON_LONG_OPTS
+ };
+ bool show_names = false;
+
+ SWITCH_FOREACH_OPT(opt, "n", opts, "pci-assignable-list", 0) {
+ case 'n':
+ show_names = true;
+ break;
}
- pciassignable_list();
+ pciassignable_list(show_names);
return 0;
}
-static int pciassignable_add(const char *bdf, int rebind)
+static int pciassignable_add(const char *bdf, const char *name, int rebind)
{
libxl_device_pci pci;
XLU_Config *config;
- int r = 0;
+ int r;
libxl_device_pci_init(&pci);
@@ -195,8 +206,15 @@ static int pciassignable_add(const char *bdf, int rebind)
exit(2);
}
- if (libxl_device_pci_assignable_add(ctx, &pci, rebind))
- r = 1;
+ if (name) {
+ pci.name = strdup(name);
+ if (!pci.name) {
+ fprintf(stderr, "pci-assignable-add: memory allocation failure\n");
+ exit(2);
+ }
+ }
+
+ r = libxl_device_pci_assignable_add(ctx, &pci, rebind);
libxl_device_pci_dispose(&pci);
xlu_cfg_destroy(config);
@@ -208,38 +226,61 @@ int main_pciassignable_add(int argc, char **argv)
{
int opt;
const char *bdf = NULL;
-
- SWITCH_FOREACH_OPT(opt, "", NULL, "pci-assignable-add", 1) {
- /* No options */
+ static struct option opts[] = {
+ {"name", 1, 0, 'n'},
+ COMMON_LONG_OPTS
+ };
+ const char *name = NULL;
+
+ SWITCH_FOREACH_OPT(opt, "n:", opts, "pci-assignable-add", 1) {
+ case 'n':
+ name = optarg;
+ break;
}
bdf = argv[optind];
- if (pciassignable_add(bdf, 1))
+ if (pciassignable_add(bdf, name, 1))
return EXIT_FAILURE;
return EXIT_SUCCESS;
}
-static int pciassignable_remove(const char *bdf, int rebind)
+static int pciassignable_remove(const char *ident, int rebind)
{
libxl_device_pci pci;
XLU_Config *config;
- int r = 0;
+ int r;
libxl_device_pci_init(&pci);
config = xlu_cfg_init(stderr, "command line");
if (!config) { perror("xlu_cfg_init"); exit(-1); }
- if (xlu_pci_parse_bdf(config, &pci, bdf)) {
- fprintf(stderr, "pci-assignable-remove: malformed BDF \"%s\"\n", bdf);
+ /* Try remove-by-name first */
+ pci.name = strdup(ident);
+ if (!pci.name) {
+ fprintf(stderr, "pci-assignable-add: memory allocation failure\n");
exit(2);
}
- if (libxl_device_pci_assignable_remove(ctx, &pci, rebind))
- r = 1;
+ r = libxl_device_pci_assignable_remove(ctx, &pci, rebind);
+ if (!r || r != ERROR_NOTFOUND)
+ goto out;
+
+ /* If remove-by-name failed to find the device, try remove-by-BDF */
+ free(pci.name);
+ pci.name = NULL;
+ if (xlu_pci_parse_bdf(config, &pci, ident)) {
+ fprintf(stderr,
+ "pci-assignable-remove: malformed BDF '%s'\n", ident);
+ exit(2);
+ }
+
+ r = libxl_device_pci_assignable_remove(ctx, &pci, rebind);
+
+out:
libxl_device_pci_dispose(&pci);
xlu_cfg_destroy(config);
@@ -249,7 +290,7 @@ static int pciassignable_remove(const char *bdf, int rebind)
int main_pciassignable_remove(int argc, char **argv)
{
int opt;
- const char *bdf = NULL;
+ const char *ident = NULL;
int rebind = 0;
SWITCH_FOREACH_OPT(opt, "r", NULL, "pci-assignable-remove", 1) {
@@ -258,9 +299,9 @@ int main_pciassignable_remove(int argc, char **argv)
break;
}
- bdf = argv[optind];
+ ident = argv[optind];
- if (pciassignable_remove(bdf, rebind))
+ if (pciassignable_remove(ident, rebind))
return EXIT_FAILURE;
return EXIT_SUCCESS;
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 5/7] xl: support naming of assignable devices
2021-01-05 17:46 ` [PATCH v7 5/7] xl: support naming of assignable devices Paul Durrant
@ 2021-01-21 14:49 ` Wei Liu
0 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2021-01-21 14:49 UTC (permalink / raw)
To: Paul Durrant
Cc: xen-devel, Paul Durrant, Ian Jackson, Wei Liu, Anthony PERARD
On Tue, Jan 05, 2021 at 05:46:40PM +0000, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
>
> With this patch applied 'xl pci-assignable-add' will take an optional '--name'
> parameter, 'xl pci-assignable-remove' can be passed either a BDF or a name and
> 'xl pci-assignable-list' will take a optional '--show-names' flag which
> determines whether names are displayed in its output.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v7 6/7] docs/man: modify xl-pci-configuration(5) to add 'name' field to PCI_SPEC_STRING
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
` (4 preceding siblings ...)
2021-01-05 17:46 ` [PATCH v7 5/7] xl: support naming of assignable devices Paul Durrant
@ 2021-01-05 17:46 ` Paul Durrant
2021-01-05 17:46 ` [PATCH v7 7/7] libxl / libxlu: support 'xl pci-attach/detach' by name Paul Durrant
2021-01-13 8:05 ` [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Oleksandr Andrushchenko
7 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2021-01-05 17:46 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Ian Jackson
From: Paul Durrant <pdurrant@amazon.com>
Since assignable devices can be named, a subsequent patch will support use
of a PCI_SPEC_STRING containing a 'name' parameter instead of a 'bdf'. In
this case the name will be used to look up the 'bdf' in the list of assignable
(or assigned) devices.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
---
Cc: Ian Jackson <iwj@xenproject.org>
---
docs/man/xl-pci-configuration.5.pod | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/docs/man/xl-pci-configuration.5.pod b/docs/man/xl-pci-configuration.5.pod
index 4dd73bc498..db3360307c 100644
--- a/docs/man/xl-pci-configuration.5.pod
+++ b/docs/man/xl-pci-configuration.5.pod
@@ -51,7 +51,7 @@ is not specified, or if it is specified with an empty value (whether
positionally or explicitly).
B<NOTE>: In context of B<xl pci-detach> (see L<xl(1)>), parameters other than
-B<bdf> will be ignored.
+B<bdf> or B<name> will be ignored.
=head1 Positional Parameters
@@ -70,7 +70,11 @@ B<*> to indicate all functions of a multi-function device.
=item Default Value
-None. This parameter is mandatory as it identifies the device.
+None. This parameter is mandatory in its positional form. As a non-positional
+parameter it is also mandatory unless a B<name> parameter is present, in
+which case B<bdf> must not be present since the B<name> will be used to find
+the B<bdf> in the list of assignable devices. See L<xl(1)> for more information
+on naming assignable devices.
=back
@@ -194,4 +198,21 @@ B<NOTE>: This overrides the global B<rdm> option.
=back
+=item B<name>=I<STRING>
+
+=over 4
+
+=item Description
+
+This is the name given when the B<BDF> was made assignable. See L<xl(1)> for
+more information on naming assignable devices.
+
+=item Default Value
+
+None. This parameter must not be present if a B<bdf> parameter is present.
+If a B<bdf> parameter is not present then B<name> is mandatory as it is
+required to look up the B<BDF> in the list of assignable devices.
+
+=back
+
=back
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v7 7/7] libxl / libxlu: support 'xl pci-attach/detach' by name
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
` (5 preceding siblings ...)
2021-01-05 17:46 ` [PATCH v7 6/7] docs/man: modify xl-pci-configuration(5) to add 'name' field to PCI_SPEC_STRING Paul Durrant
@ 2021-01-05 17:46 ` Paul Durrant
2021-01-21 14:50 ` Wei Liu
2021-01-13 8:05 ` [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Oleksandr Andrushchenko
7 siblings, 1 reply; 14+ messages in thread
From: Paul Durrant @ 2021-01-05 17:46 UTC (permalink / raw)
To: xen-devel; +Cc: Paul Durrant, Wei Liu, Ian Jackson, Anthony PERARD
From: Paul Durrant <pdurrant@amazon.com>
This patch modifies libxlu_pci_parse_spec_string() to parse the new 'name'
parameter of PCI_SPEC_STRING detailed in the updated documention in
xl-pci-configuration(5) and populate the 'name' field of 'libxl_device_pci'.
If the 'name' field is non-NULL then both libxl_device_pci_add() and
libxl_device_pci_remove() will use it to look up the device BDF in
the list of assignable devices.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
v7:
- Re-worked (mainly because 'libxl_device_pci' already has a 'name' field)
- Dropped Wei's A-b because of the re-work
v6:
- Re-base
- Slight modification to the patch name
- Kept Wei's A-b since modifications are small
---
tools/libs/light/libxl_pci.c | 43 ++++++++++++++++++++++++++++++++++++++-----
tools/libs/util/libxlu_pci.c | 7 ++++++-
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index 9e3a90dcda..1a1c263080 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -58,6 +58,8 @@ static void libxl_create_pci_backend_device(libxl__gc *gc,
flexarray_append(back, GCSPRINTF(PCI_BDF, pci->domain, pci->bus, pci->dev, pci->func));
if (pci->vdevfn)
flexarray_append_pair(back, GCSPRINTF("vdevfn-%d", num), GCSPRINTF("%x", pci->vdevfn));
+ if (pci->name)
+ flexarray_append_pair(back, GCSPRINTF("name-%d", num), GCSPRINTF("%s", pci->name));
flexarray_append(back, GCSPRINTF("opts-%d", num));
flexarray_append(back,
GCSPRINTF("msitranslate=%d,power_mgmt=%d,permissive=%d,rdm_policy=%s",
@@ -282,6 +284,7 @@ retry_transaction2:
xs_rm(ctx->xsh, t, GCSPRINTF("%s/vdev-%d", be_path, i));
xs_rm(ctx->xsh, t, GCSPRINTF("%s/opts-%d", be_path, i));
xs_rm(ctx->xsh, t, GCSPRINTF("%s/vdevfn-%d", be_path, i));
+ xs_rm(ctx->xsh, t, GCSPRINTF("%s/name-%d", be_path, i));
libxl__xs_printf(gc, t, num_devs_path, "%d", num - 1);
for (j = i + 1; j < num; j++) {
tmppath = GCSPRINTF("%s/state-%d", be_path, j);
@@ -314,6 +317,12 @@ retry_transaction2:
xs_write(ctx->xsh, t, GCSPRINTF("%s/vdevfn-%d", be_path, j - 1), tmp, strlen(tmp));
xs_rm(ctx->xsh, t, tmppath);
}
+ tmppath = GCSPRINTF("%s/name-%d", be_path, j);
+ tmp = libxl__xs_read(gc, t, tmppath);
+ if (tmp) {
+ xs_write(ctx->xsh, t, GCSPRINTF("%s/name-%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)
@@ -1589,6 +1598,12 @@ void libxl__device_pci_add(libxl__egc *egc, uint32_t domid,
libxl_device_pci_copy(CTX, &pas->pci, pci);
pci = &pas->pci;
+ /* If the device is named then we need to look up the BDF */
+ if (pci->name) {
+ rc = name2bdf(gc, pci);
+ if (rc) goto out;
+ }
+
pas->starting = starting;
pas->callback = device_pci_add_stubdom_done;
@@ -1739,11 +1754,19 @@ static void device_pci_add_done(libxl__egc *egc,
libxl_device_pci *pci = &pas->pci;
if (rc) {
- LOGD(ERROR, domid,
- "libxl__device_pci_add failed for "
- "PCI device %x:%x:%x.%x (rc %d)",
- pci->domain, pci->bus, pci->dev, pci->func,
- rc);
+ if (pci->name) {
+ LOGD(ERROR, domid,
+ "libxl__device_pci_add failed for "
+ "PCI device '%s' (rc %d)",
+ pci->name,
+ rc);
+ } else {
+ LOGD(ERROR, domid,
+ "libxl__device_pci_add failed for "
+ "PCI device %x:%x:%x.%x (rc %d)",
+ pci->domain, pci->bus, pci->dev, pci->func,
+ rc);
+ }
pci_info_xs_remove(gc, pci, "domid");
}
libxl_device_pci_dispose(pci);
@@ -2250,6 +2273,12 @@ static void libxl__device_pci_remove_common(libxl__egc *egc,
libxl_device_pci_copy(CTX, &prs->pci, pci);
pci = &prs->pci;
+ /* If the device is named then we need to look up the BDF */
+ if (pci->name) {
+ rc = name2bdf(gc, pci);
+ if (rc) goto out;
+ }
+
prs->force = force;
libxl__xswait_init(&prs->xswait);
libxl__ev_qmp_init(&prs->qmp);
@@ -2372,6 +2401,10 @@ static int libxl__device_pci_from_xs_be(libxl__gc *gc,
if (s)
pci->vdevfn = strtol(s, (char **) NULL, 16);
+ s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/name-%d", be_path, nr));
+ if (s)
+ pci->name = strdup(s);
+
s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/opts-%d", be_path, nr));
if (s) {
char *saveptr;
diff --git a/tools/libs/util/libxlu_pci.c b/tools/libs/util/libxlu_pci.c
index 05472a0bd1..ad88fee088 100644
--- a/tools/libs/util/libxlu_pci.c
+++ b/tools/libs/util/libxlu_pci.c
@@ -147,6 +147,7 @@ int xlu_pci_parse_spec_string(XLU_Config *cfg, libxl_device_pci *pci,
{
const char *ptr = str;
bool bdf_present = false;
+ bool name_present = false;
int ret;
/* Attempt to parse 'bdf' as positional parameter */
@@ -189,6 +190,10 @@ int xlu_pci_parse_spec_string(XLU_Config *cfg, libxl_device_pci *pci,
pci->power_mgmt = atoi(val);
} else if (!strcmp(key, "rdm_policy")) {
ret = parse_rdm_policy(cfg, &pci->rdm_policy, val);
+ } else if (!strcmp(key, "name")) {
+ name_present = true;
+ pci->name = strdup(val);
+ if (!pci->name) ret = ERROR_NOMEM;
} else {
XLU__PCI_ERR(cfg, "Unknown PCI_SPEC_STRING option: %s", key);
ret = ERROR_INVAL;
@@ -201,7 +206,7 @@ int xlu_pci_parse_spec_string(XLU_Config *cfg, libxl_device_pci *pci,
return ret;
}
- if (!bdf_present)
+ if (!(bdf_present ^ name_present))
return ERROR_INVAL;
return 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v7 7/7] libxl / libxlu: support 'xl pci-attach/detach' by name
2021-01-05 17:46 ` [PATCH v7 7/7] libxl / libxlu: support 'xl pci-attach/detach' by name Paul Durrant
@ 2021-01-21 14:50 ` Wei Liu
0 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2021-01-21 14:50 UTC (permalink / raw)
To: Paul Durrant
Cc: xen-devel, Paul Durrant, Wei Liu, Ian Jackson, Anthony PERARD
On Tue, Jan 05, 2021 at 05:46:42PM +0000, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
>
> This patch modifies libxlu_pci_parse_spec_string() to parse the new 'name'
> parameter of PCI_SPEC_STRING detailed in the updated documention in
> xl-pci-configuration(5) and populate the 'name' field of 'libxl_device_pci'.
>
> If the 'name' field is non-NULL then both libxl_device_pci_add() and
> libxl_device_pci_remove() will use it to look up the device BDF in
> the list of assignable devices.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v7 0/7] xl / libxl: named PCI pass-through devices
2021-01-05 17:46 [PATCH v7 0/7] xl / libxl: named PCI pass-through devices Paul Durrant
` (6 preceding siblings ...)
2021-01-05 17:46 ` [PATCH v7 7/7] libxl / libxlu: support 'xl pci-attach/detach' by name Paul Durrant
@ 2021-01-13 8:05 ` Oleksandr Andrushchenko
7 siblings, 0 replies; 14+ messages in thread
From: Oleksandr Andrushchenko @ 2021-01-13 8:05 UTC (permalink / raw)
To: Wei Liu; +Cc: Paul Durrant, xen-devel, Paul Durrant
Hi, Wei!
Is this series good to go? Could you please tell if it breaks anything?
Thank you in advance,
Oleksandr
On 1/5/21 7:46 PM, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
>
> These are re-worked versions of the remaining patches from v6 of the series
> that were reverted by commit ac6a0af3870b "Revert patches that break libxl
> API".
>
> Paul Durrant (7):
> docs/man: modify xl(1) in preparation for naming of assignable devices
> libxlu: introduce xlu_pci_parse_spec_string()
> libxl: stop setting 'vdevfn' in pci_struct_fill()
> libxl: add 'name' field to 'libxl_device_pci' in the IDL...
> xl: support naming of assignable devices
> docs/man: modify xl-pci-configuration(5) to add 'name' field to
> PCI_SPEC_STRING
> libxl / libxlu: support 'xl pci-attach/detach' by name
>
> docs/man/xl-pci-configuration.5.pod | 25 ++-
> docs/man/xl.1.pod.in | 19 +-
> tools/include/libxl.h | 6 +
> tools/include/libxlutil.h | 8 +-
> tools/libs/light/libxl_pci.c | 133 ++++++++++++--
> tools/libs/light/libxl_types.idl | 13 +-
> tools/libs/util/libxlu_pci.c | 353 +++++++++++++++++++-----------------
> tools/xl/xl_cmdtable.c | 16 +-
> tools/xl/xl_parse.c | 4 +-
> tools/xl/xl_pci.c | 120 ++++++++----
> 10 files changed, 457 insertions(+), 240 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread