Re: [Xen-devel] Issues with PCI-Passtrough (VT-d) in HVM with Xen 4.6

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.06.16 at 16:01, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 06/06/2016 04:41 AM, Jan Beulich wrote:
>>
>>> - on the DomU - when I run that "test console" tool, the "lspci -xxx" 
>>> output is different from before/after
>>> not much, though, only one register(?)
>>>
>>> diff lspci.before-testconsole lspci.after-testconsole
>>> 2c2
>>> < 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 40 00 00
>>> ---
>>>> 00: cf 15 00 00 02 01 00 02 00 00 00 ff 00 00 00 00
>> Okay, that's the Latency Timer field, and I think just like BARs we
>> may need to permit restoring this field. However, please note that
>> the reset being done behind the back of pciback really is the
>> problem here: pciback assumes (for a reason, as you can certainly
>> understand) that it has full control over config space of a passed
>> through device.
>>
>> And actually the latency timer would, as a side effect of enabling
>> bus mastering on the device (via the pci_set_master() call from
>> command_write()) set the Latency Timer field properly, just that
>> again pciback (and the rest of Dom0's PCI subsystem) thinks that
>> bus mastering is already enabled on the device. So perhaps in
>> permissive mode we should simply allow the latency timer field to
>> be written, just like we allow writing various of the Command
>> Register bits in that mode. Maintainers, what do you think?
> 
> Don't we currently allow it to be written unconditionally? It is no
> different from accessing Cache Line Size.

No, we don't. And no, someone must have thought differently.
This is what we have right now:

        {
         /* Any side effects of letting driver domain control cache line? */
         .offset    = PCI_CACHE_LINE_SIZE,
         .size      = 1,
         .u.b.read  = xen_pcibk_read_config_byte,
         .u.b.write = xen_pcibk_write_config_byte,
        },
        {
         .offset    = PCI_LATENCY_TIMER,
         .size      = 1,
         .u.b.read  = xen_pcibk_read_config_byte,
        },

I.e. whoever wrote this originally thought that writes to Latency
Timer should be suppressed altogether, and there may be reasons
to also suppress writes to Cache Line Size.

> Or are you suggesting to make it stricter?

Well, following the comment I indeed thought about making the
Cache Line Size one more strict. Latency Timer can't possibly be
made more strict (unless we also wanted to disallow reads). And
it is, iirc, irrelevant for PCIe anyway, so allowing writes there
might be an option, but as they ought to have no effect, it would
seem kind of pointless.

In general I think we should be rather conservative with allowing
writes to any register.

Jan

>> If we decide to go that route, I would then wonder whether
>> Cache Line Size being unconditionally writable right now would
>> also better be restricted to permissive mode.
> 
> 
> -boris




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel