Hi
Would you please clarify why the IA32 version implementation is empty?
Does it mean IA32 does not need validate?
Or IA32 should never call this function?
Anyway, I recommend to add some comment to describe it clearly.
If it should never be called, I recommend to add ASSERT(FALSE).
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
@@ -0,0 +1,17 @@
+#include <Uefi/UefiBaseType.h>
+
+#include "../SnpPageStateChange.h"
+
+/**
+ The function is used to set the page state when SEV-SNP is active. The page
state
+ transition consist of changing the page ownership in the RMP table, and using
the
+ PVALIDATE instruction to update the Validated bit in RMP table.
+
+ */
+VOID
+SevSnpValidateSystemRamInternal (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINTN NumPages
+ )
+{
+}
> -----Original Message-----
> From: Brijesh Singh <brijesh.si...@amd.com>
> Sent: Wednesday, March 24, 2021 11:32 PM
> To: devel@edk2.groups.io
> Cc: Brijesh Singh <brijesh.si...@amd.com>; James Bottomley
> <j...@linux.ibm.com>; Xu, Min M <min.m...@intel.com>; Yao, Jiewen
> <jiewen....@intel.com>; Tom Lendacky <thomas.lenda...@amd.com>; Justen,
> Jordan L <jordan.l.jus...@intel.com>; Ard Biesheuvel
> <ardb+tianoc...@kernel.org>; Laszlo Ersek <ler...@redhat.com>
> Subject: [RFC PATCH 12/19] OvmfPkg/MemEncryptSevLib: Add support to
> validate system RAM
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
>
> Many of the integrity guarantees of SEV-SNP are enforced through the
> Reverse Map Table (RMP). Each RMP entry contains the GPA at which a
> particular page of DRAM should be mapped. The guest can request the
> hypervisor to add pages in the RMP table via the Page State Change VMGEXIT
> defined in the GHCB specification section 2.5.1 and 4.1.6. Inside each RMP
> entry is a Validated flag; this flag is automatically cleared to 0 by the
> CPU hardware when a new RMP entry is created for a guest. Each VM page
> can be either validated or invalidated, as indicated by the Validated
> flag in the RMP entry. Memory access to a private page that is not
> validated generates a #VC. A VM can use the PVALIDATE instruction to
> validate the private page before using it.
>
> During the guest creation, the boot ROM memory is pre-validated by the
> AMD-SEV firmware. The MemEncryptSevSnpValidateSystemRam() can be called
> during the SEC and PEI phase to validate the detected system RAM.
>
> One of the fields in the Page State Change NAE is the RMP page size. The
> page size input parameter indicates that either a 4KB or 2MB page should
> be used while adding the RMP entry. During the validation, when possible,
> the MemEncryptSevSnpValidateSystemRam() will use the 2MB entry. A
> hypervisor backing the memory may choose to use the different page size
> in the RMP entry. In those cases, the PVALIDATE instruction should return
> SIZEMISMATCH. If a SIZEMISMATCH is detected, then validate all 512-pages
> constituting a 2MB region.
>
> Upon completion, the PVALIDATE instruction sets the rFLAGS.CF to 0 if
> instruction changed the RMP entry and to 1 if the instruction did not
> change the RMP entry. The rFlags.CF will be 1 only when a memory region
> is already validated. We should not double validate a memory
> as it could lead to a security compromise. If double validation is
> detected, terminate the boot.
>
> Cc: James Bottomley <j...@linux.ibm.com>
> Cc: Min Xu <min.m...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> ---
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 15
> ++
> OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c | 17
> ++
> OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf | 4 +
> OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c |
> 20 ++
> OvmfPkg/Library/BaseMemEncryptSevLib/SnpPageStateChange.h | 37
> +++
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c |
> 23 ++
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c |
> 254 ++++++++++++++++++++
> 7 files changed, 370 insertions(+)
>
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index 03d9eda392..47d6802b61 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -215,4 +215,19 @@ MemEncryptSevGetAddressRangeState (
> IN UINTN Length
> );
>
> +/**
> + If SEV-SNP is active then set the page state of the specified virtual
> + address range. This should be called in SEC and PEI phases only.
> +
> + @param[in] BaseAddress Base address
> + @param[in] NumPages Number of pages starting from the base
> address
> +
> +**/
> +VOID
> +EFIAPI
> +MemEncryptSevSnpValidateSystemRam (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINTN NumPages
> + );
> +
> #endif // _MEM_ENCRYPT_SEV_LIB_H_
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
> new file mode 100644
> index 0000000000..dace5c0bcf
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
> @@ -0,0 +1,17 @@
> +#include <Uefi/UefiBaseType.h>
> +
> +#include "../SnpPageStateChange.h"
> +
> +/**
> + The function is used to set the page state when SEV-SNP is active. The page
> state
> + transition consist of changing the page ownership in the RMP table, and
> using
> the
> + PVALIDATE instruction to update the Validated bit in RMP table.
> +
> + */
> +VOID
> +SevSnpValidateSystemRamInternal (
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINTN NumPages
> + )
> +{
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
> b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
> index 279c38bfbc..8595e244c2 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
> @@ -31,15 +31,19 @@
>
> [Sources]
> SecMemEncryptSevLibInternal.c
> + SnpPageStateChange.h
>
> [Sources.X64]
> X64/MemEncryptSevLib.c
> X64/SecVirtualMemory.c
> + X64/SecSnpSystemRamValidate.c
> + X64/SnpPageStateChangeInternal.c
> X64/VirtualMemory.c
> X64/VirtualMemory.h
>
> [Sources.IA32]
> Ia32/MemEncryptSevLib.c
> + Ia32/SnpPageStateChange.c
>
> [LibraryClasses]
> BaseLib
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> index 69852779e2..35a222e75e 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
> @@ -17,6 +17,8 @@
> #include <Register/Cpuid.h>
> #include <Uefi/UefiBaseType.h>
>
> +#include "SnpPageStateChange.h"
> +
> /**
> Reads and sets the status of SEV features.
>
> @@ -172,3 +174,21 @@
> MemEncryptSevLocateInitialSmramSaveStateMapPages (
> {
> return RETURN_UNSUPPORTED;
> }
> +
> +/**
> + If SEV-SNP is active then set the page state of the specified virtual
> + address range. This should be called in SEC and PEI phases only.
> +
> + @param[in] BaseAddress Base address
> + @param[in] NumPages Number of pages starting from the base
> address
> +
> +**/
> +VOID
> +EFIAPI
> +MemEncryptSevSnpValidateSystemRam (
> + IN PHYSICAL_ADDRESS BaseAddress,
> + IN UINTN NumPages
> + )
> +{
> + SevSnpValidateSystemRam (BaseAddress, NumPages);
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SnpPageStateChange.h
> b/OvmfPkg/Library/BaseMemEncryptSevLib/SnpPageStateChange.h
> new file mode 100644
> index 0000000000..3040930999
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SnpPageStateChange.h
> @@ -0,0 +1,37 @@
> +/** @file
> +
> + SEV-SNP Page Validation functions.
> +
> + Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef SNP_PAGE_STATE_INTERNAL_H_
> +#define SNP_PAGE_STATE_INTERNAL_H_
> +
> +//
> +// SEV-SNP Page states
> +//
> +typedef enum {
> + SevSnpPagePrivate,
> + SevSnpPageShared,
> +
> +} SEV_SNP_PAGE_STATE;
> +
> +VOID
> +SevSnpValidateSystemRam (
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINTN NumPages
> + );
> +
> +VOID
> +SetPageStateInternal (
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINTN NumPages,
> + IN SEV_SNP_PAGE_STATE State,
> + IN BOOLEAN UseLargeEntry
> + );
> +
> +#endif
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
> new file mode 100644
> index 0000000000..915706aad0
> --- /dev/null
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
> @@ -0,0 +1,23 @@
> +/** @file
> +
> + SEV-SNP Page Validation functions.
> +
> + Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Library/BaseLib.h>
> +
> +#include "../SnpPageStateChange.h"
> +
> +VOID
> +SevSnpValidateSystemRam (
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINTN NumPages
> + )
> +{
> + SetPageStateInternal (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
> +}
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> new file mode 100644
> index 0000000000..5a34db33fe
> --- /dev/null
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> @@ -0,0 +1,254 @@
> +/** @file
> +
> + SEV-SNP Page Validation functions.
> +
> + Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/MemEncryptSevLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/VmgExitLib.h>
> +
> +#include <Register/Amd/Ghcb.h>
> +#include <Register/Amd/Msr.h>
> +
> +#include "../SnpPageStateChange.h"
> +
> +#define IS_ALIGNED(x, y) ((((x) & (y - 1)) == 0))
> +#define PAGES_PER_LARGE_ENTRY 512
> +#define EFI_LARGE_PAGE (EFI_PAGE_SIZE * PAGES_PER_LARGE_ENTRY)
> +
> +STATIC
> +UINTN
> +MemoryStateToGhcbOp (
> + IN SEV_SNP_PAGE_STATE State
> + )
> +{
> + UINTN Cmd;
> +
> + switch (State) {
> + case SevSnpPageShared: Cmd = SNP_PAGE_STATE_SHARED; break;
> + case SevSnpPagePrivate: Cmd = SNP_PAGE_STATE_PRIVATE; break;
> + default: ASSERT(0);
> + }
> +
> + return Cmd;
> +}
> +
> +STATIC
> +VOID
> +SnpPageStateFailureTerminate (
> + VOID
> + )
> +{
> + MSR_SEV_ES_GHCB_REGISTER Msr;
> +
> + //
> + // Use the GHCB MSR Protocol to request termination by the hypervisor
> + //
> + Msr.GhcbPhysicalAddress = 0;
> + Msr.GhcbTerminate.Function = GHCB_INFO_TERMINATE_REQUEST;
> + Msr.GhcbTerminate.ReasonCodeSet = GHCB_TERMINATE_GHCB;
> + Msr.GhcbTerminate.ReasonCode = GHCB_TERMINATE_GHCB_GENERAL;
> + AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
> +
> + AsmVmgExit ();
> +
> + ASSERT (FALSE);
> + CpuDeadLoop ();
> +}
> +
> +STATIC
> +UINTN
> +IssuePvalidate (
> + IN UINTN Address,
> + IN UINTN RmpPageSize,
> + IN BOOLEAN Validate
> + )
> +{
> + IA32_EFLAGS32 EFlags;
> + UINTN Ret;
> +
> + Ret = AsmPvalidate (RmpPageSize, Validate, Address, &EFlags);
> +
> + //
> + // Check the rFlags.CF to verify that PVALIDATE updated the RMP
> + // entry. If there was a no change in the RMP entry then we are
> + // either double validating or invalidating the memory. This can
> + // lead to a security compromise.
> + //
> + if (EFlags.Bits.CF) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: Double %a detected for address 0x%Lx\n",
> + gEfiCallerBaseName,
> + __FUNCTION__,
> + Validate ? "Validate" : "Invalidate",
> + Address));
> + SnpPageStateFailureTerminate ();
> + }
> +
> + return Ret;
> +}
> +
> +/**
> + This function issues the PVALIDATE instruction to validate or invalidate the
> memory
> + range specified. If PVALIDATE returns size mismatch then it tries validating
> with
> + smaller page size.
> +
> + */
> +STATIC
> +VOID
> +PvalidateRange (
> + IN SNP_PAGE_STATE_CHANGE_INFO *Info,
> + IN UINTN StartIndex,
> + IN UINTN EndIndex,
> + IN BOOLEAN Validate
> + )
> +{
> + UINTN Address, RmpPageSize, Ret, i;
> +
> + for (; StartIndex < EndIndex; StartIndex++) {
> + Address = Info->Entry[StartIndex].GuestFrameNumber << EFI_PAGE_SHIFT;
> + RmpPageSize = Info->Entry[StartIndex].PageSize;
> +
> + Ret = IssuePvalidate (Address, RmpPageSize, Validate);
> +
> + //
> + // If we fail to validate due to size mismatch then try with the
> + // smaller page size. This senario will occur if the backing page in
> + // the RMP entry is 4K and we are validating it as a 2MB.
> + //
> + if ((Ret == PVALIDATE_RET_FAIL_SIZEMISMATCH) &&
> + (RmpPageSize == PVALIDATE_PAGE_SIZE_2M)) {
> + for (i = 0; i < PAGES_PER_LARGE_ENTRY; i++) {
> +
> + Ret = IssuePvalidate (Address, PVALIDATE_PAGE_SIZE_4K, Validate);
> + if (Ret) {
> + break;
> + }
> +
> + Address = Address + EFI_PAGE_SIZE;
> + }
> + }
> +
> + if (Ret) {
> + DEBUG ((DEBUG_ERROR, "%a:%a: Failed to %a address 0x%Lx Error
> code %d\n",
> + gEfiCallerBaseName,
> + __FUNCTION__,
> + Validate ? "Validate" : "Invalidate",
> + Address,
> + Ret));
> + SnpPageStateFailureTerminate ();
> + }
> + }
> +}
> +
> +/**
> + The function is used to set the page state when SEV-SNP is active. The page
> state
> + transition consist of changing the page ownership in the RMP table, and
> using
> the
> + PVALIDATE instruction to update the Validated bit in RMP table.
> +
> + When the UseLargeEntry is set to TRUE, then use the large RMP entry.
> + */
> +VOID
> +SetPageStateInternal (
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINTN NumPages,
> + IN SEV_SNP_PAGE_STATE State,
> + IN BOOLEAN UseLargeEntry
> + )
> +{
> + EFI_STATUS Status;
> + GHCB *Ghcb;
> + EFI_PHYSICAL_ADDRESS NextAddress, EndAddress;
> + MSR_SEV_ES_GHCB_REGISTER Msr;
> + BOOLEAN InterruptState;
> + SNP_PAGE_STATE_CHANGE_INFO *Info;
> + UINTN i, RmpPageSize;
> +
> + Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> + Ghcb = Msr.Ghcb;
> +
> + EndAddress = BaseAddress + EFI_PAGES_TO_SIZE (NumPages);
> +
> + DEBUG ((DEBUG_VERBOSE, "%a:%a Address 0x%Lx - 0x%Lx State = %a
> LargeEntry = %d\n",
> + gEfiCallerBaseName,
> + __FUNCTION__,
> + BaseAddress,
> + EndAddress,
> + State == SevSnpPageShared ? "Shared" : "Private",
> + UseLargeEntry));
> +
> + for (; BaseAddress < EndAddress; BaseAddress = NextAddress) {
> +
> + //
> + // Initialize the GHCB and setup scratch sw to point to shared buffer.
> + //
> + VmgInit (Ghcb, &InterruptState);
> + Info = (SNP_PAGE_STATE_CHANGE_INFO *) Ghcb->SharedBuffer;
> +
> + SetMem (Info, sizeof (*Info), 0);
> +
> + //
> + // Build page state change buffer
> + //
> + for (i = 0; (EndAddress > BaseAddress) && i < SNP_PAGE_STATE_MAX_ENTRY;
> + BaseAddress = NextAddress, i++) {
> + //
> + // Is this a 2MB aligned page? Check if we can use the Large RMP entry.
> + //
> + if (UseLargeEntry &&
> + IS_ALIGNED (BaseAddress, EFI_LARGE_PAGE) &&
> + ((EndAddress - BaseAddress) >> EFI_PAGE_SHIFT) >=
> PAGES_PER_LARGE_ENTRY) {
> + RmpPageSize = PVALIDATE_PAGE_SIZE_2M;
> + NextAddress = BaseAddress + EFI_LARGE_PAGE;
> + } else {
> + RmpPageSize = PVALIDATE_PAGE_SIZE_4K;
> + NextAddress = BaseAddress + EFI_PAGE_SIZE;
> + }
> +
> + Info->Entry[i].GuestFrameNumber = BaseAddress >> EFI_PAGE_SHIFT;
> + Info->Entry[i].PageSize = RmpPageSize;
> + Info->Entry[i].Op = MemoryStateToGhcbOp (State);
> + Info->Entry[i].CurrentPage = 0;
> + }
> +
> + Info->Header.CurrentEntry = 0;
> + Info->Header.EndEntry = i - 1;
> +
> + //
> + // If the request page state change is shared then invalidate the pages
> before
> + // adding the page in the RMP table.
> + //
> + if (State == SevSnpPageShared) {
> + PvalidateRange (Info, 0, i, FALSE);
> + }
> +
> + //
> + // Issue the VMGEXIT and retry if hypervisor failed to process all the
> entries.
> + //
> + Ghcb->SaveArea.SwScratch = (UINT64) Ghcb->SharedBuffer;
> + VmgSetOffsetValid (Ghcb, GhcbSwScratch);
> + while (Info->Header.CurrentEntry <= Info->Header.EndEntry) {
> + Status = VmgExit (Ghcb, SVM_EXIT_SNP_PAGE_STATE_CHANGE, 0, 0);
> + if (EFI_ERROR (Status)) {
> + SnpPageStateFailureTerminate ();
> + }
> + }
> +
> + //
> + // If the request page state change is shared then invalidate the pages
> before
> + // adding the page in the RMP table.
> + //
> + if (State == SevSnpPagePrivate) {
> + PvalidateRange (Info, 0, i, TRUE);
> + }
> +
> + VmgDone (Ghcb, InterruptState);
> + }
> +}
> --
> 2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73598): https://edk2.groups.io/g/devel/message/73598
Mute This Topic: https://groups.io/mt/81584589/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
