On 11/8/22 10:09, Yao, Jiewen wrote:
Hi Tom/Dionna
I think this patch is addressing 
https://bugzilla.tianocore.org/show_bug.cgi?id=3987.

For patch 1~3, I am OK for the API which we already agreed, such as 
EfiMemoryAcceptProtocol and EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
I have given Acked by: Jiewen Yao <jiewen....@intel.com>

For patch 4, it changed the behavior to accept all. I don’t believe it should 
be 4. It should be the latest one. (please correct me if I am wrong.)

For patch 5~6, I cannot give R-B for BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed 
here because it does not address my concern. Please refer to the discussion in 
the Bugzilla.


If you want to split the patch series and submit 1~3 as standalone one, that 
will match SEV to TDX on lazy accept part. I believe we may merge them after we 
get R-B from right person.

Just FYI, without the last half of this series, I believe that TDX will only be accepting 4GB of memory when using OvmfPkgX64.dsc (see OvmfPkg/Library/PlatformInitLib/IntelTdx.c), while SNP will be accepting all memory. This would seem to present a problem for TDX with OvmfPkgX64.dsc if the OS does not have support for unaccepted memory.

Thanks,
Tom


Thank you
Yao, Jiewen


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Lendacky, Thomas via groups.io
Sent: Tuesday, November 8, 2022 10:38 PM
To: Dionna Glaze <dionnagl...@google.com>; devel@edk2.groups.io
Cc: Ard Biescheuvel <a...@kernel.org>; Xu, Min M <min.m...@intel.com>;
Gerd Hoffmann <kra...@redhat.com>; James Bottomley
<j...@linux.ibm.com>; Yao, Jiewen <jiewen....@intel.com>; Aktas, Erdem
<erdemak...@google.com>; Andrew Fish <af...@apple.com>; Kinney,
Michael D <michael.d.kin...@intel.com>
Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
behavior

On 10/24/22 15:41, Dionna Glaze wrote:
These seven patches build on the lazy-accept patch series

"Introduce Lazy-accept for Tdx guest"

Since the above series was accepted into the EDK2 tree, can this series
also be pulled in so that both TDX and SNP can support unaccepted
memory
in the same release?

Thanks,
Tom


by adding SEV-SNP support for the MemoryAccept protocol, and
importantly making eager memory acceptance the default behavior.

We implement a standardized event group from UEFI v2.9,
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides
exactly
the right invocation point for eagerly accepting memory if eager
acceptance has not been disabled.

To make use of this event group, we add a new driver that is meant to
carry behavior that is needed for all confidential compute technologies,
not just specific platforms, CocoDxe. In CocoDxe we implement the
default safe behavior to accept all unaccepted memory and invalidate
the MemoryMap on ExitBootServices.

To allow the OS loader to prevent the eager acceptance, we add a new
protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
This protocol has one interface, Disable(). The OS loader can inform the
UEFI that it supports the unaccepted memory type and accepts the
responsibility to accept it.

All images that support unaccepted memory must now locate and call
this
new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call
the Disable
function.

Changes since v7:
   - Rebased onto lazy accept v4 patch series, so memory accept protocol
     has the EDKII prefix, and the unaccepted memory type has the BZ3937
     prefix.
   - Removed a bad #include to a header removed in v7.
   - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL
as per the
     discussion on the buganizer issue.
   - Uncrustify formatting

Changes since v6:
   - Added implementation of
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
   - Changed callback protocol of v5 to instead use the standardized event
     group for before_exit_boot_services.

Changes since v5:
   - Generic callback protocol moved to MdeModulePkg
   - Removed use of EFI_WARN_STALE_DATA and added comment that the
callback
     should only return EFI_SUCCESS or EFI_INVALID_PARAMETER.
   - Removed errant log statement and fixed formatting.

Changes since v4:
   - Commit message wording
   - Replaced direct change to DxeMain with a more generic callback
     protocol.
   - Implemented the direct change as an instance of the callback protocol
     from a new CocoDxe driver.
   - Replaced "enable" protocol with a "disable" protocol, since the name
     was confusing. The AcceptAllUnacceptedMemory protocol directly
names
     the behavior that is disabling.

Changes since v3:
   - "DxeMain accepts all memory" patch split into 3 to make each patch
     affect only one package at a time.

Changes since v2:
   - Removed the redundant memory accept interface and added the accept
     behavior to the DXE implementation of
     MemEncryptSevSnpPreValidateSystemRam.
   - Fixed missing #include in >=4GB patch.

Changes since v1:
   - Added a patch to classify SEV-SNP memory above 4GB unaccepted.
   - Fixed style problems in EfiMemoryAcceptProtocol implementation.

Cc: Ard Biescheuvel <a...@kernel.org>
Cc: "Min M. Xu" <min.m...@intel.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Cc: James Bottomley <j...@linux.ibm.com>
Cc: Tom Lendacky <thomas.lenda...@amd.com>
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Erdem Aktas <erdemak...@google.com>
Cc: Andrew Fish <af...@apple.com>
Cc: "Michael D. Kinney" <michael.d.kin...@intel.com>

Signed-off-by: Dionna Glaze <dionnagl...@google.com>

Dionna Glaze (7):
    OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
    MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
    MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
    OvmfPkg: Introduce CocoDxe driver
    MdePkg: Introduce the MemoryAcceptance protocol
    OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
    OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

   MdeModulePkg/Core/Dxe/DxeMain.inf                                  |   1 +
   MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c                            |   6 +
   MdePkg/Include/Guid/EventGroup.h                                   |   5 +
   MdePkg/Include/Protocol/MemoryAcceptance.h                         |  40
+++++
   MdePkg/MdePkg.dec                                                  |   8 +-
   OvmfPkg/AmdSev/AmdSevX64.dsc                                       |   1 +
   OvmfPkg/AmdSev/AmdSevX64.fdf                                       |   1 +
   OvmfPkg/AmdSevDxe/AmdSevDxe.c                                      |  55 
++++++-
   OvmfPkg/AmdSevDxe/AmdSevDxe.inf                                    |   3 +
   OvmfPkg/CocoDxe/CocoDxe.c                                          | 174
++++++++++++++++++++
   OvmfPkg/CocoDxe/CocoDxe.inf                                        |  46 
++++++
   OvmfPkg/IntelTdx/IntelTdxX64.dsc                                   |   1 +
   OvmfPkg/IntelTdx/IntelTdxX64.fdf                                   |   1 +

OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
c |  24 ++-
   OvmfPkg/OvmfPkgIa32X64.dsc                                         |   1 +
   OvmfPkg/OvmfPkgIa32X64.fdf                                         |   1 +
   OvmfPkg/OvmfPkgX64.dsc                                             |   1 +
   OvmfPkg/OvmfPkgX64.fdf                                             |   1 +
   OvmfPkg/PlatformPei/AmdSev.c                                       |   5 +
   19 files changed, 366 insertions(+), 9 deletions(-)
   create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
   create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
   create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96109): https://edk2.groups.io/g/devel/message/96109
Mute This Topic: https://groups.io/mt/94544528/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to