Re: [edk2-devel] [PATCH v2 14/31] OvmfPkg/AcpiPlatformDxe: Use PVH RSDP if exist

["Laszlo Ersek" <lersek@redhat.com>]

On 04/09/19 13:08, Anthony PERARD wrote:
> If the firmware have been started via the PVH entry point, a RSDP
> pointer would have been provided. Use it.
> 
> Also, use XenDetect() from the new XenPlatformLib.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc                     |  1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                  |  1 +
>  OvmfPkg/OvmfPkgX64.dsc                      |  1 +
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf |  2 +-
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h      |  6 +--
>  OvmfPkg/AcpiPlatformDxe/Xen.c               | 41 ++++++++------------
>  6 files changed, 22 insertions(+), 30 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index f55ab5a3d2..cab9764cab 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -205,6 +205,7 @@ [LibraryClasses]
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>    XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> +  XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
>  
>  !if $(TPM2_ENABLE) == TRUE
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 5c9bdf034e..a156358690 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -210,6 +210,7 @@ [LibraryClasses]
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>    XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> +  XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
>  
>  !if $(TPM2_ENABLE) == TRUE
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 2943e9e8af..9d8dc442b4 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -210,6 +210,7 @@ [LibraryClasses]
>    SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
>    OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
>    XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> +  XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
>  
>  !if $(TPM2_ENABLE) == TRUE
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> index 8440e7b343..ca54a3bd9e 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
> @@ -51,13 +51,13 @@ [LibraryClasses]
>    DebugLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> -  HobLib
>    QemuFwCfgLib
>    QemuFwCfgS3Lib
>    MemoryAllocationLib
>    BaseLib
>    DxeServicesTableLib
>    OrderedCollectionLib
> +  XenPlatformLib
>  
>  [Protocols]
>    gEfiAcpiTableProtocolGuid                     # PROTOCOL ALWAYS_CONSUMED
> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> index 83b981ee00..85f37128dd 100644
> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
> @@ -25,6 +25,7 @@
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/XenPlatformLib.h>
>  
>  #include <IndustryStandard/Acpi.h>
>  
> @@ -58,11 +59,6 @@ QemuInstallAcpiTable (
>    OUT  UINTN                         *TableKey
>    );
>  
> -BOOLEAN
> -XenDetected (
> -  VOID
> -  );
> -
>  EFI_STATUS
>  EFIAPI
>  InstallXenTables (
> diff --git a/OvmfPkg/AcpiPlatformDxe/Xen.c b/OvmfPkg/AcpiPlatformDxe/Xen.c
> index 618ac58b42..7e9a7797d7 100644
> --- a/OvmfPkg/AcpiPlatformDxe/Xen.c
> +++ b/OvmfPkg/AcpiPlatformDxe/Xen.c
> @@ -15,8 +15,6 @@
>  **/ 
>  
>  #include "AcpiPlatform.h"
> -#include <Library/HobLib.h>
> -#include <Guid/XenInfo.h>
>  #include <Library/BaseLib.h>
>  
>  #define XEN_ACPI_PHYSICAL_ADDRESS         0x000EA020
> @@ -24,28 +22,6 @@
>  
>  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *XenAcpiRsdpStructurePtr = NULL;
>  
> -/**
> -  This function detects if OVMF is running on Xen.
> -
> -**/
> -BOOLEAN
> -XenDetected (
> -  VOID
> -  )
> -{
> -  EFI_HOB_GUID_TYPE         *GuidHob;
> -
> -  //
> -  // See if a XenInfo HOB is available
> -  //
> -  GuidHob = GetFirstGuidHob (&gEfiXenInfoGuid);
> -  if (GuidHob == NULL) {
> -    return FALSE;
> -  }
> -
> -  return TRUE;
> -}
> -
>  /**
>    Get the address of Xen ACPI Root System Description Pointer (RSDP)
>    structure.
> @@ -66,10 +42,27 @@ GetXenAcpiRsdp (
>    EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER   *RsdpStructurePtr;
>    UINT8                                          *XenAcpiPtr;
>    UINT8                                          Sum;
> +  EFI_XEN_INFO                                   *XenInfo;
>  
>    //
>    // Detect the RSDP structure
>    //
> +
> +  //
> +  // First look for PVH one
> +  //
> +  XenInfo = XenGetInfoHOB ();
> +  ASSERT (XenInfo != NULL);
> +  if (XenInfo->RsdpPvh != NULL) {
> +    DEBUG ((DEBUG_INFO, "AcpiPlatformDxe: Use ACPI RSDP table at 0x%08x\n",
> +            XenInfo->RsdpPvh));
> +    *RsdpPtr = XenInfo->RsdpPvh;
> +    return EFI_SUCCESS;
> +  }
> +
> +  //
> +  // Otherwise, look for the HVM one
> +  //
>    for (XenAcpiPtr = (UINT8*)(UINTN) XEN_ACPI_PHYSICAL_ADDRESS;
>         XenAcpiPtr < (UINT8*)(UINTN) XEN_BIOS_PHYSICAL_END;
>         XenAcpiPtr += 0x10) {
> 

Can you please split this patch in two:

(1) the first patch should only rebase AcpiPlatformDxe to the new
library, without functional changes.

(2) the second patch should add the new feature, of looking for the PVH
RSDP.

(3) In the 2nd patch, pls mention Xen in the subject line, such as:

  OvmfPkg/AcpiPlatformDxe: Use Xen PVH RSDP if it exist

Thanks!
Laszlo