Date   

Re: [PATCH RFC v2 05/28] MdePkg: Add AsmPvalidate() support

Brijesh Singh
 

On 5/4/21 2:07 PM, Brijesh Singh via groups.io wrote:
Return EFI_UNSUPPORTED (0x8000_0003), or even EFI_NO_MAPPING
(0x8000_0017), for value 6 (FAIL_SIZEMISMATCH).
I am not sure if we really want to do this. You will see later in the
patches that in some cases the PVALIDATE will return a failure and we
will need to know the failure code to determine the next steps.
Especially this particular error code is used later. This error happens
when the page size of the backing pages does not match with the
pvalidated size. In those cases we need to retry the PVALIDATE with
lower page size so that a validation succeed. One such a example is:

- Guest ask hypervisor to add the page as 2M in RMP table.

- Hypervisor added the page as 512 4K pages - because it was not able to
find a large backing pages.

- Guest attempts to pvalidate the page as a 2M. The pvalidate will
return a failure saying its a size mismatch between the requested
pvalidated and RMP table. The recommendation is that guest should try
with a smaller page size.

I would prefer to pass the pvalidate error as-is to caller so that it
can make the correct decision.
I am perfectly fine if the function return UINTN and then use #define
instead of the enum to define the PVALIDATE return code. So that caller
can check the error code. Let me know your thought on #define instead of
the enum.

-Brijesh


Re: No ShellBinPkg with newer releases on github

Laszlo Ersek
 

On 05/03/21 21:35, Rebecca Cran wrote:
I noticed the ShellBinPkg zip file has stopped being uploaded to
https://github.com/tianocore/edk2/releases for newer tags: the most
recent release that has it is edk2-stable202002.

Was that deliberate? I can easily build it myself, but was wondering
since I don't recall seeing any messages about it _not_ being generated
any more.
https://bugzilla.tianocore.org/show_bug.cgi?id=3178

(At this point, the title of that bug should be updated so it say
"edk2-stable202105".)

Laszlo


Re: Booting UEFI with QEMU arm

Laszlo Ersek
 

On 05/04/21 13:32, Pintu Agarwal wrote:
Hi,
Is it possible to boot UEFI on a qemu arm based emulator?
Since I am working from home and my uefi related work leads to device crash
many times, which becomes painful for me.

So I wanted to do all my experiments on a qemu environment before I verify
the final changes on a real hardware.
So, I am just wondering is this possible?

According to my search I see its possible but still I could not find a
proper way to do it.
Please help me if someone has already tried it.

I already have the qemu-arm setup on my Linux/Ubuntu PC.
From "qemu-system-arm", it seems like you really mean 32-bit ARM.

Yes, there is support for that in edk2.

Build:

$ git submodule update --init --force
$ . edksetup.sh
$ nice make -C "$EDK_TOOLS_PATH" -j $(getconf _NPROCESSORS_ONLN)
$ build -a ARM -b DEBUG -p ArmVirtPkg/ArmVirtQemu.dsc -t GCC5
$ for FD in QEMU_EFI.fd QEMU_VARS.fd; do
cat -- \
"$WORKSPACE/Build/ArmVirtQemu-ARM/DEBUG_GCC5/FV/$FD" \
/dev/zero \
| head -c 64m >| /tmp/"$FD.padded"
done

Run:

$ if ! [ -e vars.fd ]; then
cp /tmp/QEMU_VARS.fd.padded vars.fd
fi
$ qemu-system-arm \
-M virt \
-cpu cortex-a15 \
-m 1024 \
-serial stdio \
-drive if=pflash,unit=0,format=raw,readonly=on,file=/tmp/QEMU_EFI.fd.padded \
-drive if=pflash,unit=1,format=raw,file=vars.fd \
-device virtio-gpu-pci \
-device id=xhci0,driver=qemu-xhci \
-device usb-kbd,bus=xhci0.0 \
-boot menu=on,splash-time=5000 \
[...]

HTH
Laszlo



Thanks,
Pintu






Re: [PATCH RFC v2 05/28] MdePkg: Add AsmPvalidate() support

Brijesh Singh
 

On 5/4/21 8:58 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%7C6ceeec6c984d468bb87908d90f04b789%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557335220626621%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=jF44vk%2FBUVukpuNg4vrcRqHnEa7nO%2FGPEe9ti720cnE%3D&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>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
MdePkg/Include/Library/BaseLib.h | 37 +++++++++++++++++
MdePkg/Library/BaseLib/BaseLib.inf | 1 +
MdePkg/Library/BaseLib/X64/Pvalidate.nasm | 43 ++++++++++++++++++++
3 files changed, 81 insertions(+)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 7253997a6f..92ce695e93 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -7518,5 +7518,42 @@ PatchInstructionX86 (
IN UINTN ValueSize
);

+/**
+ Execute a PVALIDATE instruction to validate or rescnids validation of a guest
(1) typo: "rescnids"
Noted.




+ page's RMP entry.
+
+ Upon completion, in addition to the return value the instruction also updates
+ the eFlags. A caller must check both the return code as well as eFlags to
+ determine if the RMP entry has been updated.
+
+ The function is available on x64.
(2) Please write "X64"; that's how the architecture is usually mentioned
in both the UEFI spec and in edk2.
Noted.




+
+ @param[in] Address The guest virtual address to validate.
+ @param[in] PageSize The page size to use.
+ @param[i] Validate Validate or rescinds.
+ @param[out] Eflags The value of Eflags after PVALIDATE completion.
(3) Typo: "[i]" should be "[in]".
Noted.




(4) The order of parameters listed in this comment block differs from
the actual parameter list.

The ECC plugin of the edk2 CI will catch this issue anyway. So, before
submitting the patch set to the list, please submit a personal PR on
github.com against the main repo, just to run CI on your patches.
Interestingly I did ran the series with edk2 CI and everything passed
before I submitted for the review. But, I will go ahead and fix the order.



+
+ @retval PvalidateRetValue The return value from the PVALIDATE instruction.
More on the return value / type later, below.

+**/
+typedef enum {
+ PvalidatePageSize4K = 0,
+ PvalidatePageSize2MB,
+} PVALIDATE_PAGE_SIZE;
+
+typedef enum {
+ PvalidateRetSuccess = 0,
+ PvalidateRetFailInput = 1,
+ PvalidateRetFailSizemismatch = 6,
+} PVALIDATE_RET_VALUE;
+
(5) These typedefs do not belong between the function leading comment
and the function declaration. Please hoist the typedefs just above the
leading comment, and add a separate comment for the typedefs -- using
the proper comment style for typedefs, too.


+PVALIDATE_RET_VALUE
(6) In my opinion, using an enum for an EFIAPI function's return type is
problematic. According to the UEFI spec (v2.9), "Table 2-3 Common UEFI
Data Types", <Enumerated Type> may correspond to INT32 or UINT32. I
don't like that ambiguity here. The spec also says that such types
should never be used at least as structure fields.

I'm perfectly fine with functions in standard (ISO) C programs returning
enums, but I think the situation is less clear in UEFI. I don't recall
standard interfaces (spec-level, or even edk2 / MdePkg interfaces) that
return enums.

I suggest the following instead. Drop the PVALIDATE_RET_VALUE enum
altogether. Specify EFI_STATUS as the return type, in the declaration of
the function.
Works with me.



In the UEFI spec, Appendix D specifies the numeric values of the status
codes. Furthermore, there are examples for NASM sources using EFI_*
status codes *numerically* in edk2; minimally:

- IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
- IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32/SecEntry.nasm

Thus, please modify the assembly source code in this patch to return
EFI_SUCCESS (already value 0, conveniently) if the instruction succeeds.
Works with me.


Return EFI_INVALID_PARAMETER (0x8000_0002) in case the instruction fails
with error code 1 (FAIL_INPUT).
Works with me.



Return EFI_UNSUPPORTED (0x8000_0003), or even EFI_NO_MAPPING
(0x8000_0017), for value 6 (FAIL_SIZEMISMATCH).
I am not sure if we really want to do this. You will see later in the
patches that in some cases the PVALIDATE will return a failure and we
will need to know the failure code to determine the next steps.
Especially this particular error code is used later. This error happens
when the page size of the backing pages does not match with the
pvalidated size. In those cases we need to retry the PVALIDATE with
lower page size so that a validation succeed. One such a example is:

- Guest ask hypervisor to add the page as 2M in RMP table.

- Hypervisor added the page as 512 4K pages - because it was not able to
find a large backing pages.

- Guest attempts to pvalidate the page as a 2M. The pvalidate will
return a failure saying its a size mismatch between the requested
pvalidated and RMP table. The recommendation is that guest should try
with a smaller page size.

I would prefer to pass the pvalidate error as-is to caller so that it
can make the correct decision.



The leading comment block of the function is supposed to explain these
associations:

@retval EFI_SUCCESS Successful completion (regardless of
whether the Validated bit changed state).
@retval INVALID_PARAMETER Invalid input parameters (FAIL_INPUT).
@retval EFI_UNSUPPORTED Page size mismatch between guest (2M) and
RMP entry (4K) (FAIL_SIZEMISMATCH).

(Passing in the PVALIDATE_PAGE_SIZE enum, as a parameter, should be
fine, BTW)


(7) According to the AMD APM, "Support for this instruction is indicated
by CPUID Fn8000_001F_EAX[SNP]=1".

Presumably, if the (physical, or emulated) hardware does not support
PVALIDATE, an #UD is raised. That condition should be explained in the
function's leading comment. (Mention the CPUID and the #UD, I guess.)
Noted.

+EFIAPI
+AsmPvalidate (
+ IN PVALIDATE_PAGE_SIZE PageSize,
+ IN BOOLEAN Validate,
+ IN UINTN Address,
(8) This should be EFI_PHYSICAL_ADDRESS, not UINTN.
Noted.



+ OUT IA32_EFLAGS32 *Eflags
+ );
+
#endif // defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
#endif // !defined (__BASE_LIB__)
(9) Unless you foresee particular uses for eflags *other than* CF, I
would suggest replacing the Eflags output parameter with

OUT BOOLEAN *RmpEntryUpdated

The function would still only have 4 parameters, which shouldn't be
difficult to handle in the assembly implementation (i.e. write to the
UINT8 (= BOOLEAN) object referenced by "RmpEntryUpdated"). EFIAPI means
that the first four params are passed in RCX, RDX, R8, R9.
Works with me, thats what I ended up doing for the guest kernel patches
and will do the same for OVMF as well.

Thus far, I can see only one AsmPvalidate() call: in IssuePvalidate(),
from patch #21 ("OvmfPkg/MemEncryptSevLib: Add support to validate
system RAM"). And there, CF looks sufficient.


(10) The instruction is X64 only, but you are providing the declaration
even if MDE_CPU_IA32 is #defined. That seems wrong; even the declaration
should be invisible in that case. Please declare the function for
MDE_CPU_X64 only.
Noted.


diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index b76f3af380..d33b4a8f7d 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -321,6 +321,7 @@
X64/XGetBv.nasm
X64/XSetBv.nasm
X64/VmgExit.nasm
+ X64/Pvalidate.nasm
ChkStkGcc.c | GCC

[Sources.EBC]
(11) This list of source files is already not sorted alphabetically,
unfortunately. But we can still do better than this: I suggest inserting
"X64/Pvalidate.nasm" just before "X64/RdRand.nasm".
Noted.



(12) Your git setup seems less than ideal for formatting edk2 patches.
The @@ hunk header above does not show the INF file section being
modified. It should look something like this:

@@ -317,6 +317,7 @@ [Sources.X64]
^^^^^^^^^^^^^

Please run the "BaseTools/Scripts/SetupGit.py" script in your working
tree.
Will do.



Alternatively, please see "xfuncname" at
<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FLaszlo%2527s-unkempt-git-guide-for-edk2-contributors-and-maintainers%23contrib-05&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6ceeec6c984d468bb87908d90f04b789%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557335220626621%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=W16GsCzqxFZlH7DBOIFY0EJy%2FS6DTSCt0qEVOMLjjRs%3D&amp;reserved=0>.

This is of course not a bug in the patch, but fixing your setup will
help with the next round of review.


diff --git a/MdePkg/Library/BaseLib/X64/Pvalidate.nasm b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
new file mode 100644
index 0000000000..f2aba114ac
--- /dev/null
+++ b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
@@ -0,0 +1,43 @@
+;-----------------------------------------------------------------------------
+;
+; Copyright (c) 2020-2021, AMD. All rights reserved.<BR>
(13) I believe we don't introduce new files with copyright notices
referring to the past. IOW, I think you should only say "2021" here.
Noted.


+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+; Pvalidate.Asm
+;
+; Abstract:
+;
+; AsmPvalidate function
+;
+; Notes:
+;
(14) I defer to the MdePkg maintainers on this, but "Module Name" is
plain wrong, and the Abstract is useless. Either fix those up please
("Abstract" could be a copy of the corrected leading comment block), or
just drop them both.


+;-----------------------------------------------------------------------------
+
+ SECTION .text
+
+;-----------------------------------------------------------------------------
+; PvalidateRetValue
+; EFIAPI
+; AsmPvalidate (
+; IN UINT32 RmpPageSize
+; IN UINT32 Validate,
+; IN UINTN Address,
+; OUT UINTN *Eflags,
+; )
+;-----------------------------------------------------------------------------
(15) Please update this accordingly to the corrected function
specification.
Noted.



+global ASM_PFX(AsmPvalidate)
+ASM_PFX(AsmPvalidate):
+ mov rax, r8
+
+ ; PVALIDATE instruction opcode
+ DB 0xF2, 0x0F, 0x01, 0xFF
This is bad practice; we make every effort to avoid DB-encoded
instructions.

We have two PVALIDATE instances in the patch set (... that I can see
immediateyl); the first here, and the other in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm" (from patch #17,
"OvmfPkg/ResetVector: Invalidate the GHCB page"). Therefore, hiding the
encoding of PVALIDATE behind a NASM macro definitely makes sense.

(16a) Please file a NASM feature request for PVALIDATE at
<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.nasm.us%2F&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6ceeec6c984d468bb87908d90f04b789%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557335220636620%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=VtpkUnBunOdo3n86u3dBhoJZPxLN8M95ClyYUtC7v9o%3D&amp;reserved=0>.
Will do.



(16b) In the present MdePkg patch, please extend the file

MdePkg/Include/X64/Nasm.inc

as follows:

diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc
index 527f71e9eb4d..ff37f1e35707 100644
--- a/MdePkg/Include/X64/Nasm.inc
+++ b/MdePkg/Include/X64/Nasm.inc
@@ -33,6 +33,15 @@
DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8
%endmacro

+;
+; Macro for the PVALIDATE instruction, defined in AMD publication #24594
+; revision 3.32. NASM feature request URL:
+; <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.nasm.us%2Fshow_bug.cgi%3Fid%3DFIXME&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6ceeec6c984d468bb87908d90f04b789%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557335220636620%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=WeN3YmLN6OEpY8azwBt0ixogffkCCSk70P2Y%2Bff0Hlo%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
Sure, I will add the new macro and use it.


(16c) Please replace the FIXME placeholder above with the actual NASM BZ
number (from (16a)).

(16d) In the "MdePkg/Library/BaseLib/X64/Pvalidate.nasm" source file,
and also (later) in the "OvmfPkg/ResetVector/Ia32/PageTables64.asm"
source file, please use the PVALIDATE macro, in place of the naked DBs.


Back to your patch:

On 04/30/21 13:51, Brijesh Singh wrote:
+
+ ; Read the Eflags
+ pushfq
+ pop r8
+ mov [r9], r8
+
+ ; The PVALIDATE instruction returns the status in rax register.
+ ret
(17) The assembly code should be updated to match the new interface
contract (parameter order, parameter types, return values).
Sure, I will update based on your feedback.



I'll continue reviewing the series later this week (hopefully tomorrow).

thanks for all your review feedback.


Re: [PATCH RFC v2 04/28] MdePkg: Define the Page State Change VMGEXIT structures

Brijesh Singh
 

