All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <JGross@suse.com>,
	juwalter@gmail.com, xen-devel@lists.xen.org,
	David Vrabel <david.vrabel@citrix.com>,
	Sylwester Sosnowski <syso@datanoise.net>
Subject: Re: Issues with PCI-Passtrough (VT-d) in HVM with Xen 4.6
Date: Mon, 06 Jun 2016 08:21:17 -0600	[thread overview]
Message-ID: <5755A2FD02000078000F213B@prv-mh.provo.novell.com> (raw)
In-Reply-To: <57558235.4020502@oracle.com>

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

  reply	other threads:[~2016-06-06 14:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-02 19:59 Issues with PCI-Passtrough (VT-d) in HVM with Xen 4.6 Sylwester Sosnowski
2016-06-02 20:06 ` Konrad Rzeszutek Wilk
2016-06-02 20:11   ` Sylwester Sosnowski
2016-06-03  7:24 ` Sylwester Sosnowski
2016-06-03 11:52 ` Jan Beulich
2016-06-03 12:02   ` Jürgen Walter
2016-06-03 13:26     ` Jan Beulich
2016-06-03 14:20       ` Jan Beulich
2016-06-04 14:36         ` Jürgen Walter
2016-06-06  7:59           ` Jan Beulich
2016-06-04 15:15       ` Jürgen Walter
2016-06-06  8:41         ` Jan Beulich
2016-06-06  9:09           ` Jürgen Walter
2016-06-06  9:43             ` Jan Beulich
2016-06-06 14:01           ` Boris Ostrovsky
2016-06-06 14:21             ` Jan Beulich [this message]
2016-06-06 14:45               ` Boris Ostrovsky
  -- strict thread matches above, loose matches on Subject: below --
2016-06-15 10:45 Andrey Grodzovsky
2016-06-15 13:14 ` Jan Beulich
2016-06-18  3:24   ` Andrey Grodzovsky
2016-06-20  6:57     ` Jan Beulich
2016-06-02 19:49 Sylwester Sosnowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5755A2FD02000078000F213B@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=JGross@suse.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=david.vrabel@citrix.com \
    --cc=juwalter@gmail.com \
    --cc=syso@datanoise.net \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.