Date   

[PATCH 10/13] OvmfPkg/AmdSevDxe: use MemEncryptSevClearMmioPageEncMask() to clear EncMask

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Use the MemEncryptSevClearMmioPageEncMask() to clear memory encryption mask
for the Mmio and NonExistent address range.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 689bfb376d03..80831b81facf 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -53,11 +53,10 @@ AmdSevDxeEntryPoint (
Desc = &AllDescMap[Index];
if (Desc->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo ||
Desc->GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
- Status = MemEncryptSevClearPageEncMask (
+ Status = MemEncryptSevClearMmioPageEncMask (
0,
Desc->BaseAddress,
- EFI_SIZE_TO_PAGES (Desc->Length),
- FALSE
+ EFI_SIZE_TO_PAGES (Desc->Length)
);
ASSERT_EFI_ERROR (Status);
}
@@ -73,11 +72,10 @@ AmdSevDxeEntryPoint (
// the range.
//
if (PcdGet16 (PcdOvmfHostBridgePciDevId) == INTEL_Q35_MCH_DEVICE_ID) {
- Status = MemEncryptSevClearPageEncMask (
+ Status = MemEncryptSevClearMmioPageEncMask (
0,
FixedPcdGet64 (PcdPciExpressBaseAddress),
- EFI_SIZE_TO_PAGES (SIZE_256MB),
- FALSE
+ EFI_SIZE_TO_PAGES (SIZE_256MB)
);

ASSERT_EFI_ERROR (Status);
--
2.17.1


[PATCH 09/13] OvmfPkg/BaseMemEncryptSevLib: introduce MemEncryptSevClearMmioPageEncMask()

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 simplifies version of
MemEncryptSevClearPageEncMask() -- it does not flush the caches after
clearing the page encryption mask.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <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 +++++++++++++++++++
5 files changed, 145 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 UINTN Length
);

+/**
+ 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] NumPages The number of pages from start memory
+ region.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the
+ memory region.
+ @retval RETURN_INVALID_PARAMETER Number 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 UINTN NumPages
+ );
+
#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 UINTN Length
);

+/**
+ 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_PARAMETER Length is zero.
+ @retval RETURN_UNSUPPORTED Clearing the memory encyrption attribute
+ is not supported
+**/
+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevClearMmioPageEncMask (
+ IN PHYSICAL_ADDRESS Cr3BaseAddress,
+ IN PHYSICAL_ADDRESS PhysicalAddress,
+ 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] NumPages The number of pages from start memory
+ region.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the
+ memory region.
+ @retval RETURN_INVALID_PARAMETER Number 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 UINTN NumPages
+ )
+{
+ //
+ // Memory encryption bit is not accessible in 32-bit mode
+ //
+ return RETURN_UNSUPPORTED;
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
index 4fea6a6be0ac..a2bf698bcde7 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
@@ -118,3 +118,36 @@ MemEncryptSevGetAddressRangeState (
Length
);
}
+
+/**
+ 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] NumPages The number of pages from start memory
+ region.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the
+ memory region.
+ @retval RETURN_INVALID_PARAMETER Number 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 UINTN NumPages
+ )
+{
+ return InternalMemEncryptSevClearMmioPageEncMask (
+ Cr3BaseAddress,
+ BaseAddress,
+ EFI_PAGES_TO_SIZE (NumPages)
+ );
+
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index d3455e812bd1..a18d336a8789 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -891,3 +891,36 @@ InternalMemEncryptSevSetMemoryEncrypted (
Flush
);
}
+
+/**
+ 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_PARAMETER Length is zero.
+ @retval RETURN_UNSUPPORTED Clearing the memory encyrption attribute
+ is not supported
+**/
+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevClearMmioPageEncMask (
+ IN PHYSICAL_ADDRESS Cr3BaseAddress,
+ IN PHYSICAL_ADDRESS PhysicalAddress,
+ IN UINTN Length
+ )
+{
+ return SetMemoryEncDec (
+ Cr3BaseAddress,
+ PhysicalAddress,
+ Length,
+ ClearCBit,
+ FALSE
+ );
+}
--
2.17.1


[PATCH 08/13] MdePkg/BaseLib: add support for RMPADJUST instruction

Brijesh Singh
 

From: Tom Lendacky <thomas.lendacky@...>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The RMPADJUST instruction will be used by the SEV-SNP guest to modify the
RMP permissions for a guest page. See AMD APM volume 3 for further
details.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Library/BaseLib/BaseLib.inf | 1 +
MdePkg/Include/Library/BaseLib.h | 36 +++++++++++++++++++-
MdePkg/Include/X64/Nasm.inc | 8 +++++
MdePkg/Library/BaseLib/X64/RmpAdjust.nasm | 40 +++++++++++++++++++++++
4 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644 MdePkg/Library/BaseLib/X64/RmpAdjust.nasm

diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index 89a52f72c08a..6ccb8997b7e8 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -319,6 +319,7 @@ [Sources.X64]
X64/DisablePaging64.nasm
X64/Pvalidate.nasm
X64/RdRand.nasm
+ X64/RmpAdjust.nasm
X64/XGetBv.nasm
X64/XSetBv.nasm
X64/VmgExit.nasm
diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index f177034af6a1..04e58f995b9a 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -4857,9 +4857,43 @@ AsmPvalidate (
IN BOOLEAN Validate,
IN PHYSICAL_ADDRESS Address
);
+
+//
+// RDX settings for RMPADJUST
+//
+#define RMPADJUST_VMPL_MAX 3
+#define RMPADJUST_VMPL_MASK 0xFF
+#define RMPADJUST_VMPL_SHIFT 0
+#define RMPADJUST_PERMISSION_MASK_MASK 0xFF
+#define RMPADJUST_PERMISSION_MASK_SHIFT 8
+#define RMPADJUST_VMSA_PAGE_BIT BIT16
+
+/**
+ Adjusts the permissions of an SEV-SNP guest page.
+
+ Executes a RMPADJUST instruction with the register state specified by Rax,
+ Rcx and Rdx. Returns Eax. This function is only available x64.
+
+ The instruction is available only when CPUID Fn8000_001F_EAX[SNP]=1.
+
+ @param[in] Rax The value to load into RAX before executing the RMPADJUST
+ instruction.
+ @param[in] Rcx The value to load into RCX before executing the RMPADJUST
+ instruction.
+ @param[in] Rdx The value to load into RDX before executing the RMPADJUST
+ instruction.
+
+ @return Eax
+**/
+UINTN
+EFIAPI
+AsmRmpAdjust (
+ IN UINTN Rax,
+ IN UINTN Rcx,
+ IN UINTN Rdx
+ );
#endif

-
#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
///
/// IA32 and x64 Specific Functions.
diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc
index 528bb3385609..cfb14edc9449 100644
--- a/MdePkg/Include/X64/Nasm.inc
+++ b/MdePkg/Include/X64/Nasm.inc
@@ -41,6 +41,14 @@
DB 0xF2, 0x0F, 0x01, 0xFF
%endmacro

+;
+; Macro for the RMPADJUST instruction, defined in AMD APM volume 3.
+; NASM feature request URL: https://bugzilla.nasm.us/show_bug.cgi?id=3392754
+;
+%macro RMPADJUST 0
+ DB 0xF3, 0x0F, 0x01, 0xFE
+%endmacro
+
; NASM provides built-in macros STRUC and ENDSTRUC for structure definition.
; For example, to define a structure called mytype containing a longword,
; a word, a byte and a string of bytes, you might code
diff --git a/MdePkg/Library/BaseLib/X64/RmpAdjust.nasm b/MdePkg/Library/BaseLib/X64/RmpAdjust.nasm
new file mode 100644
index 000000000000..f2c295b67c9c
--- /dev/null
+++ b/MdePkg/Library/BaseLib/X64/RmpAdjust.nasm
@@ -0,0 +1,40 @@
+;-----------------------------------------------------------------------------
+;
+; Copyright (c) 2021, Advanced Micro Devices, Inc. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+; RmpAdjust.Asm
+;
+; Abstract:
+;
+; AsmRmpAdjust function
+;
+; Notes:
+;
+;-----------------------------------------------------------------------------
+
+%include "Nasm.inc"
+
+ SECTION .text
+
+;-----------------------------------------------------------------------------
+; UINTN
+; EFIAPI
+; AsmRmpAdjust (
+; IN UINTN Rax,
+; IN UINTN Rcx,
+; IN UINTN Rdx
+; )
+;-----------------------------------------------------------------------------
+global ASM_PFX(AsmRmpAdjust)
+ASM_PFX(AsmRmpAdjust):
+ mov rax, rcx ; Input Rax is in RCX by calling convention
+ mov rcx, rdx ; Input Rcx is in RDX by calling convention
+ mov rdx, r8 ; Input Rdx is in R8 by calling convention
+
+ RMPADJUST
+
+ ; RMPADJUST returns the status in the EAX register.
+ ret
--
2.17.1


[PATCH 07/13] MdePkg/BaseLib: add support for PVALIDATE instruction

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The PVALIDATE instruction validates or rescinds validation of a guest
page RMP entry. Upon completion, a return code is stored in EAX, rFLAGS
bits OF, ZF, AF, PF and SF are set based on this return code. If the
instruction completed succesfully, the rFLAGS bit CF indicates if the
contents of the RMP entry were changed or not.

For more information about the instruction see AMD APM volume 3.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Library/BaseLib/BaseLib.inf | 1 +
MdePkg/Include/Library/BaseLib.h | 46 +++++++++++++++++++++++
MdePkg/Include/X64/Nasm.inc | 8 ++++
MdePkg/Library/BaseLib/X64/Pvalidate.nasm | 42 +++++++++++++++++++++
4 files changed, 97 insertions(+)
create mode 100644 MdePkg/Library/BaseLib/X64/Pvalidate.nasm

diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index b76f3af380ea..89a52f72c08a 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -317,6 +317,7 @@ [Sources.X64]
X64/GccInlinePriv.c | GCC
X64/EnableDisableInterrupts.nasm
X64/DisablePaging64.nasm
+ X64/Pvalidate.nasm
X64/RdRand.nasm
X64/XGetBv.nasm
X64/XSetBv.nasm
diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 7253997a6f8c..f177034af6a1 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -4813,6 +4813,52 @@ SpeculationBarrier (
VOID
);

+#if defined (MDE_CPU_X64)
+//
+// The page size for the PVALIDATE instruction
+//
+typedef enum {
+ PvalidatePageSize4K = 0,
+ PvalidatePageSize2MB,
+} PVALIDATE_PAGE_SIZE;
+
+//
+// PVALIDATE Return Code.
+//
+#define PVALIDATE_RET_SUCCESS 0
+#define PVALIDATE_RET_FAIL_INPUT 1
+#define PVALIDATE_RET_SIZE_MISMATCH 6
+
+//
+// The PVALIDATE instruction did not made any changes to the RMP entry.
+//
+#define PVALIDATE_RET_NO_RMPUPDATE 255
+
+/**
+ Execute a PVALIDATE instruction to validate or rescinds validation of a guest
+ page's RMP entry.
+
+ The instruction is available only when CPUID Fn8000_001F_EAX[SNP]=1.
+
+ The function is available on X64.
+
+ @param[in] PageSize The page size to use.
+ @param[in] Validate Validate or rescinds.
+ @param[in] Address The guest virtual address to validate.
+
+ @retval The return value from the PVALIDATE instruction, and
+ PVALIDATE_RET_NO_RMPUPDATE when there was no change in
+ the RMP entry.
+**/
+UINTN
+EFIAPI
+AsmPvalidate (
+ IN PVALIDATE_PAGE_SIZE PageSize,
+ IN BOOLEAN Validate,
+ IN PHYSICAL_ADDRESS Address
+ );
+#endif
+

#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
///
diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc
index 527f71e9eb4d..528bb3385609 100644
--- a/MdePkg/Include/X64/Nasm.inc
+++ b/MdePkg/Include/X64/Nasm.inc
@@ -33,6 +33,14 @@
DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8
%endmacro

+;
+; Macro for the PVALIDATE instruction, defined in AMD APM volume 3.
+; NASM feature request URL: https://bugzilla.nasm.us/show_bug.cgi?id=3392753
+;
+%macro PVALIDATE 0
+ DB 0xF2, 0x0F, 0x01, 0xFF
+%endmacro
+
; NASM provides built-in macros STRUC and ENDSTRUC for structure definition.
; For example, to define a structure called mytype containing a longword,
; a word, a byte and a string of bytes, you might code
diff --git a/MdePkg/Library/BaseLib/X64/Pvalidate.nasm b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
new file mode 100644
index 000000000000..b20dac7e6831
--- /dev/null
+++ b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
@@ -0,0 +1,42 @@
+;-----------------------------------------------------------------------------
+;
+; Copyright (c) 2021, AMD. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;-----------------------------------------------------------------------------
+
+%include "Nasm.inc"
+
+ SECTION .text
+
+;-----------------------------------------------------------------------------
+; UINTN
+; EFIAPI
+; AsmPvalidate (
+; IN UINT32 RmpPageSize
+; IN UINT32 Validate,
+; IN PHYSICAL_ADDRESS Address
+; )
+;-----------------------------------------------------------------------------
+global ASM_PFX(AsmPvalidate)
+ASM_PFX(AsmPvalidate):
+ mov rax, r8
+
+ PVALIDATE
+
+ ; Save the carry flag.
+ setb dl
+
+ ; The PVALIDATE instruction returns the status in rax register.
+ cmp rax, 0
+ jne PvalidateExit
+
+ ; Check the carry flag to determine if RMP entry was updated.
+ cmp dl, 0
+ jz PvalidateExit
+
+ ; Return the PVALIDATE_RET_NO_RMPUPDATE.
+ mov rax, 255
+
+PvalidateExit:
+ ret
--
2.17.1


[PATCH 06/13] MdePkg/Register/Amd: define GHCB macros for SNP AP creation

Brijesh Singh
 

From: Tom Lendacky <thomas.lendacky@...>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Version 2 of GHCB introduces NAE for creating AP when SEV-SNP is
enabled in the guest VM. See the GHCB spec section for additional
details.

While at it, define the VMSA state save area that are required for
creating the AP. The save area format is defined in AMD APM volume
2 (Table B-4).

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Ghcb.h | 70 ++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index a15b4b7e2760..956cefbc003c 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -55,6 +55,7 @@
#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
#define SVM_EXIT_SNP_PAGE_STATE_CHANGE 0x80000010ULL
+#define SVM_EXIT_SNP_AP_CREATION 0x80000013ULL
#define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL
#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL

@@ -83,6 +84,12 @@
#define IOIO_SEG_ES 0
#define IOIO_SEG_DS (BIT11 | BIT10)

+//
+// AP Creation Information
+//
+#define SVM_VMGEXIT_SNP_AP_CREATE_ON_INIT 0
+#define SVM_VMGEXIT_SNP_AP_CREATE 1
+#define SVM_VMGEXIT_SNP_AP_DESTROY 2

typedef PACKED struct {
UINT8 Reserved1[203];
@@ -195,4 +202,67 @@ typedef struct {
SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
} SNP_PAGE_STATE_CHANGE_INFO;

+//
+// SEV-ES save area mapping structures used for SEV-SNP AP Creation.
+// Only the fields required to be set to a non-zero value are defined.
+//
+#pragma pack(1)
+typedef struct {
+ UINT16 Selector;
+ UINT16 Attributes;
+ UINT32 Limit;
+ UINT64 Base;
+} SEV_ES_SEGMENT_REGISTER;
+#pragma pack()
+
+#define SEV_ES_RESET_CS_ATTRIBUTES (BIT7 | BIT4 | BIT3 | BIT1)
+#define SEV_ES_RESET_DS_ATTRIBUTES (BIT7 | BIT4 | BIT1)
+#define SEV_ES_RESET_ES_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES
+#define SEV_ES_RESET_FS_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES
+#define SEV_ES_RESET_GS_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES
+#define SEV_ES_RESET_SS_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES
+
+#define SEV_ES_RESET_GDTR_ATTRIBUTES 0
+#define SEV_ES_RESET_LDTR_ATTRIBUTES (BIT7 | 2)
+#define SEV_ES_RESET_IDTR_ATTRIBUTES 0
+#define SEV_ES_RESET_TR_ATTRIBUTES (BIT7 | 3)
+
+#pragma pack(1)
+typedef struct {
+ SEV_ES_SEGMENT_REGISTER Es;
+ SEV_ES_SEGMENT_REGISTER Cs;
+ SEV_ES_SEGMENT_REGISTER Ss;
+ SEV_ES_SEGMENT_REGISTER Ds;
+ SEV_ES_SEGMENT_REGISTER Fs;
+ SEV_ES_SEGMENT_REGISTER Gs;
+ SEV_ES_SEGMENT_REGISTER Gdtr;
+ SEV_ES_SEGMENT_REGISTER Ldtr;
+ SEV_ES_SEGMENT_REGISTER Idtr;
+ SEV_ES_SEGMENT_REGISTER Tr;
+ UINT8 Reserved1[42];
+ UINT8 Vmpl;
+ UINT8 Reserved2[5];
+ UINT64 Efer;
+ UINT8 Reserved3[112];
+ UINT64 Cr4;
+ UINT8 Reserved4[8];
+ UINT64 Cr0;
+ UINT64 Dr7;
+ UINT64 Dr6;
+ UINT64 Rflags;
+ UINT64 Rip;
+ UINT8 Reserved5[232];
+ UINT64 GPat;
+ UINT8 Reserved6[320];
+ UINT64 SevFeatures;
+ UINT8 Reserved7[48];
+ UINT64 XCr0;
+ UINT8 Reserved8[24];
+ UINT32 Mxcsr;
+ UINT64 X87Ftw;
+ UINT64 Reserved9[8];
+ UINT64 X87Fcw;
+} SEV_ES_SAVE_AREA;
+#pragma pack()
+
#endif
--
2.17.1


[PATCH 05/13] MdePkg/Register/Amd: define GHCB macro for the Page State Change

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The Page State Change NAE exit will be used by the SEV-SNP guest to
request a page state change using the GHCB protocol. See the GHCB
spec section 4.1.6 and 2.3.1 for more detail on the structure
definitions.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 15 ++++++++++++
MdePkg/Include/Register/Amd/Ghcb.h | 33 ++++++++++++++++++++++++++
2 files changed, 48 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index 542e4cdf4782..62014854d9b7 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -58,6 +58,19 @@ typedef union {
UINT64 GuestFrameNumber:52;
} GhcbGpaRegister;

+ struct {
+ UINT64 Function:12;
+ UINT64 GuestFrameNumber:40;
+ UINT64 Operation:4;
+ UINT64 Reserved:8;
+ } SnpPageStateChangeRequest;
+
+ struct {
+ UINT32 Function:12;
+ UINT32 Reserved:20;
+ UINT32 ErrorCode;
+ } SnpPageStateChangeResponse;
+
VOID *Ghcb;

UINT64 GhcbPhysicalAddress;
@@ -69,6 +82,8 @@ typedef union {
#define GHCB_INFO_CPUID_RESPONSE 5
#define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST 18
#define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE 19
+#define GHCB_INFO_SNP_PAGE_STATE_CHANGE_REQUEST 20
+#define GHCB_INFO_SNP_PAGE_STATE_CHANGE_RESPONSE 21
#define GHCB_HYPERVISOR_FEATURES_REQUEST 128
#define GHCB_HYPERVISOR_FEATURES_RESPONSE 129
#define GHCB_INFO_TERMINATE_REQUEST 256
diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 326b11479779..a15b4b7e2760 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -54,6 +54,7 @@
#define SVM_EXIT_NMI_COMPLETE 0x80000003ULL
#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
+#define SVM_EXIT_SNP_PAGE_STATE_CHANGE 0x80000010ULL
#define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL
#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL

@@ -162,4 +163,36 @@ typedef union {
#define GHCB_HV_FEATURES_SNP_AP_CREATE (GHCB_HV_FEATURES_SNP | BIT1)
#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION (GHCB_HV_FEATURES_SNP_AP_CREATE | BIT2)
#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER (GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION | BIT3)
+
+//
+// SNP Page State Change.
+//
+// Note that the PSMASH and UNSMASH operations are not supported when using the MSR protocol.
+//
+#define SNP_PAGE_STATE_PRIVATE 1
+#define SNP_PAGE_STATE_SHARED 2
+#define SNP_PAGE_STATE_PSMASH 3
+#define SNP_PAGE_STATE_UNSMASH 4
+
+typedef struct {
+ UINT64 CurrentPage:12;
+ UINT64 GuestFrameNumber:40;
+ UINT64 Operation:4;
+ UINT64 PageSize:1;
+ UINT64 Reserved: 7;
+} SNP_PAGE_STATE_ENTRY;
+
+typedef struct {
+ UINT16 CurrentEntry;
+ UINT16 EndEntry;
+ UINT32 Reserved;
+} SNP_PAGE_STATE_HEADER;
+
+#define SNP_PAGE_STATE_MAX_ENTRY 253
+
+typedef struct {
+ SNP_PAGE_STATE_HEADER Header;
+ SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
+} SNP_PAGE_STATE_CHANGE_INFO;
+
#endif
--
2.17.1


[PATCH 04/13] MdePkg/Register/Amd: define GHCB macro for Register GPA structure

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

An SEV-SNP guest is required to perform the GHCB GPA registration. See
the GHCB specification for further details.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Reviewed-by: Laszlo Ersek <lersek@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index cdb8f588ccf8..542e4cdf4782 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -53,6 +53,11 @@ typedef union {
UINT64 Features:52;
} GhcbHypervisorFeatures;

+ struct {
+ UINT64 Function:12;
+ UINT64 GuestFrameNumber:52;
+ } GhcbGpaRegister;
+
VOID *Ghcb;

UINT64 GhcbPhysicalAddress;
@@ -62,6 +67,8 @@ typedef union {
#define GHCB_INFO_SEV_INFO_GET 2
#define GHCB_INFO_CPUID_REQUEST 4
#define GHCB_INFO_CPUID_RESPONSE 5
+#define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST 18
+#define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE 19
#define GHCB_HYPERVISOR_FEATURES_REQUEST 128
#define GHCB_HYPERVISOR_FEATURES_RESPONSE 129
#define GHCB_INFO_TERMINATE_REQUEST 256
--
2.17.1


[PATCH 03/13] MdePkg/Register/Amd: define GHCB macros for hypervisor feature detection

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Version 2 of GHCB introduces advertisement of features that are supported
by the hypervisor. See the GHCB spec section 2.2 for an additional details.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Reviewd-by: Laszlo Ersek <lersek@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 7 +++++++
MdePkg/Include/Register/Amd/Ghcb.h | 8 ++++++++
2 files changed, 15 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index 7368ce7af02a..cdb8f588ccf8 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -48,6 +48,11 @@ typedef union {
UINT32 Reserved2:32;
} GhcbTerminate;

+ struct {
+ UINT64 Function:12;
+ UINT64 Features:52;
+ } GhcbHypervisorFeatures;
+
VOID *Ghcb;

UINT64 GhcbPhysicalAddress;
@@ -57,6 +62,8 @@ typedef union {
#define GHCB_INFO_SEV_INFO_GET 2
#define GHCB_INFO_CPUID_REQUEST 4
#define GHCB_INFO_CPUID_RESPONSE 5
+#define GHCB_HYPERVISOR_FEATURES_REQUEST 128
+#define GHCB_HYPERVISOR_FEATURES_RESPONSE 129
#define GHCB_INFO_TERMINATE_REQUEST 256

#define GHCB_TERMINATE_GHCB 0
diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 712dc8e769c0..326b11479779 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -54,6 +54,7 @@
#define SVM_EXIT_NMI_COMPLETE 0x80000003ULL
#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
+#define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL
#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL

//
@@ -154,4 +155,11 @@ typedef union {
#define GHCB_EVENT_INJECTION_TYPE_EXCEPTION 3
#define GHCB_EVENT_INJECTION_TYPE_SOFT_INT 4

+//
+// Hypervisor features dections
+//
+#define GHCB_HV_FEATURES_SNP BIT0
+#define GHCB_HV_FEATURES_SNP_AP_CREATE (GHCB_HV_FEATURES_SNP | BIT1)
+#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION (GHCB_HV_FEATURES_SNP_AP_CREATE | BIT2)
+#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER (GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION | BIT3)
#endif
--
2.17.1


[PATCH 01/13] MdePkg/Register/Amd: expand the SEV MSR to include the SNP definition

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Define the SEV-SNP MSR bits.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index e4db09c5184c..716d52fd508d 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -87,7 +87,12 @@ typedef union {
///
UINT32 SevEsBit:1;

- UINT32 Reserved:30;
+ ///
+ /// [Bit 2] Secure Nested Paging (SevSnp) is enabled
+ ///
+ UINT32 SevSnpBit:1;
+
+ UINT32 Reserved2:29;
} Bits;
///
/// All bit fields as a 32-bit value
--
2.17.1


[PATCH 02/13] MdePkg/Amd: add white spaces to retain alignment for future expansion

Brijesh Singh
 

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Version 2 of the GHCB spec introduces several new SNP-specific NAEs.
Unfortunately, the names for those NAEs break the alignment. Add some
white spaces so that the SNP support patches do not break the alignment.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 10 +++++-----
MdePkg/Include/Register/Amd/Ghcb.h | 12 ++++++------
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index 716d52fd508d..7368ce7af02a 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -53,11 +53,11 @@ typedef union {
UINT64 GhcbPhysicalAddress;
} MSR_SEV_ES_GHCB_REGISTER;

-#define GHCB_INFO_SEV_INFO 1
-#define GHCB_INFO_SEV_INFO_GET 2
-#define GHCB_INFO_CPUID_REQUEST 4
-#define GHCB_INFO_CPUID_RESPONSE 5
-#define GHCB_INFO_TERMINATE_REQUEST 256
+#define GHCB_INFO_SEV_INFO 1
+#define GHCB_INFO_SEV_INFO_GET 2
+#define GHCB_INFO_CPUID_REQUEST 4
+#define GHCB_INFO_CPUID_RESPONSE 5
+#define GHCB_INFO_TERMINATE_REQUEST 256

#define GHCB_TERMINATE_GHCB 0
#define GHCB_TERMINATE_GHCB_GENERAL 0
diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index ccdb662af7a7..712dc8e769c0 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -49,12 +49,12 @@
//
// VMG Special Exit Codes
//
-#define SVM_EXIT_MMIO_READ 0x80000001ULL
-#define SVM_EXIT_MMIO_WRITE 0x80000002ULL
-#define SVM_EXIT_NMI_COMPLETE 0x80000003ULL
-#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
-#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
-#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL
+#define SVM_EXIT_MMIO_READ 0x80000001ULL
+#define SVM_EXIT_MMIO_WRITE 0x80000002ULL
+#define SVM_EXIT_NMI_COMPLETE 0x80000003ULL
+#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
+#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
+#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL

//
// IOIO Exit Information
--
2.17.1


[PATCH 00/13] Add GHCBv2 macro and helpers

Brijesh Singh
 

This series is taken from the SNP RFC. This series defines the GHCBv2
macros and NAE events. Additionally, it also introduces a helper to
clear the page encryption mask from the Mmio region.

The series is based on the commit:
f297b7f20010 UnitTestFrameworkPkg: Sample unit test hangs when running in OVMF/QEMU

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Brijesh Singh (11):
MdePkg/Register/Amd: expand the SEV MSR to include the SNP definition
MdePkg/Amd: add white spaces to retain alignment for future expansion
MdePkg/Register/Amd: define GHCB macros for hypervisor feature
detection
MdePkg/Register/Amd: define GHCB macro for Register GPA structure
MdePkg/Register/Amd: define GHCB macro for the Page State Change
MdePkg/BaseLib: add support for PVALIDATE instruction
OvmfPkg/BaseMemEncryptSevLib: introduce
MemEncryptSevClearMmioPageEncMask()
OvmfPkg/AmdSevDxe: use MemEncryptSevClearMmioPageEncMask() to clear
EncMask
OvmfPkg/QemuFlashFvbServicesRuntimeDxe: use Mmio helper to clear enc
mask
OvmfPkg/TpmMmioSevDecryptPei: use MemEncryptSevClearMmioPageEncMask()
OvmfPkg/BaseMemEncryptSevLib: remove Flush parameter

Tom Lendacky (2):
MdePkg/Register/Amd: define GHCB macros for SNP AP creation
MdePkg/BaseLib: add support for RMPADJUST instruction

MdePkg/Library/BaseLib/BaseLib.inf | 2 +
MdePkg/Include/Library/BaseLib.h | 80 ++++++++++++
MdePkg/Include/Register/Amd/Fam17Msr.h | 46 ++++++-
MdePkg/Include/Register/Amd/Ghcb.h | 123 +++++++++++++++++-
OvmfPkg/Include/Library/MemEncryptSevLib.h | 35 +++--
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 33 +++--
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 13 +-
OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 6 +-
.../Ia32/MemEncryptSevLib.c | 41 ++++--
.../X64/MemEncryptSevLib.c | 49 +++++--
.../X64/PeiDxeVirtualMemory.c | 63 +++++++--
.../X64/SecVirtualMemory.c | 8 +-
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 3 +-
OvmfPkg/PlatformPei/AmdSev.c | 3 +-
.../FwBlockServiceDxe.c | 5 +-
.../QemuFlashSmm.c | 5 +-
.../TpmMmioSevDecryptPeim.c | 5 +-
MdePkg/Include/X64/Nasm.inc | 16 +++
MdePkg/Library/BaseLib/X64/Pvalidate.nasm | 42 ++++++
MdePkg/Library/BaseLib/X64/RmpAdjust.nasm | 40 ++++++
20 files changed, 526 insertions(+), 92 deletions(-)
create mode 100644 MdePkg/Library/BaseLib/X64/Pvalidate.nasm
create mode 100644 MdePkg/Library/BaseLib/X64/RmpAdjust.nasm

--
2.17.1


Re: [PATCH v1 1/1] ArmPkg: Update SCMI Base Protocol version to 0x20000

Ard Biesheuvel
 

Hello Pierre,

On Thu, 6 May 2021 at 12:43, <Pierre.Gondois@...> wrote:

From: Nicola Mazzucato <nicola.mazzucato@...>

The SCP-firmware has moved to full support for SCMIv2 which means that
the base protocol can be either compliant with SCMI v1 or v2.

Allow any version between SCMI v1.0 and SCMI v2.0 to be compatible
with the current implementation.

Signed-off-by: Nicola Mazzucato <nicola.mazzucato@...>
SOB is not an attribution of authorship, it is a statement made by the
person contributing the patch asserting that the contribution complies
with the license.

IOW, you can keep the From: line, but please change the SOB to your
name and email.

Thanks,
Ard.

---
The changes can be seen at: https://github.com/PierreARM/edk2/tree/1732_Update_SCMI_version_v1

ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c | 10 ++++++----
ArmPkg/Include/Protocol/ArmScmiBaseProtocol.h | 10 +++++-----
2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c b/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c
index d5890a7633a2..fb4e79aa3610 100644
--- a/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c
+++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c
@@ -4,9 +4,9 @@

SPDX-License-Identifier: BSD-2-Clause-Patent

- System Control and Management Interface V1.0
- http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
- DEN0056A_System_Control_and_Management_Interface.pdf
+ @par Specification Reference:
+ - Arm System Control and Management Interface - Platform Design Document
+ (https://developer.arm.com/documentation/den0056/)
**/

#include <Base.h>
@@ -86,7 +86,9 @@ ArmScmiDxeEntryPoint (
return Status;
}

- if (Version != BASE_PROTOCOL_VERSION) {
+ // Accept any version between SCMI v1.0 and SCMI v2.0
+ if ((Version < BASE_PROTOCOL_VERSION_V1) ||
+ (Version > BASE_PROTOCOL_VERSION_V2)) {
ASSERT (FALSE);
return EFI_UNSUPPORTED;
}
diff --git a/ArmPkg/Include/Protocol/ArmScmiBaseProtocol.h b/ArmPkg/Include/Protocol/ArmScmiBaseProtocol.h
index 73ad3e32a2f5..c4b81c0f56d3 100644
--- a/ArmPkg/Include/Protocol/ArmScmiBaseProtocol.h
+++ b/ArmPkg/Include/Protocol/ArmScmiBaseProtocol.h
@@ -4,9 +4,9 @@

SPDX-License-Identifier: BSD-2-Clause-Patent

- System Control and Management Interface V1.0
- http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
- DEN0056A_System_Control_and_Management_Interface.pdf
+ @par Specification Reference:
+ - Arm System Control and Management Interface - Platform Design Document
+ (https://developer.arm.com/documentation/den0056/)
**/

#ifndef ARM_SCMI_BASE_PROTOCOL_H_
@@ -14,7 +14,8 @@

#include <Protocol/ArmScmi.h>

-#define BASE_PROTOCOL_VERSION 0x10000
+#define BASE_PROTOCOL_VERSION_V1 0x10000
+#define BASE_PROTOCOL_VERSION_V2 0x20000

#define NUM_PROTOCOL_MASK 0xFFU
#define NUM_AGENT_MASK 0xFFU
@@ -165,4 +166,3 @@ typedef enum {
} SCMI_MESSAGE_ID_BASE;

#endif /* ARM_SCMI_BASE_PROTOCOL_H_ */
-
--
2.17.1


Re: [PATCH v2 0/2] MdePkg,SecurityPkg: Add support to RngDxe and BaseRngLib for AARCH64 RNDR

Ard Biesheuvel
 

On Fri, 7 May 2021 at 16:23, Rebecca Cran <rebecca@...> wrote:

Update MdePkg BaseRngLib and SecurityPkg RngDxe to add support for
the AARCH64 RNDR instruction.

Changes from v1 to v2:

o Added a PCD, gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm to
specify which algorighm the platform supports.
o Moved ArmRndr() and ArmRndrrs() into BaseRngLib.
o Added Doxygen headers.
o Merged X64 and AARCH64 *GetBytes functions into a single RngGetBytes
function.
o Updated constructors to return EFI_STATUS instead of RETURN_STATUS.
o Added ArchIsRngSupported function that gets called before each call to
ArchGetRandomNumber*.

Rebecca Cran (2):
MdePkg/BaseRngLib: Add support for ARMv8.5 RNG instructions
SecurityPkg: Add support for RngDxe on AARCH64
I don't maintain any of the packages involved, but the changes look fine to me.

Acked-by: Ard Biesheuvel <ardb@...>


MdePkg/MdePkg.dec | 9 +-
SecurityPkg/SecurityPkg.dec | 2 +
MdePkg/MdePkg.dsc | 4 +-
SecurityPkg/SecurityPkg.dsc | 11 +-
MdePkg/Library/BaseRngLib/BaseRngLib.inf | 23 ++-
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 24 ++-
MdePkg/Library/BaseRngLib/AArch64/ArmRng.h | 59 +++++++
MdePkg/Library/BaseRngLib/BaseRngLibInternals.h | 79 +++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.h | 0
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.h | 17 --
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h | 117 ++++++++++++++
MdePkg/Library/BaseRngLib/AArch64/Rndr.c | 139 ++++++++++++++++
MdePkg/Library/BaseRngLib/BaseRng.c | 87 +++++-----
MdePkg/Library/BaseRngLib/Rand/RdRand.c | 131 +++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c | 127 +++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.c | 0
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.c | 45 +-----
SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c | 146 +++++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c | 170 ++++++++------------
MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S | 31 ++++
MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm | 30 ++++
MdePkg/Library/BaseRngLib/AArch64/ArmRng.S | 61 +++++++
MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm | 64 ++++++++
MdePkg/Library/BaseRngLib/BaseRngLib.uni | 6 +-
24 files changed, 1152 insertions(+), 230 deletions(-)
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
create mode 100644 MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.h (100%)
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.h (72%)
create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/Rndr.c
create mode 100644 MdePkg/Library/BaseRngLib/Rand/RdRand.c
create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.c (100%)
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.c (71%)
create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmRng.S
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm

--
2.26.2


Re: [PATCH RFC v2 09/28] OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase

Laszlo Ersek
 

On 05/07/21 17:19, Brijesh Singh wrote:

On 5/7/21 10:10 AM, Laszlo Ersek wrote:

Sounds good. What's your thought if I take out patch 1 - 9 from this RFC
series and submit them as non-RFC for the further review and acceptance
? The patch# 1-9 are basically prepatch before we get into SNP specific
bits.
More precisely, that means patches 1-8 (because patch#9 should be
replaced by the module-scope override, and also moved to just before
what is currently patch#21).

Other than that, I agree, this is a good idea. I've anyway thought that
the MdePkg stuff (5 patches) could be / should be merged up-front in
separation, and then the subsequent 3 patches for OvmfPkg are basically
refactoring. We can record the resultant commit range (8 commits) in
TianoCore#3275, and keep the BZ open for the rest of the work. So go
ahead please.
Yes, I will keep patch#9 in SNP series.

FYI, I will add couple of more patches in MdePkg to define the macros
for AP creation and RMPAJUST instruction. Now that GHCB spec is final,
we are working to get the AP creation implemented for the next version.
If the spec is final, then extending the MdePkg patches makes sense, yes.

Thanks
Laszlo


Re: [PATCH RFC v2 09/28] OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase

Brijesh Singh
 

On 5/7/21 10:10 AM, Laszlo Ersek wrote:

Sounds good. What's your thought if I take out patch 1 - 9 from this RFC
series and submit them as non-RFC for the further review and acceptance
? The patch# 1-9 are basically prepatch before we get into SNP specific
bits.
More precisely, that means patches 1-8 (because patch#9 should be
replaced by the module-scope override, and also moved to just before
what is currently patch#21).

Other than that, I agree, this is a good idea. I've anyway thought that
the MdePkg stuff (5 patches) could be / should be merged up-front in
separation, and then the subsequent 3 patches for OvmfPkg are basically
refactoring. We can record the resultant commit range (8 commits) in
TianoCore#3275, and keep the BZ open for the rest of the work. So go
ahead please.
Yes, I will keep patch#9 in SNP series.

FYI, I will add couple of more patches in MdePkg to define the macros
for AP creation and RMPAJUST instruction. Now that GHCB spec is final,
we are working to get the AP creation implemented for the next version.

-Brijesh


Re: [PATCH RFC v2 09/28] OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase

Laszlo Ersek
 

On 05/07/21 15:29, Brijesh Singh wrote:

On 5/6/21 9:08 AM, Laszlo Ersek wrote:
On 04/30/21 13:51, Brijesh Singh wrote:
BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C01d3e5c5268043c18bdf08d910987251%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637559069206222390%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=CN2hZrjsKzfSqMAxcQLtoHTUqBOlZmdDEO9vY9XT%2FTQ%3D&amp;reserved=0

Commit 85b8eac59b8c5bd9c7eb9afdb64357ce1aa2e803 added support to ensure
that MMIO is only performed against the un-encrypted memory. If MMIO
is performed against encrypted memory, a #GP is raised.

The VmgExitLib library depends on ApicTimerLib to get the APIC base
address so that it can exclude the APIC range from the un-encrypted
check. The OvmfPkg provides ApicTimerLib for the DXE phase. The
constructor AcpiTimerLibConstructor() used in the ApicTimerLib uses
the PciRead to get the PMBASE register. The PciRead() will cause an
MMIO access.

The AmdSevDxe driver clears the memory encryption attribute from the
MMIO ranges. However, if VmgExitLib is linked to AmdSevDxe driver then the
AcpiTimerLibConstructor() will be called before AmdSevDxe driver can
clear the encryption attributes for the MMIO regions.

Exclude the PMBASE register from the encrypted check so that we
can link VmgExitLib to the MemEncryptSevLib; which gets linked to
AmdSevDxe driver.
The above explanation is inexact. There are several typos ("APIC" is
incorrect, "ACPI" would be correct, for the TimerLib instance in
question), but that's really just a side observation.

The precise explanation is the following library instance dependency
chain:

OvmfPkg/AmdSevDxe/AmdSevDxe.inf
-----> MemEncryptSevLib class
-----> "OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf" instance
-[*]-> VmgExitLib class
-----> "OvmfPkg/Library/VmgExitLib/VmgExitLib.inf" instance
-----> LocalApicLib class
-----> "UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf" instance
-----> TimerLib class
-----> "OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf" instance
-----> PciLib class
-----> "OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf" instance
-----> PciExpressLib class
-----> "MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf" instance

The link (or dependency) marked with [*] is introduced in patch #26
("OvmfPkg/MemEncryptSevLib: Change the page state in the RMP table").
That's the change that triggers the symptom. (In combination with you
testing on Q35, because on i440fx, the DxePciLibI440FxQ35 lib instance
accesses PCI config space via the 0xCF8, 0xCFC IO Ports, and those are
unaffected by SEV-ES.)

The symptom is somewhat "unjustified", because at the end of the series,
the AmdSevDxe driver makes no calls to actual TimerLib APIs (I checked
-- I disassembled the "AmdSevDxe.debug" file with "objdump -S", and
there is no call to any API declared in the "TimerLib.h" class header).
However, the ECAM (MMCONFIG) access is still triggered, because the
AcpiTimerLibConstructor() function, in
"OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c", is the constructor for
the "OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf" instance, and
AcpiTimerLibConstructor() calls PciRead32().

If you check the "OvmfPkg/OvmfPkgX64.dsc" file, you'll find that the
PciLib class is resolved to
"MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf" by default, and to
"OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf" for the
following module types:

- DXE_DRIVER,
- DXE_RUNTIME_DRIVER,
- SMM_CORE,
- DXE_SMM_DRIVER,
- UEFI_DRIVER,
- UEFI_APPLICATION.

The consequence is that modules strictly after the DXE_CORE get
dynamically enabled extended config space access (ECAM) on Q35 via the
PciLib class, whereas all modules strictly before the DXE_CORE, and the
DXE_CORE itself, are restricted to normal config space (IO Ports 0xCF8 /
0xCFC) via the PciLib class.

AmdSevDxe is a DXE_DRIVER, so it used to get DxePciLibI440FxQ35 as well.

The solution should be simple. In the AmdSevDxe driver specifically, we
need no access to extended PCI config space. Accessing normal PCI config
space, via IO Ports 0xCF8 / 0xCFC, should suffice. That can be achieved
with the following module-scope override:
Thanks Laszlo, I was not aware of the module-scope override. I will go
with this approach and make sure it works after the inclusion of the
VmgExitLib.



diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 8d9a0a077601..45a02b236633 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -966,7 +966,10 @@ [Components]
!endif

OvmfPkg/PlatformDxe/Platform.inf
- OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+ OvmfPkg/AmdSevDxe/AmdSevDxe.inf {
+ <LibraryClasses>
+ PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
+ }
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
(

For consistency, all DSC files that include
"OvmfPkg/AmdSevDxe/AmdSevDxe.inf" should be modified similarly:

- OvmfPkg/AmdSev/AmdSevX64.dsc
- OvmfPkg/Bhyve/BhyveX64.dsc
- OvmfPkg/OvmfPkgIa32X64.dsc
- OvmfPkg/OvmfPkgX64.dsc
- OvmfPkg/OvmfXen.dsc

)

Therefore, please try dropping this patch, and modifying patch#26
instead -- the above module-scope override (for 5 DSC files) should be
squashed into patch#26, *and* the explanation I provided above should be
included in the commit message of patch#26.

... Correction: you have an independent bug in the series that affects
my above analysis. Namely, you *seem* to add the VmgExitLib dependency
to the "OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf"
library instance, in patch#26. That's where you modify the INF file. But
that's wrong: in patch#21 ("OvmfPkg/MemEncryptSevLib: Add support to
validate system RAM"), you add a VmgInit() call to the same library
instance, via the new file
"OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c".

The bug in that patch is clear from the fact that you introduce an
#include <Library/VmgExitLib.h> directive, but that's not mirrored by an
appropriate [LibraryClasses] change in the "DxeMemEncryptSevLib.inf"
file. (The other two lib instance INF files, "SecMemEncryptSevLib.inf"
and "PeiMemEncryptSevLib.inf" *are* modified as needed.)

So you even need to move some stuff from patch#26 to patch#21, and
*then* squash the above module-scope override (and explanation) into
patch#21.

A significant amount of work is needed on this series. I'll stop
reviewing RFC v2 here, because I don't want to look at the remaining
patches deeply as long as code movements etc are going to affect them.
Please post the next version -- assuming no other reviewer would like to
finish reviewing this version first!

Sounds good. What's your thought if I take out patch 1 - 9 from this RFC
series and submit them as non-RFC for the further review and acceptance
? The patch# 1-9 are basically prepatch before we get into SNP specific
bits.
More precisely, that means patches 1-8 (because patch#9 should be
replaced by the module-scope override, and also moved to just before
what is currently patch#21).

Other than that, I agree, this is a good idea. I've anyway thought that
the MdePkg stuff (5 patches) could be / should be merged up-front in
separation, and then the subsequent 3 patches for OvmfPkg are basically
refactoring. We can record the resultant commit range (8 commits) in
TianoCore#3275, and keep the BZ open for the rest of the work. So go
ahead please.

Thanks!
Laszlo




Thanks
Laszlo

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 4 ++
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 7 +++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 45 ++++++++++++++++++++
3 files changed, 56 insertions(+)

diff --git a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
index e6f6ea7972..22435a0590 100644
--- a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
@@ -27,6 +27,7 @@
SecVmgExitVcHandler.c

[Packages]
+ MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
UefiCpuPkg/UefiCpuPkg.dec
@@ -42,4 +43,7 @@
[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress

+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
index c66c68726c..d3175c260e 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
@@ -27,6 +27,7 @@
PeiDxeVmgExitVcHandler.c

[Packages]
+ MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
UefiCpuPkg/UefiCpuPkg.dec
@@ -37,4 +38,10 @@
DebugLib
LocalApicLib
MemEncryptSevLib
+ PcdLib

+[FixedPcd]
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+
+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 24259060fd..01ac5d8c19 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -14,7 +14,10 @@
#include <Library/VmgExitLib.h>
#include <Register/Amd/Msr.h>
#include <Register/Intel/Cpuid.h>
+#include <IndustryStandard/Q35MchIch9.h>
+#include <IndustryStandard/I440FxPiix4.h>
#include <IndustryStandard/InstructionParsing.h>
+#include <Library/PcdLib.h>

#include "VmgExitVcHandler.h"

@@ -596,6 +599,40 @@ UnsupportedExit (
return Status;
}

+STATIC
+BOOLEAN
+IsPmbaBaseAddress (
+ IN UINTN Address
+ )
+{
+ UINT16 HostBridgeDevId;
+ UINTN Pmba;
+
+ //
+ // Query Host Bridge DID to determine platform type
+ //
+ HostBridgeDevId = PcdGet16 (PcdOvmfHostBridgePciDevId);
+ switch (HostBridgeDevId) {
+ case INTEL_82441_DEVICE_ID:
+ Pmba = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA);
+ break;
+ case INTEL_Q35_MCH_DEVICE_ID:
+ Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);
+ //
+ // Add the MMCONFIG base address to get the Pmba base access address
+ //
+ Pmba += FixedPcdGet64 (PcdPciExpressBaseAddress);
+ break;
+ default:
+ return FALSE;
+ }
+
+ // Round up the offset to page size
+ Pmba = Pmba & ~(SIZE_4KB - 1);
+
+ return (Address == Pmba);
+}
+
/**
Validate that the MMIO memory access is not to encrypted memory.

@@ -640,6 +677,14 @@ ValidateMmioMemory (
return 0;
}

+ //
+ // Allow PMBASE accesses (which will have the encryption bit set before
+ // AmdSevDxe runs in the DXE phase)
+ //
+ if (IsPmbaBaseAddress (Address)) {
+ return 0;
+ }
+
//
// Any state other than unencrypted is an error, issue a #GP.
//


[PATCH v2 2/2] SecurityPkg: Add support for RngDxe on AARCH64

Rebecca Cran <rebecca@...>
 

AARCH64 support has been added to BaseRngLib via the optional
ARMv8.5 FEAT_RNG.

Refactor RngDxe to support AARCH64, note support for it in the
VALID_ARCHITECTURES line of RngDxe.inf and enable it in SecurityPkg.dsc.

Signed-off-by: Rebecca Cran <rebecca@...>
---
SecurityPkg/SecurityPkg.dec | 2 +
SecurityPkg/SecurityPkg.dsc | 11 +-
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 24 ++-
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.h | 0
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.h | 17 --
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h | 117 ++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c | 127 +++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.c | 0
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.c | 45 +-----
SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c | 146 +++++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c | 170 ++++++++------------
11 files changed, 483 insertions(+), 176 deletions(-)

diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index dfbbb0365a2b..a45104fe3e6c 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -297,6 +297,8 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass|0x0303100A|UINT32|0x00010030
gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationFail|0x0303100B|UINT32|0x00010031

+ gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0}|VOID*|0x00010032
+
[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## Image verification policy for OptionRom. Only following values are valid:<BR><BR>
# NOTE: Do NOT use 0x5 and 0x2 since it violates the UEFI specification and has been removed.<BR>
diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 12ccd1634941..bd4b810bce61 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -259,6 +259,12 @@ [Components]
[Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf

+[Components.IA32, Components.X64, Components.AARCH64]
+ #
+ # Random Number Generator
+ #
+ SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
[Components.IA32, Components.X64]
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf

@@ -334,11 +340,6 @@ [Components.IA32, Components.X64]
SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/StandaloneMmTcg2PhysicalPresenceLib.inf

- #
- # Random Number Generator
- #
- SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
-
#
# Opal Password solution
#
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
index 99d6f6b35fc2..f3300971993f 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
@@ -26,15 +26,22 @@ [Defines]
#
# The following information is for reference only and not required by the build tools.
#
-# VALID_ARCHITECTURES = IA32 X64
+# VALID_ARCHITECTURES = IA32 X64 AARCH64
#

[Sources.common]
RngDxe.c
- RdRand.c
- RdRand.h
- AesCore.c
- AesCore.h
+ RngDxeInternals.h
+
+[Sources.IA32, Sources.X64]
+ Rand/RngDxe.c
+ Rand/RdRand.c
+ Rand/RdRand.h
+ Rand/AesCore.c
+ Rand/AesCore.h
+
+[Sources.AARCH64]
+ AArch64/RngDxe.c

[Packages]
MdePkg/MdePkg.dec
@@ -50,12 +57,19 @@ [LibraryClasses]
RngLib

[Guids]
+ gEfiRngAlgorithmSp80090Hash256Guid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
+ gEfiRngAlgorithmSp80090Hmac256Guid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
gEfiRngAlgorithmSp80090Ctr256Guid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
+ gEfiRngAlgorithmX9313DesGuid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
+ gEfiRngAlgorithmX931AesGuid ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG
gEfiRngAlgorithmRaw ## SOMETIMES_PRODUCES ## GUID # Unique ID of the algorithm for RNG

[Protocols]
gEfiRngProtocolGuid ## PRODUCES

+[Pcd]
+ gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm ## CONSUMES
+
[Depex]
TRUE

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AesCore.h b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/AesCore.h
similarity index 100%
rename from SecurityPkg/RandomNumberGenerator/RngDxe/AesCore.h
rename to SecurityPkg/RandomNumberGenerator/RngDxe/Rand/AesCore.h
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.h b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h
similarity index 72%
rename from SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.h
rename to SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h
index 12ab1f34ec6d..072378e062e7 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.h
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.h
@@ -23,23 +23,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/TimerLib.h>
#include <Protocol/Rng.h>

-/**
- Calls RDRAND to fill a buffer of arbitrary size with random bytes.
-
- @param[in] Length Size of the buffer, in bytes, to fill with.
- @param[out] RandBuffer Pointer to the buffer to store the random result.
-
- @retval EFI_SUCCESS Random bytes generation succeeded.
- @retval EFI_NOT_READY Failed to request random bytes.
-
-**/
-EFI_STATUS
-EFIAPI
-RdRandGetBytes (
- IN UINTN Length,
- OUT UINT8 *RandBuffer
- );
-
/**
Generate high-quality entropy source through RDRAND.

diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
new file mode 100644
index 000000000000..2660ed5875e0
--- /dev/null
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
@@ -0,0 +1,117 @@
+/** @file
+ Function prototypes for UEFI Random Number Generator protocol support.
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef RNGDXE_INTERNALS_H_
+#define RNGDXE_INTERNALS_H_
+
+/**
+ Returns information about the random number generation implementation.
+
+ @param[in] This A pointer to the EFI_RNG_PROTOCOL instance.
+ @param[in,out] RNGAlgorithmListSize On input, the size in bytes of RNGAlgorithmList.
+ On output with a return code of EFI_SUCCESS, the size
+ in bytes of the data returned in RNGAlgorithmList. On output
+ with a return code of EFI_BUFFER_TOO_SMALL,
+ the size of RNGAlgorithmList required to obtain the list.
+ @param[out] RNGAlgorithmList A caller-allocated memory buffer filled by the driver
+ with one EFI_RNG_ALGORITHM element for each supported
+ RNG algorithm. The list must not change across multiple
+ calls to the same driver. The first algorithm in the list
+ is the default algorithm for the driver.
+
+ @retval EFI_SUCCESS The RNG algorithm list was returned successfully.
+ @retval EFI_UNSUPPORTED The services is not supported by this driver.
+ @retval EFI_DEVICE_ERROR The list of algorithms could not be retrieved due to a
+ hardware or firmware error.
+ @retval EFI_INVALID_PARAMETER One or more of the parameters are incorrect.
+ @retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetInfo (
+ IN EFI_RNG_PROTOCOL *This,
+ IN OUT UINTN *RNGAlgorithmListSize,
+ OUT EFI_RNG_ALGORITHM *RNGAlgorithmList
+ );
+
+/**
+ Produces and returns an RNG value using either the default or specified RNG algorithm.
+
+ @param[in] This A pointer to the EFI_RNG_PROTOCOL instance.
+ @param[in] RNGAlgorithm A pointer to the EFI_RNG_ALGORITHM that identifies the RNG
+ algorithm to use. May be NULL in which case the function will
+ use its default RNG algorithm.
+ @param[in] RNGValueLength The length in bytes of the memory buffer pointed to by
+ RNGValue. The driver shall return exactly this numbers of bytes.
+ @param[out] RNGValue A caller-allocated memory buffer filled by the driver with the
+ resulting RNG value.
+
+ @retval EFI_SUCCESS The RNG value was returned successfully.
+ @retval EFI_UNSUPPORTED The algorithm specified by RNGAlgorithm is not supported by
+ this driver.
+ @retval EFI_DEVICE_ERROR An RNG value could not be retrieved due to a hardware or
+ firmware error.
+ @retval EFI_NOT_READY There is not enough random data available to satisfy the length
+ requested by RNGValueLength.
+ @retval EFI_INVALID_PARAMETER RNGValue is NULL or RNGValueLength is zero.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetRNG (
+ IN EFI_RNG_PROTOCOL *This,
+ IN EFI_RNG_ALGORITHM *RNGAlgorithm, OPTIONAL
+ IN UINTN RNGValueLength,
+ OUT UINT8 *RNGValue
+ );
+
+/**
+ Returns information about the random number generation implementation.
+
+ @param[in,out] RNGAlgorithmListSize On input, the size in bytes of RNGAlgorithmList.
+ On output with a return code of EFI_SUCCESS, the size
+ in bytes of the data returned in RNGAlgorithmList. On output
+ with a return code of EFI_BUFFER_TOO_SMALL,
+ the size of RNGAlgorithmList required to obtain the list.
+ @param[out] RNGAlgorithmList A caller-allocated memory buffer filled by the driver
+ with one EFI_RNG_ALGORITHM element for each supported
+ RNG algorithm. The list must not change across multiple
+ calls to the same driver. The first algorithm in the list
+ is the default algorithm for the driver.
+
+ @retval EFI_SUCCESS The RNG algorithm list was returned successfully.
+ @retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
+
+**/
+UINTN
+EFIAPI
+ArchGetSupportedRngAlgorithms (
+ IN OUT UINTN *RNGAlgorithmListSize,
+ OUT EFI_RNG_ALGORITHM *RNGAlgorithmList
+ );
+
+/**
+ Runs CPU RNG instruction to fill a buffer of arbitrary size with random bytes.
+
+ @param[in] Length Size of the buffer, in bytes, to fill with.
+ @param[out] RandBuffer Pointer to the buffer to store the random result.
+
+ @retval EFI_SUCCESS Random bytes generation succeeded.
+ @retval EFI_NOT_READY Failed to request random bytes.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetBytes (
+ IN UINTN Length,
+ OUT UINT8 *RandBuffer
+ );
+
+#endif // RNGDXE_INTERNALS_H_
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
new file mode 100644
index 000000000000..2810a9eb94ad
--- /dev/null
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
@@ -0,0 +1,127 @@
+/** @file
+ RNG Driver to produce the UEFI Random Number Generator protocol.
+
+ The driver will use the RNDR instruction to produce random numbers.
+
+ RNG Algorithms defined in UEFI 2.4:
+ - EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
+ - EFI_RNG_ALGORITHM_RAW - Unsupported
+ - EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID
+ - EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID
+ - EFI_RNG_ALGORITHM_X9_31_3DES_GUID - Unsupported
+ - EFI_RNG_ALGORITHM_X9_31_AES_GUID - Unsupported
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+ Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+ (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/TimerLib.h>
+#include <Protocol/Rng.h>
+
+#include "RngDxeInternals.h"
+
+/**
+ Produces and returns an RNG value using either the default or specified RNG algorithm.
+
+ @param[in] This A pointer to the EFI_RNG_PROTOCOL instance.
+ @param[in] RNGAlgorithm A pointer to the EFI_RNG_ALGORITHM that identifies the RNG
+ algorithm to use. May be NULL in which case the function will
+ use its default RNG algorithm.
+ @param[in] RNGValueLength The length in bytes of the memory buffer pointed to by
+ RNGValue. The driver shall return exactly this numbers of bytes.
+ @param[out] RNGValue A caller-allocated memory buffer filled by the driver with the
+ resulting RNG value.
+
+ @retval EFI_SUCCESS The RNG value was returned successfully.
+ @retval EFI_UNSUPPORTED The algorithm specified by RNGAlgorithm is not supported by
+ this driver.
+ @retval EFI_DEVICE_ERROR An RNG value could not be retrieved due to a hardware or
+ firmware error.
+ @retval EFI_NOT_READY There is not enough random data available to satisfy the length
+ requested by RNGValueLength.
+ @retval EFI_INVALID_PARAMETER RNGValue is NULL or RNGValueLength is zero.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetRNG (
+ IN EFI_RNG_PROTOCOL *This,
+ IN EFI_RNG_ALGORITHM *RNGAlgorithm, OPTIONAL
+ IN UINTN RNGValueLength,
+ OUT UINT8 *RNGValue
+ )
+{
+ EFI_STATUS Status;
+
+ if ((RNGValueLength == 0) || (RNGValue == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (RNGAlgorithm == NULL) {
+ //
+ // Use the default RNG algorithm if RNGAlgorithm is NULL.
+ //
+ RNGAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+ }
+
+ if (CompareGuid (RNGAlgorithm, PcdGetPtr (PcdCpuRngSupportedAlgorithm))) {
+ Status = RngGetBytes (RNGValueLength, RNGValue);
+ return Status;
+ }
+
+ //
+ // Other algorithms are unsupported by this driver.
+ //
+ return EFI_UNSUPPORTED;
+}
+
+/**
+ Returns information about the random number generation implementation.
+
+ @param[in,out] RNGAlgorithmListSize On input, the size in bytes of RNGAlgorithmList.
+ On output with a return code of EFI_SUCCESS, the size
+ in bytes of the data returned in RNGAlgorithmList. On output
+ with a return code of EFI_BUFFER_TOO_SMALL,
+ the size of RNGAlgorithmList required to obtain the list.
+ @param[out] RNGAlgorithmList A caller-allocated memory buffer filled by the driver
+ with one EFI_RNG_ALGORITHM element for each supported
+ RNG algorithm. The list must not change across multiple
+ calls to the same driver. The first algorithm in the list
+ is the default algorithm for the driver.
+
+ @retval EFI_SUCCESS The RNG algorithm list was returned successfully.
+ @retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
+
+**/
+UINTN
+EFIAPI
+ArchGetSupportedRngAlgorithms (
+ IN OUT UINTN *RNGAlgorithmListSize,
+ OUT EFI_RNG_ALGORITHM *RNGAlgorithmList
+ )
+{
+ UINTN RequiredSize;
+ EFI_RNG_ALGORITHM *CpuRngSupportedAlgorithm;
+
+ RequiredSize = sizeof (EFI_RNG_ALGORITHM);
+
+ if (*RNGAlgorithmListSize < RequiredSize) {
+ *RNGAlgorithmListSize = RequiredSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+
+ CopyMem(&RNGAlgorithmList[0], CpuRngSupportedAlgorithm, sizeof (EFI_RNG_ALGORITHM));
+
+ *RNGAlgorithmListSize = RequiredSize;
+ return EFI_SUCCESS;
+}
+
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/AesCore.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/AesCore.c
similarity index 100%
rename from SecurityPkg/RandomNumberGenerator/RngDxe/AesCore.c
rename to SecurityPkg/RandomNumberGenerator/RngDxe/Rand/AesCore.c
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
similarity index 71%
rename from SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.c
rename to SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
index e7dd5ab18111..83025a47d43d 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RdRand.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RdRand.c
@@ -8,48 +8,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include <Library/RngLib.h>

-#include "RdRand.h"
#include "AesCore.h"
-
-/**
- Calls RDRAND to fill a buffer of arbitrary size with random bytes.
-
- @param[in] Length Size of the buffer, in bytes, to fill with.
- @param[out] RandBuffer Pointer to the buffer to store the random result.
-
- @retval EFI_SUCCESS Random bytes generation succeeded.
- @retval EFI_NOT_READY Failed to request random bytes.
-
-**/
-EFI_STATUS
-EFIAPI
-RdRandGetBytes (
- IN UINTN Length,
- OUT UINT8 *RandBuffer
- )
-{
- BOOLEAN IsRandom;
- UINT64 TempRand[2];
-
- while (Length > 0) {
- IsRandom = GetRandomNumber128 (TempRand);
- if (!IsRandom) {
- return EFI_NOT_READY;
- }
- if (Length >= sizeof (TempRand)) {
- WriteUnaligned64 ((UINT64*)RandBuffer, TempRand[0]);
- RandBuffer += sizeof (UINT64);
- WriteUnaligned64 ((UINT64*)RandBuffer, TempRand[1]);
- RandBuffer += sizeof (UINT64);
- Length -= sizeof (TempRand);
- } else {
- CopyMem (RandBuffer, TempRand, Length);
- Length = 0;
- }
- }
-
- return EFI_SUCCESS;
-}
+#include "RdRand.h"
+#include "RngDxeInternals.h"

/**
Creates a 128bit random value that is fully forward and backward prediction resistant,
@@ -92,7 +53,7 @@ RdRandGetSeed128 (
//
for (Index = 0; Index < 32; Index++) {
MicroSecondDelay (10);
- Status = RdRandGetBytes (16, RandByte);
+ Status = RngGetBytes (16, RandByte);
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
new file mode 100644
index 000000000000..6b628a9f8bc6
--- /dev/null
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
@@ -0,0 +1,146 @@
+/** @file
+ RNG Driver to produce the UEFI Random Number Generator protocol.
+
+ The driver will use the new RDRAND instruction to produce high-quality, high-performance
+ entropy and random number.
+
+ RNG Algorithms defined in UEFI 2.4:
+ - EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID - Supported
+ (RDRAND implements a hardware NIST SP800-90 AES-CTR-256 based DRBG)
+ - EFI_RNG_ALGORITHM_RAW - Supported
+ (Structuring RDRAND invocation can be guaranteed as high-quality entropy source)
+ - EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID - Unsupported
+ - EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID - Unsupported
+ - EFI_RNG_ALGORITHM_X9_31_3DES_GUID - Unsupported
+ - EFI_RNG_ALGORITHM_X9_31_AES_GUID - Unsupported
+
+ Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+ (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "RdRand.h"
+#include "RngDxeInternals.h"
+
+/**
+ Produces and returns an RNG value using either the default or specified RNG algorithm.
+
+ @param[in] This A pointer to the EFI_RNG_PROTOCOL instance.
+ @param[in] RNGAlgorithm A pointer to the EFI_RNG_ALGORITHM that identifies the RNG
+ algorithm to use. May be NULL in which case the function will
+ use its default RNG algorithm.
+ @param[in] RNGValueLength The length in bytes of the memory buffer pointed to by
+ RNGValue. The driver shall return exactly this numbers of bytes.
+ @param[out] RNGValue A caller-allocated memory buffer filled by the driver with the
+ resulting RNG value.
+
+ @retval EFI_SUCCESS The RNG value was returned successfully.
+ @retval EFI_UNSUPPORTED The algorithm specified by RNGAlgorithm is not supported by
+ this driver.
+ @retval EFI_DEVICE_ERROR An RNG value could not be retrieved due to a hardware or
+ firmware error.
+ @retval EFI_NOT_READY There is not enough random data available to satisfy the length
+ requested by RNGValueLength.
+ @retval EFI_INVALID_PARAMETER RNGValue is NULL or RNGValueLength is zero.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetRNG (
+ IN EFI_RNG_PROTOCOL *This,
+ IN EFI_RNG_ALGORITHM *RNGAlgorithm, OPTIONAL
+ IN UINTN RNGValueLength,
+ OUT UINT8 *RNGValue
+ )
+{
+ EFI_STATUS Status;
+
+ if ((RNGValueLength == 0) || (RNGValue == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = EFI_UNSUPPORTED;
+ if (RNGAlgorithm == NULL) {
+ //
+ // Use the default RNG algorithm if RNGAlgorithm is NULL.
+ //
+ RNGAlgorithm = &gEfiRngAlgorithmSp80090Ctr256Guid;
+ }
+
+ //
+ // NIST SP800-90-AES-CTR-256 supported by RDRAND
+ //
+ if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmSp80090Ctr256Guid)) {
+ Status = RngGetBytes (RNGValueLength, RNGValue);
+ return Status;
+ }
+
+ //
+ // The "raw" algorithm is intended to provide entropy directly
+ //
+ if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmRaw)) {
+ //
+ // When a DRBG is used on the output of a entropy source,
+ // its security level must be at least 256 bits according to UEFI Spec.
+ //
+ if (RNGValueLength < 32) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = RdRandGenerateEntropy (RNGValueLength, RNGValue);
+ return Status;
+ }
+
+ //
+ // Other algorithms were unsupported by this driver.
+ //
+ return Status;
+}
+
+/**
+ Returns information about the random number generation implementation.
+
+ @param[in,out] RNGAlgorithmListSize On input, the size in bytes of RNGAlgorithmList.
+ On output with a return code of EFI_SUCCESS, the size
+ in bytes of the data returned in RNGAlgorithmList. On output
+ with a return code of EFI_BUFFER_TOO_SMALL,
+ the size of RNGAlgorithmList required to obtain the list.
+ @param[out] RNGAlgorithmList A caller-allocated memory buffer filled by the driver
+ with one EFI_RNG_ALGORITHM element for each supported
+ RNG algorithm. The list must not change across multiple
+ calls to the same driver. The first algorithm in the list
+ is the default algorithm for the driver.
+
+ @retval EFI_SUCCESS The RNG algorithm list was returned successfully.
+ @retval EFI_BUFFER_TOO_SMALL The buffer RNGAlgorithmList is too small to hold the result.
+
+**/
+UINTN
+EFIAPI
+ArchGetSupportedRngAlgorithms (
+ IN OUT UINTN *RNGAlgorithmListSize,
+ OUT EFI_RNG_ALGORITHM *RNGAlgorithmList
+ )
+{
+ UINTN RequiredSize;
+ EFI_RNG_ALGORITHM *CpuRngSupportedAlgorithm;
+
+ RequiredSize = 2 * sizeof (EFI_RNG_ALGORITHM);
+
+ if (*RNGAlgorithmListSize < RequiredSize) {
+ *RNGAlgorithmListSize = RequiredSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ CpuRngSupportedAlgorithm = PcdGetPtr (PcdCpuRngSupportedAlgorithm);
+
+ CopyMem(&RNGAlgorithmList[0], CpuRngSupportedAlgorithm, sizeof (EFI_RNG_ALGORITHM));
+
+ // x86 platforms also support EFI_RNG_ALGORITHM_RAW via RDSEED
+ CopyMem(&RNGAlgorithmList[1], &gEfiRngAlgorithmRaw, sizeof (EFI_RNG_ALGORITHM));
+
+ *RNGAlgorithmListSize = RequiredSize;
+ return EFI_SUCCESS;
+}
+
diff --git a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
index 13d3dbd0bfbe..b959c70536ea 100644
--- a/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
+++ b/SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c
@@ -1,34 +1,32 @@
/** @file
RNG Driver to produce the UEFI Random Number Generator protocol.

- The driver will use the new RDRAND instruction to produce high-quality, high-performance
- entropy and random number.
+ The driver uses CPU RNG instructions to produce high-quality,
+ high-performance entropy and random number.

RNG Algorithms defined in UEFI 2.4:
- - EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID - Supported
- (RDRAND implements a hardware NIST SP800-90 AES-CTR-256 based DRBG)
- - EFI_RNG_ALGORITHM_RAW - Supported
- (Structuring RDRAND invocation can be guaranteed as high-quality entropy source)
- - EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID - Unsupported
- - EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID - Unsupported
- - EFI_RNG_ALGORITHM_X9_31_3DES_GUID - Unsupported
- - EFI_RNG_ALGORITHM_X9_31_AES_GUID - Unsupported
+ - EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID
+ - EFI_RNG_ALGORITHM_RAW
+ - EFI_RNG_ALGORITHM_SP800_90_HMAC_256_GUID
+ - EFI_RNG_ALGORITHM_SP800_90_HASH_256_GUID
+ - EFI_RNG_ALGORITHM_X9_31_3DES_GUID
+ - EFI_RNG_ALGORITHM_X9_31_AES_GUID

Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

-#include "RdRand.h"
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/RngLib.h>
+#include <Library/TimerLib.h>
+#include <Protocol/Rng.h>

-//
-// Supported RNG Algorithms list by this driver.
-//
-EFI_RNG_ALGORITHM mSupportedRngAlgorithms[] = {
- EFI_RNG_ALGORITHM_SP800_90_CTR_256_GUID,
- EFI_RNG_ALGORITHM_RAW
-};
+#include "RngDxeInternals.h"

/**
Returns information about the random number generation implementation.
@@ -62,106 +60,23 @@ RngGetInfo (
)
{
EFI_STATUS Status;
- UINTN RequiredSize;

if ((This == NULL) || (RNGAlgorithmListSize == NULL)) {
return EFI_INVALID_PARAMETER;
}

- RequiredSize = sizeof (mSupportedRngAlgorithms);
- if (*RNGAlgorithmListSize < RequiredSize) {
- Status = EFI_BUFFER_TOO_SMALL;
+ //
+ // Return algorithm list supported by driver.
+ //
+ if (RNGAlgorithmList != NULL) {
+ Status = ArchGetSupportedRngAlgorithms (RNGAlgorithmListSize, RNGAlgorithmList);
} else {
- //
- // Return algorithm list supported by driver.
- //
- if (RNGAlgorithmList != NULL) {
- CopyMem (RNGAlgorithmList, mSupportedRngAlgorithms, RequiredSize);
- Status = EFI_SUCCESS;
- } else {
- Status = EFI_INVALID_PARAMETER;
- }
+ Status = EFI_INVALID_PARAMETER;
}
- *RNGAlgorithmListSize = RequiredSize;

return Status;
}

-/**
- Produces and returns an RNG value using either the default or specified RNG algorithm.
-
- @param[in] This A pointer to the EFI_RNG_PROTOCOL instance.
- @param[in] RNGAlgorithm A pointer to the EFI_RNG_ALGORITHM that identifies the RNG
- algorithm to use. May be NULL in which case the function will
- use its default RNG algorithm.
- @param[in] RNGValueLength The length in bytes of the memory buffer pointed to by
- RNGValue. The driver shall return exactly this numbers of bytes.
- @param[out] RNGValue A caller-allocated memory buffer filled by the driver with the
- resulting RNG value.
-
- @retval EFI_SUCCESS The RNG value was returned successfully.
- @retval EFI_UNSUPPORTED The algorithm specified by RNGAlgorithm is not supported by
- this driver.
- @retval EFI_DEVICE_ERROR An RNG value could not be retrieved due to a hardware or
- firmware error.
- @retval EFI_NOT_READY There is not enough random data available to satisfy the length
- requested by RNGValueLength.
- @retval EFI_INVALID_PARAMETER RNGValue is NULL or RNGValueLength is zero.
-
-**/
-EFI_STATUS
-EFIAPI
-RngGetRNG (
- IN EFI_RNG_PROTOCOL *This,
- IN EFI_RNG_ALGORITHM *RNGAlgorithm, OPTIONAL
- IN UINTN RNGValueLength,
- OUT UINT8 *RNGValue
- )
-{
- EFI_STATUS Status;
-
- if ((RNGValueLength == 0) || (RNGValue == NULL)) {
- return EFI_INVALID_PARAMETER;
- }
-
- Status = EFI_UNSUPPORTED;
- if (RNGAlgorithm == NULL) {
- //
- // Use the default RNG algorithm if RNGAlgorithm is NULL.
- //
- RNGAlgorithm = &gEfiRngAlgorithmSp80090Ctr256Guid;
- }
-
- //
- // NIST SP800-90-AES-CTR-256 supported by RDRAND
- //
- if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmSp80090Ctr256Guid)) {
- Status = RdRandGetBytes (RNGValueLength, RNGValue);
- return Status;
- }
-
- //
- // The "raw" algorithm is intended to provide entropy directly
- //
- if (CompareGuid (RNGAlgorithm, &gEfiRngAlgorithmRaw)) {
- //
- // When a DRBG is used on the output of a entropy source,
- // its security level must be at least 256 bits according to UEFI Spec.
- //
- if (RNGValueLength < 32) {
- return EFI_INVALID_PARAMETER;
- }
-
- Status = RdRandGenerateEntropy (RNGValueLength, RNGValue);
- return Status;
- }
-
- //
- // Other algorithms were unsupported by this driver.
- //
- return Status;
-}
-
//
// The Random Number Generator (RNG) protocol
//
@@ -204,3 +119,44 @@ RngDriverEntry (

return Status;
}
+
+
+/**
+ Calls RDRAND to fill a buffer of arbitrary size with random bytes.
+
+ @param[in] Length Size of the buffer, in bytes, to fill with.
+ @param[out] RandBuffer Pointer to the buffer to store the random result.
+
+ @retval EFI_SUCCESS Random bytes generation succeeded.
+ @retval EFI_NOT_READY Failed to request random bytes.
+
+**/
+EFI_STATUS
+EFIAPI
+RngGetBytes (
+ IN UINTN Length,
+ OUT UINT8 *RandBuffer
+ )
+{
+ BOOLEAN IsRandom;
+ UINT64 TempRand[2];
+
+ while (Length > 0) {
+ IsRandom = GetRandomNumber128 (TempRand);
+ if (!IsRandom) {
+ return EFI_NOT_READY;
+ }
+ if (Length >= sizeof (TempRand)) {
+ WriteUnaligned64 ((UINT64*)RandBuffer, TempRand[0]);
+ RandBuffer += sizeof (UINT64);
+ WriteUnaligned64 ((UINT64*)RandBuffer, TempRand[1]);
+ RandBuffer += sizeof (UINT64);
+ Length -= sizeof (TempRand);
+ } else {
+ CopyMem (RandBuffer, TempRand, Length);
+ Length = 0;
+ }
+ }
+
+ return EFI_SUCCESS;
+}
--
2.26.2


[PATCH v2 1/2] MdePkg/BaseRngLib: Add support for ARMv8.5 RNG instructions

Rebecca Cran <rebecca@...>
 

Make BaseRngLib more generic by moving x86 specific functionality from
BaseRng.c into Rand/RdRand.c, and adding AArch64/Rndr.c, which supports
the optional ARMv8.5 RNG instructions RNDR and RNDRRS that are a part of
FEAT_RNG.

Add support for the optional ARMv8.5 RNDR and RNDRRS instructions that
are a part of FEAT_RNG to BaseLib, and add a function to read the ISAR0
register which indicates whether the CPU supports FEAT_RNG.

Signed-off-by: Rebecca Cran <rebecca@...>
---
MdePkg/MdePkg.dec | 9 +-
MdePkg/MdePkg.dsc | 4 +-
MdePkg/Library/BaseRngLib/BaseRngLib.inf | 23 +++-
MdePkg/Library/BaseRngLib/AArch64/ArmRng.h | 59 +++++++++
MdePkg/Library/BaseRngLib/BaseRngLibInternals.h | 79 +++++++++++
MdePkg/Library/BaseRngLib/AArch64/Rndr.c | 139 ++++++++++++++++++++
MdePkg/Library/BaseRngLib/BaseRng.c | 87 ++++++------
MdePkg/Library/BaseRngLib/Rand/RdRand.c | 131 ++++++++++++++++++
MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S | 31 +++++
MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm | 30 +++++
MdePkg/Library/BaseRngLib/AArch64/ArmRng.S | 61 +++++++++
MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm | 64 +++++++++
MdePkg/Library/BaseRngLib/BaseRngLib.uni | 6 +-
13 files changed, 669 insertions(+), 54 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 8965e903e093..b49f88d8e18f 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -267,6 +267,11 @@ [LibraryClasses]
#
RegisterFilterLib|Include/Library/RegisterFilterLib.h

+[LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH64]
+ ## @libraryclass Provides services to generate random number.
+ #
+ RngLib|Include/Library/RngLib.h
+
[LibraryClasses.IA32, LibraryClasses.X64]
## @libraryclass Abstracts both S/W SMI generation and detection.
##
@@ -288,10 +293,6 @@ [LibraryClasses.IA32, LibraryClasses.X64]
#
SmmPeriodicSmiLib|Include/Library/SmmPeriodicSmiLib.h

- ## @libraryclass Provides services to generate random number.
- #
- RngLib|Include/Library/RngLib.h
-
## @libraryclass Provides services to log the SMI handler registration.
SmiHandlerProfileLib|Include/Library/SmiHandlerProfileLib.h

diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index d363419006ea..a94959169b2f 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -145,6 +145,9 @@ [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
MdePkg/Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLibSmm.inf
MdePkg/Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLibUefiShell.inf

+[Components.IA32, Components.X64, Components.AARCH64]
+ MdePkg/Library/BaseRngLib/BaseRngLib.inf
+
[Components.IA32, Components.X64]
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
@@ -168,7 +171,6 @@ [Components.IA32, Components.X64]
MdePkg/Library/BaseS3StallLib/BaseS3StallLib.inf
MdePkg/Library/SmmMemLib/SmmMemLib.inf
MdePkg/Library/SmmIoLib/SmmIoLib.inf
- MdePkg/Library/BaseRngLib/BaseRngLib.inf
MdePkg/Library/SmmPciExpressLib/SmmPciExpressLib.inf
MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.inf
MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
diff --git a/MdePkg/Library/BaseRngLib/BaseRngLib.inf b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
index 31740751c69c..1fcceb941495 100644
--- a/MdePkg/Library/BaseRngLib/BaseRngLib.inf
+++ b/MdePkg/Library/BaseRngLib/BaseRngLib.inf
@@ -1,9 +1,10 @@
## @file
# Instance of RNG (Random Number Generator) Library.
#
-# BaseRng Library that uses CPU RdRand instruction access to provide
-# high-quality random numbers.
+# BaseRng Library that uses CPU RNG instructions (e.g. RdRand) to
+# provide random numbers.
#
+# Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
# Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,11 +23,25 @@ [Defines]
CONSTRUCTOR = BaseRngLibConstructor

#
-# VALID_ARCHITECTURES = IA32 X64
+# VALID_ARCHITECTURES = IA32 X64 AARCH64
#

-[Sources.Ia32, Sources.X64]
+[Sources]
BaseRng.c
+ BaseRngLibInternals.h
+
+[Sources.Ia32, Sources.X64]
+ Rand/RdRand.c
+
+[Sources.AARCH64]
+ AArch64/Rndr.c
+ AArch64/ArmRng.h
+
+ AArch64/ArmReadIdIsar0.S | GCC
+ AArch64/ArmRng.S | GCC
+
+ AArch64/ArmReadIdIsar0.asm | MSFT
+ AArch64/ArmRng.asm | MSFT

[Packages]
MdePkg/MdePkg.dec
diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
new file mode 100644
index 000000000000..595bd87ba60c
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
@@ -0,0 +1,59 @@
+/** @file
+ Random number generator service that uses the RNDR instruction
+ to provide pseudorandom numbers.
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef ARM_RNG_H_
+#define ARM_RNG_H_
+
+/**
+ Generates a random number using RNDR.
+ Returns TRUE on success; FALSE on failure.
+
+ @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArmRndr (
+ OUT UINT64 *Rand
+ );
+
+/**
+ Generates a random number using RNDRRS.
+ Returns TRUE on success; FALSE on failure.
+
+ @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArmRndrrs (
+ OUT UINT64 *Rand
+ );
+
+/**
+ Reads the ID_AA64ISAR0 Register.
+
+ @return The contents of the ID_AA64ISAR0 register.
+
+**/
+UINT64
+EFIAPI
+ArmReadIdIsar0 (
+ VOID
+ );
+
+#endif /* ARM_RNG_H_ */
+
diff --git a/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
new file mode 100644
index 000000000000..338ba6ea5313
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
@@ -0,0 +1,79 @@
+/** @file
+
+ Architecture specific interface to RNG functionality.
+
+Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef BASE_RNGLIB_INTERNALS_H_
+
+/**
+ Generates a 16-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber16 (
+ OUT UINT16 *Rand
+ );
+
+/**
+ Generates a 32-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber32 (
+ OUT UINT32 *Rand
+ );
+
+/**
+ Generates a 64-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber64 (
+ OUT UINT64 *Rand
+ );
+
+/**
+ Checks whether the RNG instruction is supported.
+
+ @retval TRUE RNG instruction is supported.
+ @retval FALSE RNG instruction is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+ArchIsRngSupported (
+ VOID
+ );
+
+#if defined (MDE_CPU_AARCH64)
+
+// RNDR, Random Number
+#define RNDR S3_3_C2_C4_0
+#define RNDRRS S3_3_C2_C4_1
+
+#endif
+
+#endif // BASE_RNGLIB_INTERNALS_H_
diff --git a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
new file mode 100644
index 000000000000..d658ad2bea89
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c
@@ -0,0 +1,139 @@
+/** @file
+ Random number generator service that uses the RNDR instruction
+ to provide pseudorandom numbers.
+
+ Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+ Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/RngLib.h>
+
+#include "ArmRng.h"
+#include "BaseRngLibInternals.h"
+
+STATIC BOOLEAN mRndrSupported;
+
+//
+// Bit mask used to determine if RNDR instruction is supported.
+//
+#define RNDR_MASK ((UINT64)MAX_UINT16 << 60U)
+
+/**
+ The constructor function checks whether or not RNDR instruction is supported
+ by the host hardware.
+
+ The constructor function checks whether or not RNDR instruction is supported.
+ It will ASSERT() if RNDR instruction is not supported.
+ It will always return EFI_SUCCESS.
+
+ @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+BaseRngLibConstructor (
+ VOID
+ )
+{
+ UINT64 Isar0;
+ //
+ // Determine RNDR support by examining bits 63:60 of the ISAR0 register returned by
+ // MSR. A non-zero value indicates that the processor supports the RNDR instruction.
+ //
+ Isar0 = ArmReadIdIsar0 ();
+ ASSERT ((Isar0 & RNDR_MASK) != 0);
+
+ mRndrSupported = ((Isar0 & RNDR_MASK) != 0);
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Generates a 16-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber16 (
+ OUT UINT16 *Rand
+ )
+{
+ UINT64 Rand64;
+
+ if (ArchGetRandomNumber64 (&Rand64)) {
+ *Rand = Rand64 & MAX_UINT16;
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
+/**
+ Generates a 32-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber32 (
+ OUT UINT32 *Rand
+ )
+{
+ UINT64 Rand64;
+
+ if (ArchGetRandomNumber64 (&Rand64)) {
+ *Rand = Rand64 & MAX_UINT32;
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
+/**
+ Generates a 64-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber64 (
+ OUT UINT64 *Rand
+ )
+{
+ return ArmRndr (Rand);
+}
+
+/**
+ Checks whether RNDR and RNDRRS (FEAT_RNG) are supported.
+
+ @retval TRUE RNDR and RNDRRS are supported.
+ @retval FALSE RNDR and RNDRRS are not supported.
+
+**/
+BOOLEAN
+EFIAPI
+ArchIsRngSupported (
+ VOID
+ )
+{
+ return mRndrSupported;
+}
diff --git a/MdePkg/Library/BaseRngLib/BaseRng.c b/MdePkg/Library/BaseRngLib/BaseRng.c
index 7ad7aec9d38f..5b63d8f7146b 100644
--- a/MdePkg/Library/BaseRngLib/BaseRng.c
+++ b/MdePkg/Library/BaseRngLib/BaseRng.c
@@ -1,8 +1,10 @@
/** @file
- Random number generator services that uses RdRand instruction access
- to provide high-quality random numbers.
+ Random number generator services that uses CPU RNG instructions to
+ provide random numbers.

+Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -10,46 +12,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseLib.h>
#include <Library/DebugLib.h>

-//
-// Bit mask used to determine if RdRand instruction is supported.
-//
-#define RDRAND_MASK BIT30
+#include "BaseRngLibInternals.h"

//
// Limited retry number when valid random data is returned.
// Uses the recommended value defined in Section 7.3.17 of "Intel 64 and IA-32
-// Architectures Software Developer's Mannual".
+// Architectures Software Developer's Manual".
//
-#define RDRAND_RETRY_LIMIT 10
+#define GETRANDOM_RETRY_LIMIT 10

-/**
- The constructor function checks whether or not RDRAND instruction is supported
- by the host hardware.
-
- The constructor function checks whether or not RDRAND instruction is supported.
- It will ASSERT() if RDRAND instruction is not supported.
- It will always return RETURN_SUCCESS.
-
- @retval RETURN_SUCCESS The constructor always returns EFI_SUCCESS.
-
-**/
-RETURN_STATUS
-EFIAPI
-BaseRngLibConstructor (
- VOID
- )
-{
- UINT32 RegEcx;
-
- //
- // Determine RDRAND support by examining bit 30 of the ECX register returned by
- // CPUID. A value of 1 indicates that processor support RDRAND instruction.
- //
- AsmCpuid (1, 0, 0, &RegEcx, 0);
- ASSERT ((RegEcx & RDRAND_MASK) == RDRAND_MASK);
-
- return RETURN_SUCCESS;
-}

/**
Generates a 16-bit random number.
@@ -72,11 +43,19 @@ GetRandomNumber16 (

ASSERT (Rand != NULL);

+ if (Rand == NULL) {
+ return FALSE;
+ }
+
+ if (!ArchIsRngSupported ()) {
+ return FALSE;
+ }
+
//
// A loop to fetch a 16 bit random value with a retry count limit.
//
- for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
- if (AsmRdRand16 (Rand)) {
+ for (Index = 0; Index < GETRANDOM_RETRY_LIMIT; Index++) {
+ if (ArchGetRandomNumber16 (Rand)) {
return TRUE;
}
}
@@ -105,11 +84,19 @@ GetRandomNumber32 (

ASSERT (Rand != NULL);

+ if (Rand == NULL) {
+ return FALSE;
+ }
+
+ if (!ArchIsRngSupported ()) {
+ return FALSE;
+ }
+
//
// A loop to fetch a 32 bit random value with a retry count limit.
//
- for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
- if (AsmRdRand32 (Rand)) {
+ for (Index = 0; Index < GETRANDOM_RETRY_LIMIT; Index++) {
+ if (ArchGetRandomNumber32 (Rand)) {
return TRUE;
}
}
@@ -138,11 +125,19 @@ GetRandomNumber64 (

ASSERT (Rand != NULL);

+ if (Rand == NULL) {
+ return FALSE;
+ }
+
+ if (!ArchIsRngSupported ()) {
+ return FALSE;
+ }
+
//
// A loop to fetch a 64 bit random value with a retry count limit.
//
- for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
- if (AsmRdRand64 (Rand)) {
+ for (Index = 0; Index < GETRANDOM_RETRY_LIMIT; Index++) {
+ if (ArchGetRandomNumber64 (Rand)) {
return TRUE;
}
}
@@ -169,6 +164,14 @@ GetRandomNumber128 (
{
ASSERT (Rand != NULL);

+ if (Rand == NULL) {
+ return FALSE;
+ }
+
+ if (!ArchIsRngSupported ()) {
+ return FALSE;
+ }
+
//
// Read first 64 bits
//
diff --git a/MdePkg/Library/BaseRngLib/Rand/RdRand.c b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
new file mode 100644
index 000000000000..09fb875ac3f9
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/Rand/RdRand.c
@@ -0,0 +1,131 @@
+/** @file
+ Random number generator services that uses RdRand instruction access
+ to provide high-quality random numbers.
+
+Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+
+#include "BaseRngLibInternals.h"
+
+//
+// Bit mask used to determine if RdRand instruction is supported.
+//
+#define RDRAND_MASK BIT30
+
+
+STATIC BOOLEAN mRdRandSupported;
+
+/**
+ The constructor function checks whether or not RDRAND instruction is supported
+ by the host hardware.
+
+ The constructor function checks whether or not RDRAND instruction is supported.
+ It will ASSERT() if RDRAND instruction is not supported.
+ It will always return EFI_SUCCESS.
+
+ @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+BaseRngLibConstructor (
+ VOID
+ )
+{
+ UINT32 RegEcx;
+
+ //
+ // Determine RDRAND support by examining bit 30 of the ECX register returned by
+ // CPUID. A value of 1 indicates that processor support RDRAND instruction.
+ //
+ AsmCpuid (1, 0, 0, &RegEcx, 0);
+ ASSERT ((RegEcx & RDRAND_MASK) == RDRAND_MASK);
+
+ mRdRandSupported = ((RegEcx & RDRAND_MASK) == RDRAND_MASK);
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Generates a 16-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber16 (
+ OUT UINT16 *Rand
+ )
+{
+ return AsmRdRand16 (Rand);
+}
+
+/**
+ Generates a 32-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber32 (
+ OUT UINT32 *Rand
+ )
+{
+ return AsmRdRand32 (Rand);
+}
+
+/**
+ Generates a 64-bit random number.
+
+ @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+ @retval TRUE Random number generated successfully.
+ @retval FALSE Failed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+ArchGetRandomNumber64 (
+ OUT UINT64 *Rand
+ )
+{
+ return AsmRdRand64 (Rand);
+}
+
+/**
+ Checks whether RDRAND is supported.
+
+ @retval TRUE RDRAND is supported.
+ @retval FALSE RDRAND is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+ArchIsRngSupported (
+ VOID
+ )
+{
+ /*
+ Existing software depends on this always returning TRUE, so for
+ now hard-code it.
+
+ return mRdRandSupported;
+ */
+ return TRUE;
+}
diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S b/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S
new file mode 100644
index 000000000000..82a00d362212
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S
@@ -0,0 +1,31 @@
+#------------------------------------------------------------------------------
+#
+# ArmReadIdIsar0() for AArch64
+#
+# Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#------------------------------------------------------------------------------
+
+.text
+.p2align 2
+GCC_ASM_EXPORT(ArmReadIdIsar0)
+
+#/**
+# Reads the ID_AA64ISAR0 Register.
+#
+# @return The contents of the ID_AA64ISAR0 register.
+#
+#**/
+#UINT64
+#EFIAPI
+#ArmReadIdIsar0 (
+# VOID
+# );
+#
+ASM_PFX(ArmReadIdIsar0):
+ mrs x0, id_aa64isar0_el1 // Read ID_AA64ISAR0 Register
+ ret
+
+
diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm b/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm
new file mode 100644
index 000000000000..1d9f9a808c0c
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm
@@ -0,0 +1,30 @@
+;------------------------------------------------------------------------------
+;
+; ArmReadIdIsar0() for AArch64
+;
+; Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+;
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;------------------------------------------------------------------------------
+
+ EXPORT ArmReadIdIsar0
+ AREA BaseLib_LowLevel, CODE, READONLY
+
+;/**
+; Reads the ID_AA64ISAR0 Register.
+;
+; @return The contents of the ID_AA64ISAR0 register.
+;
+;**/
+;UINT64
+;EFIAPI
+;ArmReadIdIsar0 (
+; VOID
+; );
+;
+ArmReadIdIsar0
+ mrs x0, id_aa64isar0_el1 // Read ID_AA64ISAR0 Register
+ ret
+
+ END
diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmRng.S b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.S
new file mode 100644
index 000000000000..4b9898dadc52
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.S
@@ -0,0 +1,61 @@
+#------------------------------------------------------------------------------
+#
+# ArmRndr() and ArmRndrrs() for AArch64
+#
+# Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#------------------------------------------------------------------------------
+
+#include "BaseRngLibInternals.h"
+
+.text
+.p2align 2
+GCC_ASM_EXPORT(ArmRndr)
+GCC_ASM_EXPORT(ArmRndrrs)
+
+#/**
+# Generates a random number using RNDR.
+# Returns TRUE on success; FALSE on failure.
+#
+# @param[out] Rand Buffer pointer to store the 64-bit random value.
+#
+# @retval TRUE Random number generated successfully.
+# @retval FALSE Failed to generate the random number.
+#
+#**/
+#BOOLEAN
+#EFIAPI
+#ArmRndr (
+# OUT UINT64 *Rand
+# );
+#
+ASM_PFX(ArmRndr):
+ mrs x1, RNDR
+ str x1, [x0]
+ cset x0, ne // RNDR sets NZCV to 0b0100 on failure
+ ret
+
+
+#/**
+# Generates a random number using RNDRRS
+# Returns TRUE on success; FALSE on failure.
+#
+# @param[out] Rand Buffer pointer to store the 64-bit random value.
+#
+# @retval TRUE Random number generated successfully.
+# @retval FALSE Failed to generate the random number.
+#
+#**/
+#BOOLEAN
+#EFIAPI
+#ArmRndrrs (
+# OUT UINT64 *Rand
+# );
+#
+ASM_PFX(ArmRndrrs):
+ mrs x1, RNDRRS
+ str x1, [x0]
+ cset x0, ne // RNDRRS sets NZCV to 0b0100 on failure
+ ret
diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm
new file mode 100644
index 000000000000..e3feb56adbcf
--- /dev/null
+++ b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm
@@ -0,0 +1,64 @@
+;------------------------------------------------------------------------------
+;
+; ArmRndr() and ArmRndrrs() for AArch64
+;
+; Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
+;
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;------------------------------------------------------------------------------
+
+#include "BaseRngLibInternals.h"
+
+ EXPORT ArmRndr
+ EXPORT ArmRndrrs
+ AREA BaseLib_LowLevel, CODE, READONLY
+
+
+;/**
+; Generates a random number using RNDR.
+; Returns TRUE on success; FALSE on failure.
+;
+; @param[out] Rand Buffer pointer to store the 64-bit random value.
+;
+; @retval TRUE Random number generated successfully.
+; @retval FALSE Failed to generate the random number.
+;
+;**/
+;BOOLEAN
+;EFIAPI
+;ArmRndr (
+; OUT UINT64 *Rand
+; );
+;
+ArmRndr
+ mrs x1, RNDR
+ str x1, [x0]
+ cset x0, ne // RNDR sets NZCV to 0b0100 on failure
+ ret
+
+ END
+
+;/**
+; Generates a random number using RNDRRS.
+; Returns TRUE on success; FALSE on failure.
+;
+; @param[out] Rand Buffer pointer to store the 64-bit random value.
+;
+; @retval TRUE Random number generated successfully.
+; @retval FALSE Failed to generate the random number.
+;
+;**/
+;BOOLEAN
+;EFIAPI
+;ArmRndrrs (
+; OUT UINT64 *Rand
+; );
+;
+ArmRndrrs
+ mrs x1, RNDRRS
+ str x1, [x0]
+ cset x0, ne // RNDRRS sets NZCV to 0b0100 on failure
+ ret
+
+ END
diff --git a/MdePkg/Library/BaseRngLib/BaseRngLib.uni b/MdePkg/Library/BaseRngLib/BaseRngLib.uni
index f3ed954c5209..de5d4f9dd869 100644
--- a/MdePkg/Library/BaseRngLib/BaseRngLib.uni
+++ b/MdePkg/Library/BaseRngLib/BaseRngLib.uni
@@ -1,8 +1,8 @@
// /** @file
// Instance of RNG (Random Number Generator) Library.
//
-// BaseRng Library that uses CPU RdRand instruction access to provide
-// high-quality random numbers.
+// BaseRng Library that uses CPU RNG instructions to provide
+// random numbers.
//
// Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
//
@@ -13,5 +13,5 @@

#string STR_MODULE_ABSTRACT #language en-US "Instance of RNG Library"

-#string STR_MODULE_DESCRIPTION #language en-US "BaseRng Library that uses CPU RdRand instruction access to provide high-quality random numbers"
+#string STR_MODULE_DESCRIPTION #language en-US "BaseRng Library that uses CPU RNG instructions to provide random numbers"

--
2.26.2


[PATCH v2 0/2] MdePkg,SecurityPkg: Add support to RngDxe and BaseRngLib for AARCH64 RNDR

Rebecca Cran <rebecca@...>
 

Update MdePkg BaseRngLib and SecurityPkg RngDxe to add support for
the AARCH64 RNDR instruction.

Changes from v1 to v2:

o Added a PCD, gEfiSecurityPkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm to
specify which algorighm the platform supports.
o Moved ArmRndr() and ArmRndrrs() into BaseRngLib.
o Added Doxygen headers.
o Merged X64 and AARCH64 *GetBytes functions into a single RngGetBytes
function.
o Updated constructors to return EFI_STATUS instead of RETURN_STATUS.
o Added ArchIsRngSupported function that gets called before each call to
ArchGetRandomNumber*.

Rebecca Cran (2):
MdePkg/BaseRngLib: Add support for ARMv8.5 RNG instructions
SecurityPkg: Add support for RngDxe on AARCH64

MdePkg/MdePkg.dec | 9 +-
SecurityPkg/SecurityPkg.dec | 2 +
MdePkg/MdePkg.dsc | 4 +-
SecurityPkg/SecurityPkg.dsc | 11 +-
MdePkg/Library/BaseRngLib/BaseRngLib.inf | 23 ++-
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf | 24 ++-
MdePkg/Library/BaseRngLib/AArch64/ArmRng.h | 59 +++++++
MdePkg/Library/BaseRngLib/BaseRngLibInternals.h | 79 +++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.h | 0
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.h | 17 --
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h | 117 ++++++++++++++
MdePkg/Library/BaseRngLib/AArch64/Rndr.c | 139 ++++++++++++++++
MdePkg/Library/BaseRngLib/BaseRng.c | 87 +++++-----
MdePkg/Library/BaseRngLib/Rand/RdRand.c | 131 +++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c | 127 +++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.c | 0
SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.c | 45 +-----
SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c | 146 +++++++++++++++++
SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.c | 170 ++++++++------------
MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S | 31 ++++
MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm | 30 ++++
MdePkg/Library/BaseRngLib/AArch64/ArmRng.S | 61 +++++++
MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm | 64 ++++++++
MdePkg/Library/BaseRngLib/BaseRngLib.uni | 6 +-
24 files changed, 1152 insertions(+), 230 deletions(-)
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmRng.h
create mode 100644 MdePkg/Library/BaseRngLib/BaseRngLibInternals.h
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.h (100%)
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.h (72%)
create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/RngDxeInternals.h
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/Rndr.c
create mode 100644 MdePkg/Library/BaseRngLib/Rand/RdRand.c
create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/AArch64/RngDxe.c
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/AesCore.c (100%)
rename SecurityPkg/RandomNumberGenerator/RngDxe/{ => Rand}/RdRand.c (71%)
create mode 100644 SecurityPkg/RandomNumberGenerator/RngDxe/Rand/RngDxe.c
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmRng.S
create mode 100644 MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm

--
2.26.2


Re: [PATCH RFC v2 09/28] OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase

Brijesh Singh
 

On 5/6/21 9:08 AM, Laszlo Ersek wrote:
On 04/30/21 13:51, Brijesh Singh wrote:
BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C01d3e5c5268043c18bdf08d910987251%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637559069206222390%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=CN2hZrjsKzfSqMAxcQLtoHTUqBOlZmdDEO9vY9XT%2FTQ%3D&amp;reserved=0

Commit 85b8eac59b8c5bd9c7eb9afdb64357ce1aa2e803 added support to ensure
that MMIO is only performed against the un-encrypted memory. If MMIO
is performed against encrypted memory, a #GP is raised.

The VmgExitLib library depends on ApicTimerLib to get the APIC base
address so that it can exclude the APIC range from the un-encrypted
check. The OvmfPkg provides ApicTimerLib for the DXE phase. The
constructor AcpiTimerLibConstructor() used in the ApicTimerLib uses
the PciRead to get the PMBASE register. The PciRead() will cause an
MMIO access.

The AmdSevDxe driver clears the memory encryption attribute from the
MMIO ranges. However, if VmgExitLib is linked to AmdSevDxe driver then the
AcpiTimerLibConstructor() will be called before AmdSevDxe driver can
clear the encryption attributes for the MMIO regions.

Exclude the PMBASE register from the encrypted check so that we
can link VmgExitLib to the MemEncryptSevLib; which gets linked to
AmdSevDxe driver.
The above explanation is inexact. There are several typos ("APIC" is
incorrect, "ACPI" would be correct, for the TimerLib instance in
question), but that's really just a side observation.

The precise explanation is the following library instance dependency
chain:

OvmfPkg/AmdSevDxe/AmdSevDxe.inf
-----> MemEncryptSevLib class
-----> "OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf" instance
-[*]-> VmgExitLib class
-----> "OvmfPkg/Library/VmgExitLib/VmgExitLib.inf" instance
-----> LocalApicLib class
-----> "UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf" instance
-----> TimerLib class
-----> "OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf" instance
-----> PciLib class
-----> "OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf" instance
-----> PciExpressLib class
-----> "MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf" instance

The link (or dependency) marked with [*] is introduced in patch #26
("OvmfPkg/MemEncryptSevLib: Change the page state in the RMP table").
That's the change that triggers the symptom. (In combination with you
testing on Q35, because on i440fx, the DxePciLibI440FxQ35 lib instance
accesses PCI config space via the 0xCF8, 0xCFC IO Ports, and those are
unaffected by SEV-ES.)

The symptom is somewhat "unjustified", because at the end of the series,
the AmdSevDxe driver makes no calls to actual TimerLib APIs (I checked
-- I disassembled the "AmdSevDxe.debug" file with "objdump -S", and
there is no call to any API declared in the "TimerLib.h" class header).
However, the ECAM (MMCONFIG) access is still triggered, because the
AcpiTimerLibConstructor() function, in
"OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c", is the constructor for
the "OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf" instance, and
AcpiTimerLibConstructor() calls PciRead32().

If you check the "OvmfPkg/OvmfPkgX64.dsc" file, you'll find that the
PciLib class is resolved to
"MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf" by default, and to
"OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf" for the
following module types:

- DXE_DRIVER,
- DXE_RUNTIME_DRIVER,
- SMM_CORE,
- DXE_SMM_DRIVER,
- UEFI_DRIVER,
- UEFI_APPLICATION.

The consequence is that modules strictly after the DXE_CORE get
dynamically enabled extended config space access (ECAM) on Q35 via the
PciLib class, whereas all modules strictly before the DXE_CORE, and the
DXE_CORE itself, are restricted to normal config space (IO Ports 0xCF8 /
0xCFC) via the PciLib class.

AmdSevDxe is a DXE_DRIVER, so it used to get DxePciLibI440FxQ35 as well.

The solution should be simple. In the AmdSevDxe driver specifically, we
need no access to extended PCI config space. Accessing normal PCI config
space, via IO Ports 0xCF8 / 0xCFC, should suffice. That can be achieved
with the following module-scope override:
Thanks Laszlo, I was not aware of the module-scope override. I will go
with this approach and make sure it works after the inclusion of the
VmgExitLib.



diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 8d9a0a077601..45a02b236633 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -966,7 +966,10 @@ [Components]
!endif

OvmfPkg/PlatformDxe/Platform.inf
- OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+ OvmfPkg/AmdSevDxe/AmdSevDxe.inf {
+ <LibraryClasses>
+ PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
+ }
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf
(

For consistency, all DSC files that include
"OvmfPkg/AmdSevDxe/AmdSevDxe.inf" should be modified similarly:

- OvmfPkg/AmdSev/AmdSevX64.dsc
- OvmfPkg/Bhyve/BhyveX64.dsc
- OvmfPkg/OvmfPkgIa32X64.dsc
- OvmfPkg/OvmfPkgX64.dsc
- OvmfPkg/OvmfXen.dsc

)

Therefore, please try dropping this patch, and modifying patch#26
instead -- the above module-scope override (for 5 DSC files) should be
squashed into patch#26, *and* the explanation I provided above should be
included in the commit message of patch#26.

... Correction: you have an independent bug in the series that affects
my above analysis. Namely, you *seem* to add the VmgExitLib dependency
to the "OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf"
library instance, in patch#26. That's where you modify the INF file. But
that's wrong: in patch#21 ("OvmfPkg/MemEncryptSevLib: Add support to
validate system RAM"), you add a VmgInit() call to the same library
instance, via the new file
"OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c".

The bug in that patch is clear from the fact that you introduce an
#include <Library/VmgExitLib.h> directive, but that's not mirrored by an
appropriate [LibraryClasses] change in the "DxeMemEncryptSevLib.inf"
file. (The other two lib instance INF files, "SecMemEncryptSevLib.inf"
and "PeiMemEncryptSevLib.inf" *are* modified as needed.)

So you even need to move some stuff from patch#26 to patch#21, and
*then* squash the above module-scope override (and explanation) into
patch#21.

A significant amount of work is needed on this series. I'll stop
reviewing RFC v2 here, because I don't want to look at the remaining
patches deeply as long as code movements etc are going to affect them.
Please post the next version -- assuming no other reviewer would like to
finish reviewing this version first!

Sounds good. What's your thought if I take out patch 1 - 9 from this RFC
series and submit them as non-RFC for the further review and acceptance
? The patch# 1-9 are basically prepatch before we get into SNP specific
bits.


Thanks
Laszlo

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 4 ++
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 7 +++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 45 ++++++++++++++++++++
3 files changed, 56 insertions(+)

diff --git a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
index e6f6ea7972..22435a0590 100644
--- a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
@@ -27,6 +27,7 @@
SecVmgExitVcHandler.c

[Packages]
+ MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
UefiCpuPkg/UefiCpuPkg.dec
@@ -42,4 +43,7 @@
[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress

+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
index c66c68726c..d3175c260e 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
@@ -27,6 +27,7 @@
PeiDxeVmgExitVcHandler.c

[Packages]
+ MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
UefiCpuPkg/UefiCpuPkg.dec
@@ -37,4 +38,10 @@
DebugLib
LocalApicLib
MemEncryptSevLib
+ PcdLib

+[FixedPcd]
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+
+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 24259060fd..01ac5d8c19 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -14,7 +14,10 @@
#include <Library/VmgExitLib.h>
#include <Register/Amd/Msr.h>
#include <Register/Intel/Cpuid.h>
+#include <IndustryStandard/Q35MchIch9.h>
+#include <IndustryStandard/I440FxPiix4.h>
#include <IndustryStandard/InstructionParsing.h>
+#include <Library/PcdLib.h>

#include "VmgExitVcHandler.h"

@@ -596,6 +599,40 @@ UnsupportedExit (
return Status;
}

+STATIC
+BOOLEAN
+IsPmbaBaseAddress (
+ IN UINTN Address
+ )
+{
+ UINT16 HostBridgeDevId;
+ UINTN Pmba;
+
+ //
+ // Query Host Bridge DID to determine platform type
+ //
+ HostBridgeDevId = PcdGet16 (PcdOvmfHostBridgePciDevId);
+ switch (HostBridgeDevId) {
+ case INTEL_82441_DEVICE_ID:
+ Pmba = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA);
+ break;
+ case INTEL_Q35_MCH_DEVICE_ID:
+ Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);
+ //
+ // Add the MMCONFIG base address to get the Pmba base access address
+ //
+ Pmba += FixedPcdGet64 (PcdPciExpressBaseAddress);
+ break;
+ default:
+ return FALSE;
+ }
+
+ // Round up the offset to page size
+ Pmba = Pmba & ~(SIZE_4KB - 1);
+
+ return (Address == Pmba);
+}
+
/**
Validate that the MMIO memory access is not to encrypted memory.

@@ -640,6 +677,14 @@ ValidateMmioMemory (
return 0;
}

+ //
+ // Allow PMBASE accesses (which will have the encryption bit set before
+ // AmdSevDxe runs in the DXE phase)
+ //
+ if (IsPmbaBaseAddress (Address)) {
+ return 0;
+ }
+
//
// Any state other than unencrypted is an error, issue a #GP.
//

17461 - 17480 of 92219