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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Roger.

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