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

[Chao Gao <chao.gao@xxxxxxxxx>]

On Thu, Apr 05, 2018 at 11:08:59AM +0100, George Dunlap wrote:
>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.

pba_shared_quirk is fine with me. I will use it.

Thanks
Chao

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