On 5/4/21 7:33 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%7Cbb2eaad10f574a464cb008d90ef8e5b4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557284470753463%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=fpm8afTb4UwW13njwD9WzHWIZZhjUCdmm3Vkt9GJ2SI%3D&amp;reserved=0

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@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>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 15 ++++++++++
MdePkg/Include/Register/Amd/Ghcb.h | 29 ++++++++++++++++++++
2 files changed, 44 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index e19bd04b6c..432cee2feb 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;
This matches section 2.3.1 in rev 2.00.

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
Matches section 2.3.1.

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 2d64a4c28f..1e7c0daed3 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
Matches "Table 5. List of Supported Non-Automatic Events".

@@ -160,4 +161,32 @@ 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
(1) Comment style.
Noted.



+#define SNP_PAGE_STATE_MAX_NPAGES 4095
+#define SNP_PAGE_STATE_MAX_ENTRY 253
+#define SNP_PAGE_STATE_PRIVATE 1
+#define SNP_PAGE_STATE_SHARED 2
+#define SNP_PAGE_STATE_PSMASH 3
+#define SNP_PAGE_STATE_UNSMASH 4
(2) The PSMASH and UNSMASH operations are not documented in the rev 2.00
spec, in the GHCB MSR protocol. That's probably because PSMASH and
UNSMASH can only be defined in terms of 2MB pages, and
GHCB_INFO_SNP_PAGE_STATE_CHANGE_REQUEST is suitable only for individual,
4KB pages. I think it would be useful to point out somehow here that
PSMASH and UNSMASH are restricted to the GHCB shared area protocol
(perhaps extend the leading comment on this block of macros).
I will add something in comment to clarify that UNMASH and PSMASH are
not available in MSR protocol.



(3) I don't understand what "MAX_NPAGES" stands for (4095). The rest of
the series never uses the macro, and I can't associate it with anything
from the spec. If the macro is supposed to relate to the 4KB / 2MB page
smashing / splitting, then its replacement text should be 512. Unless
the macro corresponds to a definition in the spec, I think we should
drop it.
That macro is replaced is no longer used, I ended up creating a more
meaningful macro (SNP_PAGES_STATE_MAX_ENTRY). I will remove it MAX_NPAGES.

+
+typedef PACKED struct {
+ UINT64 CurrentPage:12;
+ UINT64 GuestFrameNumber:40;
+ UINT64 Op:4;
+ UINT64 PageSize:1;
+ UINT64 Rsvd: 7;
+} SNP_PAGE_STATE_ENTRY;
+
+typedef PACKED struct {
+ UINT16 CurrentEntry;
+ UINT16 EndEntry;
+ UINT32 Rsvd;
+} SNP_PAGE_STATE_HEADER;
(4) We tend to write

#pragma pack (1)
...
#pragma pack ()

rather than PACKED -- but anyway, is packing really necessary? "Natural
alignment" is required in edk2. I'm OK with packing, but I think the
pragma is the preferred form.
Noted.


(5) Please spell out both "Rsvd" fields above as "Reserved".
Noted.



(6) Stray space character in "Rsvd: 7".
Noted.



(7) The field name "Op" is inconsistent with the other field name
"Operation".
Noted.



(8) I think there is a bug (typo) in the rev 2.00 spec, in 4.1.6 "SNP
Page State Change": it says

... calculated from the supplied guest physical frame number (GFN) for
the requested page size (GPA = GFN << 12).

But, if you can choose 2MB page size in the request, then the (GPA = GFN
<< 12) formula is not g
I think Tom already clarified it on his latest response.


(9) If my understanding of the spec is correct, "EndEntry" has
*inclusive* meaning. That's unusual. Any particular reason for not
making "EndEntry" exclusive (in the spec)?
Sometimes guest may need to fill only few entries. The "EndEntry" will
give hint to hypervisor that it should stop processing after it reached
to the EndEntry.


+
+typedef struct {
+ SNP_PAGE_STATE_HEADER Header;
+ SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
+} SNP_PAGE_STATE_CHANGE_INFO;
+
#endif
Yes, this looks OK. Size is 2+2+4+253*8 = 2032 bytes, which matches the
size of GHCB.SharedBuffer.

(10) However, *if* you decide to declare SNP_PAGE_STATE_ENTRY and
SNP_PAGE_STATE_HEADER explicitly as packed, then you should do the same
for SNP_PAGE_STATE_CHANGE_INFO.
Noted.



(11) Like I mentioned earlier, it's probably helpful if you start the
subject line with

MdePkg/Register/Amd: ...

on all of these MdePkg patches. If that becomes too tight, for some of
the MdePkg patches, then I suggest "MdePkg/Amd: ..." (i.e., drop
"Register").
Noted.

Thanks

Brijesh


Re: [PATCH RFC v2 04/28] MdePkg: Define the Page State Change VMGEXIT structures

Laszlo Ersek
 

On 05/04/21 16:48, Tom Lendacky wrote:


On 5/4/21 8:59 AM, Laszlo Ersek wrote:
On 05/04/21 14:33, 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%7Cthomas.lendacky%40amd.com%7Cf400bca14b6f4f138a1908d90f05090f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557336582189771%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=bjXNVWGpurRGUZkjemvDQR%2FYnEQRG9ENN22jUjtkNP0%3D&amp;reserved=0

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@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>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 15 ++++++++++
MdePkg/Include/Register/Amd/Ghcb.h | 29 ++++++++++++++++++++
2 files changed, 44 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index e19bd04b6c..432cee2feb 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;
This matches section 2.3.1 in rev 2.00.

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
Matches section 2.3.1.

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 2d64a4c28f..1e7c0daed3 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
Matches "Table 5. List of Supported Non-Automatic Events".

@@ -160,4 +161,32 @@ 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
(1) Comment style.

+#define SNP_PAGE_STATE_MAX_NPAGES 4095
+#define SNP_PAGE_STATE_MAX_ENTRY 253
+#define SNP_PAGE_STATE_PRIVATE 1
+#define SNP_PAGE_STATE_SHARED 2
+#define SNP_PAGE_STATE_PSMASH 3
+#define SNP_PAGE_STATE_UNSMASH 4
(2) The PSMASH and UNSMASH operations are not documented in the rev 2.00
spec, in the GHCB MSR protocol. That's probably because PSMASH and
UNSMASH can only be defined in terms of 2MB pages, and
GHCB_INFO_SNP_PAGE_STATE_CHANGE_REQUEST is suitable only for individual,
4KB pages. I think it would be useful to point out somehow here that
PSMASH and UNSMASH are restricted to the GHCB shared area protocol
(perhaps extend the leading comment on this block of macros).

(3) I don't understand what "MAX_NPAGES" stands for (4095). The rest of
the series never uses the macro, and I can't associate it with anything
from the spec. If the macro is supposed to relate to the 4KB / 2MB page
smashing / splitting, then its replacement text should be 512. Unless
the macro corresponds to a definition in the spec, I think we should
drop it.

+
+typedef PACKED struct {
+ UINT64 CurrentPage:12;
+ UINT64 GuestFrameNumber:40;
+ UINT64 Op:4;
+ UINT64 PageSize:1;
+ UINT64 Rsvd: 7;
+} SNP_PAGE_STATE_ENTRY;
+
+typedef PACKED struct {
+ UINT16 CurrentEntry;
+ UINT16 EndEntry;
+ UINT32 Rsvd;
+} SNP_PAGE_STATE_HEADER;
(4) We tend to write

#pragma pack (1)
...
#pragma pack ()

rather than PACKED -- but anyway, is packing really necessary? "Natural
alignment" is required in edk2. I'm OK with packing, but I think the
pragma is the preferred form.

(5) Please spell out both "Rsvd" fields above as "Reserved".

(6) Stray space character in "Rsvd: 7".

(7) The field name "Op" is inconsistent with the other field name
"Operation".

(8) I think there is a bug (typo) in the rev 2.00 spec, in 4.1.6 "SNP
Page State Change": it says

... calculated from the supplied guest physical frame number (GFN) for
the requested page size (GPA = GFN << 12).

But, if you can choose 2MB page size in the request, then the (GPA = GFN
<< 12) formula is not g
Sorry, unfinished sentence: I meant that the formula was not generally
correct.
Actually, for any page size, the GPA for any GFN is GFN << 12.

For the SNP Page State Change NAE event, it is up to the hypervisor to
ensure that the GFN/GPA supplied is aligned appropriately for the
requested page size (see 4.1.6 of the GHCB spec where the page operations
are defined). A GFN is naturally 4K aligned, so only a 2MB page size needs
GFN/GPA alignment validation.
Thanks. I guess I was confused by the term "for the requested page size".

For the SNP Page State Change MSR protocol, only a 4K page size is supported.
Yes, that's clear.

Thanks!
Laszlo


Re: [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

Laszlo Ersek
 

On 05/04/21 10:31, Sami Mujawar wrote:
Hi Laszlo,

On Thu, Apr 22, 2021 at 06:56 AM, Laszlo Ersek wrote:


5) "Cloud Hypervisor" is not something that I can justifiably spend
much time on. I'm willing to review this series at the level at which
I've reviewed (for example) XenPVH or Bhyve in the past, mainly focusing
on style and potential regressions. However, that's not enough for the
long term: someone from ARM (or elsewhere) will have to step up for
permanent reviewership. Please add a patch for extending
"Maintainers.txt" appropriately. Example subsystems:
I can help to review the 'Cloud Hypervisor' patches and will send out a patch to update the reviewership once the initial series is merged.
OK, thanks!


Hi Jainyong,

I could not find the remaining patches from your v1 series. Can you forward them to me, please?
Indeed, the v1 patches have not reached the list.

Thanks,
Laszlo


I can review and provide feedback so that they are addressed in your v2 series.

Regards,

Sami Mujawar


[PATCH v3 1/1] CryptoPkg: BaseCryptLib: Add RSA PSS verify support

Agrawal, Sachin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3314

This patch uses Openssl's EVP API's to perform RSASSA-PSS verification
of a binary blob.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>

Signed-off-by: Sachin Agrawal <sachin.agrawal@intel.com>
---

Notes:
v3:
- Fixed gcc compilation error [CI System]

CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c | 146 +++++++++++++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c | 46 +++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 169 +++++++++++++++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSignNull.c | 60 ++++++
CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssNull.c | 46 +++++
CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssSignNull.c | 60 ++++++
CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c | 66 +++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c | 1 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c | 191 ++++++++++++++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c | 2 +
CryptoPkg/Include/Library/BaseCryptLib.h | 74 ++++++++
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf | 2 +
CryptoPkg/Private/Protocol/Crypto.h | 78 ++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h | 3 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf | 1 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf | 1 +
21 files changed, 956 insertions(+)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
new file mode 100644
index 000000000000..af7cdafa4c47
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
@@ -0,0 +1,146 @@
+/** @file
+ RSA Asymmetric Cipher Wrapper Implementation over OpenSSL.
+
+ This file implements following APIs which provide basic capabilities for RSA:
+ 1) RsaPssVerify
+
+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+
+#include <openssl/bn.h>
+#include <openssl/rsa.h>
+#include <openssl/objects.h>
+#include <openssl/evp.h>
+
+
+/**
+ Retrieve a pointer to EVP message digest object.
+
+ @param[in] DigestLen Length of the message digest.
+
+**/
+STATIC
+const
+EVP_MD*
+GetEvpMD (
+ IN UINT16 DigestLen
+ )
+{
+ switch (DigestLen){
+ case SHA256_DIGEST_SIZE:
+ return EVP_sha256();
+ break;
+ case SHA384_DIGEST_SIZE:
+ return EVP_sha384();
+ break;
+ case SHA512_DIGEST_SIZE:
+ return EVP_sha512();
+ break;
+ default:
+ return NULL;
+ }
+}
+
+
+/**
+ Verifies the RSA signature with RSASSA-PSS signature scheme defined in RFC 8017.
+ Implementation determines salt length automatically from the signature encoding.
+ Mask generation function is the same as the message digest algorithm.
+ Salt length should atleast be equal to digest length.
+
+ @param[in] RsaContext Pointer to RSA context for signature verification.
+ @param[in] Message Pointer to octet message to be verified.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] Signature Pointer to RSASSA-PSS signature to be verified.
+ @param[in] SigSize Size of signature in bytes.
+ @param[in] DigestLen Length of digest for RSA operation.
+ @param[in] SaltLen Salt length for PSS encoding.
+
+ @retval TRUE Valid signature encoded in RSASSA-PSS.
+ @retval FALSE Invalid signature or invalid RSA context.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssVerify (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN CONST UINT8 *Signature,
+ IN UINTN SigSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen
+ )
+{
+ BOOLEAN Result;
+ EVP_PKEY *pEvpRsaKey = NULL;
+ EVP_MD_CTX *pEvpVerifyCtx = NULL;
+ EVP_PKEY_CTX *pKeyCtx = NULL;
+ CONST EVP_MD *HashAlg = NULL;
+
+ if (RsaContext == NULL) {
+ return FALSE;
+ }
+ if (Message == NULL || MsgSize == 0 || MsgSize > INT_MAX) {
+ return FALSE;
+ }
+ if (Signature == NULL || SigSize == 0 || SigSize > INT_MAX) {
+ return FALSE;
+ }
+ if (SaltLen < DigestLen) {
+ return FALSE;
+ }
+
+ HashAlg = GetEvpMD(DigestLen);
+
+ if (HashAlg == NULL) {
+ return FALSE;
+ }
+
+ pEvpRsaKey = EVP_PKEY_new();
+ if (pEvpRsaKey == NULL) {
+ goto _Exit;
+ }
+
+ EVP_PKEY_set1_RSA(pEvpRsaKey, RsaContext);
+
+ pEvpVerifyCtx = EVP_MD_CTX_create();
+ if (pEvpVerifyCtx == NULL) {
+ goto _Exit;
+ }
+
+ Result = EVP_DigestVerifyInit(pEvpVerifyCtx, &pKeyCtx, HashAlg, NULL, pEvpRsaKey) > 0;
+ if (pKeyCtx == NULL) {
+ goto _Exit;
+ }
+
+ if (Result) {
+ Result = EVP_PKEY_CTX_set_rsa_padding(pKeyCtx, RSA_PKCS1_PSS_PADDING) > 0;
+ }
+ if (Result) {
+ Result = EVP_PKEY_CTX_set_rsa_pss_saltlen(pKeyCtx, SaltLen) > 0;
+ }
+ if (Result) {
+ Result = EVP_PKEY_CTX_set_rsa_mgf1_md(pKeyCtx, HashAlg) > 0;
+ }
+ if (Result) {
+ Result = EVP_DigestVerifyUpdate(pEvpVerifyCtx, Message, (UINT32)MsgSize) > 0;
+ }
+ if (Result) {
+ Result = EVP_DigestVerifyFinal(pEvpVerifyCtx, Signature, (UINT32)SigSize) > 0;
+ }
+
+_Exit :
+ if (pEvpRsaKey) {
+ EVP_PKEY_free(pEvpRsaKey);
+ }
+ if (pEvpVerifyCtx) {
+ EVP_MD_CTX_destroy(pEvpVerifyCtx);
+ }
+
+ return Result;
+}
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c
new file mode 100644
index 000000000000..69c6889fbc4b
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c
@@ -0,0 +1,46 @@
+/** @file
+ RSA-PSS Asymmetric Cipher Wrapper Implementation over OpenSSL.
+
+ This file does not provide real capabilities for following APIs in RSA handling:
+ 1) RsaPssVerify
+
+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+
+/**
+ Verifies the RSA signature with RSASSA-PSS signature scheme defined in RFC 8017.
+ Implementation determines salt length automatically from the signature encoding.
+ Mask generation function is the same as the message digest algorithm.
+ Salt length should atleast be equal to digest length.
+
+ @param[in] RsaContext Pointer to RSA context for signature verification.
+ @param[in] Message Pointer to octet message to be verified.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] Signature Pointer to RSASSA-PSS signature to be verified.
+ @param[in] SigSize Size of signature in bytes.
+ @param[in] DigestLen Length of digest for RSA operation.
+ @param[in] SaltLen Salt length for PSS encoding.
+
+ @retval TRUE Valid signature encoded in RSASSA-PSS.
+ @retval FALSE Invalid signature or invalid RSA context.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssVerify (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN CONST UINT8 *Signature,
+ IN UINTN SigSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen
+ )
+{
+ ASSERT (FALSE);
+ return FALSE;
+}
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
new file mode 100644
index 000000000000..1ed076e4192c
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
@@ -0,0 +1,169 @@
+/** @file
+ RSA PSS Asymmetric Cipher Wrapper Implementation over OpenSSL.
+
+ This file implements following APIs which provide basic capabilities for RSA:
+ 1) RsaPssSign
+
+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+
+#include <openssl/bn.h>
+#include <openssl/rsa.h>
+#include <openssl/objects.h>
+#include <openssl/evp.h>
+
+
+/**
+ Retrieve a pointer to EVP message digest object.
+
+ @param[in] DigestLen Length of the message digest.
+
+**/
+STATIC
+const
+EVP_MD*
+GetEvpMD (
+ IN UINT16 DigestLen
+ )
+{
+ switch (DigestLen){
+ case SHA256_DIGEST_SIZE:
+ return EVP_sha256();
+ break;
+ case SHA384_DIGEST_SIZE:
+ return EVP_sha384();
+ break;
+ case SHA512_DIGEST_SIZE:
+ return EVP_sha512();
+ break;
+ default:
+ return NULL;
+ }
+}
+
+
+/**
+ Carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme.
+
+ This function carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme defined in
+ RFC 8017.
+ Mask generation function is the same as the message digest algorithm.
+ If the Signature buffer is too small to hold the contents of signature, FALSE
+ is returned and SigSize is set to the required buffer size to obtain the signature.
+
+ If RsaContext is NULL, then return FALSE.
+ If Message is NULL, then return FALSE.
+ If MsgSize is zero or > INT_MAX, then return FALSE.
+ If DigestLen is NOT 32, 48 or 64, return FALSE.
+ If SaltLen is < DigestLen, then return FALSE.
+ If SigSize is large enough but Signature is NULL, then return FALSE.
+ If this interface is not supported, then return FALSE.
+
+ @param[in] RsaContext Pointer to RSA context for signature generation.
+ @param[in] Message Pointer to octet message to be signed.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] DigestLen Length of the digest in bytes to be used for RSA signature operation.
+ @param[in] SaltLen Length of the salt in bytes to be used for PSS encoding.
+ @param[out] Signature Pointer to buffer to receive RSA PSS signature.
+ @param[in, out] SigSize On input, the size of Signature buffer in bytes.
+ On output, the size of data returned in Signature buffer in bytes.
+
+ @retval TRUE Signature successfully generated in RSASSA-PSS.
+ @retval FALSE Signature generation failed.
+ @retval FALSE SigSize is too small.
+ @retval FALSE This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssSign (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen,
+ OUT UINT8 *Signature,
+ IN OUT UINTN *SigSize
+ )
+{
+ BOOLEAN Result;
+ UINTN RsaSigSize;
+ EVP_PKEY *pEvpRsaKey = NULL;
+ EVP_MD_CTX *pEvpVerifyCtx = NULL;
+ EVP_PKEY_CTX *pKeyCtx = NULL;
+ CONST EVP_MD *HashAlg = NULL;
+
+ if (RsaContext == NULL) {
+ return FALSE;
+ }
+ if (Message == NULL || MsgSize == 0 || MsgSize > INT_MAX) {
+ return FALSE;
+ }
+
+ RsaSigSize = RSA_size (RsaContext);
+ if (*SigSize < RsaSigSize) {
+ *SigSize = RsaSigSize;
+ return FALSE;
+ }
+
+ if (Signature == NULL) {
+ return FALSE;
+ }
+
+ if (SaltLen < DigestLen) {
+ return FALSE;
+ }
+
+ HashAlg = GetEvpMD(DigestLen);
+
+ if (HashAlg == NULL) {
+ return FALSE;
+ }
+
+ pEvpRsaKey = EVP_PKEY_new();
+ if (pEvpRsaKey == NULL) {
+ goto _Exit;
+ }
+
+ EVP_PKEY_set1_RSA(pEvpRsaKey, RsaContext);
+
+ pEvpVerifyCtx = EVP_MD_CTX_create();
+ if (pEvpVerifyCtx == NULL) {
+ goto _Exit;
+ }
+
+ Result = EVP_DigestSignInit(pEvpVerifyCtx, &pKeyCtx, HashAlg, NULL, pEvpRsaKey) > 0;
+ if (pKeyCtx == NULL) {
+ goto _Exit;
+ }
+
+ if (Result) {
+ Result = EVP_PKEY_CTX_set_rsa_padding(pKeyCtx, RSA_PKCS1_PSS_PADDING) > 0;
+ }
+ if (Result) {
+ Result = EVP_PKEY_CTX_set_rsa_pss_saltlen(pKeyCtx, SaltLen) > 0;
+ }
+ if (Result) {
+ Result = EVP_PKEY_CTX_set_rsa_mgf1_md(pKeyCtx, HashAlg) > 0;
+ }
+ if (Result) {
+ Result = EVP_DigestSignUpdate(pEvpVerifyCtx, Message, (UINT32)MsgSize) > 0;
+ }
+ if (Result) {
+ Result = EVP_DigestSignFinal(pEvpVerifyCtx, Signature, SigSize) > 0;
+ }
+
+_Exit :
+ if (pEvpRsaKey) {
+ EVP_PKEY_free(pEvpRsaKey);
+ }
+ if (pEvpVerifyCtx) {
+ EVP_MD_CTX_destroy(pEvpVerifyCtx);
+ }
+
+ return Result;
+}
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSignNull.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSignNull.c
new file mode 100644
index 000000000000..4ed2dfce992a
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSignNull.c
@@ -0,0 +1,60 @@
+/** @file
+ RSA-PSS Asymmetric Cipher Wrapper Implementation over OpenSSL.
+
+ This file does not provide real capabilities for following APIs in RSA handling:
+ 1) RsaPssSign
+
+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+
+/**
+ Carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme.
+
+ This function carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme defined in
+ RFC 8017.
+ Mask generation function is the same as the message digest algorithm.
+ If the Signature buffer is too small to hold the contents of signature, FALSE
+ is returned and SigSize is set to the required buffer size to obtain the signature.
+
+ If RsaContext is NULL, then return FALSE.
+ If Message is NULL, then return FALSE.
+ If MsgSize is zero or > INT_MAX, then return FALSE.
+ If DigestLen is NOT 32, 48 or 64, return FALSE.
+ If SaltLen is < DigestLen, then return FALSE.
+ If SigSize is large enough but Signature is NULL, then return FALSE.
+ If this interface is not supported, then return FALSE.
+
+ @param[in] RsaContext Pointer to RSA context for signature generation.
+ @param[in] Message Pointer to octet message to be signed.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] DigestLen Length of the digest in bytes to be used for RSA signature operation.
+ @param[in] SaltLen Length of the salt in bytes to be used for PSS encoding.
+ @param[out] Signature Pointer to buffer to receive RSA PSS signature.
+ @param[in, out] SigSize On input, the size of Signature buffer in bytes.
+ On output, the size of data returned in Signature buffer in bytes.
+
+ @retval TRUE Signature successfully generated in RSASSA-PSS.
+ @retval FALSE Signature generation failed.
+ @retval FALSE SigSize is too small.
+ @retval FALSE This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssSign (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen,
+ OUT UINT8 *Signature,
+ IN OUT UINTN *SigSize
+ )
+{
+ ASSERT (FALSE);
+ return FALSE;
+}
diff --git a/CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssNull.c b/CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssNull.c
new file mode 100644
index 000000000000..69c6889fbc4b
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssNull.c
@@ -0,0 +1,46 @@
+/** @file
+ RSA-PSS Asymmetric Cipher Wrapper Implementation over OpenSSL.
+
+ This file does not provide real capabilities for following APIs in RSA handling:
+ 1) RsaPssVerify
+
+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+
+/**
+ Verifies the RSA signature with RSASSA-PSS signature scheme defined in RFC 8017.
+ Implementation determines salt length automatically from the signature encoding.
+ Mask generation function is the same as the message digest algorithm.
+ Salt length should atleast be equal to digest length.
+
+ @param[in] RsaContext Pointer to RSA context for signature verification.
+ @param[in] Message Pointer to octet message to be verified.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] Signature Pointer to RSASSA-PSS signature to be verified.
+ @param[in] SigSize Size of signature in bytes.
+ @param[in] DigestLen Length of digest for RSA operation.
+ @param[in] SaltLen Salt length for PSS encoding.
+
+ @retval TRUE Valid signature encoded in RSASSA-PSS.
+ @retval FALSE Invalid signature or invalid RSA context.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssVerify (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN CONST UINT8 *Signature,
+ IN UINTN SigSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen
+ )
+{
+ ASSERT (FALSE);
+ return FALSE;
+}
diff --git a/CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssSignNull.c b/CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssSignNull.c
new file mode 100644
index 000000000000..4ed2dfce992a
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssSignNull.c
@@ -0,0 +1,60 @@
+/** @file
+ RSA-PSS Asymmetric Cipher Wrapper Implementation over OpenSSL.
+
+ This file does not provide real capabilities for following APIs in RSA handling:
+ 1) RsaPssSign
+
+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+
+/**
+ Carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme.
+
+ This function carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme defined in
+ RFC 8017.
+ Mask generation function is the same as the message digest algorithm.
+ If the Signature buffer is too small to hold the contents of signature, FALSE
+ is returned and SigSize is set to the required buffer size to obtain the signature.
+
+ If RsaContext is NULL, then return FALSE.
+ If Message is NULL, then return FALSE.
+ If MsgSize is zero or > INT_MAX, then return FALSE.
+ If DigestLen is NOT 32, 48 or 64, return FALSE.
+ If SaltLen is < DigestLen, then return FALSE.
+ If SigSize is large enough but Signature is NULL, then return FALSE.
+ If this interface is not supported, then return FALSE.
+
+ @param[in] RsaContext Pointer to RSA context for signature generation.
+ @param[in] Message Pointer to octet message to be signed.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] DigestLen Length of the digest in bytes to be used for RSA signature operation.
+ @param[in] SaltLen Length of the salt in bytes to be used for PSS encoding.
+ @param[out] Signature Pointer to buffer to receive RSA PSS signature.
+ @param[in, out] SigSize On input, the size of Signature buffer in bytes.
+ On output, the size of data returned in Signature buffer in bytes.
+
+ @retval TRUE Signature successfully generated in RSASSA-PSS.
+ @retval FALSE Signature generation failed.
+ @retval FALSE SigSize is too small.
+ @retval FALSE This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssSign (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen,
+ OUT UINT8 *Signature,
+ IN OUT UINTN *SigSize
+ )
+{
+ ASSERT (FALSE);
+ return FALSE;
+}
diff --git a/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c b/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c
index 8b43d1363cb9..412fbdbff52c 100644
--- a/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c
+++ b/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c
@@ -1552,6 +1552,72 @@ RsaPkcs1Verify (
CALL_CRYPTO_SERVICE (RsaPkcs1Verify, (RsaContext, MessageHash, HashSize, Signature, SigSize), FALSE);
}

+/**
+ Verifies the RSA signature with RSASSA-PSS signature scheme defined in RFC 8017.
+ Implementation determines salt length automatically from the signature encoding.
+ Mask generation function is the same as the message digest algorithm.
+ Salt length should atleast be equal to digest length.
+
+ @param[in] RsaContext Pointer to RSA context for signature verification.
+ @param[in] Message Pointer to octet message to be verified.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] Signature Pointer to RSASSA-PSS signature to be verified.
+ @param[in] SigSize Size of signature in bytes.
+ @param[in] DigestLen Length of digest for RSA operation.
+ @param[in] SaltLen Salt length for PSS encoding.
+
+ @retval TRUE Valid signature encoded in RSASSA-PSS.
+ @retval FALSE Invalid signature or invalid RSA context.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssVerify (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN CONST UINT8 *Signature,
+ IN UINTN SigSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen
+ )
+{
+ CALL_CRYPTO_SERVICE (RsaPssVerify, (RsaContext, Message, MsgSize, Signature, SigSize, DigestLen, SaltLen), FALSE);
+}
+
+/**
+ Verifies the RSA signature with RSASSA-PSS signature scheme defined in RFC 8017.
+ Implementation determines salt length automatically from the signature encoding.
+ Mask generation function is the same as the message digest algorithm.
+ Salt length should atleast be equal to digest length.
+
+ @param[in] RsaContext Pointer to RSA context for signature verification.
+ @param[in] Message Pointer to octet message to be verified.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] Signature Pointer to RSASSA-PSS signature to be verified.
+ @param[in] SigSize Size of signature in bytes.
+ @param[in] DigestLen Length of digest for RSA operation.
+ @param[in] SaltLen Salt length for PSS encoding.
+
+ @retval TRUE Valid signature encoded in RSASSA-PSS.
+ @retval FALSE Invalid signature or invalid RSA context.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssSign (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen,
+ OUT UINT8 *Signature,
+ IN OUT UINTN *SigSize
+ )
+{
+ CALL_CRYPTO_SERVICE (RsaPssSign, (RsaContext, Message, MsgSize, DigestLen, SaltLen, Signature, SigSize), FALSE);
+}
+
/**
Retrieve the RSA Private Key from the password-protected PEM key data.

diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c
index b7fcea3ff7e4..3873de973064 100644
--- a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c
+++ b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c
@@ -16,6 +16,7 @@ SUITE_DESC mSuiteDesc[] = {
{"HMAC verify tests", "CryptoPkg.BaseCryptLib", NULL, NULL, &mHmacTestNum, mHmacTest},
{"BlockCipher verify tests", "CryptoPkg.BaseCryptLib", NULL, NULL, &mBlockCipherTestNum, mBlockCipherTest},
{"RSA verify tests", "CryptoPkg.BaseCryptLib", NULL, NULL, &mRsaTestNum, mRsaTest},
+ {"RSA PSS verify tests", "CryptoPkg.BaseCryptLib", NULL, NULL, &mRsaPssTestNum, mRsaPssTest},
{"RSACert verify tests", "CryptoPkg.BaseCryptLib", NULL, NULL, &mRsaCertTestNum, mRsaCertTest},
{"PKCS7 verify tests", "CryptoPkg.BaseCryptLib", NULL, NULL, &mPkcs7TestNum, mPkcs7Test},
{"PKCS5 verify tests", "CryptoPkg.BaseCryptLib", NULL, NULL, &mPkcs5TestNum, mPkcs5Test},
diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c
new file mode 100644
index 000000000000..5ac2f325fbdd
--- /dev/null
+++ b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c
@@ -0,0 +1,191 @@
+/** @file
+ Application for RSA PSS Primitives Validation.
+
+Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "TestBaseCryptLib.h"
+
+//
+// RSA PSS test vectors from NIST FIPS 186-3 RSA files
+//
+
+//
+// Public Modulus of RSA Key
+//
+UINT8 RsaPssN[]={
+ 0xa4, 0x7d, 0x04, 0xe7, 0xca, 0xcd, 0xba, 0x4e, 0xa2, 0x6e, 0xca, 0x8a, 0x4c, 0x6e, 0x14, 0x56,
+ 0x3c, 0x2c, 0xe0, 0x3b, 0x62, 0x3b, 0x76, 0x8c, 0x0d, 0x49, 0x86, 0x8a, 0x57, 0x12, 0x13, 0x01,
+ 0xdb, 0xf7, 0x83, 0xd8, 0x2f, 0x4c, 0x05, 0x5e, 0x73, 0x96, 0x0e, 0x70, 0x55, 0x01, 0x87, 0xd0,
+ 0xaf, 0x62, 0xac, 0x34, 0x96, 0xf0, 0xa3, 0xd9, 0x10, 0x3c, 0x2e, 0xb7, 0x91, 0x9a, 0x72, 0x75,
+ 0x2f, 0xa7, 0xce, 0x8c, 0x68, 0x8d, 0x81, 0xe3, 0xae, 0xe9, 0x94, 0x68, 0x88, 0x7a, 0x15, 0x28,
+ 0x8a, 0xfb, 0xb7, 0xac, 0xb8, 0x45, 0xb7, 0xc5, 0x22, 0xb5, 0xc6, 0x4e, 0x67, 0x8f, 0xcd, 0x3d,
+ 0x22, 0xfe, 0xb8, 0x4b, 0x44, 0x27, 0x27, 0x00, 0xbe, 0x52, 0x7d, 0x2b, 0x20, 0x25, 0xa3, 0xf8,
+ 0x3c, 0x23, 0x83, 0xbf, 0x6a, 0x39, 0xcf, 0x5b, 0x4e, 0x48, 0xb3, 0xcf, 0x2f, 0x56, 0xee, 0xf0,
+ 0xdf, 0xff, 0x18, 0x55, 0x5e, 0x31, 0x03, 0x7b, 0x91, 0x52, 0x48, 0x69, 0x48, 0x76, 0xf3, 0x04,
+ 0x78, 0x14, 0x41, 0x51, 0x64, 0xf2, 0xc6, 0x60, 0x88, 0x1e, 0x69, 0x4b, 0x58, 0xc2, 0x80, 0x38,
+ 0xa0, 0x32, 0xad, 0x25, 0x63, 0x4a, 0xad, 0x7b, 0x39, 0x17, 0x1d, 0xee, 0x36, 0x8e, 0x3d, 0x59,
+ 0xbf, 0xb7, 0x29, 0x9e, 0x46, 0x01, 0xd4, 0x58, 0x7e, 0x68, 0xca, 0xaf, 0x8d, 0xb4, 0x57, 0xb7,
+ 0x5a, 0xf4, 0x2f, 0xc0, 0xcf, 0x1a, 0xe7, 0xca, 0xce, 0xd2, 0x86, 0xd7, 0x7f, 0xac, 0x6c, 0xed,
+ 0xb0, 0x3a, 0xd9, 0x4f, 0x14, 0x33, 0xd2, 0xc9, 0x4d, 0x08, 0xe6, 0x0b, 0xc1, 0xfd, 0xef, 0x05,
+ 0x43, 0xcd, 0x29, 0x51, 0xe7, 0x65, 0xb3, 0x82, 0x30, 0xfd, 0xd1, 0x8d, 0xe5, 0xd2, 0xca, 0x62,
+ 0x7d, 0xdc, 0x03, 0x2f, 0xe0, 0x5b, 0xbd, 0x2f, 0xf2, 0x1e, 0x2d, 0xb1, 0xc2, 0xf9, 0x4d, 0x8b,
+ };
+
+//
+// Public Exponent of RSA Key
+//
+UINT8 RsaPssE[]={ 0x10, 0xe4, 0x3f };
+
+//
+// Private Exponent of RSA Key
+//
+UINT8 RsaPssD[]={
+ 0x11, 0xa0, 0xdd, 0x28, 0x5f, 0x66, 0x47, 0x1a, 0x8d, 0xa3, 0x0b, 0xcb, 0x8c, 0x24, 0xa1, 0xd5,
+ 0xc8, 0xdb, 0x94, 0x2f, 0xc9, 0x92, 0x07, 0x97, 0xca, 0x44, 0x24, 0x60, 0xa8, 0x00, 0xb7, 0x5b,
+ 0xbc, 0x73, 0x8b, 0xeb, 0x8e, 0xe0, 0xe8, 0x74, 0xb0, 0x53, 0xe6, 0x47, 0x07, 0xdf, 0x4c, 0xfc,
+ 0x78, 0x37, 0xc4, 0x0e, 0x5b, 0xe6, 0x8b, 0x8a, 0x8e, 0x1d, 0x01, 0x45, 0x16, 0x9c, 0xa6, 0x27,
+ 0x1d, 0x81, 0x88, 0x7e, 0x19, 0xa1, 0xcd, 0x95, 0xb2, 0xfd, 0x0d, 0xe0, 0xdb, 0xa3, 0x47, 0xfe,
+ 0x63, 0x7b, 0xcc, 0x6c, 0xdc, 0x24, 0xee, 0xbe, 0x03, 0xc2, 0x4d, 0x4c, 0xf3, 0xa5, 0xc6, 0x15,
+ 0x4d, 0x78, 0xf1, 0x41, 0xfe, 0x34, 0x16, 0x99, 0x24, 0xd0, 0xf8, 0x95, 0x33, 0x65, 0x8e, 0xac,
+ 0xfd, 0xea, 0xe9, 0x9c, 0xe1, 0xa8, 0x80, 0x27, 0xc1, 0x8f, 0xf9, 0x26, 0x53, 0xa8, 0x35, 0xaa,
+ 0x38, 0x91, 0xbf, 0xff, 0xcd, 0x38, 0x8f, 0xfc, 0x23, 0x88, 0xce, 0x2b, 0x10, 0x56, 0x85, 0x43,
+ 0x75, 0x05, 0x02, 0xcc, 0xbc, 0x69, 0xc0, 0x08, 0x8f, 0x1d, 0x69, 0x0e, 0x97, 0xa5, 0xf5, 0xbd,
+ 0xd1, 0x88, 0x8c, 0xd2, 0xfa, 0xa4, 0x3c, 0x04, 0xae, 0x24, 0x53, 0x95, 0x22, 0xdd, 0xe2, 0xd9,
+ 0xc2, 0x02, 0xf6, 0x55, 0xfc, 0x55, 0x75, 0x44, 0x40, 0xb5, 0x3a, 0x15, 0x32, 0xaa, 0xb4, 0x78,
+ 0x51, 0xf6, 0x0b, 0x7a, 0x06, 0x7e, 0x24, 0x0b, 0x73, 0x8e, 0x1b, 0x1d, 0xaa, 0xe6, 0xca, 0x0d,
+ 0x59, 0xee, 0xae, 0x27, 0x68, 0x6c, 0xd8, 0x88, 0x57, 0xe9, 0xad, 0xad, 0xc2, 0xd4, 0xb8, 0x2b,
+ 0x07, 0xa6, 0x1a, 0x35, 0x84, 0x56, 0xaa, 0xf8, 0x07, 0x66, 0x96, 0x93, 0xff, 0xb1, 0x3c, 0x99,
+ 0x64, 0xa6, 0x36, 0x54, 0xca, 0xdc, 0x81, 0xee, 0x59, 0xdf, 0x51, 0x1c, 0xa3, 0xa4, 0xbd, 0x67,
+ };
+
+//
+// Binary message to be signed and verified
+//
+UINT8 PssMessage[]={
+ 0xe0, 0x02, 0x37, 0x7a, 0xff, 0xb0, 0x4f, 0x0f, 0xe4, 0x59, 0x8d, 0xe9, 0xd9, 0x2d, 0x31, 0xd6,
+ 0xc7, 0x86, 0x04, 0x0d, 0x57, 0x76, 0x97, 0x65, 0x56, 0xa2, 0xcf, 0xc5, 0x5e, 0x54, 0xa1, 0xdc,
+ 0xb3, 0xcb, 0x1b, 0x12, 0x6b, 0xd6, 0xa4, 0xbe, 0xd2, 0xa1, 0x84, 0x99, 0x0c, 0xce, 0xa7, 0x73,
+ 0xfc, 0xc7, 0x9d, 0x24, 0x65, 0x53, 0xe6, 0xc6, 0x4f, 0x68, 0x6d, 0x21, 0xad, 0x41, 0x52, 0x67,
+ 0x3c, 0xaf, 0xec, 0x22, 0xae, 0xb4, 0x0f, 0x6a, 0x08, 0x4e, 0x8a, 0x5b, 0x49, 0x91, 0xf4, 0xc6,
+ 0x4c, 0xf8, 0xa9, 0x27, 0xef, 0xfd, 0x0f, 0xd7, 0x75, 0xe7, 0x1e, 0x83, 0x29, 0xe4, 0x1f, 0xdd,
+ 0x44, 0x57, 0xb3, 0x91, 0x11, 0x73, 0x18, 0x7b, 0x4f, 0x09, 0xa8, 0x17, 0xd7, 0x9e, 0xa2, 0x39,
+ 0x7f, 0xc1, 0x2d, 0xfe, 0x3d, 0x9c, 0x9a, 0x02, 0x90, 0xc8, 0xea, 0xd3, 0x1b, 0x66, 0x90, 0xa6,
+ };
+
+//
+// Binary message to be signed and verified
+//
+UINT8 PssSalt[]={
+ 0xd6, 0x6f, 0x72, 0xf1, 0x0b, 0x69, 0x00, 0x1a, 0x5b, 0x59, 0xcf, 0x10, 0x92, 0xad, 0x27, 0x4d,
+ 0x50, 0x56, 0xc4, 0xe9, 0x5c, 0xcc, 0xcf, 0xbe, 0x3b, 0x53, 0x0d, 0xcb, 0x02, 0x7e, 0x57, 0xd6
+ };
+
+//
+// RSASSA-PSS Signature over above message using above keys, salt and SHA256 digest(and MGF1) algo.
+//
+UINT8 TestVectorSignature[]={
+ 0x4f, 0x9b, 0x42, 0x5c, 0x20, 0x58, 0x46, 0x0e, 0x4a, 0xb2, 0xf5, 0xc9, 0x63, 0x84, 0xda, 0x23,
+ 0x27, 0xfd, 0x29, 0x15, 0x0f, 0x01, 0x95, 0x5a, 0x76, 0xb4, 0xef, 0xe9, 0x56, 0xaf, 0x06, 0xdc,
+ 0x08, 0x77, 0x9a, 0x37, 0x4e, 0xe4, 0x60, 0x7e, 0xab, 0x61, 0xa9, 0x3a, 0xdc, 0x56, 0x08, 0xf4,
+ 0xec, 0x36, 0xe4, 0x7f, 0x2a, 0x0f, 0x75, 0x4e, 0x8f, 0xf8, 0x39, 0xa8, 0xa1, 0x9b, 0x1d, 0xb1,
+ 0xe8, 0x84, 0xea, 0x4c, 0xf3, 0x48, 0xcd, 0x45, 0x50, 0x69, 0xeb, 0x87, 0xaf, 0xd5, 0x36, 0x45,
+ 0xb4, 0x4e, 0x28, 0xa0, 0xa5, 0x68, 0x08, 0xf5, 0x03, 0x1d, 0xa5, 0xba, 0x91, 0x12, 0x76, 0x8d,
+ 0xfb, 0xfc, 0xa4, 0x4e, 0xbe, 0x63, 0xa0, 0xc0, 0x57, 0x2b, 0x73, 0x1d, 0x66, 0x12, 0x2f, 0xb7,
+ 0x16, 0x09, 0xbe, 0x14, 0x80, 0xfa, 0xa4, 0xe4, 0xf7, 0x5e, 0x43, 0x95, 0x51, 0x59, 0xd7, 0x0f,
+ 0x08, 0x1e, 0x2a, 0x32, 0xfb, 0xb1, 0x9a, 0x48, 0xb9, 0xf1, 0x62, 0xcf, 0x6b, 0x2f, 0xb4, 0x45,
+ 0xd2, 0xd6, 0x99, 0x4b, 0xc5, 0x89, 0x10, 0xa2, 0x6b, 0x59, 0x43, 0x47, 0x78, 0x03, 0xcd, 0xaa,
+ 0xa1, 0xbd, 0x74, 0xb0, 0xda, 0x0a, 0x5d, 0x05, 0x3d, 0x8b, 0x1d, 0xc5, 0x93, 0x09, 0x1d, 0xb5,
+ 0x38, 0x83, 0x83, 0xc2, 0x60, 0x79, 0xf3, 0x44, 0xe2, 0xae, 0xa6, 0x00, 0xd0, 0xe3, 0x24, 0x16,
+ 0x4b, 0x45, 0x0f, 0x7b, 0x9b, 0x46, 0x51, 0x11, 0xb7, 0x26, 0x5f, 0x3b, 0x1b, 0x06, 0x30, 0x89,
+ 0xae, 0x7e, 0x26, 0x23, 0xfc, 0x0f, 0xda, 0x80, 0x52, 0xcf, 0x4b, 0xf3, 0x37, 0x91, 0x02, 0xfb,
+ 0xf7, 0x1d, 0x7c, 0x98, 0xe8, 0x25, 0x86, 0x64, 0xce, 0xed, 0x63, 0x7d, 0x20, 0xf9, 0x5f, 0xf0,
+ 0x11, 0x18, 0x81, 0xe6, 0x50, 0xce, 0x61, 0xf2, 0x51, 0xd9, 0xc3, 0xa6, 0x29, 0xef, 0x22, 0x2d,
+ };
+
+
+VOID *mRsa;
+
+UNIT_TEST_STATUS
+EFIAPI
+TestVerifyRsaPssPreReq (
+ UNIT_TEST_CONTEXT Context
+ )
+{
+ mRsa = RsaNew ();
+
+ if (mRsa == NULL) {
+ return UNIT_TEST_ERROR_TEST_FAILED;
+ }
+
+ return UNIT_TEST_PASSED;
+}
+
+VOID
+EFIAPI
+TestVerifyRsaPssCleanUp (
+ UNIT_TEST_CONTEXT Context
+ )
+{
+ if (mRsa != NULL) {
+ RsaFree (mRsa);
+ mRsa = NULL;
+ }
+}
+
+
+UNIT_TEST_STATUS
+EFIAPI
+TestVerifyRsaPssSignVerify (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT8 *Signature;
+ UINTN SigSize;
+ BOOLEAN Status;
+
+ Status = RsaSetKey (mRsa, RsaKeyN, RsaPssN, sizeof (RsaPssN));
+ UT_ASSERT_TRUE (Status);
+
+ Status = RsaSetKey (mRsa, RsaKeyE, RsaPssE, sizeof (RsaPssE));
+ UT_ASSERT_TRUE (Status);
+
+ Status = RsaSetKey (mRsa, RsaKeyD, RsaPssD, sizeof (RsaPssD));
+ UT_ASSERT_TRUE (Status);
+
+ SigSize = 0;
+ Status = RsaPssSign (mRsa, PssMessage, sizeof(PssMessage), SHA256_DIGEST_SIZE, SHA256_DIGEST_SIZE, NULL, &SigSize);
+ UT_ASSERT_FALSE (Status);
+ UT_ASSERT_NOT_EQUAL (SigSize, 0);
+
+ Signature = AllocatePool (SigSize);
+ Status = RsaPssSign (mRsa, PssMessage, sizeof(PssMessage), SHA256_DIGEST_SIZE, SHA256_DIGEST_SIZE, Signature, &SigSize);
+ UT_ASSERT_TRUE (Status);
+
+ //
+ // Verify RSA PSS encoded Signature generated in above step
+ //
+ Status = RsaPssVerify (mRsa, PssMessage, sizeof(PssMessage), Signature, SigSize, SHA256_DIGEST_SIZE, SHA256_DIGEST_SIZE);
+ UT_ASSERT_TRUE (Status);
+
+ //
+ // Verify NIST FIPS 186-3 RSA test vector signature
+ //
+ Status = RsaPssVerify (mRsa, PssMessage, sizeof(PssMessage), TestVectorSignature, sizeof(TestVectorSignature), SHA256_DIGEST_SIZE, SHA256_DIGEST_SIZE);
+ UT_ASSERT_TRUE (Status);
+
+ FreePool(Signature);
+ return UNIT_TEST_PASSED;
+}
+
+
+TEST_DESC mRsaPssTest[] = {
+ //
+ // -----Description--------------------------------------Class----------------------Function---------------------------------Pre---------------------Post---------Context
+ //
+ {"TestVerifyRsaPssSignVerify()", "CryptoPkg.BaseCryptLib.Rsa", TestVerifyRsaPssSignVerify, TestVerifyRsaPssPreReq, TestVerifyRsaPssCleanUp, NULL},
+};
+
+UINTN mRsaPssTestNum = ARRAY_SIZE(mRsaPssTest);
diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c
index 7ce20d2e778f..0969b6aea660 100644
--- a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c
+++ b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c
@@ -295,6 +295,8 @@ TestVerifyRsaPkcs1SignVerify (
Status = RsaPkcs1Verify (mRsa, HashValue, HashSize, Signature, SigSize);
UT_ASSERT_TRUE (Status);

+ FreePool(Signature);
+
return UNIT_TEST_PASSED;
}

diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
index 496121e6a4ed..8c7d5922ef96 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -1363,6 +1363,80 @@ RsaPkcs1Verify (
IN UINTN SigSize
);

+/**
+ Carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme.
+
+ This function carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme defined in
+ RFC 8017.
+ Mask generation function is the same as the message digest algorithm.
+ If the Signature buffer is too small to hold the contents of signature, FALSE
+ is returned and SigSize is set to the required buffer size to obtain the signature.
+
+ If RsaContext is NULL, then return FALSE.
+ If Message is NULL, then return FALSE.
+ If MsgSize is zero or > INT_MAX, then return FALSE.
+ If DigestLen is NOT 32, 48 or 64, return FALSE.
+ If SaltLen is < DigestLen, then return FALSE.
+ If SigSize is large enough but Signature is NULL, then return FALSE.
+ If this interface is not supported, then return FALSE.
+
+ @param[in] RsaContext Pointer to RSA context for signature generation.
+ @param[in] Message Pointer to octet message to be signed.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] DigestLen Length of the digest in bytes to be used for RSA signature operation.
+ @param[in] SaltLen Length of the salt in bytes to be used for PSS encoding.
+ @param[out] Signature Pointer to buffer to receive RSA PSS signature.
+ @param[in, out] SigSize On input, the size of Signature buffer in bytes.
+ On output, the size of data returned in Signature buffer in bytes.
+
+ @retval TRUE Signature successfully generated in RSASSA-PSS.
+ @retval FALSE Signature generation failed.
+ @retval FALSE SigSize is too small.
+ @retval FALSE This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssSign (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen,
+ OUT UINT8 *Signature,
+ IN OUT UINTN *SigSize
+ );
+
+/**
+ Verifies the RSA signature with RSASSA-PSS signature scheme defined in RFC 8017.
+ Implementation determines salt length automatically from the signature encoding.
+ Mask generation function is the same as the message digest algorithm.
+ Salt length should atleast be equal to digest length.
+
+ @param[in] RsaContext Pointer to RSA context for signature verification.
+ @param[in] Message Pointer to octet message to be verified.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] Signature Pointer to RSASSA-PSS signature to be verified.
+ @param[in] SigSize Size of signature in bytes.
+ @param[in] DigestLen Length of digest for RSA operation.
+ @param[in] SaltLen Salt length for PSS encoding.
+
+ @retval TRUE Valid signature encoded in RSASSA-PSS.
+ @retval FALSE Invalid signature or invalid RSA context.
+
+**/
+BOOLEAN
+EFIAPI
+RsaPssVerify (
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN CONST UINT8 *Signature,
+ IN UINTN SigSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen
+ );
+
/**
Retrieve the RSA Private Key from the password-protected PEM key data.

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 4aae2aba95d6..49703fa4c963 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -49,6 +49,8 @@
Pk/CryptX509.c
Pk/CryptAuthenticode.c
Pk/CryptTs.c
+ Pk/CryptRsaPss.c
+ Pk/CryptRsaPssSign.c
Pem/CryptPem.c

SysCall/CrtWrapper.c
diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
index 7509e4273028..0cab5f3ce36c 100644
--- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
@@ -55,6 +55,8 @@
Pk/CryptX509Null.c
Pk/CryptAuthenticodeNull.c
Pk/CryptTsNull.c
+ Pk/CryptRsaPss.c
+ Pk/CryptRsaPssSignNull.c
Pem/CryptPemNull.c
Rand/CryptRandNull.c

diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
index 70c985ec93dc..3d3a6fb94a77 100644
--- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
@@ -55,6 +55,8 @@
Pk/CryptX509.c
Pk/CryptAuthenticodeNull.c
Pk/CryptTsNull.c
+ Pk/CryptRsaPssNull.c
+ Pk/CryptRsaPssSignNull.c
Pem/CryptPem.c

SysCall/CrtWrapper.c
diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
index 91ec3e03bf5e..07c376ce04bb 100644
--- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
@@ -53,6 +53,8 @@
Pk/CryptX509.c
Pk/CryptAuthenticodeNull.c
Pk/CryptTsNull.c
+ Pk/CryptRsaPss.c
+ Pk/CryptRsaPssSignNull.c
Pem/CryptPem.c

SysCall/CrtWrapper.c
diff --git a/CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf
index db506c32f724..b98f9635b27b 100644
--- a/CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf
@@ -44,6 +44,8 @@
Pk/CryptAuthenticode.c
Pk/CryptTs.c
Pem/CryptPem.c
+ Pk/CryptRsaPss.c
+ Pk/CryptRsaPssSign.c

SysCall/UnitTestHostCrtWrapper.c

diff --git a/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf b/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
index 689af4fedd68..faf959827b90 100644
--- a/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
+++ b/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
@@ -50,6 +50,8 @@
Pk/CryptTsNull.c
Pem/CryptPemNull.c
Rand/CryptRandNull.c
+ Pk/CryptRsaPssNull.c
+ Pk/CryptRsaPssSignNull.c

[Packages]
MdePkg/MdePkg.dec
diff --git a/CryptoPkg/Private/Protocol/Crypto.h b/CryptoPkg/Private/Protocol/Crypto.h
index 17930a77a60e..e304302c9445 100644
--- a/CryptoPkg/Private/Protocol/Crypto.h
+++ b/CryptoPkg/Private/Protocol/Crypto.h
@@ -3408,6 +3408,81 @@ EFI_STATUS
IN OUT UINTN *DataSize
);

+/**
+ Carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme.
+
+ This function carries out the RSA-SSA signature generation with EMSA-PSS encoding scheme defined in
+ RFC 8017.
+ Mask generation function is the same as the message digest algorithm.
+ If the Signature buffer is too small to hold the contents of signature, FALSE
+ is returned and SigSize is set to the required buffer size to obtain the signature.
+
+ If RsaContext is NULL, then return FALSE.
+ If Message is NULL, then return FALSE.
+ If MsgSize is zero or > INT_MAX, then return FALSE.
+ If DigestLen is NOT 32, 48 or 64, return FALSE.
+ If SaltLen is < DigestLen, then return FALSE.
+ If SigSize is large enough but Signature is NULL, then return FALSE.
+ If this interface is not supported, then return FALSE.
+
+ @param[in] RsaContext Pointer to RSA context for signature generation.
+ @param[in] Message Pointer to octet message to be signed.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] DigestLen Length of the digest in bytes to be used for RSA signature operation.
+ @param[in] SaltLen Length of the salt in bytes to be used for PSS encoding.
+ @param[out] Signature Pointer to buffer to receive RSA PSS signature.
+ @param[in, out] SigSize On input, the size of Signature buffer in bytes.
+ On output, the size of data returned in Signature buffer in bytes.
+
+ @retval TRUE Signature successfully generated in RSASSA-PSS.
+ @retval FALSE Signature generation failed.
+ @retval FALSE SigSize is too small.
+ @retval FALSE This interface is not supported.
+
+**/
+typedef
+BOOLEAN
+(EFIAPI* EDKII_CRYPTO_RSA_PSS_SIGN)(
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen,
+ OUT UINT8 *Signature,
+ IN OUT UINTN *SigSize
+ );
+
+/**
+ Verifies the RSA signature with RSASSA-PSS signature scheme defined in RFC 8017.
+ Implementation determines salt length automatically from the signature encoding.
+ Mask generation function is the same as the message digest algorithm.
+ Salt length should atleast be equal to digest length.
+
+ @param[in] RsaContext Pointer to RSA context for signature verification.
+ @param[in] Message Pointer to octet message to be verified.
+ @param[in] MsgSize Size of the message in bytes.
+ @param[in] Signature Pointer to RSASSA-PSS signature to be verified.
+ @param[in] SigSize Size of signature in bytes.
+ @param[in] DigestLen Length of digest for RSA operation.
+ @param[in] SaltLen Salt length for PSS encoding.
+
+ @retval TRUE Valid signature encoded in RSASSA-PSS.
+ @retval FALSE Invalid signature or invalid RSA context.
+
+**/
+typedef
+BOOLEAN
+(EFIAPI* EDKII_CRYPTO_RSA_PSS_VERIFY)(
+ IN VOID *RsaContext,
+ IN CONST UINT8 *Message,
+ IN UINTN MsgSize,
+ IN CONST UINT8 *Signature,
+ IN UINTN SigSize,
+ IN UINT16 DigestLen,
+ IN UINT16 SaltLen
+ );
+
+

