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.

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/IntelTdx/IntelTdxX64.dsc | 8 ++++++++
 OvmfPkg/OvmfPkg.dec              | 4 ++++
 OvmfPkg/OvmfPkgX64.dsc           | 9 +++++++++
 3 files changed, 21 insertions(+)

diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 43ab8bd089d9..c6195ab9c9d7 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -62,6 +62,11 @@
   #
   DEFINE UP_CPU_DXE_GUID  = 6490f1c5-ebcc-4665-8892-0075b9bb49b7
 
+  #
+  # Define the size of lazy accepted memory. The unit is MB.
+  #
+  DEFINE LAZY_ACCEPT_PARTIAL_MEM = 512
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
@@ -450,6 +455,9 @@
   # Point to the MdeModulePkg/Application/UiApp/UiApp.inf
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
 
+  # The partial memory size in Lazy accept
+  gUefiOvmfPkgTokenSpaceGuid.PcdLazyAcceptPartialMemorySize|$(LAZY_ACCEPT_PARTIAL_MEM)
+
 ################################################################################
 #
 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 5fe487f82d1a..7444765fe760 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -399,6 +399,10 @@
   ## The Tdx accept page size. 0x1000(4k),0x200000(2M)
   gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize|0x200000|UINT32|0x65
 
+  ## The partial memory size in Lazy accept. Its unit is MB.
+  ## The default value is 0 which means lazy accept is turned off.
+  gUefiOvmfPkgTokenSpaceGuid.PcdLazyAcceptPartialMemorySize|0|UINT64|0x68
+
 [PcdsDynamic, PcdsDynamicEx]
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 1448f925b782..398981b6dad8 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -78,6 +78,12 @@
   DEFINE UP_CPU_PEI_GUID  = 280251c4-1d09-4035-9062-839acb5f18c1
   DEFINE UP_CPU_DXE_GUID  = 6490f1c5-ebcc-4665-8892-0075b9bb49b7
 
+  #
+  # Define the size of lazy accepted memory. The unit is MB.
+  # In OvmfPkgX64, the lazy accept page is disabled by default.
+  #
+  DEFINE LAZY_ACCEPT_PARTIAL_MEM = 0
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS             = -DMDEPKG_NDEBUG
   INTEL:RELEASE_*_*_CC_FLAGS           = /D MDEPKG_NDEBUG
@@ -601,6 +607,9 @@
   # Point to the MdeModulePkg/Application/UiApp/UiApp.inf
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
 
+  # The partial memory size in Lazy accept
+  gUefiOvmfPkgTokenSpaceGuid.PcdLazyAcceptPartialMemorySize|$(LAZY_ACCEPT_PARTIAL_MEM)
+
 ################################################################################
 #
 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform
-- 
2.29.2.windows.2



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

Since there will be guest OSes that don't support unaccepted memory type (at least initially), and what will be run won't be known at firmware build time, can PcdLazyAcceptPartialMemorySize be initialized by a QemuFwCfg option instead of a FixedPcd? I started a different thread on this, but perhaps it was lost.


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

Hi, Glaze
We’re discussing it in another thread. I have added you into that mail thread.
Your comments is always welcomed.

Min

From: dionnaglaze via groups.io <dionnaglaze=google.com@groups.io>
Sent: Wednesday, June 15, 2022 12:10 AM
To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 07/14] OvmfPkg: Add PCD and DEFINEs for Lazy Accept page.

Since there will be guest OSes that don't support unaccepted memory type (at least initially), and what will be run won't be known at firmware build time, can PcdLazyAcceptPartialMemorySize be initialized by a QemuFwCfg option instead of a FixedPcd? I started a different thread on this, but perhaps it was lost.


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

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?
Also what exactly does this option mean?  Is this the minimum amount of
memory accepted by the firmware?

take care,
  Gerd



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

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.
> 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.

Thanks 
Min


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

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?

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.

> > 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?

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.

For guests which don't support lazy accept we could offer a (runtime)
option to simply accept all memory in ExitBootServices.

take care,
  Gerd



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

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


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

  Hi,

> > 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.

Yes, it'll most likely work for > 90% of the use cases.

The remaining cases will come some day though, so having a plan for that
would be good IMHO.

> > 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.

That'll only work with direct kernel boot, when some boot loader
is in your boot workflow this will not work.

> > 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?

