Re: [PATCH] xen/mcfg: Call PHYSDEVOP_pci_mmcfg_reserved before PCI enumeration

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Sep 21, 2015 at 08:58:44AM -0700, Ed Swierk wrote:
> After testing this change on different platforms, I'm finding some
> complications.
> 
> As I understand it, the BIOS is supposed to mark the MMCONFIG area
> reserved in the E820 table no matter what. And if the ACPI DSDT
> includes an MCFG record, then it should also include a PNP0C0x record
> for the MMCONFIG area.
> 
> Some BIOSes do just one of these. For example, the Intel S2600GL/GZ
> BIOS has the correct ACPI records, but does not have the area reserved
> in the E820 table. Linux is forgiving enough to use the MMCONFIG area
> in this case.
> 
> Other BIOSes do neither. For example, the Intel S2600WT BIOS doesn't
> even include the PNP0C0x record in the ACPI table, when TXT is
> enabled. On this platform the only way to get Linux to see extended
> config space at all is by passing memmap=256M$0x80000000 on the kernel
> command line.
> 
> My change works in the latter case, since the memmap parameter causes
> it to add the MMCONFIG area in pci_arch_init(). But in the former
> case, where the ACPI tables aren't parsed until acpi_init(),
> xen_mcfg_late() must come later, unless you also use the memmap kernel
> command line parameter.
> 
> The fundamental problem is that Xen tries to access extended config
> space in pci_add_device(), before the Dom0 finally figures out where
> MMCONFIG area is and makes the pci_mmcfg_reserved hypercall. The only
> robust solution seems to be for Xen to defer extended config space
> accesses. It's not clear to me how late is late enough, however.

Or it can deal with PCI config spaces changing.

I wrote some patches (floating around ) where we would walk the full PCI
structure in the xen_mcfg_late() and go over all of them and do
the Xen pci_add_device hypercall.

It (the hypercall) would then need to update its own entries on the subsequent
call. However I never got it completletly working because of:
 - Linux can do 'pci=reassign_devices' which renumbers the bus addresses and
   Xen's idea of the "old" and "new" pci devices goes out the window.
   (for that to work I wrote an patchset that implements this functionality
   in Xen so Linux's pci=reassign_devices in effect will be a nop).
 - Never figured out how much data we should save in the Xen's
   struct pci_device to see if we are 'stale'. Looking back I think
   we just need to do the interogation of the PCI capabilities and see
   if they have somehow changed (the ones we care about).


> 
> --Ed
> 
> 
> On Tue, Sep 15, 2015 at 4:29 PM, Ed Swierk <eswierk@skyportsystems.com> wrote:
> > On systems where the ACPI DSDT advertises the PCI MMCONFIG area but the
> > E820 table does not reserve it, it's up to Dom0 to inform Xen via
> > PHYSDEVOP_pci_mmcfg_reserved.  This needs to happen before Xen tries to
> > access extended capabilities like SRIOV in pci_add_device(), which is
> > triggered when Linux enumerates PCI devices in acpi_init().  Changing
> > xen_mcfg_late() to arch_initcall_sync ensures it gets called before
> > acpi_init(), but after pci_mmcfg_list is populated by pci_arch_init().
> >
> > Without this change, Xen 4.4 and 4.5 emit WARN messsages from
> > msix_capability_init() when setting up Intel 82599 VFs, since vf_rlen has
> > not been initialized by pci_add_device().  And on Xen 4.5, Xen nukes the
> > DomU due to "Potentially insecure use of MSI-X" when the VF driver loads
> > in the DomU.  Both problems are fixed by this change.
> >
> > Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
> > ---
> >  drivers/xen/pci.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/xen/pci.c b/drivers/xen/pci.c
> > index 7494dbe..7b5bbdb 100644
> > --- a/drivers/xen/pci.c
> > +++ b/drivers/xen/pci.c
> > @@ -253,7 +253,9 @@ static int __init xen_mcfg_late(void)
> >         return 0;
> >  }
> >  /*
> > - * Needs to be done after acpi_init which are subsys_initcall.
> > + * Needs to be called after pci_arch_init (arch_initcall) populates
> > + * pci_mmcfg_list, but before acpi_init (subsys_initcall) triggers
> > + * pci_add_device() in Xen, since the latter probes extended capabilities.
> >   */
> > -subsys_initcall_sync(xen_mcfg_late);
> > +arch_initcall_sync(xen_mcfg_late);
> >  #endif
> > --
> > 1.9.1
> >
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel