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 12:25:26PM +0100, Roger Pau Monné wrote:
>On Thu, Apr 05, 2018 at 07:00:41PM +0800, Chao Gao wrote:
>> On Thu, Apr 05, 2018 at 10:34:39AM +0100, Roger Pau Monné wrote:
>> >On Wed, Apr 04, 2018 at 11:29:39PM +0800, Chao Gao wrote:
>> >> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
>> >> index 5567990..2abf2cf 100644
>> >> --- a/xen/arch/x86/msi.c
>> >> +++ b/xen/arch/x86/msi.c
>> >> @@ -992,7 +992,9 @@ static int msix_capability_init(struct pci_dev *dev,
>> >>          if ( rangeset_add_range(mmio_ro_ranges, msix->table.first,
>> >>                                  msix->table.last) )
>> >>              WARN();
>> >> -        if ( rangeset_add_range(mmio_ro_ranges, msix->pba.first,
>> >> +
>> >> +        if ( !msix->pba_quirk_enabled &&
>> >> +             rangeset_add_range(mmio_ro_ranges, msix->pba.first,
>> >>                                  msix->pba.last) )
>> >>              WARN();
>> >
>> >This will work fine as long as the PBA is not in the same page as the
>> >MSI-X table. In such case you will also need changes to QEMU (see
>> >pci_msix_write), so that writes to the memory in the same page as the
>> >MSI-X/PBA tables are forwarded to the underlying hardware.
>> >
>> >You should add least add something like:
>> >
>> >if ( msix->pba_quirk_enabled &&
>> >     msix->table.first <= msix->pba.last &&
>> >     msix->pba.first <= msix->table.last )
>> >{
>> >    printk("PBA write not allowed to dev %04x:%02x:%02x.%u due to MSI-X 
>> > table overlap\n");
>> >    return -ENXIO;
>> >}
>> >
>> >Or similar if the QEMU side is not fixed.
>> >
>> >Note that in order to fix the QEMU side you would probably have to add
>> >a flag to xl 'pci' config option and pass it to both QEMU and Xen.
>> 
>> Thanks for your comments.
>> 
>> First of all, I don't intend to also support devices which has MSI-X
>> table, MSI-X PBA and other MSI-X irrelevant registers in the same page.
>> Because as you said, it cleary violates MSI-X spec. IMO, we can extend
>> the workaround when we found such a device.
>>
>> I also had the same concern with yours. But after careful thinking, I
>> found it wouldn't be a problem. If MSI-X table resides the same pages
>> with MSI-X PBA, we will mark the pages as RO when handling MSI-X table.
>> As a consequence, guest isn't able to write MSI-X table directly in this
>> case. Hence, it won't affect MSI-X table emulation and furthermore the
>> quirk won't override the decision, marking the page RO, made for other
>> reasons.
>
>My suggestion is not because it would be dangerous from a security
>PoV, it's simply because the quirk won't be applied, and hence we need
>to notify the user that the desired quirk has not been applied.

Got it. It is reasonable.

Thanks
Chao


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