Re: [PATCH] pci: a workaround for nonstandard PCI devices whose PBA shares

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: George Dunlap <george.dunlap@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Tim Deegan <tim@xen.org>, Ian Jackson <ian.jackson@eu.citrix.com>,
	xen-devel@lists.xen.org, Julien Grall <julien.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Chao Gao <chao.gao@intel.com>
Subject: Re: [PATCH] pci: a workaround for nonstandard PCI devices whose PBA shares
Date: Thu, 5 Apr 2018 10:59:35 +0100	[thread overview]
Message-ID: <20180405095935.guryybld2um7njsv@MacBook-Pro-de-Roger.local> (raw)
In-Reply-To: <6d08270e-f7f7-a9f9-67ae-9af2a4d4f134@citrix.com>

On Thu, Apr 05, 2018 at 10:52:09AM +0100, George Dunlap wrote:
> On 04/05/2018 10:46 AM, Roger Pau Monné wrote:
> > On Thu, Apr 05, 2018 at 10:40:37AM +0100, George Dunlap wrote:
> >> On 04/05/2018 10:34 AM, Roger Pau Monné wrote:
> >>> On Wed, Apr 04, 2018 at 11:29:39PM +0800, Chao Gao wrote:
> >>>> ... the same page with other registers which are not relevant to MSI-X. Xen
> >>>> marks pages where PBA resides as read-only. When assigning such devices to
> >>>> guest, device driver writes MSI-X irrelevant registers on those pages would
> >>>> lead to an EPT violation and the guest is destroyed because no handler is
> >>>> registered for those address range. In order to make guest capable to use such
> >>>> kind of devices, trapping very frequent write accesses is not a good idea for
> >>>> it would significantly impact the performance.
> >>>>
> >>>> This patch provides a workaround with caveat. Specifically, an option is
> >>>> introduced to specify a list of devices. For those devices, Xen doesn't
> >>>> control the access right to pages where PBA resides. Hence, guest device
> >>>> driver is able to write those pages and functions well. Note that adding an
> >>>> untrusted device to this option may endanger security of the entire system.
> >>>>
> >>>> Signed-off-by: Chao Gao <chao.gao@intel.com>
> >>>> ---
> >>>>  docs/misc/xen-command-line.markdown | 10 +++++++++
> >>>>  xen/arch/x86/msi.c                  |  7 ++++--
> >>>>  xen/drivers/passthrough/pci.c       | 45 +++++++++++++++++++++++++++++++++++--
> >>>>  xen/include/asm-x86/msi.h           |  1 +
> >>>>  4 files changed, 59 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> >>>> index b353352..e382513 100644
> >>>> --- a/docs/misc/xen-command-line.markdown
> >>>> +++ b/docs/misc/xen-command-line.markdown
> >>>> @@ -1423,6 +1423,16 @@ Defaults to booting secondary processors.
> >>>>  
> >>>>  > Default: `on`
> >>>>  
> >>>> +### pba\_quirk
> >>>
> >>> pba_write_allowed would be better, pba_quirk is too generic IMO.
> >>
> >> 'quirk' was I think requested by Jan; and my understanding is that the
> >> word clearly indicates that the behavior in question is a workaround for
> >> hardware which is not compliant with the appropriate specification.  If
> >> you grep the source tree for 'quirk' you'll find a fairly large number.
> >>
> >> pba_shared_quirk might be slightly more descriptive.
> > 
> > pba_write_quirk?
> > 
> > I just think it should be slightly more descriptive than pba_quirk in
> > case Xen has to add further PBA-related quirks in the future.
> 
> "shared" tells you something about the quirk itself: The PBA is shared
> across multiple devices.  "write" tells you about the work-around:
> unsafe writes to the PBA region are allowed.

I don't think the PBA page is shared with multiple devices in any
case. The problem here is that the PBA page contains other registers
(from the same device as the PBA) that must be RW instead of RO.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel