Re: [PATCH v9 0/4] Add safe unaccepted memory behavior

["Yao, Jiewen" <jiewen.yao@intel.com>]

Hi Dionna
I think I understand your intention.
I believe we need OS side and UEFI standard sign-off for this *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the consumer, right?
If so, I suggest you maintain the work in a edk2-stage area for https://github.com/tianocore/edk2-staging.

EDKII main branch is for production. MdePkg can only include the API definition approved by UEFI standard.
EDK2 staging is a place for POC / collaboration. That is why I think edk2 staging is more proper place for this feature.

Without OS and UEFI standard sign-off, I don't think this BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to EDKII main branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.


Thank you
Yao, Jiewen

> -----Original Message-----
> From: Dionna Glaze <dionnaglaze@google.com>
> Sent: Friday, January 13, 2023 8:14 AM
> To: devel@edk2.groups.io
> Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; James Bottomley <jejb@linux.ibm.com>; Tom
> Lendacky <Thomas.Lendacky@amd.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Aktas, Erdem <erdemaktas@google.com>; Andrew
> Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v9 0/4] Add safe unaccepted memory behavior
> 
> We make eager memory acceptance the default behavior at ExitBootServices
> by using the standard-enforced behavior that if the call returns an
> error code, then the map key is incorrect and the caller must re-call
> GetMemoryMap to ensure the contents are correct.
> 
> Eager memory acceptance is implemented by using the UEFI v2.9-added
> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES to check a support
> condition
> before changing all unaccepted memory type regions to conventional
> memory after first using the MemoryAccept protocol to accept all memory
> in each region. This update to the memory map only happens once, since
> there are no extra unaccepted memory regions to change on the forced
> second call to ExitBootServices.
> 
> The new acceptance logic is technology-agnostic and usable across TEE
> technologies, so this patch series introduces a Confidenial Compute DXE
> driver called CocoDxe.
> 
> To allow the OS loader to prevent the eager acceptance, and thus pass
> the before-mentioned "support condition", 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.
> 
> The MemoryAcceptance protocol is necessary for safe rollout of the
> unaccepted memory type, given the gradual update of guest OS kernels.
> OVMF cannot rely on the following implication
> 
>   (MemEncryptSevIsEnabled () || MemEncryptTdxIsEnabled ())
> 
>   implies
> 
>   unaccepted memory is supported by the guest
> 
> This implication does not hold for e.g., upstream Linux, and will not
> hold generally since both SEV-SNP and TDX define unaccepted memory
> support as optional rather than required.
> 
> 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 v8:
>  - First 3 patches removed since they were submitted separately.
>  - Later patches rebased on edk2/master and modified to work with the
>    current locations and namings of the unaccepted memory constants.
> 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 <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
> 
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> 
> Dionna Glaze (4):
>   OvmfPkg: Introduce CocoDxe driver
>   MdePkg: Introduce the MemoryAcceptance protocol
>   OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
>   OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> 
>  MdePkg/Include/Protocol/MemoryAcceptance.h |  40 +++++
>  MdePkg/MdePkg.dec                          |   3 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc               |   1 +
>  OvmfPkg/AmdSev/AmdSevX64.fdf               |   1 +
>  OvmfPkg/CocoDxe/CocoDxe.c                  | 175 +++++++++++++++++++++
>  OvmfPkg/CocoDxe/CocoDxe.inf                |  46 ++++++
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc           |   1 +
>  OvmfPkg/IntelTdx/IntelTdxX64.fdf           |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                 |   1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf                 |   1 +
>  OvmfPkg/OvmfPkgX64.dsc                     |   1 +
>  OvmfPkg/OvmfPkgX64.fdf                     |   1 +
>  OvmfPkg/PlatformPei/AmdSev.c               |   5 +
>  13 files changed, 277 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
>  create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
>  create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
> 
> --
> 2.39.0.314.g84b9a713c41-goog