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:23:39 -0700 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4A5C23008CDF; Thu, 11 Apr 2019 12:23:39 +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 A082F5D6A9; Thu, 11 Apr 2019 12:23:37 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 14/31] OvmfPkg/AcpiPlatformDxe: Use PVH RSDP if exist From: "Laszlo Ersek" 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> Message-ID: <972252b3-4de1-acaa-03d3-5a7d27a68630@redhat.com> Date: Thu, 11 Apr 2019 14:23:36 +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: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.48]); Thu, 11 Apr 2019 12:23:39 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 04/11/19 14:20, Laszlo Ersek wrote: > 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)); (4) sorry, missed this one: please - either print pointers with %p, - or convert them with (UINT64)(UINTN)XenInfo->RsdpPvh and log them with "0x%016Lx". Thanks, Laszlo >> + *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 >