On 05/07/21 22:38, Brijesh Singh wrote:
> From: Tom 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
> Cc: Min Xu
> Cc: Jiewen Yao
> Cc: Tom Lendacky
> Cc: Jordan Justen
> Cc: Ard Biesheuvel
> Cc: Laszlo Ersek
> Cc: Erdem Aktas
> Cc: Michael D Kinney
> Cc: Liming Gao
> Cc: Zhiguang Liu
> Signed-off-by: Tom Lendacky
> Signed-off-by: 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_ADDRESSAddress
>);
> +
> +//
> +// 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.
(1) trivial typo: IMO it should be "on X64" (preposition missing, and
X64 should be upper case).
> +
> + 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
(2) Not a "hard requirement", just something I thought I'd raise: both
the spec, and the leading comment (twice), say that the return code is
in EAX (not RAX). Would it make sense to use UINT32 for the return type
of the function?
(3) Since we are talking return codes... For PVALIDATE, the previous
patch introduces macros for the return codes. I haven't looked at
RMPADJUST before, but now it seems like SEV-ES-related machine
instructions come with a "global" status code table: 0 for SUCCESS, 1
for FAIL_INPUT, 6 for FAIL_SIZEMISMATCH (<-- all of these are shared by
PVALIDATE and RMPADJUST), and now FAIL_PERMISSION (2) for RMPADJUST only.
So now I wonder if these macros belong in an AMD-specific header file...
Anyway, I definitely defer to Liming and Mike on this MdePkg content.
> +EFIAPI
> +AsmRmpAdjust (
> + IN UINTN Rax,
> + IN UINTN Rcx,
> + IN UINTN Rdx
> + );
(4) Given that we really call these R*x, shouldn't we make them
explicitly UINT64? I don't think there's an interpretation for RAX that
is *not* 64-bit.
> #endif
>
> -
(5) Indeed, this newline is superfluous, I just didn't want to obsess
about it under patch #7. If you agree it's unneeded, then please drop it
from patch#7.
> #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/