///
/// EDK II Crypto Protocol
@@ -3593,6 +3668,9 @@ struct _EDKII_CRYPTO_PROTOCOL {
EDKII_CRYPTO_TLS_GET_HOST_PUBLIC_CERT TlsGetHostPublicCert;
EDKII_CRYPTO_TLS_GET_HOST_PRIVATE_KEY TlsGetHostPrivateKey;
EDKII_CRYPTO_TLS_GET_CERT_REVOCATION_LIST TlsGetCertRevocationList;
+ /// RSA PSS
+ EDKII_CRYPTO_RSA_PSS_SIGN RsaPssSign;
+ EDKII_CRYPTO_RSA_PSS_VERIFY RsaPssVerify;
};

extern GUID gEdkiiCryptoProtocolGuid;
diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h
index 9d1cb150a113..25c1379f1a77 100644
--- a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h
+++ b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h
@@ -83,6 +83,9 @@ extern TEST_DESC mPrngTest[];
extern UINTN mOaepTestNum;
extern TEST_DESC mOaepTest[];

+extern UINTN mRsaPssTestNum;
+extern TEST_DESC mRsaPssTest[];
+
/** Creates a framework you can use */
EFI_STATUS
EFIAPI
diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf
index 300b98e40b33..00c869265080 100644
--- a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf
+++ b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf
@@ -34,6 +34,7 @@
RandTests.c
Pkcs7EkuTests.c
OaepEncryptTests.c
+ RsaPssTests.c

