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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 04/05/2018 10:59 AM, Roger Pau Monné wrote:
> 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@xxxxxxxxx>
>>>>>> ---
>>>>>>  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.

Yes, I realized that after I'd clicked 'send'.  "Shared" still makes
sense though: the pba shares a page with registers which must be kept RO.

 -George

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