On 05/13/21 01:46, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
>
> The Flush parameter is used to provide a hint whether the specified range
> is Mmio address. Now that we have a dedicated helper to clear the
> memory encryption mask for the Mmio address range, its safe to remove the
> Flush parameter from MemEncryptSev{Set,Clear}PageEncMask().
>
> Since the address specified in the MemEncryptSev{Set,Clear}PageEncMask()
> points to a system RAM, thus a cache flush is required during the
> encryption mask update.
>
> 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>
> Cc: Erdem Aktas <erdemak...@google.com>
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> ---
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 10 ++--------
> .../BaseMemEncryptSevLib/X64/VirtualMemory.h | 10 ++--------
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 3 +--
> OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 6 ++----
> .../BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c | 10 ++--------
> .../BaseMemEncryptSevLib/X64/MemEncryptSevLib.c | 16 ++++------------
> .../X64/PeiDxeVirtualMemory.c | 14 ++++----------
> .../BaseMemEncryptSevLib/X64/SecVirtualMemory.c | 8 ++------
> .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 3 +--
> OvmfPkg/PlatformPei/AmdSev.c | 3 +--
> 10 files changed, 21 insertions(+), 62 deletions(-)
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
(v2 patches 10 through 12 are OK as well)
Thanks
Laszlo
>
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index b91490d5d44d..76d06c206c8b 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -100,8 +100,6 @@ MemEncryptSevIsEnabled (
> address of a memory region.
> @param[in] NumPages The number of pages from start memory
> region.
> - @param[in] Flush Flush the caches before clearing the
> bit
> - (mostly TRUE except MMIO addresses)
>
> @retval RETURN_SUCCESS The attributes were cleared for the
> memory region.
> @@ -114,8 +112,7 @@ EFIAPI
> MemEncryptSevClearPageEncMask (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS BaseAddress,
> - IN UINTN NumPages,
> - IN BOOLEAN Flush
> + IN UINTN NumPages
> );
>
> /**
> @@ -128,8 +125,6 @@ MemEncryptSevClearPageEncMask (
> address of a memory region.
> @param[in] NumPages The number of pages from start memory
> region.
> - @param[in] Flush Flush the caches before setting the bit
> - (mostly TRUE except MMIO addresses)
>
> @retval RETURN_SUCCESS The attributes were set for the memory
> region.
> @@ -142,8 +137,7 @@ EFIAPI
> MemEncryptSevSetPageEncMask (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS BaseAddress,
> - IN UINTN NumPages,
> - IN BOOLEAN Flush
> + IN UINTN NumPages
> );
>
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> index 8dc39e647b90..21bbbd1c4f9c 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> @@ -58,8 +58,6 @@ InternalGetMemEncryptionAddressMask (
> @param[in] PhysicalAddress The physical address that is the start
> address of a memory region.
> @param[in] Length The length of memory region
> - @param[in] Flush Flush the caches before applying the
> - encryption mask
>
> @retval RETURN_SUCCESS The attributes were cleared for the
> memory region.
> @@ -72,8 +70,7 @@ EFIAPI
> InternalMemEncryptSevSetMemoryDecrypted (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS PhysicalAddress,
> - IN UINTN Length,
> - IN BOOLEAN Flush
> + IN UINTN Length
> );
>
> /**
> @@ -85,8 +82,6 @@ InternalMemEncryptSevSetMemoryDecrypted (
> @param[in] PhysicalAddress The physical address that is the start
> address of a memory region.
> @param[in] Length The length of memory region
> - @param[in] Flush Flush the caches before applying the
> - encryption mask
>
> @retval RETURN_SUCCESS The attributes were set for the memory
> region.
> @@ -99,8 +94,7 @@ EFIAPI
> InternalMemEncryptSevSetMemoryEncrypted (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS PhysicalAddress,
> - IN UINTN Length,
> - IN BOOLEAN Flush
> + IN UINTN Length
> );
>
> /**
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 80831b81facf..c66c4e9b9272 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -120,8 +120,7 @@ AmdSevDxeEntryPoint (
> Status = MemEncryptSevClearPageEncMask (
> 0, // Cr3BaseAddress -- use current CR3
> MapPagesBase, // BaseAddress
> - MapPagesCount, // NumPages
> - TRUE // Flush
> + MapPagesCount // NumPages
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "%a: MemEncryptSevClearPageEncMask(): %r\n",
> diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
> index 49ffa2448811..b30628078f73 100644
> --- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
> +++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
> @@ -252,8 +252,7 @@ IoMmuMap (
> Status = MemEncryptSevClearPageEncMask (
> 0,
> MapInfo->PlainTextAddress,
> - MapInfo->NumberOfPages,
> - TRUE
> + MapInfo->NumberOfPages
> );
> ASSERT_EFI_ERROR (Status);
> if (EFI_ERROR (Status)) {
> @@ -407,8 +406,7 @@ IoMmuUnmapWorker (
> Status = MemEncryptSevSetPageEncMask (
> 0,
> MapInfo->PlainTextAddress,
> - MapInfo->NumberOfPages,
> - TRUE
> + MapInfo->NumberOfPages
> );
> ASSERT_EFI_ERROR (Status);
> if (EFI_ERROR (Status)) {
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> index 169d3118e44f..be260e0d1014 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> @@ -25,8 +25,6 @@
> address of a memory region.
> @param[in] NumPages The number of pages from start memory
> region.
> - @param[in] Flush Flush the caches before clearing the
> bit
> - (mostly TRUE except MMIO addresses)
>
> @retval RETURN_SUCCESS The attributes were cleared for the
> memory region.
> @@ -39,8 +37,7 @@ EFIAPI
> MemEncryptSevClearPageEncMask (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS BaseAddress,
> - IN UINTN NumPages,
> - IN BOOLEAN Flush
> + IN UINTN NumPages
> )
> {
> //
> @@ -59,8 +56,6 @@ MemEncryptSevClearPageEncMask (
> address of a memory region.
> @param[in] NumPages The number of pages from start memory
> region.
> - @param[in] Flush Flush the caches before setting the bit
> - (mostly TRUE except MMIO addresses)
>
> @retval RETURN_SUCCESS The attributes were set for the memory
> region.
> @@ -73,8 +68,7 @@ EFIAPI
> MemEncryptSevSetPageEncMask (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS BaseAddress,
> - IN UINTN NumPages,
> - IN BOOLEAN Flush
> + IN UINTN NumPages
> )
> {
> //
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> index a2bf698bcde7..a57e8fd37fa7 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
> @@ -27,8 +27,6 @@
> address of a memory region.
> @param[in] NumPages The number of pages from start memory
> region.
> - @param[in] Flush Flush the caches before clearing the
> bit
> - (mostly TRUE except MMIO addresses)
>
> @retval RETURN_SUCCESS The attributes were cleared for the
> memory region.
> @@ -41,15 +39,13 @@ EFIAPI
> MemEncryptSevClearPageEncMask (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS BaseAddress,
> - IN UINTN NumPages,
> - IN BOOLEAN Flush
> + IN UINTN NumPages
> )
> {
> return InternalMemEncryptSevSetMemoryDecrypted (
> Cr3BaseAddress,
> BaseAddress,
> - EFI_PAGES_TO_SIZE (NumPages),
> - Flush
> + EFI_PAGES_TO_SIZE (NumPages)
> );
> }
>
> @@ -63,8 +59,6 @@ MemEncryptSevClearPageEncMask (
> address of a memory region.
> @param[in] NumPages The number of pages from start memory
> region.
> - @param[in] Flush Flush the caches before setting the bit
> - (mostly TRUE except MMIO addresses)
>
> @retval RETURN_SUCCESS The attributes were set for the memory
> region.
> @@ -77,15 +71,13 @@ EFIAPI
> MemEncryptSevSetPageEncMask (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS BaseAddress,
> - IN UINTN NumPages,
> - IN BOOLEAN Flush
> + IN UINTN NumPages
> )
> {
> return InternalMemEncryptSevSetMemoryEncrypted (
> Cr3BaseAddress,
> BaseAddress,
> - EFI_PAGES_TO_SIZE (NumPages),
> - Flush
> + EFI_PAGES_TO_SIZE (NumPages)
> );
> }
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> index a18d336a8789..c696745f9d26 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -828,8 +828,6 @@ SetMemoryEncDec (
> @param[in] PhysicalAddress The physical address that is the start
> address of a memory region.
> @param[in] Length The length of memory region
> - @param[in] Flush Flush the caches before applying the
> - encryption mask
>
> @retval RETURN_SUCCESS The attributes were cleared for the
> memory region.
> @@ -842,8 +840,7 @@ EFIAPI
> InternalMemEncryptSevSetMemoryDecrypted (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS PhysicalAddress,
> - IN UINTN Length,
> - IN BOOLEAN Flush
> + IN UINTN Length
> )
> {
>
> @@ -852,7 +849,7 @@ InternalMemEncryptSevSetMemoryDecrypted (
> PhysicalAddress,
> Length,
> ClearCBit,
> - Flush
> + TRUE
> );
> }
>
> @@ -865,8 +862,6 @@ InternalMemEncryptSevSetMemoryDecrypted (
> @param[in] PhysicalAddress The physical address that is the start
> address of a memory region.
> @param[in] Length The length of memory region
> - @param[in] Flush Flush the caches before applying the
> - encryption mask
>
> @retval RETURN_SUCCESS The attributes were set for the memory
> region.
> @@ -879,8 +874,7 @@ EFIAPI
> InternalMemEncryptSevSetMemoryEncrypted (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS PhysicalAddress,
> - IN UINTN Length,
> - IN BOOLEAN Flush
> + IN UINTN Length
> )
> {
> return SetMemoryEncDec (
> @@ -888,7 +882,7 @@ InternalMemEncryptSevSetMemoryEncrypted (
> PhysicalAddress,
> Length,
> SetCBit,
> - Flush
> + TRUE
> );
> }
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
> index e0d3a15e8503..a155c8729bfb 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c
> @@ -42,8 +42,6 @@ InternalGetMemEncryptionAddressMask (
> @param[in] PhysicalAddress The physical address that is the start
> address of a memory region.
> @param[in] Length The length of memory region
> - @param[in] Flush Flush the caches before applying the
> - encryption mask
>
> @retval RETURN_SUCCESS The attributes were cleared for the
> memory region.
> @@ -56,8 +54,7 @@ EFIAPI
> InternalMemEncryptSevSetMemoryDecrypted (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS PhysicalAddress,
> - IN UINTN Length,
> - IN BOOLEAN Flush
> + IN UINTN Length
> )
> {
> //
> @@ -89,8 +86,7 @@ EFIAPI
> InternalMemEncryptSevSetMemoryEncrypted (
> IN PHYSICAL_ADDRESS Cr3BaseAddress,
> IN PHYSICAL_ADDRESS PhysicalAddress,
> - IN UINTN Length,
> - IN BOOLEAN Flush
> + IN UINTN Length
> )
> {
> //
> diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> index fdf2380974fa..c7cc5b0389c8 100644
> --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> @@ -283,8 +283,7 @@ SmmCpuFeaturesSmmRelocationComplete (
> Status = MemEncryptSevSetPageEncMask (
> 0, // Cr3BaseAddress -- use current CR3
> MapPagesBase, // BaseAddress
> - MapPagesCount, // NumPages
> - TRUE // Flush
> + MapPagesCount // NumPages
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "%a: MemEncryptSevSetPageEncMask(): %r\n",
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index dddffdebda4b..a8bf610022ba 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -72,8 +72,7 @@ AmdSevEsInitialize (
> DecryptStatus = MemEncryptSevClearPageEncMask (
> 0,
> GhcbBasePa + EFI_PAGES_TO_SIZE (PageCount),
> - 1,
> - TRUE
> + 1
> );
> ASSERT_RETURN_ERROR (DecryptStatus);
> }
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75165): https://edk2.groups.io/g/devel/message/75165
Mute This Topic: https://groups.io/mt/82787295/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