[Packages]
MdePkg/MdePkg.dec
diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf
index d5e7e0d01446..ca789aa6ada3 100644
--- a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf
+++ b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf
@@ -35,6 +35,7 @@
RandTests.c
Pkcs7EkuTests.c
OaepEncryptTests.c
+ RsaPssTests.c

[Packages]
MdePkg/MdePkg.dec
--
2.14.3.windows.1


[PATCH v3 0/1] CryptoPkg: Add RSA PSS verify support

Agrawal, Sachin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3314

This patch uses Openssl's EVP API's to perform RSASSA-PSS verification
of a binary blob.

Patch v1 Cover Letter : https://edk2.groups.io/g/devel/message/74286?p=,,,20,0,0,0::Created,,sachin,20,2,0,82225507
Patch v2 Cover Letter : https://edk2.groups.io/g/devel/message/74565?p=,,,20,0,0,0::Created,,sachin,20,2,0,82434091

https://github.com/sagraw2/edk2/tree/pss_1
https://github.com/sagraw2/edk2/tree/pss_2
https://github.com/sagraw2/edk2/tree/pss_3

Updates from v2:
- Fixed gcc compilation error [CI System]

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>

Sachin Agrawal (1):
CryptoPkg: BaseCryptLib: Add RSA PSS verify support

CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c | 146 +++++++++++++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c | 46 +++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 169 +++++++++++++++++
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSignNull.c | 60 ++++++
CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssNull.c | 46 +++++
CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssSignNull.c | 60 ++++++
CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c | 66 +++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c | 1 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c | 191 ++++++++++++++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c | 2 +
CryptoPkg/Include/Library/BaseCryptLib.h | 74 ++++++++
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf | 2 +
CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf | 2 +
CryptoPkg/Private/Protocol/Crypto.h | 78 ++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h | 3 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf | 1 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf | 1 +
21 files changed, 956 insertions(+)
create mode 100644 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
create mode 100644 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssNull.c
create mode 100644 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
create mode 100644 CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSignNull.c
create mode 100644 CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssNull.c
create mode 100644 CryptoPkg/Library/BaseCryptLibNull/Pk/CryptRsaPssSignNull.c
create mode 100644 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c

--
2.14.3.windows.1


[PATCH 5/5] StandaloneMmPkg: build for 32bit arm machines

Etienne Carriere
 

This change allows to build StandaloneMmPkg components for 32bit Arm
StandaloneMm firmware.

This change mainly moves AArch64/ source files to Arm/ side directory
for several components: StandaloneMmCpu, StandaloneMmCoreEntryPoint
and StandaloneMmMemLib. The source file is built for both 32b and 64b
Arm targets.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
StandaloneMmPkg/Core/StandaloneMmCore.inf | 2 +-
StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/EventHandle.c | 12 ++++++++++--
StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/StandaloneMmCpu.c | 2 +-
StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/StandaloneMmCpu.h | 0
StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/StandaloneMmCpu.inf | 0
StandaloneMmPkg/Include/Library/{AArch64 => Arm}/StandaloneMmCoreEntryPoint.h | 0
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/{AArch64 => Arm}/CreateHobList.c | 2 +-
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/{AArch64 => Arm}/SetPermissions.c | 2 +-
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/{AArch64 => Arm}/StandaloneMmCoreEntryPoint.c | 16 ++++++++--------
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 14 +++++++-------
StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{AArch64 => Arm}/StandaloneMmCoreHobLib.c | 0
StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{AArch64 => Arm}/StandaloneMmCoreHobLibInternal.c | 0
StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf | 8 ++++----
StandaloneMmPkg/Library/StandaloneMmMemLib/{AArch64/StandaloneMmMemLibInternal.c => ArmStandaloneMmMemLibInternal.c} | 9 ++++++++-
StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf | 6 +++---
StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf | 2 +-
StandaloneMmPkg/StandaloneMmPkg.dsc | 8 ++++----
17 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf
index 87bf6e9440..56042b7b39 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
@@ -17,7 +17,7 @@
PI_SPECIFICATION_VERSION = 0x00010032
ENTRY_POINT = StandaloneMmMain

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

[Sources]
StandaloneMmCore.c
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
similarity index 92%
rename from StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
rename to StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
index 63fbe26642..2d7fd81133 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c
@@ -2,6 +2,7 @@

Copyright (c) 2016 HP Development Company, L.P.
Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
+ Copyright (c) 2021, Linaro Limited

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

@@ -27,6 +28,13 @@

#include "StandaloneMmCpu.h"

