George Dunlap
2012-Mar-27 17:03 UTC
[PATCH] xl, libxl: Add per-device and global permissive config options for pci passthrough
By default pciback only allows PV guests to write "known safe" values
into
PCI config space. But many devices require writes to other areas of config
space in order to operate properly. One way to do that is with the
"quirks"
interface, which specifies areas known safe to a particular device; the
other way is to mark a device as "permissive", which tells pciback to
allow
all config space writes for that domain and device.
This adds a "permissive" flag to the libxl_pci struct and teaches
libxl how
to write the appropriate value into sysfs to enable the permissive feature for
devices being passed through. It also adds the permissive config options either
on a per-device basis, or as a global option in the xl command-line.
Because of the potential stability and security implications of enabling
permissive, the flag is left off by default.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
diff -r f744e82ea740 -r 980c34bd5e51 docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5 Wed Feb 29 16:30:34 2012 +0000
+++ b/docs/man/xl.cfg.pod.5 Tue Mar 27 18:02:41 2012 +0100
@@ -294,10 +294,25 @@ XXX
XXX
+=item B<permissive=BOOLEAN>
+
+By default pciback only allows PV guests to write "known safe" values
into
+PCI config space. But many devices require writes to other areas of config
+space in order to operate properly. This tells the pciback driver to
+allow all writes to PCI config space for this domain and this device. This
+option should be enabled with caution, as there may be stability or security
+implications of doing so.
+
=back
=back
+=item B<pci_permissive=BOOLEAN>
+
+Changes the default value of ''permissive'' for all PCI devices
for this
+VM. This can still be overriden on a per-device basis. See the
+"pci=" section for more information on the "permissive"
flag.
+
=back
=head2 Paravirtualised (PV) Guest Specific Options
diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Wed Feb 29 16:30:34 2012 +0000
+++ b/tools/libxl/libxl_pci.c Tue Mar 27 18:02:41 2012 +0100
@@ -165,6 +165,8 @@ int libxl_device_pci_parse_bdf(libxl_ctx
pcidev->msitranslate = atoi(tok);
}else if ( !strcmp(optkey, "power_mgmt") ) {
pcidev->power_mgmt = atoi(tok);
+ }else if ( !strcmp(optkey, "permissive") ) {
+ pcidev->permissive = atoi(tok);
}else{
LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
"Unknown PCI BDF option: %s", optkey);
@@ -198,7 +200,10 @@ static void libxl_create_pci_backend_dev
if (pcidev->vdevfn)
flexarray_append_pair(back, libxl__sprintf(gc, "vdevfn-%d",
num), libxl__sprintf(gc, "%x", pcidev->vdevfn));
flexarray_append(back, libxl__sprintf(gc, "opts-%d", num));
- flexarray_append(back, libxl__sprintf(gc,
"msitranslate=%d,power_mgmt=%d", pcidev->msitranslate,
pcidev->power_mgmt));
+ flexarray_append(back,
+ libxl__sprintf(gc,
"msitranslate=%d,power_mgmt=%d,permissive=%d",
+ pcidev->msitranslate, pcidev->power_mgmt,
+ pcidev->permissive));
flexarray_append_pair(back, libxl__sprintf(gc, "state-%d", num),
libxl__sprintf(gc, "%d", 1));
}
@@ -708,6 +713,20 @@ static int do_pci_add(libxl__gc *gc, uin
}
}
fclose(f);
+
+ /* Don''t restrict writes to the PCI config space from this VM
*/
+ if(pcidev->permissive) {
+ sysfs_path = libxl__sprintf(gc,
SYSFS_PCIBACK_DRIVER"/permissive");
+ f = fopen(sysfs_path, "w");
+ if (f == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn''t
open %s",
+ sysfs_path);
+ goto out;
+ }
+ fprintf(f, PCI_BDF, pcidev->domain, pcidev->bus,
+ pcidev->dev, pcidev->func);
+ fclose(f);
+ }
break;
}
default:
@@ -1101,6 +1120,9 @@ static void libxl__device_pci_from_xs_be
} else if (!strcmp(p, "power_mgmt")) {
p = strtok_r(NULL, ",=", &saveptr);
pci->power_mgmt = atoi(p);
+ } else if (!strcmp(p, "permissive")) {
+ p = strtok_r(NULL, ",=", &saveptr);
+ pci->permissive = atoi(p);
}
} while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL);
}
diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Wed Feb 29 16:30:34 2012 +0000
+++ b/tools/libxl/libxl_types.idl Tue Mar 27 18:02:41 2012 +0100
@@ -352,6 +352,7 @@ libxl_device_pci = Struct("device_pci",
("vfunc_mask", uint32),
("msitranslate", bool),
("power_mgmt", bool),
+ ("permissive", bool),
])
libxl_diskinfo = Struct("diskinfo", [
diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed Feb 29 16:30:34 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c Tue Mar 27 18:02:41 2012 +0100
@@ -518,6 +518,7 @@ static void parse_config_data(const char
XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
int pci_power_mgmt = 0;
int pci_msitranslate = 1;
+ int pci_permissive = 0;
int e;
libxl_domain_create_info *c_info = &d_config->c_info;
@@ -986,6 +987,9 @@ skip_vfb:
if (!xlu_cfg_get_long (config, "pci_power_mgmt", &l, 0))
pci_power_mgmt = l;
+ if (!xlu_cfg_get_long (config, "pci_permissive", &l, 0))
+ pci_permissive = l;
+
/* To be reworked (automatically enabled) once the auto ballooning
* after guest starts is done (with PCI devices passed in). */
if (c_info->type == LIBXL_DOMAIN_TYPE_PV) {
@@ -1005,6 +1009,7 @@ skip_vfb:
pcidev->msitranslate = pci_msitranslate;
pcidev->power_mgmt = pci_power_mgmt;
+ pcidev->permissive = pci_permissive;
if (!libxl_device_pci_parse_bdf(ctx, pcidev, buf))
d_config->num_pcidevs++;
}
diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c Wed Feb 29 16:30:34 2012 +0000
+++ b/tools/libxl/xl_sxp.c Tue Mar 27 18:02:41 2012 +0100
@@ -199,9 +199,10 @@ void printf_info_sexp(int domid, libxl_d
d_config->pcidevs[i].domain, d_config->pcidevs[i].bus,
d_config->pcidevs[i].dev, d_config->pcidevs[i].func,
d_config->pcidevs[i].vdevfn);
- printf("\t\t\t(opts msitranslate %d power_mgmt %d)\n",
+ printf("\t\t\t(opts msitranslate %d power_mgmt %d permissive
%d)\n",
d_config->pcidevs[i].msitranslate,
- d_config->pcidevs[i].power_mgmt);
+ d_config->pcidevs[i].power_mgmt,
+ d_config->pcidevs[i].permissive);
printf("\t\t)\n");
printf("\t)\n");
}
Ian Campbell
2012-Mar-28 09:20 UTC
Re: [PATCH] xl, libxl: Add per-device and global permissive config options for pci passthrough
On Tue, 2012-03-27 at 18:03 +0100, George Dunlap wrote:> By default pciback only allows PV guests to write "known safe" values into > PCI config space. But many devices require writes to other areas of config > space in order to operate properly. One way to do that is with the "quirks" > interface, which specifies areas known safe to a particular device; the > other way is to mark a device as "permissive", which tells pciback to allow > all config space writes for that domain and device. > > This adds a "permissive" flag to the libxl_pci struct and teaches libxl how > to write the appropriate value into sysfs to enable the permissive feature for > devices being passed through. It also adds the permissive config options either > on a per-device basis, or as a global option in the xl command-line. > > Because of the potential stability and security implications of enabling > permissive, the flag is left off by default. > > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> > > diff -r f744e82ea740 -r 980c34bd5e51 docs/man/xl.cfg.pod.5 > --- a/docs/man/xl.cfg.pod.5 Wed Feb 29 16:30:34 2012 +0000 > +++ b/docs/man/xl.cfg.pod.5 Tue Mar 27 18:02:41 2012 +0100 > @@ -294,10 +294,25 @@ XXX > > XXX > > +=item B<permissive=BOOLEAN> > + > +By default pciback only allows PV guests to write "known safe" values into > +PCI config space. But many devices require writes to other areas of config > +space in order to operate properly. This tells the pciback driver to > +allow all writes to PCI config space for this domain and this device. This > +option should be enabled with caution, as there may be stability or security > +implications of doing so. > + > =back > > =back > > +=item B<pci_permissive=BOOLEAN> > + > +Changes the default value of ''permissive'' for all PCI devices for this > +VM. This can still be overriden on a per-device basis. See the > +"pci=" section for more information on the "permissive" flag. > + > =back > > =head2 Paravirtualised (PV) Guest Specific Options > diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/libxl_pci.c > --- a/tools/libxl/libxl_pci.c Wed Feb 29 16:30:34 2012 +0000 > +++ b/tools/libxl/libxl_pci.c Tue Mar 27 18:02:41 2012 +0100 > @@ -165,6 +165,8 @@ int libxl_device_pci_parse_bdf(libxl_ctxThis should probably actually be in libxlu rather than libxl. But no need to fix that here. (I''ll send a follow up if you like).> pcidev->msitranslate = atoi(tok); > }else if ( !strcmp(optkey, "power_mgmt") ) { > pcidev->power_mgmt = atoi(tok); > + }else if ( !strcmp(optkey, "permissive") ) { > + pcidev->permissive = atoi(tok); > }else{ > LIBXL__LOG(ctx, LIBXL__LOG_WARNING, > "Unknown PCI BDF option: %s", optkey); > @@ -198,7 +200,10 @@ static void libxl_create_pci_backend_dev > if (pcidev->vdevfn) > flexarray_append_pair(back, libxl__sprintf(gc, "vdevfn-%d", num), libxl__sprintf(gc, "%x", pcidev->vdevfn)); > flexarray_append(back, libxl__sprintf(gc, "opts-%d", num)); > - flexarray_append(back, libxl__sprintf(gc, "msitranslate=%d,power_mgmt=%d", pcidev->msitranslate, pcidev->power_mgmt)); > + flexarray_append(back, > + libxl__sprintf(gc, "msitranslate=%d,power_mgmt=%d,permissive=%d", > + pcidev->msitranslate, pcidev->power_mgmt, > + pcidev->permissive)); > flexarray_append_pair(back, libxl__sprintf(gc, "state-%d", num), libxl__sprintf(gc, "%d", 1)); > } > > @@ -708,6 +713,20 @@ static int do_pci_add(libxl__gc *gc, uin > } > } > fclose(f); > + > + /* Don''t restrict writes to the PCI config space from this VM */ > + if(pcidev->permissive) { > + sysfs_path = libxl__sprintf(gc, SYSFS_PCIBACK_DRIVER"/permissive"); > + f = fopen(sysfs_path, "w"); > + if (f == NULL) { > + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn''t open %s", > + sysfs_path); > + goto out; > + } > + fprintf(f, PCI_BDF, pcidev->domain, pcidev->bus, > + pcidev->dev, pcidev->func); > + fclose(f); > + } > break; > } > default: > @@ -1101,6 +1120,9 @@ static void libxl__device_pci_from_xs_be > } else if (!strcmp(p, "power_mgmt")) { > p = strtok_r(NULL, ",=", &saveptr); > pci->power_mgmt = atoi(p); > + } else if (!strcmp(p, "permissive")) { > + p = strtok_r(NULL, ",=", &saveptr); > + pci->permissive = atoi(p); > } > } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL); > } > diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/libxl_types.idl > --- a/tools/libxl/libxl_types.idl Wed Feb 29 16:30:34 2012 +0000 > +++ b/tools/libxl/libxl_types.idl Tue Mar 27 18:02:41 2012 +0100 > @@ -352,6 +352,7 @@ libxl_device_pci = Struct("device_pci", > ("vfunc_mask", uint32), > ("msitranslate", bool), > ("power_mgmt", bool), > + ("permissive", bool), > ]) > > libxl_diskinfo = Struct("diskinfo", [ > diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Wed Feb 29 16:30:34 2012 +0000 > +++ b/tools/libxl/xl_cmdimpl.c Tue Mar 27 18:02:41 2012 +0100 > @@ -518,6 +518,7 @@ static void parse_config_data(const char > XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids; > int pci_power_mgmt = 0; > int pci_msitranslate = 1; > + int pci_permissive = 0; > int e; > > libxl_domain_create_info *c_info = &d_config->c_info; > @@ -986,6 +987,9 @@ skip_vfb: > if (!xlu_cfg_get_long (config, "pci_power_mgmt", &l, 0)) > pci_power_mgmt = l; > > + if (!xlu_cfg_get_long (config, "pci_permissive", &l, 0)) > + pci_permissive = l; > + > /* To be reworked (automatically enabled) once the auto ballooning > * after guest starts is done (with PCI devices passed in). */ > if (c_info->type == LIBXL_DOMAIN_TYPE_PV) { > @@ -1005,6 +1009,7 @@ skip_vfb: > > pcidev->msitranslate = pci_msitranslate; > pcidev->power_mgmt = pci_power_mgmt; > + pcidev->permissive = pci_permissive; > if (!libxl_device_pci_parse_bdf(ctx, pcidev, buf)) > d_config->num_pcidevs++; > } > diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/xl_sxp.c > --- a/tools/libxl/xl_sxp.c Wed Feb 29 16:30:34 2012 +0000 > +++ b/tools/libxl/xl_sxp.c Tue Mar 27 18:02:41 2012 +0100This file/function is provided only for compatibility with xend, is this the precise syntax used by xend? There is no need to update this SXP unless someone reports and incompatibility (although if you happen to know what the xend output is you may as well). Otherwise: Acked-by: Ian Campbell <ian.campbell@citrix.com>> @@ -199,9 +199,10 @@ void printf_info_sexp(int domid, libxl_d > d_config->pcidevs[i].domain, d_config->pcidevs[i].bus, > d_config->pcidevs[i].dev, d_config->pcidevs[i].func, > d_config->pcidevs[i].vdevfn); > - printf("\t\t\t(opts msitranslate %d power_mgmt %d)\n", > + printf("\t\t\t(opts msitranslate %d power_mgmt %d permissive %d)\n", > d_config->pcidevs[i].msitranslate, > - d_config->pcidevs[i].power_mgmt); > + d_config->pcidevs[i].power_mgmt, > + d_config->pcidevs[i].permissive); > printf("\t\t)\n"); > printf("\t)\n"); > } > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
George Dunlap
2012-Mar-28 10:34 UTC
Re: [PATCH] xl, libxl: Add per-device and global permissive config options for pci passthrough
On Wed, Mar 28, 2012 at 10:20 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Tue, 2012-03-27 at 18:03 +0100, George Dunlap wrote: >> By default pciback only allows PV guests to write "known safe" values into >> PCI config space. But many devices require writes to other areas of config >> space in order to operate properly. One way to do that is with the "quirks" >> interface, which specifies areas known safe to a particular device; the >> other way is to mark a device as "permissive", which tells pciback to allow >> all config space writes for that domain and device. >> >> This adds a "permissive" flag to the libxl_pci struct and teaches libxl how >> to write the appropriate value into sysfs to enable the permissive feature for >> devices being passed through. It also adds the permissive config options either >> on a per-device basis, or as a global option in the xl command-line. >> >> Because of the potential stability and security implications of enabling >> permissive, the flag is left off by default. >> >> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> >> >> diff -r f744e82ea740 -r 980c34bd5e51 docs/man/xl.cfg.pod.5 >> --- a/docs/man/xl.cfg.pod.5 Wed Feb 29 16:30:34 2012 +0000 >> +++ b/docs/man/xl.cfg.pod.5 Tue Mar 27 18:02:41 2012 +0100 >> @@ -294,10 +294,25 @@ XXX >> >> XXX >> >> +=item B<permissive=BOOLEAN> >> + >> +By default pciback only allows PV guests to write "known safe" values into >> +PCI config space. But many devices require writes to other areas of config >> +space in order to operate properly. This tells the pciback driver to >> +allow all writes to PCI config space for this domain and this device. This >> +option should be enabled with caution, as there may be stability or security >> +implications of doing so. >> + >> =back >> >> =back >> >> +=item B<pci_permissive=BOOLEAN> >> + >> +Changes the default value of ''permissive'' for all PCI devices for this >> +VM. This can still be overriden on a per-device basis. See the >> +"pci=" section for more information on the "permissive" flag. >> + >> =back >> >> =head2 Paravirtualised (PV) Guest Specific Options >> diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/libxl_pci.c >> --- a/tools/libxl/libxl_pci.c Wed Feb 29 16:30:34 2012 +0000 >> +++ b/tools/libxl/libxl_pci.c Tue Mar 27 18:02:41 2012 +0100 >> @@ -165,6 +165,8 @@ int libxl_device_pci_parse_bdf(libxl_ctx > > This should probably actually be in libxlu rather than libxl. But no > need to fix that here. (I''ll send a follow up if you like). > >> pcidev->msitranslate = atoi(tok); >> }else if ( !strcmp(optkey, "power_mgmt") ) { >> pcidev->power_mgmt = atoi(tok); >> + }else if ( !strcmp(optkey, "permissive") ) { >> + pcidev->permissive = atoi(tok); >> }else{ >> LIBXL__LOG(ctx, LIBXL__LOG_WARNING, >> "Unknown PCI BDF option: %s", optkey); >> @@ -198,7 +200,10 @@ static void libxl_create_pci_backend_dev >> if (pcidev->vdevfn) >> flexarray_append_pair(back, libxl__sprintf(gc, "vdevfn-%d", num), libxl__sprintf(gc, "%x", pcidev->vdevfn)); >> flexarray_append(back, libxl__sprintf(gc, "opts-%d", num)); >> - flexarray_append(back, libxl__sprintf(gc, "msitranslate=%d,power_mgmt=%d", pcidev->msitranslate, pcidev->power_mgmt)); >> + flexarray_append(back, >> + libxl__sprintf(gc, "msitranslate=%d,power_mgmt=%d,permissive=%d", >> + pcidev->msitranslate, pcidev->power_mgmt, >> + pcidev->permissive)); >> flexarray_append_pair(back, libxl__sprintf(gc, "state-%d", num), libxl__sprintf(gc, "%d", 1)); >> } >> >> @@ -708,6 +713,20 @@ static int do_pci_add(libxl__gc *gc, uin >> } >> } >> fclose(f); >> + >> + /* Don''t restrict writes to the PCI config space from this VM */ >> + if(pcidev->permissive) { >> + sysfs_path = libxl__sprintf(gc, SYSFS_PCIBACK_DRIVER"/permissive"); >> + f = fopen(sysfs_path, "w"); >> + if (f == NULL) { >> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn''t open %s", >> + sysfs_path); >> + goto out; >> + } >> + fprintf(f, PCI_BDF, pcidev->domain, pcidev->bus, >> + pcidev->dev, pcidev->func); >> + fclose(f); >> + } >> break; >> } >> default: >> @@ -1101,6 +1120,9 @@ static void libxl__device_pci_from_xs_be >> } else if (!strcmp(p, "power_mgmt")) { >> p = strtok_r(NULL, ",=", &saveptr); >> pci->power_mgmt = atoi(p); >> + } else if (!strcmp(p, "permissive")) { >> + p = strtok_r(NULL, ",=", &saveptr); >> + pci->permissive = atoi(p); >> } >> } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL); >> } >> diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/libxl_types.idl >> --- a/tools/libxl/libxl_types.idl Wed Feb 29 16:30:34 2012 +0000 >> +++ b/tools/libxl/libxl_types.idl Tue Mar 27 18:02:41 2012 +0100 >> @@ -352,6 +352,7 @@ libxl_device_pci = Struct("device_pci", >> ("vfunc_mask", uint32), >> ("msitranslate", bool), >> ("power_mgmt", bool), >> + ("permissive", bool), >> ]) >> >> libxl_diskinfo = Struct("diskinfo", [ >> diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/xl_cmdimpl.c >> --- a/tools/libxl/xl_cmdimpl.c Wed Feb 29 16:30:34 2012 +0000 >> +++ b/tools/libxl/xl_cmdimpl.c Tue Mar 27 18:02:41 2012 +0100 >> @@ -518,6 +518,7 @@ static void parse_config_data(const char >> XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids; >> int pci_power_mgmt = 0; >> int pci_msitranslate = 1; >> + int pci_permissive = 0; >> int e; >> >> libxl_domain_create_info *c_info = &d_config->c_info; >> @@ -986,6 +987,9 @@ skip_vfb: >> if (!xlu_cfg_get_long (config, "pci_power_mgmt", &l, 0)) >> pci_power_mgmt = l; >> >> + if (!xlu_cfg_get_long (config, "pci_permissive", &l, 0)) >> + pci_permissive = l; >> + >> /* To be reworked (automatically enabled) once the auto ballooning >> * after guest starts is done (with PCI devices passed in). */ >> if (c_info->type == LIBXL_DOMAIN_TYPE_PV) { >> @@ -1005,6 +1009,7 @@ skip_vfb: >> >> pcidev->msitranslate = pci_msitranslate; >> pcidev->power_mgmt = pci_power_mgmt; >> + pcidev->permissive = pci_permissive; >> if (!libxl_device_pci_parse_bdf(ctx, pcidev, buf)) >> d_config->num_pcidevs++; >> } >> diff -r f744e82ea740 -r 980c34bd5e51 tools/libxl/xl_sxp.c >> --- a/tools/libxl/xl_sxp.c Wed Feb 29 16:30:34 2012 +0000 >> +++ b/tools/libxl/xl_sxp.c Tue Mar 27 18:02:41 2012 +0100 > > This file/function is provided only for compatibility with xend, is this > the precise syntax used by xend? There is no need to update this SXP > unless someone reports and incompatibility (although if you happen to > know what the xend output is you may as well).Ah, right -- in that case, this hunk should probably be taken out, as xend uses a different method of marking a device permissive. I''m going to write a patch that adds an option to automatically do unbinding and binding; so I''ll just re-send this one as part of a series with that, and a patch that moves all the parsing stuff to libxlu. -George> > Otherwise: > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > >> @@ -199,9 +199,10 @@ void printf_info_sexp(int domid, libxl_d >> d_config->pcidevs[i].domain, d_config->pcidevs[i].bus, >> d_config->pcidevs[i].dev, d_config->pcidevs[i].func, >> d_config->pcidevs[i].vdevfn); >> - printf("\t\t\t(opts msitranslate %d power_mgmt %d)\n", >> + printf("\t\t\t(opts msitranslate %d power_mgmt %d permissive %d)\n", >> d_config->pcidevs[i].msitranslate, >> - d_config->pcidevs[i].power_mgmt); >> + d_config->pcidevs[i].power_mgmt, >> + d_config->pcidevs[i].permissive); >> printf("\t\t)\n"); >> printf("\t)\n"); >> } >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Jackson
2012-Mar-29 14:34 UTC
Re: [PATCH] xl, libxl: Add per-device and global permissive config options for pci passthrough
George Dunlap writes ("[Xen-devel] [PATCH] xl, libxl: Add per-device and
global permissive config options for pci
passthrough"):> + /* Don''t restrict writes to the PCI config space from
this VM */
> + if(pcidev->permissive) {
Missing space after "if".
> + sysfs_path = libxl__sprintf(gc,
SYSFS_PCIBACK_DRIVER"/permissive");
> + f = fopen(sysfs_path, "w");
...> + fprintf(f, PCI_BDF, pcidev->domain, pcidev->bus,
> + pcidev->dev, pcidev->func);
> + fclose(f);
Do this fprintf and fclose need error checking at all ? If the actual
write fails, atm you will ignore it.
I agree with most of the other comments.
TBH although it would be nice to move this parsing to libxlu I don''t
think you should be required to do it as part of this patch. But I''d
encourage you to do so :-).
Thanks,
Ian.