Re: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformPei: consider AP stacks for pei memory cap

["Gerd Hoffmann" <kraxel@redhat.com>]

  Hi,

> > +  if (MemoryCap > MAX_UINT32) {
> > +    MemoryCap = MAX_UINT32;
> > +  }
> > +
> 
> (2) This doesn't look good, for two reasons.
> 
> First, if UINTN is UINT32, then we're too late to check. (If it's intentional that the code be dead on IA32, then that should be documented.)

IA32 has an early exit in this function.

Looking again I see this applies to 32-bit DXE only, so with mixed
32-bit PEI / 64-bit DXE we can still land here.  Hmm, so yes, you
have a point here.

In practice we don't come even close to MAX_UINT32.  With 4096 vcpus the
amount of memory needed for the AP stacks sums up to 128 MB.  The page
table memory wouldn't grow that big too because OVMF will use at most
1 TB of address space if gigabyte pages are not available (to limit page
table memory and boot time needed to set them all up).

> Second, while returning MAX_UINT32 from this function is safe, it's also obscure. After the call site, we've got
> 
>     MemoryBase = PlatformInfoHob->S3Supported && PlatformInfoHob->SmmSmramRequire ?
>                  PcdGet32 (PcdOvmfDecompressionScratchEnd) :
>                  PcdGet32 (PcdOvmfDxeMemFvBase) + PcdGet32 (PcdOvmfDxeMemFvSize);
>     MemorySize = LowerMemorySize - MemoryBase;
>     if (MemorySize > PeiMemoryCap) {
>       MemoryBase = LowerMemorySize - PeiMemoryCap;
>       MemorySize = PeiMemoryCap;
>     }

MemorySize is UINT64.  So I guess the easiest would be to just switch
MemoryCap variable and GetPeiMemoryCap() return value to UINT64 too.
Avoids all the thinking and arguing whenever UINT32 is safe or not.

And maybe add an else branch to log a warning in case
MemorySize < PeiMemoryCap.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114903): https://edk2.groups.io/g/devel/message/114903
Mute This Topic: https://groups.io/mt/104073297/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-