Re: [PATCH v3 1/5] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data

["Laszlo Ersek" <lersek@redhat.com>]

On 1/18/23 16:06, Ni, Ray wrote:

> #pragma pack(1)
> typedef struct {
>   UINT32    CpuIndex;
>   UINT32    NumberOfCpus;  // align to EFI_SEC_PLATFORM_INFORMATION_RECORD2.NumberOfCpus
>   UINT64    SmBase[];
> } SMM_BASE_HOB_DATA;
> #pragma pack()
> 
> For system with less than 8K CPUs, one HOB is produced. CpuIndex is set to 0 indicating
> the HOB describes the CPU from 0 to NumberOfCpus-1.
> 
> The HOB list may contains multiple such HOB instances each describing the information for
> CPU from CpuIndex to CpuIndex + NumberOfCpus - 1.
> The instance order in the HOB list is random so consumer cannot assume the CpuIndex
> of first instance is 0.

When using discontiguous blocks that are limited to ~64KB each:

- The consumer won't be able to access elements of the "conceptual" big
array in a truly random (= random-access) fashion. There won't be a
single contiguous domain of valid subscripts. It's "bank switching", and
switching banks should be avoided IMO. It effectively replaces the
vector data structure with a linked list. The consequence is that the
consumer will have to either (a) build a new (temporary, or permanent)
index table of sorts, for implementing the "conceptual" big array as a
factual contiguous array, or (b) traverse the HOB list multiple times.

- If the element size of the array increases (which is otherwise
possible to do compatibly, e.g. by placing a GUID and/or revision# in
the HOB), the number of elements that fit in a single HOB decreases. I
think that's an artifact that needlessly complicates debugging, and
maybe performance too (it might increase the number of full-list
traversals).

- With relatively many elements fitting into a single HOB, on most
platforms, just one HOB is going to be used. While that may be good for
performance, it is not good for code coverage (testing). The quirky
indexing method will not be exercised by most platforms.

What's wrong with:

- restricting the creation of all such HOBs after
"gEfiPeiMemoryDiscoveredPpiGuid"

- using a page allocation, for representing the array contiguously

- in the HOB, only storing the address of the page allocation.

Page allocations performed after gEfiPeiMemoryDiscoveredPpiGuid will not
be moved, so the address in the HOB would be stable.

Laszlo