Re: [Xen-devel] [PATCH] MdeModulePkg: mark completion of PCI enumeration in PciEnumeratorLight

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Feb 12, 2015 at 11:46:04AM +0100, Laszlo Ersek wrote:
> On 02/11/15 21:23, Wei Liu wrote:
> > I had an issue when trying to boot Xen HVM guest with latest OVMF
> > master. Guest crashed with memory violation, and the bisection pointed
> > to 66b280df2 ("OvmfPkg: AcpiPlatformDxe: make dependency on PCI
> > enumeration explicit"). That commit made AcpiPlatformDxe depend on PCI
> > enumeration using gEfiPciEnumerationCompleteProtocolGuid, which is a
> > very reasonable change.
> > 
> > The real culprit is that Xen HVM is using PciEnumeratorLight which
> > doesn't install gEfiPciEnumerationCompleteProtocolGuid. This, in
> > combination with 66b280df2, makes AcpiPlatformDxe not able to be loaded,
> > resulting in guest crash.
> > 
> > The fix is to install gEfiPciEnumerationCompleteProtocolGuid in
> > PciEnumeratorLight.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Cc: Feng Tian <feng.tian@xxxxxxxxx>
> > Cc: Anthony Perard <anthony.perard@xxxxxxxxxx>
> > Cc: Laszlo Ersek <lersek@xxxxxxxxxx>
> > Cc: Jordan Justen <jordan.l.justen@xxxxxxxxx>
> > ---
> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c 
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > index 9e7ac74..7659585 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > @@ -2256,6 +2256,7 @@ PciEnumeratorLight (
> >  {
> >  
> >    EFI_STATUS                        Status;
> > +  EFI_HANDLE                        HostBridgeHandle;
> >    EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   *PciRootBridgeIo;
> >    PCI_IO_DEVICE                     *RootBridgeDev;
> >    UINT16                            MinBus;
> > @@ -2288,6 +2289,11 @@ PciEnumeratorLight (
> >      return Status;
> >    }
> >  
> > +  //
> > +  // Get the host bridge handle
> > +  //
> > +  HostBridgeHandle = PciRootBridgeIo->ParentHandle;
> > +
> >    Status = PciRootBridgeIo->Configuration (PciRootBridgeIo, (VOID **) 
> > &Descriptors);
> >  
> >    if (EFI_ERROR (Status)) {
> > @@ -2348,7 +2354,14 @@ PciEnumeratorLight (
> >      Descriptors++;
> >    }
> >  
> > -  return EFI_SUCCESS;
> > +  Status = gBS->InstallProtocolInterface (
> > +                  &HostBridgeHandle,
> > +                  &gEfiPciEnumerationCompleteProtocolGuid,
> > +                  EFI_NATIVE_INTERFACE,
> > +                  NULL
> > +                  );
> > +
> > +  return Status;
> >  }
> >  
> >  /**
> > 
> 
> I think this could result in several installations of
> gEfiPciEnumerationCompleteProtocolGuid. See the beginning of the
> PciEnumerator() function.
> 
> If gFullEnumeration is TRUE to begin with (which is the QEMU case), then
> the protocol will be installed at the end of PciEnumerator() first. At
> that point we flip gFullEnumeration to FALSE, which implies that further
> invocations of PciEnumerator() are possible and allowed. So at the next
> call gFullEnumeration will be false, ie. we'll call
> PciEnumeratorLight(). With the above patch PciEnumeratorLight() would
> add the 2nd (and possibly further) instances of
> EfiPciEnumerationCompleteProtocol.
> 

Right. Xen guest starts with gFullEnumeration set to FALSE which means
that protocol never gets installed.

> I'll submit an alternative patch for OvmfPkg soon.
> 

Thanks. I'm just about to write a patch as Ray suggested but now I will
wait for your patch. :-)

Wei.

> Thanks
> Laszlo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel