Re: [PATCH 07/14] OvmfPkg: Add PCD and DEFINEs for Lazy Accept page.

["Min Xu" <min.m.xu@intel.com>]

On June 8, 2022 2:18 PM, Gerd Hoffmann wrote:
> On Wed, Jun 08, 2022 at 12:06:28AM +0000, Xu, Min M wrote:
> > On June 7, 2022 6:46 PM, Gerd Hoffmann wrote:
> > > On Mon, Jun 06, 2022 at 10:59:55AM +0800, Min Xu wrote:
> > > > From: Min M Xu <min.m.xu@intel.com>
> > > >
> > > > RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3937
> > > >
> > > > Lazy accept page can be controlled in build time like below:
> > > >   -D LAZY_ACCEPT_PARTIAL_MEM=512
> > > >
> > > > The unit is MB. If it is 0 then it means Lazy-accept is turned off.
> > > >
> > > > Lazy-accept is turned off by default in OvmfPkgX64.
> > > > Lazy-accept is turned on with 512MB by default in IntelTdxX64.
> > >
> > > Care to explain?  What is the point in not using lazy accept in
> OvmfPkgX64?
> 
> > It's an Open to the community that if lazy accept should be enabled in
> > OvmfPkgX64. I would like to hear the suggestions/comments from you
> > guys. So as the first step, it is disabled.
> 
> I think OvmfPkgX64 and IntelTdxX64 configuration should be identical, and
> I'm wondering whenever this should be a compile time option in the first
> place.  Can we simply use lazy accept unconditionally?
Not all the guest OS support lazy accept. That's why there is a PCD which is used to control this feature.
IntelTdxX64 is a newly added pkg and it hasn't been widely used. So it is low risk to enable the lazy-accept by default.
But OvmfPkgX64 is more general. So at the first stage I think it may be better to disable lazy-accept by default. If someone understand what lazy-accept is and what guest os should be used, then he can enable lazy-accept in OvmfPkgX64 by adding the compile time option.
Dionna Glaze has proposed a new fw_cfg to control the lazy-accept.
> 
> With the edk2 memory management code being able to handle unaccepted
> memory I just don't see the point in supporting different options in early
> firmware setup.  I think sec/pei should accept the memory needed to run
> dxe and not more.  Anything beyond that can be accepted later on demand
> via TdxDxe.
In current implementation by Tdx guest, we accept 512MB in SEC phase and install EfiMemoryAcceptProtocol TdxDxe driver. 512MB is more memories needed by DXE. 
That is because: Tdx guest now accepts memory in 2MB page size. Accepting 512MB is not much slower than accepting 96MB. And 512MB memory can satisfy most of the usage scenarios without accepting more memories later on demand.
Of course other architecture CPUs can have their own implementation. 
> 
> > > Also what exactly does this option mean?  Is this the minimum amount
> > > of memory accepted by the firmware?
> 
> > Yes, this option defines the minimum amount of memory accepted by the
> > firmware. For example, if it is 512MB, then there will be such amount
> > memory accepted by the firmware before jump to OS.
> 
> Where does the 512MB figure come from?
512MB is an experiment value. It includes the minimum DXE memories (~96MB) and the memories to load a normal kernel-image/initrd. In our internal test, 512MB can satisfy most of the usage scenarios without accepting more memories later. And it has no serious performance impact.
> 
> Using a fixed amount of memory doesn't look very robust to me.  Is it
> possible to accept memory when the guest calls ExitBootServices?  That way
> we could guarantee a minimum amount of *free* accepted memory being
> available, to make sure the linux kernel has enough memory to decompress
> itself, allocate memory management data structures and get its own lazy
> accept support going.
With the help of fw_cfg, we can know the size of QemuFwCfgItemKernelSize/ QemuFwCfgItemInitrdSize/QemuFwCfgItemKernelSetupSize. So I am thinking we can calculate the minimum size of accepted memory (PEI/DXE/KernelSize/InitrdSize/KernelSetupSize) before it jumps to OS.  If we can calculate the minimum memory size before jumping to OS, does it mean the on-demand accept is not needed? Actually I am not conformable to change code of AllocatePages/AllocatePools.
What's your thought?
> 
> For guests which don't support lazy accept we could offer a (runtime) option
> to simply accept all memory in ExitBootServices.
What is the runtime option? Some new fw_cfg?

Thanks
Min