Re: [Xen-devel] [edk2] [PATCH] OvmfPkg: AcpiPlatformDxe: PCI enumeration may be disabled

[Jordan Justen <jordan.l.justen@xxxxxxxxx>]

I think gEfiPciEnumerationCompleteProtocolGuid should be installed by
MdeModulePkg/Bus/Pci/PciBusDxe, even when PcdPciDisableBusEnumeration
is set.

Ray's main feedback seemed to be that we need to make sure PciBusDxe
only installs the protocol once. (I'll also reply to the other related
patch thread.)

-Jordan

On 2015-02-12 04:16:07, Laszlo Ersek wrote:
> SVN r16411 delayed ACPI table installation until PCI enumeration was
> complete, because on QEMU the ACPI-related fw_cfg files should only be
> downloaded after PCI enumeration.
> 
> However, InitializeXen() in "OvmfPkg/PlatformPei/Xen.c" sets
> PcdPciDisableBusEnumeration to TRUE. This causes
> PciBusDriverBindingStart() in "MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c" to
> set gFullEnumeration to FALSE, which in turn makes PciEnumerator() in
> "MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c" branch to
> PciEnumeratorLight(). The installation of
> EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL at the end of PciEnumerator() is not
> reached.
> 
> Which means that starting with SVN r16411, AcpiPlatformDxe is never
> dispatched on Xen.
> 
> This patch replaces the EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL depex with a
> matching protocol registration callback for the PCI enumeration enabled
> (ie. QEMU) case. When PCI enumeration is disabled (ie. when running on
> Xen), AcpiPlatformDxe doesn't wait for
> EFI_PCI_ENUMERATION_COMPLETE_PROTOCOL.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
> ---
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf |  4 +-
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c      | 84 
> +++++++++++++++++++++++------
>  2 files changed, 72 insertions(+), 16 deletions(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf 
> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> index 53292bf..6b2c9d2 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> @@ -56,16 +56,18 @@
>  
>  [Protocols]
>    gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
> +  gEfiPciEnumerationCompleteProtocolGuid        # PROTOCOL SOMETIMES_CONSUMED
>  
>  [Guids]
>    gEfiXenInfoGuid
>  
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiTableStorageFile
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
>    gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
>  
>  [Depex]
> -  gEfiAcpiTableProtocolGuid AND gEfiPciEnumerationCompleteProtocolGuid
> +  gEfiAcpiTableProtocolGuid
>  
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c 
> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> index 11f0ca8..9823eba 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c
> @@ -12,6 +12,7 @@
>  
>  **/
>  
> +#include <Protocol/PciEnumerationComplete.h>
>  #include "AcpiPlatform.h"
>  
>  EFI_STATUS
> @@ -221,6 +222,47 @@ FindAcpiTablesInFv (
>    return EFI_SUCCESS;
>  }
>  
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +InstallTables (
> +  VOID
> +  )
> +{
> +  EFI_STATUS              Status;
> +  EFI_ACPI_TABLE_PROTOCOL *AcpiTable;
> +
> +  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid,
> +                  NULL /* Registration */, (VOID **)&AcpiTable);
> +  if (EFI_ERROR (Status)) {
> +    return EFI_ABORTED;
> +  }
> +
> +  if (XenDetected ()) {
> +    Status = InstallXenTables (AcpiTable);
> +  } else {
> +    Status = InstallAllQemuLinkedTables (AcpiTable);
> +  }
> +
> +  if (EFI_ERROR (Status)) {
> +    Status = FindAcpiTablesInFv (AcpiTable);
> +  }
> +
> +  return Status;
> +}
> +
> +STATIC
> +VOID
> +EFIAPI
> +OnPciEnumerated (
> +  IN EFI_EVENT Event,
> +  IN VOID      *Context
> +  )
> +{
> +  InstallTables ();
> +  gBS->CloseEvent (Event);
> +}
> +
>  /**
>    Entrypoint of Acpi Platform driver.
>  
> @@ -239,31 +281,43 @@ AcpiPlatformEntryPoint (
>    IN EFI_SYSTEM_TABLE   *SystemTable
>    )
>  {
> -  EFI_STATUS                         Status;
> -  EFI_ACPI_TABLE_PROTOCOL            *AcpiTable;
> +  EFI_STATUS Status;
> +  VOID       *Interface;
> +  EFI_EVENT  PciEnumerated;
> +  VOID       *Registration;
>  
>    //
> -  // Find the AcpiTable protocol
> +  // If PCI enumeration has been disabled, or it has already completed, 
> install
> +  // the tables at once, and let the entry point's return code reflect the 
> full
> +  // functionality.
>    //
> -  Status = gBS->LocateProtocol (
> -                  &gEfiAcpiTableProtocolGuid,
> -                  NULL,
> -                  (VOID**)&AcpiTable
> -                  );
> -  if (EFI_ERROR (Status)) {
> -    return EFI_ABORTED;
> +  if (PcdGetBool (PcdPciDisableBusEnumeration)) {
> +    return InstallTables ();
>    }
>  
> -  if (XenDetected ()) {
> -    Status = InstallXenTables (AcpiTable);
> -  } else {
> -    Status = InstallAllQemuLinkedTables (AcpiTable);
> +  Status = gBS->LocateProtocol (&gEfiPciEnumerationCompleteProtocolGuid,
> +                  NULL /* Registration */, &Interface);
> +  if (!EFI_ERROR (Status)) {
> +    return InstallTables ();
>    }
> +  ASSERT (Status == EFI_NOT_FOUND);
>  
> +  //
> +  // Otherwise, delay the installation until PCI enumeration is complete. The
> +  // entry point's return status will only reflect the callback setup.
> +  //
> +  Status = gBS->CreateEvent (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, 
> OnPciEnumerated,
> +                  NULL /* Context */, &PciEnumerated);
>    if (EFI_ERROR (Status)) {
> -    Status = FindAcpiTablesInFv (AcpiTable);
> +    return Status;
>    }
>  
> +  Status = gBS->RegisterProtocolNotify (
> +                  &gEfiPciEnumerationCompleteProtocolGuid, PciEnumerated,
> +                  &Registration);
> +  if (EFI_ERROR (Status)) {
> +    gBS->CloseEvent (PciEnumerated);
> +  }
>    return Status;
>  }
>  
> -- 
> 1.8.3.1
> 
> 
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming. The Go Parallel Website,
> sponsored by Intel and developed in partnership with Slashdot Media, is your
> hub for all things parallel software development, from weekly thought
> leadership blogs to news, videos, case studies, tutorials and more. Take a
> look and join the conversation now. http://goparallel.sourceforge.net/
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

Attachment: signature.asc
Description: signature

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