[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@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
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


Laszlo Ersek
 

On 05/07/21 22:38, Brijesh Singh wrote:
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@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
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.
(1) Typo: should be "did not make".

+//
+#define PVALIDATE_RET_NO_RMPUPDATE 255
+
+/**
+ Execute a PVALIDATE instruction to validate or rescinds validation of a guest
(2) should be "to validate or to rescind validation" (infinitive form).

+ 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.
(3) If you use the imperative for "validate", then "rescinds"
(indicative) reads strangely.

+ @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.
(4) @retval is only usable with actual return values (constants). If you
provide a natural language explanation, then @return is the proper
doxygen directive.

You can combine these BTW, for example:

@retval PVALIDATE_RET_SUCCESS The PVALIDATE instruction
succeeded, and updated the RMP
entry.
@retval PVALIDATE_RET_NO_RMPUPDATE The PVALIDATE instruction
succeeded, but did not update the
RMP entry.
@return Failure codes from the PVALIDATE
instruction.

+**/
+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
Thanks for filing the NASM BZ!

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
+; )
(5) This prototype does not match the one from the header file.

I guess it's reasonable to replace the enum type and the BOOLEAN type
with UINT32, in the assembly source code. But then I don't understand
why PHYSICAL_ADDRESS is not replaced with UINT64 -- that would only be
consistent with the other replacements.

Furthermore, the parameter *names* PageSize and RmpPageSize do not match.


+;-----------------------------------------------------------------------------
+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
This looks OK. I'm not very used to reading assembly, so "setc" (set if
carry) instead of the equivalent "setb" (set if below) would have been
easier to parse.

Similarly, "je" (jump if equal) would be easier to read than the
equivalent "jz" (jump if zero), especially after using "jne" (and not
"jnz") with the previous "cmp".

But, the assembly does look correct to me, so there's no need to change it.

Thanks!
Laszlo


Brijesh Singh
 

On 5/11/21 5:29 AM, Laszlo Ersek wrote:
On 05/07/21 22:38, 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%7Cc5e03d8f093c430d01b608d91467bc91%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563258060234952%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=QFATwHj5Jtoughp2bLTtFChUuKCWzQBSOGcZb2I%2B534%3D&amp;reserved=0

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@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
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.
(1) Typo: should be "did not make".
Noted.

+//
+#define PVALIDATE_RET_NO_RMPUPDATE 255
+
+/**
+ Execute a PVALIDATE instruction to validate or rescinds validation of a guest
(2) should be "to validate or to rescind validation" (infinitive form).
Noted.

+ 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.
(3) If you use the imperative for "validate", then "rescinds"
(indicative) reads strangely.
I will use validate or invalidate in future patches.

+ @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.
(4) @retval is only usable with actual return values (constants). If you
provide a natural language explanation, then @return is the proper
doxygen directive.

You can combine these BTW, for example:

@retval PVALIDATE_RET_SUCCESS The PVALIDATE instruction
succeeded, and updated the RMP
entry.
@retval PVALIDATE_RET_NO_RMPUPDATE The PVALIDATE instruction
succeeded, but did not update the
RMP entry.
@return Failure codes from the PVALIDATE
instruction.
Will do.


+**/
+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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.nasm.us%2Fshow_bug.cgi%3Fid%3D3392753&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc5e03d8f093c430d01b608d91467bc91%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563258060234952%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Sk1vgmvOHjp2VVlONWIqDMZnb5nfV5%2Fl9pegv9AQgmg%3D&amp;reserved=0
+;
+%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
Thanks for filing the NASM BZ!

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
+; )
(5) This prototype does not match the one from the header file.

I guess it's reasonable to replace the enum type and the BOOLEAN type
with UINT32, in the assembly source code. But then I don't understand
why PHYSICAL_ADDRESS is not replaced with UINT64 -- that would only be
consistent with the other replacements.

Furthermore, the parameter *names* PageSize and RmpPageSize do not match.
Will fix in next rev.
+;-----------------------------------------------------------------------------
+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
This looks OK. I'm not very used to reading assembly, so "setc" (set if
carry) instead of the equivalent "setb" (set if below) would have been
easier to parse.

Similarly, "je" (jump if equal) would be easier to read than the
equivalent "jz" (jump if zero), especially after using "jne" (and not
"jnz") with the previous "cmp".

But, the assembly does look correct to me, so there's no need to change it.

Thanks!
Laszlo