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.

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 (#96083): https://edk2.groups.io/g/devel/message/96083
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