Re: [PATCH 2 of 2] xl, libxl: Add per-device and global permissive config options for pci passthrough

[George Dunlap <george.dunlap@eu.citrix.com>]

On 02/04/12 16:20, Ian Jackson wrote:
> George Dunlap writes ("[Xen-devel] [PATCH 2 of 2] 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.  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.
> Is this security warning not overly mealy-mouthed ?  Surely it should
> be more definite.
I'm not sure how we can make it more definite.  What's possible (i.e., 
the security implications) entirely depends on the card; and what's 
likely (i.e., the stability implications) entirely depends on the card 
and the driver.  Short of giving a short discourse on the vices of 
various cards PCI config space (which is entirely inappropriate for a 
man page, IMHO), I'm not sure what more we can say.
>> +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.
> And this should mention it as well I think.
I thought it was unnecessary to duplicate, but I can do so if you prefer.
>
>> +                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "write to %s returned %d",
> Please keep the lines to 75-80 characters at most.
Ack.
>
> I think you should consider breakibg out the sysfs writing function
> and refactoring with the very similar code in libxl__device_pci_reset,
> rather than introducing yet another clone.
I shall consider it. :-)

  -George