| Message ID | 20190409110844.14746-15-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Specific platform to run OVMF in Xen PVH and HVM guests | expand |
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
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@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)); (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 >
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@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)); > > (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 >> >
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) {
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(-)