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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 06/06/2016 10:21 AM, Jan Beulich wrote:
>>>> On 06.06.16 at 16:01, <boris.ostrovsky@oracle.com> 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). 

Oh, yes, I completely mis-read the chunk that you quoted above. Didn't
notice that timer doesn't have a write op.

> 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.

Yes.

-boris

>
> 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@lists.xen.org
http://lists.xen.org/xen-devel