|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [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@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>
>> @@ -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@xxxxxxxxxxxxx
>> http://lists.xen.org/xen-devel
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |