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] -=-=-=-=-=-=-=-=-=-=-=-