+#ifdef MDE_CPU_AARCH64
+#define ARM_SMC_ID_MM_COMMUNICATE ARM_SMC_ID_MM_COMMUNICATE_AARCH64
+#endif
+#ifdef MDE_CPU_ARM
+#define ARM_SMC_ID_MM_COMMUNICATE ARM_SMC_ID_MM_COMMUNICATE_AARCH32
+#endif
+
EFI_STATUS
EFIAPI
MmFoundationEntryRegister (
@@ -92,8 +100,8 @@ PiMmStandaloneArmTfCpuDriverEntry (
// receipt of a synchronous MM request. Use the Event ID to distinguish
// between synchronous and asynchronous events.
//
- if ((ARM_SMC_ID_MM_COMMUNICATE_AARCH64 != EventId) &&
- (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64 != EventId)) {
+ if ((ARM_SMC_ID_MM_COMMUNICATE != EventId) &&
+ (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != EventId)) {
DEBUG ((DEBUG_INFO, "UnRecognized Event - 0x%x\n", EventId));
return EFI_INVALID_PARAMETER;
}
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
similarity index 96%
rename from StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
rename to StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
index d4590bcd19..10097f792f 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c
@@ -10,7 +10,7 @@

#include <Base.h>
#include <Pi/PiMmCis.h>
-#include <Library/AArch64/StandaloneMmCoreEntryPoint.h>
+#include <Library/Arm/StandaloneMmCoreEntryPoint.h>
#include <Library/DebugLib.h>
#include <Library/ArmSvcLib.h>
#include <Library/ArmLib.h>
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
similarity index 100%
rename from StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
rename to StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
similarity index 100%
rename from StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
rename to StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
diff --git a/StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
similarity index 100%
rename from StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h
rename to StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
similarity index 97%
rename from StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
rename to StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
index 4d4cf3d5ff..85f8194687 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
@@ -14,7 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/MmramMemoryReserve.h>
#include <Guid/MpInformation.h>

-#include <Library/AArch64/StandaloneMmCoreEntryPoint.h>
+#include <Library/Arm/StandaloneMmCoreEntryPoint.h>
#include <Library/ArmMmuLib.h>
#include <Library/ArmSvcLib.h>
#include <Library/DebugLib.h>
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
similarity index 96%
rename from StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
rename to StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
index 4a380df4a6..cd4b90823e 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
@@ -14,7 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/MmramMemoryReserve.h>
#include <Guid/MpInformation.h>

-#include <Library/AArch64/StandaloneMmCoreEntryPoint.h>
+#include <Library/Arm/StandaloneMmCoreEntryPoint.h>
#include <Library/ArmMmuLib.h>
#include <Library/ArmSvcLib.h>
#include <Library/DebugLib.h>
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
similarity index 94%
rename from StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
rename to StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index b445d6942e..e199e81bbd 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -10,7 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

#include <PiMm.h>

-#include <Library/AArch64/StandaloneMmCoreEntryPoint.h>
+#include <Library/Arm/StandaloneMmCoreEntryPoint.h>

#include <PiPei.h>
#include <Guid/MmramMemoryReserve.h>
@@ -182,13 +182,13 @@ DelegatedEventLoop (
}

if (FfaEnabled) {
- EventCompleteSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64;
+ EventCompleteSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP;
EventCompleteSvcArgs->Arg1 = 0;
EventCompleteSvcArgs->Arg2 = 0;
- EventCompleteSvcArgs->Arg3 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
+ EventCompleteSvcArgs->Arg3 = ARM_SVC_ID_SP_EVENT_COMPLETE;
EventCompleteSvcArgs->Arg4 = SvcStatus;
} else {
- EventCompleteSvcArgs->Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
+ EventCompleteSvcArgs->Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE;
EventCompleteSvcArgs->Arg1 = SvcStatus;
}
}
@@ -273,13 +273,13 @@ InitArmSvcArgs (
)
{
if (FeaturePcdGet (PcdFfaEnable)) {
- InitMmFoundationSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64;
+ InitMmFoundationSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP;
InitMmFoundationSvcArgs->Arg1 = 0;
InitMmFoundationSvcArgs->Arg2 = 0;
- InitMmFoundationSvcArgs->Arg3 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
+ InitMmFoundationSvcArgs->Arg3 = ARM_SVC_ID_SP_EVENT_COMPLETE;
InitMmFoundationSvcArgs->Arg4 = *Ret;
} else {
- InitMmFoundationSvcArgs->Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
+ InitMmFoundationSvcArgs->Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE;
InitMmFoundationSvcArgs->Arg1 = *Ret;
}
}
@@ -395,7 +395,7 @@ _ModuleEntryPoint (
//
ProcessModuleEntryPointList (HobStart);

- DEBUG ((DEBUG_INFO, "Shared Cpu Driver EP 0x%lx\n", (UINT64) CpuDriverEntryPoint));
+ DEBUG ((DEBUG_INFO, "Shared Cpu Driver EP %p\n", (void *) CpuDriverEntryPoint));

finish:
if (Status == RETURN_UNSUPPORTED) {
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 4fa426f58e..1762586cfa 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -21,10 +21,10 @@
# VALID_ARCHITECTURES = IA32 X64 IPF EBC (EBC is for build only)
#

-[Sources.AARCH64]
- AArch64/StandaloneMmCoreEntryPoint.c
- AArch64/SetPermissions.c
- AArch64/CreateHobList.c
+[Sources.AARCH64, Sources.ARM]
+ Arm/StandaloneMmCoreEntryPoint.c
+ Arm/SetPermissions.c
+ Arm/CreateHobList.c

[Sources.X64]
X64/StandaloneMmCoreEntryPoint.c
@@ -34,14 +34,14 @@
MdeModulePkg/MdeModulePkg.dec
StandaloneMmPkg/StandaloneMmPkg.dec

-[Packages.AARCH64]
+[Packages.ARM, Packages.AARCH64]
ArmPkg/ArmPkg.dec

[LibraryClasses]
BaseLib
DebugLib

-[LibraryClasses.AARCH64]
+[LibraryClasses.ARM, LibraryClasses.AARCH64]
StandaloneMmMmuLib
ArmSvcLib

@@ -51,7 +51,7 @@
gEfiStandaloneMmNonSecureBufferGuid
gEfiArmTfCpuDriverEpDescriptorGuid

-[FeaturePcd.AARCH64]
+[FeaturePcd.ARM, FeaturePcd.AARCH64]
gArmTokenSpaceGuid.PcdFfaEnable

[BuildOptions]
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
similarity index 100%
rename from StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c
rename to StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLib.c
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLibInternal.c b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLibInternal.c
similarity index 100%
rename from StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLibInternal.c
rename to StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneMmCoreHobLibInternal.c
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
index a2559920e8..34ed536480 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
@@ -22,7 +22,7 @@
LIBRARY_CLASS = HobLib|MM_CORE_STANDALONE

#
-# VALID_ARCHITECTURES = X64 AARCH64
+# VALID_ARCHITECTURES = X64 AARCH64 ARM
#
[Sources.common]
Common.c
@@ -30,9 +30,9 @@
[Sources.X64]
X64/StandaloneMmCoreHobLib.c

-[Sources.AARCH64]
- AArch64/StandaloneMmCoreHobLib.c
- AArch64/StandaloneMmCoreHobLibInternal.c
+[Sources.AARCH64, Sources.ARM]
+ Arm/StandaloneMmCoreHobLib.c
+ Arm/StandaloneMmCoreHobLibInternal.c

[Packages]
MdePkg/MdePkg.dec
diff --git a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c b/StandaloneMmPkg/Library/StandaloneMmMemLib/ArmStandaloneMmMemLibInternal.c
similarity index 86%
rename from StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
rename to StandaloneMmPkg/Library/StandaloneMmMemLib/ArmStandaloneMmMemLibInternal.c
index 4124959e04..fa7df46413 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/ArmStandaloneMmMemLibInternal.c
@@ -20,6 +20,13 @@
//
extern EFI_PHYSICAL_ADDRESS mMmMemLibInternalMaximumSupportAddress;

+#ifdef MDE_CPU_AARCH64
+#define ARM_PHYSICAL_ADDRESS_BITS 36
+#endif
+#ifdef MDE_CPU_ARM
+#define ARM_PHYSICAL_ADDRESS_BITS 32
+#endif
+
/**
Calculate and save the maximum support address.

@@ -31,7 +38,7 @@ MmMemLibInternalCalculateMaximumSupportAddress (
{
UINT8 PhysicalAddressBits;

- PhysicalAddressBits = 36;
+ PhysicalAddressBits = ARM_PHYSICAL_ADDRESS_BITS;

//
// Save the maximum support address in one global variable
diff --git a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
index 062b0d7a11..b29d97a746 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
@@ -28,7 +28,7 @@
#
# The following information is for reference only and not required by the build tools.
#
-# VALID_ARCHITECTURES = IA32 X64 AARCH64
+# VALID_ARCHITECTURES = IA32 X64 AARCH64 ARM
#

[Sources.Common]
@@ -37,8 +37,8 @@
[Sources.IA32, Sources.X64]
X86StandaloneMmMemLibInternal.c

-[Sources.AARCH64]
- AArch64/StandaloneMmMemLibInternal.c
+[Sources.AARCH64, Sources.ARM]
+ ArmStandaloneMmMemLibInternal.c

[Packages]
MdePkg/MdePkg.dec
diff --git a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
index a2a059c5d6..ffb2a6d083 100644
--- a/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
+++ b/StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
@@ -20,7 +20,7 @@
#
# The following information is for reference only and not required by the build tools.
#
-# VALID_ARCHITECTURES = AARCH64
+# VALID_ARCHITECTURES = AARCH64|ARM
#
#

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc
index 0c45df95e2..01bfbe8cc5 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -20,7 +20,7 @@
PLATFORM_VERSION = 1.0
DSC_SPECIFICATION = 0x00010011
OUTPUT_DIRECTORY = Build/StandaloneMm
- SUPPORTED_ARCHITECTURES = AARCH64|X64
+ SUPPORTED_ARCHITECTURES = AARCH64|X64|ARM
BUILD_TARGETS = DEBUG|RELEASE
SKUID_IDENTIFIER = DEFAULT

@@ -60,7 +60,7 @@
StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
VariableMmDependency|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf

-[LibraryClasses.AARCH64]
+[LibraryClasses.AARCH64, LibraryClasses.ARM]
ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
@@ -118,7 +118,7 @@
StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf

-[Components.AARCH64]
+[Components.AARCH64, Components.ARM]
StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf

@@ -131,7 +131,7 @@
# module style (EDK or EDKII) specified in [Components] section.
#
###################################################################################################
-[BuildOptions.AARCH64]
+[BuildOptions.AARCH64, BuildOptions.ARM]
GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -march=armv8-a+nofp -mstrict-align
GCC:*_*_*_CC_FLAGS = -mstrict-align

--
2.17.1


[PATCH 4/5] StandaloneMmPkg: fix pointer/int casts against 32bit architectures

Etienne Carriere
 

Use intermediate (UINTN) cast when casting int from/to pointer. This
is needed as UINT64 values cast from/to 32bit pointer for 32bit
architectures.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c | 8 ++++----
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c | 14 +++++++-------
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
index 6884095c49..d4590bcd19 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
@@ -164,8 +164,8 @@ StandaloneMmCpuInitialize (

// Share the entry point of the CPU driver
DEBUG ((DEBUG_INFO, "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
- (UINT64) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
- (UINT64) PiMmStandaloneArmTfCpuDriverEntry));
+ (UINTN) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
+ (UINTN) PiMmStandaloneArmTfCpuDriverEntry));
*(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = PiMmStandaloneArmTfCpuDriverEntry;

// Find the descriptor that contains the whereabouts of the buffer for
@@ -180,8 +180,8 @@ StandaloneMmCpuInitialize (
return Status;
}

- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINT64) NsCommBufMmramRange->PhysicalStart));
- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINT64) NsCommBufMmramRange->PhysicalSize));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINTN) NsCommBufMmramRange->PhysicalStart));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINTN) NsCommBufMmramRange->PhysicalSize));

CopyMem (&mNsCommBuffer, NsCommBufMmramRange, sizeof(EFI_MMRAM_DESCRIPTOR));
DEBUG ((DEBUG_INFO, "mNsCommBuffer: 0x%016lx - 0x%lx\n", mNsCommBuffer.CpuStart, mNsCommBuffer.PhysicalSize));
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
index e8fb96bd6e..4d4cf3d5ff 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
@@ -72,14 +72,14 @@ CreateHobListFromBootInfo (

// Create a hoblist with a PHIT and EOH
HobStart = HobConstructor (
- (VOID *) PayloadBootInfo->SpMemBase,
+ (VOID *) (UINTN) PayloadBootInfo->SpMemBase,
(UINTN) PayloadBootInfo->SpMemLimit - PayloadBootInfo->SpMemBase,
- (VOID *) PayloadBootInfo->SpHeapBase,
- (VOID *) (PayloadBootInfo->SpHeapBase + PayloadBootInfo->SpHeapSize)
+ (VOID *) (UINTN) PayloadBootInfo->SpHeapBase,
+ (VOID *) (UINTN) (PayloadBootInfo->SpHeapBase + PayloadBootInfo->SpHeapSize)
);

// Check that the Hoblist starts at the bottom of the Heap
- ASSERT (HobStart == (VOID *) PayloadBootInfo->SpHeapBase);
+ ASSERT (HobStart == (VOID *) (UINTN) PayloadBootInfo->SpHeapBase);

// Build a Boot Firmware Volume HOB
BuildFvHob (PayloadBootInfo->SpImageBase, PayloadBootInfo->SpImageSize);
@@ -190,9 +190,9 @@ CreateHobListFromBootInfo (
MmramRanges[3].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
- MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom - (EFI_PHYSICAL_ADDRESS) HobStart;
+ MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom - (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
MmramRanges[4].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 6c50f470aa..b445d6942e 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -328,7 +328,7 @@ _ModuleEntryPoint (

// Locate PE/COFF File information for the Standalone MM core module
Status = LocateStandaloneMmCorePeCoffData (
- (EFI_FIRMWARE_VOLUME_HEADER *) PayloadBootInfo->SpImageBase,
+ (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PayloadBootInfo->SpImageBase,
&TeData,
&TeDataSize
);
--
2.17.1


[PATCH 3/5] GenGv: Arm: support images entered in Thumb mode

Etienne Carriere
 

Change GenFv for Arm architecture to generate a specific jump
instruction as image entry instruction, when the target entry label
is assembled with Thumb instruction set. This is possible since
SecCoreEntryAddress value fetched from the PE32 as its LSBit set when
the entry instruction executes in Thumb mode.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
BaseTools/Source/C/GenFv/GenFvInternalLib.c | 38 +++++++++++++++-----
1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 6e296b8ad6..3af65146f6 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -34,9 +34,27 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "FvLib.h"
#include "PeCoffLib.h"

-#define ARMT_UNCONDITIONAL_JUMP_INSTRUCTION 0xEB000000
#define ARM64_UNCONDITIONAL_JUMP_INSTRUCTION 0x14000000

+/*
+ * Arm instruction to jump to Fv enry instruction in Arm or Thumb mode.
+ * From ARM Arch Ref Manual versions b/c/d, section A8.8.25 BL, BLX (immediate)
+ * BLX (encoding A2) branches to offset in Thumb instruction set mode.
+ * BL (encoding A1) branches to offset in Arm instruction set mode.
+ */
+#define ARM_JUMP_OFFSET_MAX 0xffffff
+#define ARM_JUMP_TO_ARM(Offset) (0xeb000000 | ((Offset - 8) >> 2))
+
+#define _ARM_JUMP_TO_THUMB(Imm32) (0xfa000000 | \
+ (((Imm32) & (1 << 1)) << (24 - 1)) | \
+ (((Imm32) >> 2) & 0x7fffff))
+#define ARM_JUMP_TO_THUMB(Offset) _ARM_JUMP_TO_THUMB((Offset) - 8)
+
+/*
+ * Arm instruction to retrun from exception (MOVS PC, LR)
+ */
+#define ARM_RETURN_FROM_EXCEPTION 0xE1B0F07E
+
BOOLEAN mArm = FALSE;
BOOLEAN mRiscV = FALSE;
STATIC UINT32 MaxFfsAlignment = 0;
@@ -2203,23 +2221,25 @@ Returns:
// if we found an SEC core entry point then generate a branch instruction
// to it and populate a debugger SWI entry as well
if (UpdateVectorSec) {
+ UINT32 EntryOffset;

VerboseMsg("UpdateArmResetVectorIfNeeded updating ARM SEC vector");

- // B SecEntryPoint - signed_immed_24 part +/-32MB offset
- // on ARM, the PC is always 8 ahead, so we're not really jumping from the base address, but from base address + 8
- ResetVector[0] = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress - 8) >> 2;
+ EntryOffset = (INT32)(SecCoreEntryAddress - FvInfo->BaseAddress);

- if (ResetVector[0] > 0x00FFFFFF) {
- Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 32MB of the start of the FV");
+ if (EntryOffset > ARM_JUMP_OFFSET_MAX) {
+ Error(NULL, 0, 3000, "Invalid", "SEC Entry point offset above 1MB of the start of the FV");
return EFI_ABORTED;
}

- // Add opcode for an unconditional branch with no link. i.e.: " B SecEntryPoint"
- ResetVector[0] |= ARMT_UNCONDITIONAL_JUMP_INSTRUCTION;
+ if (SecCoreEntryAddress & 1) {
+ ResetVector[0] = ARM_JUMP_TO_THUMB(EntryOffset);
+ } else {
+ ResetVector[0] = ARM_JUMP_TO_ARM(EntryOffset);
+ }

// SWI handler movs pc,lr. Just in case a debugger uses SWI
- ResetVector[2] = 0xE1B0F07E;
+ ResetVector[2] = ARM_RETURN_FROM_EXCEPTION;

// Place holder to support a common interrupt handler from ROM.
// Currently not supported. For this to be used the reset vector would not be in this FV
--
2.17.1


[PATCH 2/5] ArmPkg: prepare 32bit ARM build of StandaloneMmPkg

Etienne Carriere
 

Changes in ArmPkg to prepare building StandaloneMm firmware for
32bit Arm architectures.

Adds MmCommunicationDxe driver and ArmMmuPeiLib and
ArmmmuStandaloneMmLib libraries to the list of the standard
components build for ArmPkg on when ARM architectures.

Changes path of source file AArch64/ArmMmuStandaloneMmLib.c
and compile it for both 32bit and 64bit architectures.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
ArmPkg/ArmPkg.dec | 2 +-
ArmPkg/ArmPkg.dsc | 2 +-
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 2 +-
ArmPkg/Library/StandaloneMmMmuLib/{AArch64 => }/ArmMmuStandaloneMmLib.c | 15 ++++++++-------
ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf | 6 +++---
5 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 214b2f5892..6ed51edd03 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -137,7 +137,7 @@
# hardware coherency (i.e., no virtualization or cache coherent DMA)
gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride|FALSE|BOOLEAN|0x00000043

-[PcdsFeatureFlag.AARCH64]
+[PcdsFeatureFlag.AARCH64, PcdsFeatureFlag.ARM]
## Used to select method for requesting services from S-EL1.<BR><BR>
# TRUE - Selects FF-A calls for communication between S-EL0 and SPMC.<BR>
# FALSE - Selects SVC calls for communication between S-EL0 and SPMC.<BR>
diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 926986cf7f..4c79dadf9e 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -158,7 +158,7 @@
ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf

-[Components.AARCH64]
+[Components.AARCH64, Components.ARM]
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index b1e3095809..4ae38a9f22 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -125,7 +125,7 @@ MmCommunication2Communicate (
}

// SMC Function ID
- CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
+ CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE;

// Cookie
CommunicateSmcArgs.Arg1 = 0;
diff --git a/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
similarity index 92%
rename from ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
rename to ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
index dd014beec8..20f873e680 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c
+++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
@@ -2,6 +2,7 @@
File managing the MMU for ARMv8 architecture in S-EL0

Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
+ Copyright (c) 2021, Linaro Limited
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Reference(s):
@@ -62,7 +63,7 @@ SendMemoryPermissionRequest (
// for other Direct Request calls which are not atomic
// We therefore check only for Direct Response by the
// callee.
- if (SvcArgs->Arg0 == ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
+ if (SvcArgs->Arg0 == ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP) {
// A Direct Response means FF-A success
// Now check the payload for errors
// The callee sends back the return value
@@ -164,13 +165,13 @@ GetMemoryPermissions (
ZeroMem (&SvcArgs, sizeof (ARM_SVC_ARGS));
if (FeaturePcdGet (PcdFfaEnable)) {
// See [2], Section 10.2 FFA_MSG_SEND_DIRECT_REQ.
- SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
+ SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ;
SvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID;
SvcArgs.Arg2 = 0;
- SvcArgs.Arg3 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
+ SvcArgs.Arg3 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES;
SvcArgs.Arg4 = BaseAddress;
} else {
- SvcArgs.Arg0 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
+ SvcArgs.Arg0 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES;
SvcArgs.Arg1 = BaseAddress;
SvcArgs.Arg2 = 0;
SvcArgs.Arg3 = 0;
@@ -219,15 +220,15 @@ RequestMemoryPermissionChange (
ZeroMem (&SvcArgs, sizeof (ARM_SVC_ARGS));
if (FeaturePcdGet (PcdFfaEnable)) {
// See [2], Section 10.2 FFA_MSG_SEND_DIRECT_REQ.
- SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
+ SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ;
SvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID;
SvcArgs.Arg2 = 0;
- SvcArgs.Arg3 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
+ SvcArgs.Arg3 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES;
SvcArgs.Arg4 = BaseAddress;
SvcArgs.Arg5 = EFI_SIZE_TO_PAGES (Length);
SvcArgs.Arg6 = Permissions;
} else {
- SvcArgs.Arg0 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
+ SvcArgs.Arg0 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES;
SvcArgs.Arg1 = BaseAddress;
SvcArgs.Arg2 = EFI_SIZE_TO_PAGES (Length);
SvcArgs.Arg3 = Permissions;
diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
index 6c71fe0023..ff20e58980 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
+++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
@@ -16,14 +16,14 @@
LIBRARY_CLASS = StandaloneMmMmuLib
PI_SPECIFICATION_VERSION = 0x00010032

-[Sources.AARCH64]
- AArch64/ArmMmuStandaloneMmLib.c
+[Sources]
+ ArmMmuStandaloneMmLib.c

[Packages]
ArmPkg/ArmPkg.dec
MdePkg/MdePkg.dec

-[FeaturePcd.AARCH64]
+[FeaturePcd.ARM, FeaturePcd.AARCH64]
gArmTokenSpaceGuid.PcdFfaEnable

[LibraryClasses]
--
2.17.1


[PATCH 1/5] ArmPkg/IndustryStandard: 32b/64b agnostic FF-A and Mm SVC IDs

Etienne Carriere
 

Defines ARM_SVC_ID_FFA_* and ARM_SVC_ID_SP_* identifiers for 32bit
function IDs as per SMCCC specification. Defines also generic ARM
SVC identifier macros to wrap 32bit or 64bit identifiers upon target
built architecture.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 12 ++++++++++++
ArmPkg/Include/IndustryStandard/ArmMmSvc.h | 15 +++++++++++++++
2 files changed, 27 insertions(+)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 65b8343ade..ebcb54b28b 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -17,9 +17,21 @@
#define ARM_FFA_SVC_H_

#define ARM_SVC_ID_FFA_VERSION_AARCH32 0x84000063
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32 0x8400006F
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32 0x84000070
#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64 0xC400006F
#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64 0xC4000070

+/* Generic IDs when using AArch32 or AArch64 execution state */
+#ifdef MDE_CPU_AARCH64
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64
+#endif
+#ifdef MDE_CPU_ARM
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32
+#endif
+
#define SPM_MAJOR_VERSION_FFA 1
#define SPM_MINOR_VERSION_FFA 0

diff --git a/ArmPkg/Include/IndustryStandard/ArmMmSvc.h b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
index 33d60ccf17..deb3bc99d2 100644
--- a/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
@@ -15,10 +15,25 @@
* privileged operations on its behalf.
*/
#define ARM_SVC_ID_SPM_VERSION_AARCH32 0x84000060
+#define ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH32 0x84000061
+#define ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH32 0x84000064
+#define ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH32 0x84000065
#define ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64 0xC4000061
#define ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64 0xC4000064
#define ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64 0xC4000065

+/* Generic IDs when using AArch32 or AArch64 execution state */
+#ifdef MDE_CPU_AARCH64
+#define ARM_SVC_ID_SP_EVENT_COMPLETE ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64
+#define ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64
+#define ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64
+#endif
+#ifdef MDE_CPU_ARM
+#define ARM_SVC_ID_SP_EVENT_COMPLETE ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH32
+#define ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH32
+#define ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH32
+#endif
+
#define SET_MEM_ATTR_DATA_PERM_MASK 0x3
#define SET_MEM_ATTR_DATA_PERM_SHIFT 0
#define SET_MEM_ATTR_DATA_PERM_NO_ACCESS 0
--
2.17.1


[PATCH 0/5] Arm 32bit support in StandaloveMm

Etienne Carriere
 

This series targets building StandaloneMM package for 32bit ARM
architectures. The main parts of the changes move AArch64/*
files to paths suitable for both 32bit and 64bit Arm machines.

Because these changes move file paths, some other edk2 related
source trees must be updated. Up to my knowledge, only edk2-platforms
and edk2-codereview must be synchronised. [1] shows the changes
needed in edk2-platforms to synchronise with these changes.

I will post a patch series for edk2-platforms, including [1] and
some other changes to allow building StandaloneMmPkg for 32bit Arm
platforms.

This change is a resend of [2], rebased to latest master.

[1] https://github.com/etienne-lms/edk2-platforms/commit/becbd185167f6a115dbd3895bc6071ab04e44d9b
[2] https://edk2.groups.io/g/devel/message/72834 (with 72835, 72836, 72837 and 72838)

Etienne Carriere (5):
ArmPkg/IndustryStandard: 32b/64b agnostic FF-A and Mm SVC IDs
ArmPkg: prepare 32bit ARM build of StandaloneMmPkg
GenGv: Arm: support images entered in Thumb mode
StandaloneMmPkg: fix pointer/int casts against 32bit architectures
StandaloneMmPkg: build for 32bit arm machines

ArmPkg/ArmPkg.dec | 2 +-
ArmPkg/ArmPkg.dsc | 2 +-
.../MmCommunicationDxe/MmCommunication.c | 2 +-
ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 12 ++++++
ArmPkg/Include/IndustryStandard/ArmMmSvc.h | 15 ++++++++
.../{AArch64 => }/ArmMmuStandaloneMmLib.c | 15 ++++----
.../ArmMmuStandaloneMmLib.inf | 6 +--
BaseTools/Source/C/GenFv/GenFvInternalLib.c | 38 ++++++++++++++-----
StandaloneMmPkg/Core/StandaloneMmCore.inf | 2 +-
.../{AArch64 => }/EventHandle.c | 12 +++++-
.../{AArch64 => }/StandaloneMmCpu.c | 10 ++---
.../{AArch64 => }/StandaloneMmCpu.h | 0
.../{AArch64 => }/StandaloneMmCpu.inf | 0
.../StandaloneMmCoreEntryPoint.h | 0
.../{AArch64 => Arm}/CreateHobList.c | 16 ++++----
.../{AArch64 => Arm}/SetPermissions.c | 2 +-
.../StandaloneMmCoreEntryPoint.c | 18 ++++-----
.../StandaloneMmCoreEntryPoint.inf | 14 +++----
.../{AArch64 => Arm}/StandaloneMmCoreHobLib.c | 0
.../StandaloneMmCoreHobLibInternal.c | 0
.../StandaloneMmCoreHobLib.inf | 8 ++--
...rnal.c => ArmStandaloneMmMemLibInternal.c} | 9 ++++-
.../StandaloneMmMemLib/StandaloneMmMemLib.inf | 6 +--
.../VariableMmDependency.inf | 2 +-
StandaloneMmPkg/StandaloneMmPkg.dsc | 8 ++--
25 files changed, 131 insertions(+), 68 deletions(-)
rename ArmPkg/Library/StandaloneMmMmuLib/{AArch64 => }/ArmMmuStandaloneMmLib.c (92%)
rename StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/EventHandle.c (92%)
rename StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/StandaloneMmCpu.c (94%)
rename StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/StandaloneMmCpu.h (100%)
rename StandaloneMmPkg/Drivers/StandaloneMmCpu/{AArch64 => }/StandaloneMmCpu.inf (100%)
rename StandaloneMmPkg/Include/Library/{AArch64 => Arm}/StandaloneMmCoreEntryPoint.h (100%)
rename StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/{AArch64 => Arm}/CreateHobList.c (91%)
rename StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/{AArch64 => Arm}/SetPermissions.c (96%)
rename StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/{AArch64 => Arm}/StandaloneMmCoreEntryPoint.c (94%)
rename StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{AArch64 => Arm}/StandaloneMmCoreHobLib.c (100%)
rename StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{AArch64 => Arm}/StandaloneMmCoreHobLibInternal.c (100%)
rename StandaloneMmPkg/Library/StandaloneMmMemLib/{AArch64/StandaloneMmMemLibInternal.c => ArmStandaloneMmMemLibInternal.c} (86%)

--
2.17.1


Re: [PATCH RFC v2 04/28] MdePkg: Define the Page State Change VMGEXIT structures

Lendacky, Thomas
 

On 5/4/21 8:59 AM, Laszlo Ersek wrote:
On 05/04/21 14:33, 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%7Cthomas.lendacky%40amd.com%7Cf400bca14b6f4f138a1908d90f05090f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557336582189771%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=bjXNVWGpurRGUZkjemvDQR%2FYnEQRG9ENN22jUjtkNP0%3D&amp;reserved=0

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@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>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 15 ++++++++++
MdePkg/Include/Register/Amd/Ghcb.h | 29 ++++++++++++++++++++
2 files changed, 44 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index e19bd04b6c..432cee2feb 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;
This matches section 2.3.1 in rev 2.00.

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
Matches section 2.3.1.

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 2d64a4c28f..1e7c0daed3 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
Matches "Table 5. List of Supported Non-Automatic Events".

@@ -160,4 +161,32 @@ 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
(1) Comment style.

+#define SNP_PAGE_STATE_MAX_NPAGES 4095
+#define SNP_PAGE_STATE_MAX_ENTRY 253
+#define SNP_PAGE_STATE_PRIVATE 1
+#define SNP_PAGE_STATE_SHARED 2
+#define SNP_PAGE_STATE_PSMASH 3
+#define SNP_PAGE_STATE_UNSMASH 4
(2) The PSMASH and UNSMASH operations are not documented in the rev 2.00
spec, in the GHCB MSR protocol. That's probably because PSMASH and
UNSMASH can only be defined in terms of 2MB pages, and
GHCB_INFO_SNP_PAGE_STATE_CHANGE_REQUEST is suitable only for individual,
4KB pages. I think it would be useful to point out somehow here that
PSMASH and UNSMASH are restricted to the GHCB shared area protocol
(perhaps extend the leading comment on this block of macros).

(3) I don't understand what "MAX_NPAGES" stands for (4095). The rest of
the series never uses the macro, and I can't associate it with anything
from the spec. If the macro is supposed to relate to the 4KB / 2MB page
smashing / splitting, then its replacement text should be 512. Unless
the macro corresponds to a definition in the spec, I think we should
drop it.

+
+typedef PACKED struct {
+ UINT64 CurrentPage:12;
+ UINT64 GuestFrameNumber:40;
+ UINT64 Op:4;
+ UINT64 PageSize:1;
+ UINT64 Rsvd: 7;
+} SNP_PAGE_STATE_ENTRY;
+
+typedef PACKED struct {
+ UINT16 CurrentEntry;
+ UINT16 EndEntry;
+ UINT32 Rsvd;
+} SNP_PAGE_STATE_HEADER;
(4) We tend to write

#pragma pack (1)
...
#pragma pack ()

rather than PACKED -- but anyway, is packing really necessary? "Natural
alignment" is required in edk2. I'm OK with packing, but I think the
pragma is the preferred form.

(5) Please spell out both "Rsvd" fields above as "Reserved".

(6) Stray space character in "Rsvd: 7".

(7) The field name "Op" is inconsistent with the other field name
"Operation".

(8) I think there is a bug (typo) in the rev 2.00 spec, in 4.1.6 "SNP
Page State Change": it says

... calculated from the supplied guest physical frame number (GFN) for
the requested page size (GPA = GFN << 12).

But, if you can choose 2MB page size in the request, then the (GPA = GFN
<< 12) formula is not g
Sorry, unfinished sentence: I meant that the formula was not generally
correct.
Actually, for any page size, the GPA for any GFN is GFN << 12.

For the SNP Page State Change NAE event, it is up to the hypervisor to
ensure that the GFN/GPA supplied is aligned appropriately for the
requested page size (see 4.1.6 of the GHCB spec where the page operations
are defined). A GFN is naturally 4K aligned, so only a 2MB page size needs
GFN/GPA alignment validation.

For the SNP Page State Change MSR protocol, only a 4K page size is supported.

Thanks,
Tom


Thanks
Laszlo


(9) If my understanding of the spec is correct, "EndEntry" has
*inclusive* meaning. That's unusual. Any particular reason for not
making "EndEntry" exclusive (in the spec)?

+
+typedef struct {
+ SNP_PAGE_STATE_HEADER Header;
+ SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
+} SNP_PAGE_STATE_CHANGE_INFO;
+
#endif
Yes, this looks OK. Size is 2+2+4+253*8 = 2032 bytes, which matches the
size of GHCB.SharedBuffer.

(10) However, *if* you decide to declare SNP_PAGE_STATE_ENTRY and
SNP_PAGE_STATE_HEADER explicitly as packed, then you should do the same
for SNP_PAGE_STATE_CHANGE_INFO.

(11) Like I mentioned earlier, it's probably helpful if you start the
subject line with

MdePkg/Register/Amd: ...

on all of these MdePkg patches. If that becomes too tight, for some of
the MdePkg patches, then I suggest "MdePkg/Amd: ..." (i.e., drop
"Register").

Thanks
Laszlo


Re: [PATCH RFC v2 05/28] MdePkg: Add AsmPvalidate() support

Laszlo Ersek
 

On 05/04/21 15:58, Laszlo Ersek wrote:

The leading comment block of the function is supposed to explain these
associations:

@retval EFI_SUCCESS Successful completion (regardless of
whether the Validated bit changed state).
@retval INVALID_PARAMETER Invalid input parameters (FAIL_INPUT).
@retval EFI_UNSUPPORTED Page size mismatch between guest (2M) and
RMP entry (4K) (FAIL_SIZEMISMATCH).
Apologies, that should have been "EFI_INVALID_PARAMETER", not just
"INVALID_PARAMETER".

Thanks
Laszlo


Re: [PATCH RFC v2 04/28] MdePkg: Define the Page State Change VMGEXIT structures

Laszlo Ersek
 

On 05/04/21 14:33, Laszlo Ersek wrote:
On 04/30/21 13:51, Brijesh Singh wrote:
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@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>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 15 ++++++++++
MdePkg/Include/Register/Amd/Ghcb.h | 29 ++++++++++++++++++++
2 files changed, 44 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index e19bd04b6c..432cee2feb 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;
This matches section 2.3.1 in rev 2.00.

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
Matches section 2.3.1.

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 2d64a4c28f..1e7c0daed3 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
Matches "Table 5. List of Supported Non-Automatic Events".

@@ -160,4 +161,32 @@ 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
(1) Comment style.

+#define SNP_PAGE_STATE_MAX_NPAGES 4095
+#define SNP_PAGE_STATE_MAX_ENTRY 253
+#define SNP_PAGE_STATE_PRIVATE 1
+#define SNP_PAGE_STATE_SHARED 2
+#define SNP_PAGE_STATE_PSMASH 3
+#define SNP_PAGE_STATE_UNSMASH 4
(2) The PSMASH and UNSMASH operations are not documented in the rev 2.00
spec, in the GHCB MSR protocol. That's probably because PSMASH and
UNSMASH can only be defined in terms of 2MB pages, and
GHCB_INFO_SNP_PAGE_STATE_CHANGE_REQUEST is suitable only for individual,
4KB pages. I think it would be useful to point out somehow here that
PSMASH and UNSMASH are restricted to the GHCB shared area protocol
(perhaps extend the leading comment on this block of macros).

(3) I don't understand what "MAX_NPAGES" stands for (4095). The rest of
the series never uses the macro, and I can't associate it with anything
from the spec. If the macro is supposed to relate to the 4KB / 2MB page
smashing / splitting, then its replacement text should be 512. Unless
the macro corresponds to a definition in the spec, I think we should
drop it.

+
+typedef PACKED struct {
+ UINT64 CurrentPage:12;
+ UINT64 GuestFrameNumber:40;
+ UINT64 Op:4;
+ UINT64 PageSize:1;
+ UINT64 Rsvd: 7;
+} SNP_PAGE_STATE_ENTRY;
+
+typedef PACKED struct {
+ UINT16 CurrentEntry;
+ UINT16 EndEntry;
+ UINT32 Rsvd;
+} SNP_PAGE_STATE_HEADER;
(4) We tend to write

#pragma pack (1)
...
#pragma pack ()

rather than PACKED -- but anyway, is packing really necessary? "Natural
alignment" is required in edk2. I'm OK with packing, but I think the
pragma is the preferred form.

(5) Please spell out both "Rsvd" fields above as "Reserved".

(6) Stray space character in "Rsvd: 7".

(7) The field name "Op" is inconsistent with the other field name
"Operation".

(8) I think there is a bug (typo) in the rev 2.00 spec, in 4.1.6 "SNP
Page State Change": it says

... calculated from the supplied guest physical frame number (GFN) for
the requested page size (GPA = GFN << 12).

But, if you can choose 2MB page size in the request, then the (GPA = GFN
<< 12) formula is not g
Sorry, unfinished sentence: I meant that the formula was not generally
correct.

Thanks
Laszlo


(9) If my understanding of the spec is correct, "EndEntry" has
*inclusive* meaning. That's unusual. Any particular reason for not
making "EndEntry" exclusive (in the spec)?

+
+typedef struct {
+ SNP_PAGE_STATE_HEADER Header;
+ SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
+} SNP_PAGE_STATE_CHANGE_INFO;
+
#endif
Yes, this looks OK. Size is 2+2+4+253*8 = 2032 bytes, which matches the
size of GHCB.SharedBuffer.

(10) However, *if* you decide to declare SNP_PAGE_STATE_ENTRY and
SNP_PAGE_STATE_HEADER explicitly as packed, then you should do the same
for SNP_PAGE_STATE_CHANGE_INFO.

(11) Like I mentioned earlier, it's probably helpful if you start the
subject line with

MdePkg/Register/Amd: ...

on all of these MdePkg patches. If that becomes too tight, for some of
the MdePkg patches, then I suggest "MdePkg/Amd: ..." (i.e., drop
"Register").

Thanks
Laszlo


Re: [PATCH RFC v2 05/28] MdePkg: Add AsmPvalidate() support

Laszlo Ersek
 

On 04/30/21 13:51, 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>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
MdePkg/Include/Library/BaseLib.h | 37 +++++++++++++++++
MdePkg/Library/BaseLib/BaseLib.inf | 1 +
MdePkg/Library/BaseLib/X64/Pvalidate.nasm | 43 ++++++++++++++++++++
3 files changed, 81 insertions(+)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 7253997a6f..92ce695e93 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -7518,5 +7518,42 @@ PatchInstructionX86 (
IN UINTN ValueSize
);

+/**
+ Execute a PVALIDATE instruction to validate or rescnids validation of a guest
(1) typo: "rescnids"


+ page's RMP entry.
+
+ Upon completion, in addition to the return value the instruction also updates
+ the eFlags. A caller must check both the return code as well as eFlags to
+ determine if the RMP entry has been updated.
+
+ The function is available on x64.
(2) Please write "X64"; that's how the architecture is usually mentioned
in both the UEFI spec and in edk2.


+
+ @param[in] Address The guest virtual address to validate.
+ @param[in] PageSize The page size to use.
+ @param[i] Validate Validate or rescinds.
+ @param[out] Eflags The value of Eflags after PVALIDATE completion.
(3) Typo: "[i]" should be "[in]".


(4) The order of parameters listed in this comment block differs from
the actual parameter list.

The ECC plugin of the edk2 CI will catch this issue anyway. So, before
submitting the patch set to the list, please submit a personal PR on
github.com against the main repo, just to run CI on your patches.


+
+ @retval PvalidateRetValue The return value from the PVALIDATE instruction.
More on the return value / type later, below.

+**/
+typedef enum {
+ PvalidatePageSize4K = 0,
+ PvalidatePageSize2MB,
+} PVALIDATE_PAGE_SIZE;
+
+typedef enum {
+ PvalidateRetSuccess = 0,
+ PvalidateRetFailInput = 1,
+ PvalidateRetFailSizemismatch = 6,
+} PVALIDATE_RET_VALUE;
+
(5) These typedefs do not belong between the function leading comment
and the function declaration. Please hoist the typedefs just above the
leading comment, and add a separate comment for the typedefs -- using
the proper comment style for typedefs, too.


+PVALIDATE_RET_VALUE
(6) In my opinion, using an enum for an EFIAPI function's return type is
problematic. According to the UEFI spec (v2.9), "Table 2-3 Common UEFI
Data Types", <Enumerated Type> may correspond to INT32 or UINT32. I
don't like that ambiguity here. The spec also says that such types
should never be used at least as structure fields.

I'm perfectly fine with functions in standard (ISO) C programs returning
enums, but I think the situation is less clear in UEFI. I don't recall
standard interfaces (spec-level, or even edk2 / MdePkg interfaces) that
return enums.

I suggest the following instead. Drop the PVALIDATE_RET_VALUE enum
altogether. Specify EFI_STATUS as the return type, in the declaration of
the function.

In the UEFI spec, Appendix D specifies the numeric values of the status
codes. Furthermore, there are examples for NASM sources using EFI_*
status codes *numerically* in edk2; minimally:

- IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
- IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32/SecEntry.nasm

Thus, please modify the assembly source code in this patch to return
EFI_SUCCESS (already value 0, conveniently) if the instruction succeeds.

Return EFI_INVALID_PARAMETER (0x8000_0002) in case the instruction fails
with error code 1 (FAIL_INPUT).

Return EFI_UNSUPPORTED (0x8000_0003), or even EFI_NO_MAPPING
(0x8000_0017), for value 6 (FAIL_SIZEMISMATCH).

The leading comment block of the function is supposed to explain these
associations:

@retval EFI_SUCCESS Successful completion (regardless of
whether the Validated bit changed state).
@retval INVALID_PARAMETER Invalid input parameters (FAIL_INPUT).
@retval EFI_UNSUPPORTED Page size mismatch between guest (2M) and
RMP entry (4K) (FAIL_SIZEMISMATCH).

(Passing in the PVALIDATE_PAGE_SIZE enum, as a parameter, should be
fine, BTW)


(7) According to the AMD APM, "Support for this instruction is indicated
by CPUID Fn8000_001F_EAX[SNP]=1".

Presumably, if the (physical, or emulated) hardware does not support
PVALIDATE, an #UD is raised. That condition should be explained in the
function's leading comment. (Mention the CPUID and the #UD, I guess.)


+EFIAPI
+AsmPvalidate (
+ IN PVALIDATE_PAGE_SIZE PageSize,
+ IN BOOLEAN Validate,
+ IN UINTN Address,
(8) This should be EFI_PHYSICAL_ADDRESS, not UINTN.


+ OUT IA32_EFLAGS32 *Eflags
+ );
+
#endif // defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
#endif // !defined (__BASE_LIB__)
(9) Unless you foresee particular uses for eflags *other than* CF, I
would suggest replacing the Eflags output parameter with

OUT BOOLEAN *RmpEntryUpdated

The function would still only have 4 parameters, which shouldn't be
difficult to handle in the assembly implementation (i.e. write to the
UINT8 (= BOOLEAN) object referenced by "RmpEntryUpdated"). EFIAPI means
that the first four params are passed in RCX, RDX, R8, R9.

Thus far, I can see only one AsmPvalidate() call: in IssuePvalidate(),
from patch #21 ("OvmfPkg/MemEncryptSevLib: Add support to validate
system RAM"). And there, CF looks sufficient.


(10) The instruction is X64 only, but you are providing the declaration
even if MDE_CPU_IA32 is #defined. That seems wrong; even the declaration
should be invisible in that case. Please declare the function for
MDE_CPU_X64 only.


diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index b76f3af380..d33b4a8f7d 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -321,6 +321,7 @@
X64/XGetBv.nasm
X64/XSetBv.nasm
X64/VmgExit.nasm
+ X64/Pvalidate.nasm
ChkStkGcc.c | GCC

[Sources.EBC]
(11) This list of source files is already not sorted alphabetically,
unfortunately. But we can still do better than this: I suggest inserting
"X64/Pvalidate.nasm" just before "X64/RdRand.nasm".


(12) Your git setup seems less than ideal for formatting edk2 patches.
The @@ hunk header above does not show the INF file section being
modified. It should look something like this:

@@ -317,6 +317,7 @@ [Sources.X64]
^^^^^^^^^^^^^

Please run the "BaseTools/Scripts/SetupGit.py" script in your working
tree.

Alternatively, please see "xfuncname" at
<https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05>.

This is of course not a bug in the patch, but fixing your setup will
help with the next round of review.


diff --git a/MdePkg/Library/BaseLib/X64/Pvalidate.nasm b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
new file mode 100644
index 0000000000..f2aba114ac
--- /dev/null
+++ b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
@@ -0,0 +1,43 @@
+;-----------------------------------------------------------------------------
+;
+; Copyright (c) 2020-2021, AMD. All rights reserved.<BR>
(13) I believe we don't introduce new files with copyright notices
referring to the past. IOW, I think you should only say "2021" here.


+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+; Pvalidate.Asm
+;
+; Abstract:
+;
+; AsmPvalidate function
+;
+; Notes:
+;
(14) I defer to the MdePkg maintainers on this, but "Module Name" is
plain wrong, and the Abstract is useless. Either fix those up please
("Abstract" could be a copy of the corrected leading comment block), or
just drop them both.


+;-----------------------------------------------------------------------------
+
+ SECTION .text
+
+;-----------------------------------------------------------------------------
+; PvalidateRetValue
+; EFIAPI
+; AsmPvalidate (
+; IN UINT32 RmpPageSize
+; IN UINT32 Validate,
+; IN UINTN Address,
+; OUT UINTN *Eflags,
+; )
+;-----------------------------------------------------------------------------
(15) Please update this accordingly to the corrected function
specification.


+global ASM_PFX(AsmPvalidate)
+ASM_PFX(AsmPvalidate):
+ mov rax, r8
+
+ ; PVALIDATE instruction opcode
+ DB 0xF2, 0x0F, 0x01, 0xFF
This is bad practice; we make every effort to avoid DB-encoded
instructions.

We have two PVALIDATE instances in the patch set (... that I can see
immediateyl); the first here, and the other in
"OvmfPkg/ResetVector/Ia32/PageTables64.asm" (from patch #17,
"OvmfPkg/ResetVector: Invalidate the GHCB page"). Therefore, hiding the
encoding of PVALIDATE behind a NASM macro definitely makes sense.

(16a) Please file a NASM feature request for PVALIDATE at
<https://bugzilla.nasm.us>.

(16b) In the present MdePkg patch, please extend the file

MdePkg/Include/X64/Nasm.inc

as follows:

diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc
index 527f71e9eb4d..ff37f1e35707 100644
--- a/MdePkg/Include/X64/Nasm.inc
+++ b/MdePkg/Include/X64/Nasm.inc
@@ -33,6 +33,15 @@
DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8
%endmacro

+;
+; Macro for the PVALIDATE instruction, defined in AMD publication #24594
+; revision 3.32. NASM feature request URL:
+; <https://bugzilla.nasm.us/show_bug.cgi?id=FIXME>.
+;
+%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
(16c) Please replace the FIXME placeholder above with the actual NASM BZ
number (from (16a)).

(16d) In the "MdePkg/Library/BaseLib/X64/Pvalidate.nasm" source file,
and also (later) in the "OvmfPkg/ResetVector/Ia32/PageTables64.asm"
source file, please use the PVALIDATE macro, in place of the naked DBs.


Back to your patch:

On 04/30/21 13:51, Brijesh Singh wrote:
+
+ ; Read the Eflags
+ pushfq
+ pop r8
+ mov [r9], r8
+
+ ; The PVALIDATE instruction returns the status in rax register.
+ ret
(17) The assembly code should be updated to match the new interface
contract (parameter order, parameter types, return values).


I'll continue reviewing the series later this week (hopefully tomorrow).

Thanks,
Laszlo


Booting UEFI with QEMU arm

Pintu Agarwal <pintu.ping@...>
 

Hi,
Is it possible to boot UEFI on a qemu arm based emulator?
Since I am working from home and my uefi related work leads to device crash many times, which becomes painful for me.

So I wanted to do all my experiments on a qemu environment before I verify the final changes on a real hardware.
So, I am just wondering is this possible?

According to my search I see its possible but still I could not find a proper way to do it.
Please help me if someone has already tried it.

I already have the qemu-arm setup on my Linux/Ubuntu PC.


Thanks,
Pintu

15181 - 15200 of 89854