[edk2-devel] [PATCH v2 13/13] OvmfPkg/BaseMemEncryptSevLib: remove Flush parameter

Laszlo Ersek lersek at redhat.com
Mon May 17 03:46:45 UTC 2021


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 <jejb at linux.ibm.com>
> Cc: Min Xu <min.m.xu at intel.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Tom Lendacky <thomas.lendacky at amd.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Erdem Aktas <erdemaktas at google.com>
> Signed-off-by: Brijesh Singh <brijesh.singh at 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 <lersek at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list