Hi
> > +/**
> > + Check if it is Tdx guest
> > +
> > + @retval TRUE It is Tdx guest
> > + @retval FALSE It is not Tdx guest
> > +**/
> > +BOOLEAN
> > +PlatformPeiIsTdxGuest (
> > + VOID
> > + )
> > +{
> > + CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER
> *CcWorkAreaHeader;
> > +
> > + CcWorkAreaHeader =
> (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER
> > +*)FixedPcdGet32 (PcdOvmfWorkAreaBase);
> > + return (CcWorkAreaHeader != NULL && CcWorkAreaHeader->GuestType
> ==
> > +GUEST_TYPE_INTEL_TDX); }
>
> fwcfg patch adds a similar function. Can't we have a single function
> somewhere, or have a #define for this check, instead of doing this
> cut+paste programming?
There is TdIsEnabled() in BaseLib. So PlatformPeiIsTdxGuest is deprecated in
the next version.
>
> > +VOID
> > +EFIAPI
> > +DEBUG_HOBLIST (
> > + IN CONST VOID *HobStart
> > + )
> > +{
> > + EFI_PEI_HOB_POINTERS Hob;
> > +
> > + Hob.Raw = (UINT8 *)HobStart;
> > + //
> > + // Parse the HOB list until end of list or matching type is found.
> > + //
> > + while (!END_OF_HOB_LIST (Hob)) {
> > + DEBUG ((DEBUG_INFO, "HOB(%p) : %x %x\n", Hob, Hob.Header-
> >HobType, Hob.Header->HobLength));
> > + switch (Hob.Header->HobType) {
> > + case EFI_HOB_TYPE_RESOURCE_DESCRIPTOR:
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "\t: %x %x %llx %llx\n",
> > + Hob.ResourceDescriptor->ResourceType,
> > + Hob.ResourceDescriptor->ResourceAttribute,
> > + Hob.ResourceDescriptor->PhysicalStart,
> > + Hob.ResourceDescriptor->ResourceLength
> > + ));
> > +
> > + break;
> > + case EFI_HOB_TYPE_MEMORY_ALLOCATION:
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "\t: %llx %llx %x\n",
> > + Hob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress,
> > + Hob.MemoryAllocation->AllocDescriptor.MemoryLength,
> > + Hob.MemoryAllocation->AllocDescriptor.MemoryType
> > + ));
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + Hob.Raw = GET_NEXT_HOB (Hob);
> > + }
> > +}
>
> Likewise, I've seen this before in another patch of this series.
This will be deprecated in the next version.
>
> > diff --git a/OvmfPkg/PlatformPei/MemDetect.c
> > b/OvmfPkg/PlatformPei/MemDetect.c index 934d5c196570..9227fa260ccd
> > 100644
> > --- a/OvmfPkg/PlatformPei/MemDetect.c
> > +++ b/OvmfPkg/PlatformPei/MemDetect.c
> > @@ -36,6 +36,7 @@ Module Name:
> > #include <Library/MtrrLib.h>
> > #include <Library/QemuFwCfgLib.h>
> > #include <Library/QemuFwCfgSimpleParserLib.h>
> > +#include <Library/TdxLib.h>
> >
> > #include "Platform.h"
> > #include "Cmos.h"
> > @@ -556,7 +557,19 @@ AddressWidthInitialization (
> > mPhysMemAddressWidth = 36;
> > }
> >
> > + #if defined (MDE_CPU_X64)
> > + if (PlatformPeiIsTdxGuest ()) {
> > + if (TdSharedPageMask () == (1ULL << 47)) {
> > + mPhysMemAddressWidth = 48;
> > + } else {
> > + mPhysMemAddressWidth = 52;
> > + }
> > + }
> > +
> > + ASSERT (mPhysMemAddressWidth <= 52); #else
>
> Making this TDX-specific looks wrong to me. 5-level paging exists outside TDX
> too.
>
> Given we don't support 5-level paging (yet) I think we can just drop this and
> revisit in case 5-level paging support is added in the future.
>
> > + UINT32 Pml5Entries;
>
> Same here.
mPhysMemAddressWidth indicates the physical memory address width. Tdx guest
supports GPAW of 52 or 48. So I think here mPhysMemAddresWidth is correct.
Even mPhysMemAddress is 52, we can still use 4-level paging. So I am going to
update GetPeiMemoryCap() like below:
if (mPhysMemAddressWidth <= 39) {
Pml4Entries = 1;
PdpEntries = 1 << (mPhysMemAddressWidth - 30);
ASSERT (PdpEntries <= 0x200);
} else {
if (TdIsEnabled ()) { <-- If it is td guest, then Pml4Entries is
set to 0x200.
Pml4Entries = 0x200;
} else {
Pml4Entries = 1 << (mPhysMemAddressWidth - 39);
}
ASSERT (Pml4Entries <= 0x200);
PdpEntries = 512;
}
Thanks
Min
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85863): https://edk2.groups.io/g/devel/message/85863
Mute This Topic: https://groups.io/mt/87696595/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
