From: Min M Xu <min.m.xu@intel.com>

RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3937

In TDVF the hob list is constructed at the memory region which is the
largest one below 4GB. After lazy accept is introduced, the
MaxAcceptedMemoryAddress (which is tha max accpeted memory address in
lazy accept) should be considered.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Library/PeilessStartupLib/Hob.c       | 23 ++++++++++++++++++-
 .../PeilessStartupLib/PeilessStartupLib.inf   |  1 +
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/PeilessStartupLib/Hob.c b/OvmfPkg/Library/PeilessStartupLib/Hob.c
index a9b92b5fbaba..884490af68fd 100644
--- a/OvmfPkg/Library/PeilessStartupLib/Hob.c
+++ b/OvmfPkg/Library/PeilessStartupLib/Hob.c
@@ -21,6 +21,7 @@
 #include <Library/PlatformInitLib.h>
 #include <OvmfPlatforms.h>
 #include <Pi/PrePiHob.h>
+#include <WorkArea.h>
 #include "PeilessStartupInternal.h"
 
 /**
@@ -74,10 +75,13 @@ ConstructFwHobList (
   )
 {
   EFI_PEI_HOB_POINTERS  Hob;
+  EFI_PHYSICAL_ADDRESS  PhysicalStart;
   EFI_PHYSICAL_ADDRESS  PhysicalEnd;
   UINT64                ResourceLength;
   EFI_PHYSICAL_ADDRESS  LowMemoryStart;
   UINT64                LowMemoryLength;
+  UINT64                MaxAcceptedMemoryAddress;
+  TDX_WORK_AREA         *WorkArea;
 
   ASSERT (VmmHobList != NULL);
 
@@ -86,14 +90,31 @@ ConstructFwHobList (
   LowMemoryLength = 0;
   LowMemoryStart  = 0;
 
+  WorkArea = (TDX_WORK_AREA *)FixedPcdGet32 (PcdOvmfWorkAreaBase);
+  ASSERT (WorkArea != NULL);
+  ASSERT (WorkArea->Header.GuestType == CcGuestTypeIntelTdx);
+  MaxAcceptedMemoryAddress = WorkArea->SecTdxWorkArea.MaxAcceptedMemoryAddress;
+  if (MaxAcceptedMemoryAddress == 0) {
+    MaxAcceptedMemoryAddress = MAX_UINT64;
+  }
+
   //
   // Parse the HOB list until end of list or matching type is found.
   //
   while (!END_OF_HOB_LIST (Hob)) {
     if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
-      if (Hob.ResourceDescriptor->ResourceType == EFI_RESOURCE_MEMORY_UNACCEPTED) {
+      if ((Hob.ResourceDescriptor->ResourceType == EFI_RESOURCE_MEMORY_UNACCEPTED) && (Hob.ResourceDescriptor->PhysicalStart < MaxAcceptedMemoryAddress)) {
         PhysicalEnd    = Hob.ResourceDescriptor->PhysicalStart + Hob.ResourceDescriptor->ResourceLength;
         ResourceLength = Hob.ResourceDescriptor->ResourceLength;
+        PhysicalStart  = Hob.ResourceDescriptor->PhysicalStart;
+
+        if ((PhysicalEnd >= MaxAcceptedMemoryAddress) && (PhysicalStart < MaxAcceptedMemoryAddress)) {
+          //
+          // This memory region is split into 2 parts. The left part is accepted.
+          //
+          PhysicalEnd    = MaxAcceptedMemoryAddress;
+          ResourceLength = PhysicalEnd - PhysicalStart;
+        }
 
         if (PhysicalEnd <= BASE_4GB) {
           if (ResourceLength > LowMemoryLength) {
diff --git a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
index def50b4b019e..eed9f27d3d01 100644
--- a/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
+++ b/OvmfPkg/Library/PeilessStartupLib/PeilessStartupLib.inf
@@ -88,3 +88,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask    ## CONSUMES
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93134): https://edk2.groups.io/g/devel/message/93134
Mute This Topic: https://groups.io/mt/93474520/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

  Hi,

> +          //
> +          // This memory region is split into 2 parts. The left part is accepted.
> +          //
> +          PhysicalEnd    = MaxAcceptedMemoryAddress;
> +          ResourceLength = PhysicalEnd - PhysicalStart;

Same comment here.  Can't happen when all memory below 4G is accepted,
and I think MaxAcceptedMemoryAddress is not needed either.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93347): https://edk2.groups.io/g/devel/message/93347
Mute This Topic: https://groups.io/mt/93474520/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On September 7, 2022 1:42 PM, Gerd Hoffmann wrote:
>   Hi,
> 
> > +          //
> > +          // This memory region is split into 2 parts. The left part is accepted.
> > +          //
> > +          PhysicalEnd    = MaxAcceptedMemoryAddress;
> > +          ResourceLength = PhysicalEnd - PhysicalStart;
> 
> Same comment here.  Can't happen when all memory below 4G is accepted,
> and I think MaxAcceptedMemoryAddress is not needed either.
> 
It may happen. For example, a TD VM is created with 2G memory, then the MaxAcceptedMemoryAddress is 0x80000000. If it is created with 1G memory, MaxAcceptedMemoryAddress is 0x40000000. This information can be retrieved by walking thru the TD-Hob and read the largest address of the unaccept-mem-region under 4G. 
But I think it's easier to record the value in MaxAcceptedMemoryAddress. And it can be used when not all memory below 4G is accepted.

Thanks
Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93469): https://edk2.groups.io/g/devel/message/93469
Mute This Topic: https://groups.io/mt/93474520/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Thu, Sep 08, 2022 at 12:48:05AM +0000, Xu, Min M wrote:
> On September 7, 2022 1:42 PM, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > +          //
> > > +          // This memory region is split into 2 parts. The left part is accepted.
> > > +          //
> > > +          PhysicalEnd    = MaxAcceptedMemoryAddress;
> > > +          ResourceLength = PhysicalEnd - PhysicalStart;
> > 
> > Same comment here.  Can't happen when all memory below 4G is accepted,
> > and I think MaxAcceptedMemoryAddress is not needed either.

> It may happen. For example, a TD VM is created with 2G memory, then
> the MaxAcceptedMemoryAddress is 0x80000000. If it is created with 1G
> memory, MaxAcceptedMemoryAddress is 0x40000000. This information can
> be retrieved by walking thru the TD-Hob and read the largest address
> of the unaccept-mem-region under 4G.  But I think it's easier to
> record the value in MaxAcceptedMemoryAddress. And it can be used when
> not all memory below 4G is accepted.

Memory regions wouldn't cross the 4G border, they are either completely
below 4G or completely above 4G.  So when we accept all memory below 4G
we will never have to split a memory region and you can easily figure
whenever an region is accepted or not by checking whenever it is above
or below 4G.

I still don't see what MaxAcceptedMemoryAddress would be needed for
given that when we never have to split memory regions.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93520): https://edk2.groups.io/g/devel/message/93520
Mute This Topic: https://groups.io/mt/93474520/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-