|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [edk2-devel] [PATCH v2 14/31] OvmfPkg/AcpiPlatformDxe: Use PVH RSDP if exist
On 04/11/19 14:23, Laszlo Ersek wrote:
> 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 <anthony.perard@xxxxxxxxxx>
>>> ---
>>> 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));
>
> (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".
Sigh, I clearly need to cease reviewing this series for today. Here's
another remark:
(5) if you want to print the name of the module (such as
"AcpiPlatformDxe") in a log message, please use the "%a" format
specifier (for "ASCII string"), and use the following auto-generated
variable:
gEfiCallerBaseName
It will contain the BASE_NAME string from the module INF file.
This variable is particularly useful in files that are built into
multiple modules:
- for example, a module directory may contain two INF files, with
different BASE_NAMEs, and include a common .c file. If that .c file
produces log messages, gEfiCallerBaseName is helpful.
- the same applies even more to log messages generated from libraries --
you'll want to see the name of the driver or application that the
library is linked into. In that case, gEfiCallerBaseName refers to the
BASE_NAME of the driver/app INF file, and not the lib instance INF
file.
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
>>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |