Re: [edk2-devel] [PATCH v3 09/13] OvmfPkg/BaseMemEncryptSevLib: introduce MemEncryptSevClearMmioPageEncMask()

2021-05-28 Thread Erdem Aktas via groups.io
> +  @param[in]  BaseAddress The physical address that is the start
> +  address of a MMIO region.

Based on the code, what I understand is that the address parameters
should be "guest virtual address", not the physical address. But in
this patch, all the address parameters are named as PhysicalAddress.
Is this intentional?

-Erdem

On Wed, May 19, 2021 at 11:20 AM Brijesh Singh  wrote:
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
>
> The MemEncryptSevClearMmioPageEncMask() helper can be used for clearing
> the memory encryption mask for the Mmio region.
>
> The MemEncryptSevClearMmioPageEncMask() is a simplified version of
> MemEncryptSevClearPageEncMask() -- it does not flush the caches after
> clearing the page encryption mask.
>
> Cc: James Bottomley 
> Cc: Min Xu 
> Cc: Jiewen Yao 
> Cc: Tom Lendacky 
> Cc: Jordan Justen 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Erdem Aktas 
> Reviewed-by: Laszlo Ersek 
> Signed-off-by: Brijesh Singh 
> ---
>  OvmfPkg/Include/Library/MemEncryptSevLib.h| 25 ++
>  .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 23 +
>  .../Ia32/MemEncryptSevLib.c   | 31 +
>  .../X64/MemEncryptSevLib.c| 33 +++
>  .../X64/PeiDxeVirtualMemory.c | 33 +++
>  .../X64/SecVirtualMemory.c| 30 +
>  6 files changed, 175 insertions(+)
>
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h 
> b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index 99f15a7d1271..b91490d5d44d 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -203,4 +203,29 @@ MemEncryptSevGetAddressRangeState (
>IN UINTNLength
>);
>
> +/**
> +  This function clears memory encryption bit for the MMIO region specified by
> +  BaseAddress and NumPages.
> +
> +  @param[in]  Cr3BaseAddress  Cr3 Base Address (if zero then use
> +  current CR3)
> +  @param[in]  BaseAddress The physical address that is the start
> +  address of a MMIO region.
> +  @param[in]  NumPagesThe number of pages from start memory
> +  region.
> +
> +  @retval RETURN_SUCCESS  The attributes were cleared for the
> +  memory region.
> +  @retval RETURN_INVALID_PARAMETERNumber of pages is zero.
> +  @retval RETURN_UNSUPPORTED  Clearing the memory encryption 
> attribute
> +  is not supported
> +**/
> +RETURN_STATUS
> +EFIAPI
> +MemEncryptSevClearMmioPageEncMask (
> +  IN PHYSICAL_ADDRESS Cr3BaseAddress,
> +  IN PHYSICAL_ADDRESS BaseAddress,
> +  IN UINTNNumPages
> +  );
> +
>  #endif // _MEM_ENCRYPT_SEV_LIB_H_
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h 
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> index fe2a0b2826cd..8dc39e647b90 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> @@ -126,4 +126,27 @@ InternalMemEncryptSevGetAddressRangeState (
>IN UINTNLength
>);
>
> +/**
> +  This function clears memory encryption bit for the MMIO region specified by
> +  PhysicalAddress and Length.
> +
> +  @param[in]  Cr3BaseAddress  Cr3 Base Address (if zero then use
> +  current CR3)
> +  @param[in]  PhysicalAddress The physical address that is the start
> +  address of a MMIO region.
> +  @param[in]  Length  The length of memory region
> +
> +  @retval RETURN_SUCCESS  The attributes were cleared for the
> +  memory region.
> +  @retval RETURN_INVALID_PARAMETERLength is zero.
> +  @retval RETURN_UNSUPPORTED  Clearing the memory encyrption 
> attribute
> +  is not supported
> +**/
> +RETURN_STATUS
> +EFIAPI
> +InternalMemEncryptSevClearMmioPageEncMask (
> +  IN  PHYSICAL_ADDRESSCr3BaseAddress,
> +  IN  PHYSICAL_ADDRESSPhysicalAddress,
> +  IN  UINTN   Length
> +  );
>  #endif
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c 
> b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> index 12a5bf495bd7..169d3118e44f 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
> @@ -111,3 +111,34 @@ MemEncryptSevGetAddressRangeState (
>//
>return MemEncryptSevAddressRangeEncrypted;
>  }
> +
> +/**
> +  This function clears memory encryption bit for the MMIO region specified by
> +  

[edk2-devel] [PATCH v3 09/13] OvmfPkg/BaseMemEncryptSevLib: introduce MemEncryptSevClearMmioPageEncMask()

2021-05-19 Thread Brijesh Singh
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The MemEncryptSevClearMmioPageEncMask() helper can be used for clearing
the memory encryption mask for the Mmio region.

The MemEncryptSevClearMmioPageEncMask() is a simplified version of
MemEncryptSevClearPageEncMask() -- it does not flush the caches after
clearing the page encryption mask.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Erdem Aktas 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h| 25 ++
 .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 23 +
 .../Ia32/MemEncryptSevLib.c   | 31 +
 .../X64/MemEncryptSevLib.c| 33 +++
 .../X64/PeiDxeVirtualMemory.c | 33 +++
 .../X64/SecVirtualMemory.c| 30 +
 6 files changed, 175 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h 
b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 99f15a7d1271..b91490d5d44d 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -203,4 +203,29 @@ MemEncryptSevGetAddressRangeState (
   IN UINTNLength
   );
 
+/**
+  This function clears memory encryption bit for the MMIO region specified by
+  BaseAddress and NumPages.
+
+  @param[in]  Cr3BaseAddress  Cr3 Base Address (if zero then use
+  current CR3)
+  @param[in]  BaseAddress The physical address that is the start
+  address of a MMIO region.
+  @param[in]  NumPagesThe number of pages from start memory
+  region.
+
+  @retval RETURN_SUCCESS  The attributes were cleared for the
+  memory region.
+  @retval RETURN_INVALID_PARAMETERNumber of pages is zero.
+  @retval RETURN_UNSUPPORTED  Clearing the memory encryption attribute
+  is not supported
+**/
+RETURN_STATUS
+EFIAPI
+MemEncryptSevClearMmioPageEncMask (
+  IN PHYSICAL_ADDRESS Cr3BaseAddress,
+  IN PHYSICAL_ADDRESS BaseAddress,
+  IN UINTNNumPages
+  );
+
 #endif // _MEM_ENCRYPT_SEV_LIB_H_
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
index fe2a0b2826cd..8dc39e647b90 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
@@ -126,4 +126,27 @@ InternalMemEncryptSevGetAddressRangeState (
   IN UINTNLength
   );
 