Ideally without manual configuration (see also the reply in Dionna's thread).

I think ovmf should start in lazy accept mode unconditionally.  Accept
enough memory to reach dxe, but not more.  Accept additional memory
needed on demand.

Whenever we pass unaccepted memory to the guest os or not can be decided
rather late, I think we can wait with that until the first
BS->GetMemoryMap() call comes in.  Doing it late gives is more options
to handle things automatically and it also simplifies SEC code.

take care,
  Gerd



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

>
>
> > With the help of fw_cfg, we can know the size of
> > QemuFwCfgItemKernelSize/
> > QemuFwCfgItemInitrdSize/QemuFwCfgItemKernelSetupSize.
>
> That'll only work with direct kernel boot, when some boot loader
> is in your boot workflow this will not work.

Indeed, the TDX startup library does things very differently from how
we'd handle this for SEV-SNP. There are already 2 ranges of memory
prevalidated in SEC that have to be specially skipped over in PEI.
I tried the lazy accept changes from edk2-staging with SEV-SNP changes
to prevalidate only the PEI installed memory
https://github.com/AMDESE/ovmf/pull/4 and I just could not manage to
get the guest to boot. It'd crash in the QemuTryBootKernel part of
BDS.

Tom Lendacky's suggestion for SEV-SNP is to pre-accept all memory
under 4GB to make all that complexity go away. Only this approach
worked in my own testing. With the MMIO hole it's just validating 3GB
of memory.

>
> > > 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?
>
> Ideally without manual configuration (see also the reply in Dionna's thread).
>
> I think ovmf should start in lazy accept mode unconditionally.  Accept
> enough memory to reach dxe, but not more.  Accept additional memory
> needed on demand.

This will probably be complicated for SEV-SNP. I couldn't get it to
work as discussed above, but I wouldn't count myself a UEFI expert.

>
> Whenever we pass unaccepted memory to the guest os or not can be decided
> rather late, I think we can wait with that until the first
> BS->GetMemoryMap() call comes in.  Doing it late gives is more options
> to handle things automatically and it also simplifies SEC code.
>
> take care,
>   Gerd
>


-- 
-Dionna Glaze, PhD (she/her)


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

On June 16, 2022 4:52 AM, Dionna Glaze wrote:
> >
> >
> > > With the help of fw_cfg, we can know the size of
> > > QemuFwCfgItemKernelSize/
> > > QemuFwCfgItemInitrdSize/QemuFwCfgItemKernelSetupSize.
> >
> > That'll only work with direct kernel boot, when some boot loader is in
> > your boot workflow this will not work.
> 
> Indeed, the TDX startup library does things very differently from how we'd
> handle this for SEV-SNP. There are already 2 ranges of memory prevalidated
> in SEC that have to be specially skipped over in PEI.
> I tried the lazy accept changes from edk2-staging with SEV-SNP changes to
> prevalidate only the PEI installed memory
> https://github.com/AMDESE/ovmf/pull/4 and I just could not manage to get
> the guest to boot. It'd crash in the QemuTryBootKernel part of BDS.
> 
> Tom Lendacky's suggestion for SEV-SNP is to pre-accept all memory under
> 4GB to make all that complexity go away. Only this approach worked in my
> own testing. With the MMIO hole it's just validating 3GB of memory.
Accepting all memory under 4GB will make the things much simpler. In this way I think the accept-on-demand maybe not needed. 
A question: is there some performance impact when accepting all memory under 4GB?

Thanks
Min


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

> A question: is there some performance impact when accepting all memory under 4GB?

On SEV-SNP, we accept the HOBs 0-0xA000 and 0x10_0000-0xC00_0000,
which takes a couple seconds, which is non-negligible.
For VMs that want to boot very fast and still have access to a lot of
memory in the long run (e.g., a UEFI app as an enclave or sandbox), I
admit it's not my favorite solution.
That being said, supporting unaccepted memory in the guest means that
the 4GB solution doesn't preclude an accept-on-demand solution in the
future in the case that there is demand.

-- 
-Dionna Glaze, PhD (she/her)


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

On 6/16/22 11:44, Dionna Amalie Glaze wrote:
>> A question: is there some performance impact when accepting all memory under 4GB?
> 
> On SEV-SNP, we accept the HOBs 0-0xA000 and 0x10_0000-0xC00_0000,
> which takes a couple seconds, which is non-negligible.

Are you possibly including the memory pinning time (that happens in the 
hypervisor before the guest is started) in your measurement?

For example, I added from RDTSCP instructions in before and after the 
calls to MemEncryptSevSnpPreValidateSystemRam() and got:

*** DEBUG: SEC PreValidating RAM from 820000 to 1710000
*** DEBUG: TSC1=3057966080, TSC2=3070503712, delta = 12537632
*** DEBUG: PEI PreValidating RAM from 0 to A0000
*** DEBUG: TSC1=3430088800, TSC2=3432589504, delta = 2500704
*** DEBUG: PEI PreValidating RAM from 100000 to 80000000
*** DEBUG: TSC1=3436557536, TSC2=3496324336, delta = 59766800


This is for 2GB (0x80000000) of RAM. If I've done my calculations 
correctly for a 1600MHz TSC, that comes out to about 45 milliseconds.

Thanks,
Tom

> For VMs that want to boot very fast and still have access to a lot of
> memory in the long run (e.g., a UEFI app as an enclave or sandbox), I
> admit it's not my favorite solution.
> That being said, supporting unaccepted memory in the guest means that
> the 4GB solution doesn't preclude an accept-on-demand solution in the
> future in the case that there is demand.
> 


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

> Are you possibly including the memory pinning time (that happens in the
> hypervisor before the guest is started) in your measurement?
>

I might be, sorry. I'd go with your numbers.

> This is for 2GB (0x80000000) of RAM. If I've done my calculations
> correctly for a 1600MHz TSC, that comes out to about 45 milliseconds.
>


-- 
-Dionna Glaze, PhD (she/her)


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

  Hi,

> > Tom Lendacky's suggestion for SEV-SNP is to pre-accept all memory under
> > 4GB to make all that complexity go away. Only this approach worked in my
> > own testing. With the MMIO hole it's just validating 3GB of memory.
> Accepting all memory under 4GB will make the things much simpler. In this way I think the accept-on-demand maybe not needed. 
> A question: is there some performance impact when accepting all memory under 4GB?

That would certainly be easiest when it is acceptable from a performance
point of view.  Will also simplify the code because you don't have to
split the low memory block into accepted/unaccepted parts.

It'll be 3G (-machine pc) or 2G (-machine q35) of memory.
Is it possible to accept gigabyte pages btw?

And, yes, this might be enough that accept-on-demand is not needed any
more.

take care,
  Gerd



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

On 6/16/22 00:51, Gerd Hoffmann wrote:
>    Hi,
> 
>>> Tom Lendacky's suggestion for SEV-SNP is to pre-accept all memory under
>>> 4GB to make all that complexity go away. Only this approach worked in my
>>> own testing. With the MMIO hole it's just validating 3GB of memory.
>> Accepting all memory under 4GB will make the things much simpler. In this way I think the accept-on-demand maybe not needed.
>> A question: is there some performance impact when accepting all memory under 4GB?
> 
> That would certainly be easiest when it is acceptable from a performance
> point of view.  Will also simplify the code because you don't have to
> split the low memory block into accepted/unaccepted parts.
> 
> It'll be 3G (-machine pc) or 2G (-machine q35) of memory.
> Is it possible to accept gigabyte pages btw?

+Ashish/Mike - I thought I had added them earlier...

SNP supports 4K and 2MB. However, SNP can support multiple pages in a 
single Page State Change GHCB request. This might be better than having to 
exit once for every on-demand page.

Thanks,
Tom

> 
> And, yes, this might be enough that accept-on-demand is not needed any
> more.
> 
> take care,
>    Gerd
> 


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

On June 16, 2022 1:51 PM, Gerd Hoffmann wrote:
> > > Tom Lendacky's suggestion for SEV-SNP is to pre-accept all memory
> > > under 4GB to make all that complexity go away. Only this approach
> > > worked in my own testing. With the MMIO hole it's just validating 3GB of
> memory.
> > Accepting all memory under 4GB will make the things much simpler. In this
> way I think the accept-on-demand maybe not needed.
> > A question: is there some performance impact when accepting all memory
> under 4GB?
> 
> That would certainly be easiest when it is acceptable from a performance
> point of view.  Will also simplify the code because you don't have to split the
> low memory block into accepted/unaccepted parts.
> 
> It'll be 3G (-machine pc) or 2G (-machine q35) of memory.
> Is it possible to accept gigabyte pages btw?
Currently the supported page size are 4K and 2M. Gigabyte page size has not been supported.
> 
> And, yes, this might be enough that accept-on-demand is not needed any
> more.

Thanks
Min


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