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

["Ard Biesheuvel" <ardb@kernel.org>]

On Thu, 26 Jan 2023 at 22:17, Dionna Glaze <dionnaglaze@google.com> wrote:
>
> We make eager memory acceptance the default behavior at
> ExitBootServices for SEV-SNP machines 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 required only for SEV-SNP since it is the
> only memory-accepting virtualization technology with kernel support live
> without unaccepted memory support.
>
> To allow the OS loader to prevent the eager acceptance, and thus pass
> the before-mentioned "support condition", we add a new protocol,
> OvmfSevMemoryAcceptance.  This protocol has one interface,
> AllowUnacceptedMemory(). The OS loader can inform the UEFI that it
> supports the unaccepted memory type and accepts the responsibility to
> accept it.
>
> The OvmfSevMemoryAcceptance protocol is necessary for safe rollout of
> the unaccepted memory type in SEV-SNP-enabled kernels, given the
> gradual update of guest OS kernels.
>
> All images that support unaccepted memory must now locate and call this
> new OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL and call the
> AllowUnacceptedMemory function.
>
> Changes since v10:
>  - AmdSevDxe called AcceptMemory directly without locating the
>    MemoryAccept protocol.
>  - The protocol is no longer a candidate for standardization and has
>    moved to OvmfPkg/Include/Protocol.
> Changes since v9:
>  - Renamed protocol to SevMemoryAcceptance.
>  - Removed CocoDxe and moved all contained code to AmdSevDxe.
>  - Renamed protocol header file to reference the bugzilla number.
> 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: Add memory acceptance event in AmdSevDxe
>   MdePkg: Introduce the SevMemoryAcceptance protocol
>   OvmfPkg: Implement AcceptAllUnacceptedMemory in AmdSevDxe
>   OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
>

For the series,

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Thanks a lot for your persistence.

Queued as #3954

>  OvmfPkg/AmdSevDxe/AmdSevDxe.c                  | 123 ++++++++++++++++++++
>  OvmfPkg/AmdSevDxe/AmdSevDxe.inf                |   2 +
>  OvmfPkg/Include/Protocol/SevMemoryAcceptance.h |  42 +++++++
>  OvmfPkg/OvmfPkg.dec                            |   1 +
>  OvmfPkg/PlatformPei/AmdSev.c                   |   5 +
>  5 files changed, 173 insertions(+)
>  create mode 100644 OvmfPkg/Include/Protocol/SevMemoryAcceptance.h
>
> --
> 2.39.1.456.gfc5497dd1b-goog
>