+/**
+  This function clears memory encryption bit for the MMIO region specified by
+  PhysicalAddress and Length.
+
+  @param[in]  Cr3BaseAddress  Cr3 Base Address (if zero then use
+  current CR3)
+  @param[in]  PhysicalAddress The physical address that is the start
+  address of a MMIO region.
+  @param[in]  Length  The length of memory region
+
+  @retval RETURN_SUCCESS  The attributes were cleared for the
+  memory region.
+  @retval RETURN_INVALID_PARAMETERLength is zero.
+  @retval RETURN_UNSUPPORTED  Clearing the memory encyrption attribute
+  is not supported
+**/
+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevClearMmioPageEncMask (
+  IN  PHYSICAL_ADDRESSCr3BaseAddress,
+  IN  PHYSICAL_ADDRESSPhysicalAddress,
+  IN  UINTN   Length
+  );
 #endif
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
index 12a5bf495bd7..169d3118e44f 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
@@ -111,3 +111,34 @@ MemEncryptSevGetAddressRangeState (
   //
   return MemEncryptSevAddressRangeEncrypted;
 }
+
+/**
+  This function clears memory encryption bit for the MMIO region specified by
+  BaseAddress and NumPages.
+
+  @param[in]  Cr3BaseAddress  Cr3 Base Address (if zero then use
+  current CR3)
+  @param[in]  BaseAddress The physical address that is the start
+  address of a MMIO region.
+  @param[in]  NumPagesThe number of pages from start memory
+  region.
+
+  @retval RETURN_SUCCESS  The attributes were cleared for the
+  memory region.
+  @retval RETURN_INVALID_PARAMETERNumber of pages is zero.
+  @retval RETURN_UNSUPPORTED