From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 11 Apr 2019 05:20:26 -0700 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 93AD230B087C; Thu, 11 Apr 2019 12:20:25 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-225.rdu2.redhat.com [10.10.120.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id C6C63608BB; Thu, 11 Apr 2019 12:20:23 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 14/31] OvmfPkg/AcpiPlatformDxe: Use PVH RSDP if exist To: devel@edk2.groups.io, anthony.perard@citrix.com Cc: Jordan Justen , Ard Biesheuvel , Julien Grall , xen-devel@lists.xenproject.org References: <20190409110844.14746-1-anthony.perard@citrix.com> <20190409110844.14746-15-anthony.perard@citrix.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 11 Apr 2019 14:20:22 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190409110844.14746-15-anthony.perard@citrix.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Thu, 11 Apr 2019 12:20:25 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > --- > 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 > #include > #include > +#include > > #include > > @@ -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 > -#include > #include > > #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