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

[Laszlo Ersek <lersek@xxxxxxxxxx>]

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.

I'll submit an alternative patch for OvmfPkg soon.

Thanks
Laszlo

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