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

[Ni, Ray]

Gerd,
Can you please explain a bit more on the chunk idea?

Brian,
Page allocation is not preferred in this SMM case because the pointer
in HOB entry points to another memory. StandaloneMmIpl has to
migrate the "another memory" manually to SMRAM. I want to avoid
that.

Thanks,
Ray

> -----Original Message-----
> From: Brian J. Johnson <brian.johnson at hpe.com>
> Sent: Friday, February 3, 2023 6:29 AM
> To: devel at edk2.groups.io; kraxel at redhat.com; Ni, Ray <ray.ni at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>; Wu, Jiaxin <jiaxin.wu at intel.com>; Dong,
> Eric <eric.dong at intel.com>; Zeng, Star <star.zeng at intel.com>; Kumar, Rahul R
> <rahul.r.kumar at intel.com>; Kinney, Michael D <michael.d.kinney at intel.com>;
> Zimmer, Vincent <vincent.zimmer at intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/5] UefiCpuPkg/SmmBaseHob.h: Add SMM
> Base HOB Data
> 
> On 2/2/23 06:51, Gerd Hoffmann wrote:
> >    Hi,
> >
> >>> - 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.
> >>
> >> TRUE so I propose that the first version of the code change only expects
> >> the HOB.NumberOfCpus equals to the NumberOfCpus returned from MP
> >> service, meaning the code logic only supports single instance of the HOB.
> >> When a platform that contains >8000 cpu threads resulting in multiple HOBs
> >> produced, the expectation will break and remind us that the CpuSmm driver
> >> needs to update to handle multiple HOBs.
> >
> > Given that this is already the second case where we hit the 64k size
> > limit and I expect it will not be the last one:  I think it makes sense
> > to introduce a generic and reusable concept of chunked HOBs, so you can
> > add helper functions to HobLib for splitting and reassembling, with a
> > struct along the lines of:
> >
> > typedef struct {
> > 	// offset and size of this particular chunk
> > 	UINT32	ChunkOffset;
> > 	UINT32	ChunkSize;
> >
> > 	// number of chunks and size of all chunks combined.
> > 	UINT32	ChunkCount;
> > 	UINT32	TotalSize;
> >
> > 	// chunk data
> > 	UINT8   Data[0];
> > } EFI_HOB_CHUNK;
> >
> > take care,
> >    Gerd
> >
> 
> Gerd's suggestion could be handy.  Here's another approach when data is
> too large to fit in a HOB, which doesn't require splitting up the data:
> 
> PEI tracks page allocations by generating memory allocation HOBs
> (EFI_HOB_TYPE_MEMORY_ALLOCATION.)  The
> EFI_HOB_MEMORY_ALLOCATION_HEADER
> structure in the HOB contains a "Name" field of type EFI_GUID which can
> be used to track the purpose of that particular page allocation.  It's
> zeroed by BuildMemoryAllocationHob(), and not usually used.  But if you
> put your own GUID in there, you can use it to track which memory
> allocation HOB corresponds to your data, without having to manage a
> separate HOB with a pointer.  The allocation will be automatically
> tracked through pre-RAM PEI, post-RAM PEI, and DXE, and the pages
> (although not the HOB) will even persist into Runtime (if you use an
> EfiRuntimeServices memory type.)
> 
> That wouldn't help the OP with SMM, though.  They would still have to
> copy the pages into SMRAM somehow.
> 
> Unfortunately, neither HobLib nor AllocatePages() has an interface for
> setting the "Name" field.  But you can call AllocatePages(), then search
> the HOB list for the resulting HOB, and update it's AllocDescriptor.Name
> field.
> 
> --
> Brian J. Johnson
> Enterprise X86 Lab
> 
> Hewlett Packard Enterprise


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99519): https://edk2.groups.io/g/devel/message/99519
Mute This Topic: https://groups.io/mt/96350760/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-