Provide SEV-SNP support for running under an SVSM

During early boot phases, check for the presence of an SVSM when running as an SEV-SNP guest.

An SVSM is present if not running at VMPL0 and the 64-bit value at offset 0x148 into the secrets page is non-zero. If an SVSM is present, save the SVSM Calling Area address (CAA), located at offset 0x150 into the secrets page, and set the VMPL level of the guest, which should be non-zero, to indicate the presence of an SVSM.

Signed-off-by Tom Lendacky <[email protected]>

---
 .../arch/x86/amd-memory-encryption.rst        | 22 ++++++
 arch/x86/boot/compressed/sev.c                |  8 +++
 arch/x86/include/asm/sev-common.h             |  4 ++
 arch/x86/include/asm/sev.h                    | 25 ++++++-
 arch/x86/kernel/sev-shared.c                  | 70 +++++++++++++++++++
 arch/x86/kernel/sev.c                         |  7 ++
 6 files changed, 135 insertions(+), 1 deletion(-)

Documentation/arch/x86/amd-memory-encryption.rst +0 -0

index 414bc7402ae7..32737718d4a2 100644
@@ -130,4 +130,26 @@ SNP feature support.
 
 More details in AMD64 APM[1] Vol 2: 15.34.10 SEV_STATUS MSR

arch/x86/boot/compressed/sev.c +0 -0

 #include <asm/bootparam.h>
 #include <asm/pgtable_types.h>
 #include <asm/sev.h>

arch/x86/boot/compressed/sev.c +10 -0

+static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
+{
+	struct snp_secrets_page *secrets_page;
+	u64 caa;
+
+	BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
+
+	/*
+	 * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
+	 * higher) privilege level. Here, clear the VMPL1 permission mask of the

arch/x86/boot/compressed/sev.c +6 -0

+	if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
+		return;
+
+	/*
+	 * Not running at VMPL0, ensure everything has been properly supplied
+	 * for running under an SVSM.

Quoted 26 lines · Tom Lendacky

During early boot phases, check for the presence of an SVSM when running as an SEV-SNP guest.

An SVSM is present if not running at VMPL0 and the 64-bit value at offset 0x148 into the secrets page is non-zero. If an SVSM is present, save the SVSM Calling Area address (CAA), located at offset 0x150 into the secrets page, and set the VMPL level of the guest, which should be non-zero, to indicate the presence of an SVSM.

Signed-off-by Tom Lendacky <[email protected]>

---
  .../arch/x86/amd-memory-encryption.rst        | 22 ++++++
  arch/x86/boot/compressed/sev.c                |  8 +++
  arch/x86/include/asm/sev-common.h             |  4 ++
  arch/x86/include/asm/sev.h                    | 25 ++++++-
  arch/x86/kernel/sev-shared.c                  | 70 +++++++++++++++++++
  arch/x86/kernel/sev.c                         |  7 ++
  6 files changed, 135 insertions(+), 1 deletion(-)

Documentation/arch/x86/amd-memory-encryption.rst +0 -0

index 414bc7402ae7..32737718d4a2 100644
@@ -130,4 +130,26 @@ SNP feature support.
 
  More details in AMD64 APM[1] Vol 2: 15.34.10 SEV_STATUS MSR

Documentation/arch/x86/amd-memory-encryption.rst SNP feature support. · line 141

Quoted 10 lines

Documentation/arch/x86/amd-memory-encryption.rst +9 -0

 
+Secure VM Service Module (SVSM)
+===============================
+
+SNP provides a feature called Virtual Machine Privilege Levels (VMPL). The most
+privileged VMPL is 0 with numerically higher numbers having lesser privileges.
+More details in AMD64 APM[1] Vol 2: 15.35.7 Virtual Machine Privilege Levels.
+
+The VMPL feature provides the ability to run software services at a more
+privileged level than the guest OS is running at. This provides a secure

Too many "provides".

Documentation/arch/x86/amd-memory-encryption.rst SNP feature support. · line 152

Quoted 11 lines

Documentation/arch/x86/amd-memory-encryption.rst +11 -0

+environment for services within the guest's SNP environment, while protecting
+the service from hypervisor interference. An example of a secure service
+would be a virtual TPM (vTPM). Additionally, certain operations require the
+guest to be running at VMPL0 in order for them to be performed. For example,
+the PVALIDATE instruction is required to be executed at VMPL0.
+
+When a guest is not running at VMPL0, it needs to communicate with the software
+running at VMPL0 to perform privileged operations or to interact with secure
+services. This software running at VMPL0 is known as a Secure VM Service Module
+(SVSM). Discovery of an SVSM and the API used to communicate with it is
+documented in Secure VM Service Module for SEV-SNP Guests[2].

This paragraph needs to go second, not third.

Somehow that text is missing "restraint" and is all over the place. Lemme try to restructure it:

"SNP provides a feature called Virtual Machine Privilege Levels (VMPL) which defines four privilege levels at which guest software can run. The most privileged level is 0 and numerically higher numbers have lesser privileges. More details in the AMD64 APM[1] Vol 2, section "15.35.7 Virtual Machine Privilege Levels", docID: 24593.

When using that feature, different services can run at different protection levels, apart from the guest OS but still within the secure SNP environment. They can provide services to the guest, like a vTPM, for example.

When a guest is not running at VMPL0, it needs to communicate with the software running at VMPL0 to perform privileged operations or to interact with secure services. An example fur such a privileged operation is PVALIDATE which is *required* to be executed at VMPL0.

In this scenario, the software running at VMPL0 is usually called a Secure VM Service Module (SVSM). Discovery of an SVSM and the API used to communicate with it is documented in "Secure VM Service Module for SEV-SNP Guests", docID:
58019."

How's that?

Documentation/arch/x86/amd-memory-encryption.rst SNP feature support. · line 155

Quoted 4 lines

Documentation/arch/x86/amd-memory-encryption.rst +2 -0

+
  [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
+[2] https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf

Yeah, about those links - they get stale pretty quickly. I think it suffices to explain what the document is and what it is called so that one can find it by searching the web. See what I did above.

Quoted 8 lines

arch/x86/boot/compressed/sev.c +0 -0

index 0457a9d7e515..cb771b380a6b 100644
@@ -12,6 +12,7 @@
   */
  #include "misc.h"
 

arch/x86/boot/compressed/sev.c line 15

Quoted 1 line

arch/x86/boot/compressed/sev.c +1 -0

+#include <linux/mm.h>

Please do not include a kernel-proper header into the decompresssor. Those things are solved by exposing the shared *minimal* functionality into

arch/x86/include/asm/shared/

There are examples there.

By the looks of it:

In file included from arch/x86/boot/compressed/sev.c:130:
arch/x86/boot/compressed/../../kernel/sev-shared.c: In function ‘setup_svsm_ca’:
arch/x86/boot/compressed/../../kernel/sev-shared.c:1332:14: warning: implicit declaration of function ‘PAGE_ALIGNED’; did you mean ‘IS_ALIGNED’? [-Wimplicit-function-declaration]

  1332 |         if (!PAGE_ALIGNED(caa))
       |              ^~~~~~~~~~~~
       |              IS_ALIGNED

it'll need PAGE_ALIGNED and IS_ALIGNED into an arch/x86/include/asm/shared/mm.h header.

Quoted 4 lines

arch/x86/boot/compressed/sev.c +0 -0

  #include <asm/bootparam.h>
  #include <asm/pgtable_types.h>
  #include <asm/sev.h>

..

Quoted 10 lines

arch/x86/boot/compressed/sev.c +10 -0

+static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
+{
+	struct snp_secrets_page *secrets_page;
+	u64 caa;
+
+	BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
+
+	/*
+	 * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
+	 * higher) privilege level. Here, clear the VMPL1 permission mask of the

arch/x86/boot/compressed/sev.c

Quoted 10 lines

arch/x86/boot/compressed/sev.c +10 -0

+	 * GHCB page. If the guest is not running at VMPL0, this will fail.
+	 *
+	 * If the guest is running at VMPL0, it will succeed. Even if that operation
+	 * modifies permission bits, it is still ok to do so currently because Linux
+	 * SNP guests running at VMPL0 only run at VMPL0, so VMPL1 or higher
+	 * permission mask changes are a don't-care.
+	 *
+	 * Use __pa() since this routine is running identity mapped when called,
+	 * both by the decompressor code and the early kernel code.
+	 */

Let's not replicate that comment. Diff ontop:

arch/x86/boot/compressed/sev.c +0 -3

index cb771b380a6b..cde1890c8843 100644
@@ -576,18 +576,7 @@ void sev_enable(struct boot_params *bp)
  		if (!(get_hv_features() & GHCB_HV_FT_SNP))
  			sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
 
-		/*
-		 * Enforce running at VMPL0.
-		 *

arch/x86/boot/compressed/sev.c +0 -0

  		if (rmpadjust((unsigned long)&boot_ghcb_page, RMP_PG_SIZE_4K, 1))
  			sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NOT_VMPL0);
  	}

arch/x86/include/asm/sev.h +0 -0

index 350db22e66be..b168403c07be 100644
@@ -204,6 +204,17 @@ static __always_inline void sev_es_nmi_complete(void)
  extern int __init sev_es_efi_map_ghcbs(pgd_t *pgd);
  extern void sev_enable(struct boot_params *bp);
 

arch/x86/include/asm/sev.h +1 -0

+ */
  static inline int rmpadjust(unsigned long vaddr, bool rmp_psize, unsigned long attrs)
  {
  	int rc;

arch/x86/kernel/sev-shared.c +3 -11

index 46ea4e5e118a..9ca54bcf0e99 100644
@@ -1297,17 +1297,9 @@ static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
  	BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
 
  	/*
-	 * RMPADJUST modifies RMP permissions of a lesser-privileged (numerically
-	 * higher) privilege level. Here, clear the VMPL1 permission mask of the
-	 * GHCB page. If the guest is not running at VMPL0, this will fail.
-	 *
-	 * If the guest is running at VMPL0, it will succeed. Even if that operation
-	 * modifies permission bits, it is still ok to do so currently because Linux
-	 * SNP guests running at VMPL0 only run at VMPL0, so VMPL1 or higher
-	 * permission mask changes are a don't-care.
-	 *
-	 * Use __pa() since this routine is running identity mapped when called,
-	 * both by the decompressor code and the early kernel code.
+	 * See comment above rmpadjust() for details. Use __pa() since
+	 * this routine is running identity mapped when called both by
+	 * the decompressor code and the early kernel code.
  	 */
  	if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
  		return;

Quoted 7 lines

arch/x86/boot/compressed/sev.c +6 -0

+	if (!rmpadjust((unsigned long)__pa(&boot_ghcb_page), RMP_PG_SIZE_4K, 1))
+		return;
+
+	/*
+	 * Not running at VMPL0, ensure everything has been properly supplied
+	 * for running under an SVSM.

arch/x86/boot/compressed/sev.c

Quoted 10 lines

arch/x86/boot/compressed/sev.c +10 -0

+	 */
+	if (!cc_info || !cc_info->secrets_phys || cc_info->secrets_len != PAGE_SIZE)
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SECRETS_PAGE);
+
+	secrets_page = (struct snp_secrets_page *)cc_info->secrets_phys;
+	if (!secrets_page->svsm_size)
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_NO_SVSM);
+
+	if (!secrets_page->svsm_guest_vmpl)
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_VMPL0);

0x15C 1 byte SVSM_GUEST_VMPL Indicates the VMPL at which the guest is executing.

Do I understand it correctly that this contains the VMPL of the guest and the SVSM is running below it?

IOW, SVSM should be at VMPL0 and the guest should be a at a level determined by that value and it cannot be 0.

Just making sure I'm reading it right.

Thx.

PAGE_ALIGNED and IS_ALIGNED are from two separate header files (mm.h and align.h) which seems like a lot of extra changes for just one check.

Not sure I agree. I'd prefer to keep the comment here because it is specific to this rmpadjust() call. See below.

Right. Not sure about the "cannot", more like "must not." The specification states that the guest should run at a VMPL other than 0. If an SVSM starts the guest at VMPL0, then the SVSM would not be protected from guest.

Quoted 2 lines · Tom Lendacky

PAGE_ALIGNED and IS_ALIGNED are from two separate header files (mm.h and align.h) which seems like a lot of extra changes for just one check.

No, pls put them in a single shared/mm.h header. And no, those are not a lot of extra changes - those are changes which are moving the code in the right direction and we do them sooner rather than later, otherwise they'd pile up and we'll never be able to find time to do them - sev.c movement attempt case-in-point.

Quoted 3 lines

Not sure I agree. I'd prefer to keep the comment here because it is specific to this rmpadjust() call. See below.

Just don't replicate some versions of the same comment all over the place. Do one big comment which explains which RMPADJUST has to do with VMPL levels - perhaps over the insn - and then refer to it from the other places after adding the specific explanations for them.

Quoted 3 lines

Right. Not sure about the "cannot", more like "must not." The specification states that the guest should run at a VMPL other than 0. If an SVSM starts the guest at VMPL0, then the SVSM would not be protected from guest.

Yeah, well, you do terminate the guest if it is running at VMPL 0 *in* the presence of a SVSM so it is a "must not". Ok.

So this will be a new shared directory in the top level include directory (as PAGE_ALIGNED is defined in include/linux/mm.h), not just in the arch/x86/include directory like the others (io.h, msr.h and tdx.h). Is that what you want?

Quoted 4 lines · Tom Lendacky

So this will be a new shared directory in the top level include directory (as PAGE_ALIGNED is defined in include/linux/mm.h), not just in the arch/x86/include directory like the others (io.h, msr.h and tdx.h). Is that what you want?

You can actually do this - it is a lot easier and still clean:

arch/x86/boot/compressed/sev.c +0 -1

index cb771b380a6b..5ee53a7a060e 100644
@@ -12,7 +12,6 @@
   */
  #include "misc.h"
 
-#include <linux/mm.h>
  #include <asm/bootparam.h>
  #include <asm/pgtable_types.h>
  #include <asm/sev.h>

arch/x86/kernel/sev-shared.c +0 -0

index 46ea4e5e118a..bd4bbb30ef0c 100644

arch/x86/kernel/sev-shared.c +0 -0

  		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_CAA);
 
  	/*

Or what I originally proposed:

if (!IS_ALIGNED(caa, PAGE_SIZE))

Which also works without including mm.h.

 arch/x86/include/asm/sev-common.h |  13 ++
 arch/x86/include/asm/sev.h        |  32 +++++
 arch/x86/include/uapi/asm/svm.h   |   1 +
 arch/x86/kernel/sev-shared.c      |  94 +++++++++++++-
 arch/x86/kernel/sev.c             | 207 +++++++++++++++++++++++++-----

arch/x86/include/asm/sev-common.h +12 -0

+#define GHCB_MSR_VMPL_REQ		0x016
+#define GHCB_MSR_VMPL_REQ_LEVEL(v)			\
+	/* GHCBData[39:32] */				\
+	(((u64)(v) & GENMASK_ULL(7, 0) << 32) |		\
+	/* GHCBDdata[11:0] */				\
+	GHCB_MSR_VMPL_REQ)
+
+#define GHCB_MSR_VMPL_RESP		0x017
+#define GHCB_MSR_VMPL_RESP_VAL(v)			\
+	/* GHCBData[63:32] */				\
+	(((u64)(v) & GENMASK_ULL(63, 32)) >> 32)
+
 /* GHCB Hypervisor Feature Request/Response */
 #define GHCB_MSR_HV_FT_REQ		0x080
 #define GHCB_MSR_HV_FT_RESP		0x081

arch/x86/kernel/sev-shared.c +3 -1

index 46ea4e5e118a..6f57eb804e70 100644
@@ -18,9 +18,11 @@
 #define sev_printk_rtl(fmt, ...)	printk_ratelimited(fmt, ##__VA_ARGS__)
 #else
 #undef WARN
-#define WARN(condition, format...) (!!(condition))
+#define WARN(condition, format...)	(!!(condition))
 #define sev_printk(fmt, ...)
 #define sev_printk_rtl(fmt, ...)
+#undef vc_forward_exception
+#define vc_forward_exception(c)		panic("SNP: Hypervisor requested exception\n")
 #endif
 
 /*

arch/x86/kernel/sev-shared.c +5 -0

+	asm volatile("mov %9, %%r8\n\t"
+		     "mov %10, %%r9\n\t"
+		     "movb $1, %11\n\t"
+		     "rep; vmmcall\n\t"
+		     "mov %%r8, %3\n\t"

Ok,

I think this is very readable and clear what's going on:

static __always_inline void issue_svsm_call(struct svsm_call *call, u8 *pending)

{
         register unsigned long rax asm("rax") = call->rax;
         register unsigned long rcx asm("rcx") = call->rcx;
         register unsigned long rdx asm("rdx") = call->rdx;
         register unsigned long r8  asm("r8")  = call->r8;
         register unsigned long r9  asm("r9")  = call->r9;

         call->caa->call_pending = 1;

         asm volatile("rep; vmmcall\n\t"
                      : "+r" (rax), "+r" (rcx), "+r" (rdx), "+r" (r8), "+r" (r9));

         xchg(pending, 1);

         call->rax_out = rax;
         call->rcx_out = rcx;
         call->rdx_out = rdx;
         call->r8_out = r8;
         call->r9_out = r9;
}

and the asm looks ok but the devil's in the detail.

This isn't quite right. The xchg has to occur between pending and call->caa->call_pending.

+static int __svsm_msr_protocol(struct svsm_call *call)

arch/x86/kernel/sev-shared.c +10 -0

+}
+
+static int __svsm_ghcb_protocol(struct ghcb *ghcb, struct svsm_call *call)
+{
+	struct es_em_ctxt ctxt;
+	u8 pending;
+
+	vc_ghcb_invalidate(ghcb);
+
+	/* Fill in protocol and format specifiers */

arch/x86/kernel/sev-shared.c +6 -0

+	issue_svsm_call(call, &pending);
+
+	if (pending)
+		return -EINVAL;
+
+	switch (verify_exception_info(ghcb, &ctxt)) {

arch/x86/kernel/sev-shared.c +2 -0

+}
+
 static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
 					  struct es_em_ctxt *ctxt,
 					  u64 exit_code, u64 exit_info_1,

arch/x86/kernel/sev.c +5 -0

index a500df807e79..21f3cc40d662 100644
@@ -133,8 +133,13 @@ struct ghcb_state {
 	struct ghcb *ghcb;
 };
 
+/* For early boot SVSM communication */
+static struct svsm_ca boot_svsm_ca_page __aligned(PAGE_SIZE);
+
 static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
 static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
+static DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
+static DEFINE_PER_CPU(u64, svsm_caa_pa);
 
 struct sev_config {
 	__u64 debug		: 1,

arch/x86/kernel/sev.c +28 -0

+	       * used instead of the boot SVSM CA.
+	       *
+	       * For APs, the per-CPU SVSM CA is created as part of the AP
+	       * bringup, so this flag can be used globally for the BSP and APs.
+	       */
+	      cas_initialized	: 1,
+
 	      __reserved	: 62;
 };
 

@@ -572,9 +586,42 @@ static enum es_result vc_ioio_check(struct es_em_ctxt *ctxt, u16 port, size_t si
 	return ES_EXCEPTION;
 }
 
+static __always_inline void vc_forward_exception(struct es_em_ctxt *ctxt)
+{
+	long error_code = ctxt->fi.error_code;
+	int trapnr = ctxt->fi.vector;
+
+	ctxt->regs->orig_ax = ctxt->fi.error_code;
+
+	switch (trapnr) {
+	case X86_TRAP_GP:
+		exc_general_protection(ctxt->regs, error_code);
+		break;
+	case X86_TRAP_UD:
+		exc_invalid_op(ctxt->regs);
+		break;
+	case X86_TRAP_PF:
+		write_cr2(ctxt->fi.cr2);
+		exc_page_fault(ctxt->regs, error_code);
+		break;
+	case X86_TRAP_AC:
+		exc_alignment_check(ctxt->regs, error_code);
+		break;

arch/x86/kernel/sev.c +21 -0

+
+	if (sev_cfg.ghcbs_initialized)
+		ghcb = __sev_get_ghcb(&state);
+	else if (boot_ghcb)
+		ghcb = boot_ghcb;
+	else
+		ghcb = NULL;
+
+	do {
+		ret = ghcb ? __svsm_ghcb_protocol(ghcb, call)
+			   : __svsm_msr_protocol(call);
+	} while (ret == SVSM_ERR_BUSY);
+
+	if (sev_cfg.ghcbs_initialized)
+		__sev_put_ghcb(&state);
+
+	if (flags & X86_EFLAGS_IF)
+		native_irq_enable();
+
+	return ret;
+}

arch/x86/kernel/sev.c +5 -0

+{
+	struct svsm_call call = {};
+	int ret;
+	u64 pa;
+

arch/x86/kernel/sev.c +11 -0

+	 * RIP-relative addressing.
+	 */
+	pa = (u64)&RIP_REL_REF(boot_svsm_ca_page);
+
+	/*
+	 * Switch over to the boot SVSM CA while the current CA is still
+	 * addressable. There is no GHCB at this point so use the MSR protocol.
+	 *
+	 * SVSM_CORE_REMAP_CA call:
+	 *   RAX = 0 (Protocol=0, CallID=0)
+	 *   RCX = New CA GPA

arch/x86/kernel/sev.c +10 -6

 bool __head snp_init(struct boot_params *bp)
 {
 	struct cc_blob_sev_info *cc_info;

@@ -2108,12 +2245,7 @@ bool __head snp_init(struct boot_params *bp)
 
 	setup_cpuid_table(cc_info);
 
-	/*
-	 * Record the SVSM Calling Area address (CAA) if the guest is not
-	 * running at VMPL0. The CA will be used to communicate with the
-	 * SVSM to perform the SVSM services.
-	 */
-	setup_svsm_ca(cc_info);
+	setup_svsm(cc_info);
 
 	/*
 	 * The CC blob will be used later to access the secrets page. Cache

@@ -2306,3 +2438,12 @@ void sev_show_status(void)
 	}
 	pr_cont("\n");
 }
+
+void __init snp_remap_svsm_ca(void)
+{
+	if (!vmpl)
+		return;
+
+	/* Update the CAA to a proper kernel address */
+	boot_svsm_caa = &boot_svsm_ca_page;
+}

arch/x86/mm/mem_encrypt_amd.c +0 -0

index 422602f6039b..6155020e4d2d 100644

Quoted 1 line · Tom Lendacky

+static int __svsm_msr_protocol(struct svsm_call *call)

All those protocol functions need a verb in the name:

__svsm_do_msr_protocol
__svsm_do_ghcb_protocol

or something slicker than the silly "do".

arch/x86/kernel/sev-shared.c static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt · line 284

Quoted 10 lines

arch/x86/kernel/sev-shared.c +9 -0

+{
+	u64 val, resp;
+	u8 pending;
+
+	val = sev_es_rd_ghcb_msr();
+
+	sev_es_wr_ghcb_msr(GHCB_MSR_VMPL_REQ_LEVEL(0));
+
+	pending = 0;

Do that assignment above, at declaration.

arch/x86/kernel/sev-shared.c static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt · line 289

Quoted 6 lines

arch/x86/kernel/sev-shared.c +5 -0

+	issue_svsm_call(call, &pending);
+
+	resp = sev_es_rd_ghcb_msr();
+
+	sev_es_wr_ghcb_msr(val);

The MSR SVSM protocol is supposed to restore the MSR? A comment pls.

arch/x86/kernel/sev-shared.c static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt · line 297

Quoted 9 lines

arch/x86/kernel/sev-shared.c +8 -0

+
+	if (pending)
+		return -EINVAL;
+
+	if (GHCB_RESP_CODE(resp) != GHCB_MSR_VMPL_RESP)
+		return -EINVAL;
+
+	if (GHCB_MSR_VMPL_RESP_VAL(resp) != 0)

s/ != 0//

arch/x86/kernel/sev-shared.c static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt · line 300

Quoted 4 lines

arch/x86/kernel/sev-shared.c +3 -0

+		return -EINVAL;
+
+	return call->rax_out;

rax_out is u64, your function returns int because of the negative values. But then it'll truncate the u64 in the success case.

Quoted 15 lines

arch/x86/kernel/sev-shared.c +14 -0

+}
+
+static int __svsm_ghcb_protocol(struct ghcb *ghcb, struct svsm_call *call)
+{
+	struct es_em_ctxt ctxt;
+	u8 pending;
+
+	vc_ghcb_invalidate(ghcb);
+
+	/* Fill in protocol and format specifiers */
+	ghcb->protocol_version = ghcb_version;
+	ghcb->ghcb_usage       = GHCB_DEFAULT_USAGE;
+
+	ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_SNP_RUN_VMPL);

arch/x86/kernel/sev-shared.c static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt · line 320

Quoted 6 lines

arch/x86/kernel/sev-shared.c +6 -0

+	ghcb_set_sw_exit_info_1(ghcb, 0);
+	ghcb_set_sw_exit_info_2(ghcb, 0);
+
+	sev_es_wr_ghcb_msr(__pa(ghcb));
+
+	pending = 0;

As above.

Quoted 11 lines

arch/x86/kernel/sev-shared.c +10 -0

+	issue_svsm_call(call, &pending);
+
+	if (pending)
+		return -EINVAL;
+
+	switch (verify_exception_info(ghcb, &ctxt)) {
+	case ES_OK:
+		break;
+	case ES_EXCEPTION:
+		vc_forward_exception(&ctxt);

arch/x86/kernel/sev-shared.c static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt · line 336

Quoted 6 lines

arch/x86/kernel/sev-shared.c +6 -0

+		fallthrough;
+	default:
+		return -EINVAL;
+	}
+
+	return call->rax_out;

As above.

Quoted 28 lines

arch/x86/kernel/sev-shared.c +2 -0

+}
+
  static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
  					  struct es_em_ctxt *ctxt,
  					  u64 exit_code, u64 exit_info_1,

arch/x86/kernel/sev.c +5 -0

index a500df807e79..21f3cc40d662 100644
@@ -133,8 +133,13 @@ struct ghcb_state {
  	struct ghcb *ghcb;
  };
 
+/* For early boot SVSM communication */
+static struct svsm_ca boot_svsm_ca_page __aligned(PAGE_SIZE);
+
  static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
  static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
+static DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
+static DEFINE_PER_CPU(u64, svsm_caa_pa);
 
  struct sev_config {
  	__u64 debug		: 1,

@@ -150,6 +155,15 @@ struct sev_config {
  	       */
  	      ghcbs_initialized	: 1,
 

arch/x86/kernel/sev.c struct sev_config { · line 159

Quoted 2 lines

arch/x86/kernel/sev.c +2 -0

+	      /*
+	       * A flag used to indicate when the per-CPU SVSM CA is to be

s/A flag used //

and above.

Quoted 45 lines

arch/x86/kernel/sev.c +34 -0

+	       * used instead of the boot SVSM CA.
+	       *
+	       * For APs, the per-CPU SVSM CA is created as part of the AP
+	       * bringup, so this flag can be used globally for the BSP and APs.
+	       */
+	      cas_initialized	: 1,
+
  	      __reserved	: 62;
  };
 

@@ -572,9 +586,42 @@ static enum es_result vc_ioio_check(struct es_em_ctxt *ctxt, u16 port, size_t si
  	return ES_EXCEPTION;
  }
 
+static __always_inline void vc_forward_exception(struct es_em_ctxt *ctxt)
+{
+	long error_code = ctxt->fi.error_code;
+	int trapnr = ctxt->fi.vector;
+
+	ctxt->regs->orig_ax = ctxt->fi.error_code;
+
+	switch (trapnr) {
+	case X86_TRAP_GP:
+		exc_general_protection(ctxt->regs, error_code);
+		break;
+	case X86_TRAP_UD:
+		exc_invalid_op(ctxt->regs);
+		break;
+	case X86_TRAP_PF:
+		write_cr2(ctxt->fi.cr2);
+		exc_page_fault(ctxt->regs, error_code);
+		break;
+	case X86_TRAP_AC:
+		exc_alignment_check(ctxt->regs, error_code);
+		break;
+	default:
+		pr_emerg("Unsupported exception in #VC instruction emulation - can't continue\n");
+		BUG();
+	}
+}
+
  /* Include code shared with pre-decompression boot stage */
  #include "sev-shared.c"
 

arch/x86/kernel/sev.c static enum es_result vc_ioio_check(struct es_em_ctxt *ctxt, u16 port, size_t si · line 619

Quoted 1 line

arch/x86/kernel/sev.c +1 -0

+static struct svsm_ca *__svsm_get_caa(void)

Why the "__" prefix?

I gon't see a svsm_get_caa() variant...

Quoted 10 lines

arch/x86/kernel/sev.c +5 -0

+{
+	return sev_cfg.cas_initialized ? this_cpu_read(svsm_caa)
+				       : boot_svsm_caa;
+}
+
  static noinstr void __sev_put_ghcb(struct ghcb_state *state)
  {
  	struct sev_es_runtime_data *data;

arch/x86/kernel/sev.c static noinstr void __sev_put_ghcb(struct ghcb_state *state) · line 650

Quoted 4 lines

arch/x86/kernel/sev.c +1 -0

@@ -600,6 +647,42 @@ static noinstr void __sev_put_ghcb(struct ghcb_state *state)
  	}
  }
 
+static int svsm_protocol(struct svsm_call *call)

svsm_issue_protocol_call() or whatnot...

Btw, can all this svsm guest gunk live simply in a separate file? Or is it going to need a lot of sev.c stuff exported through an internal.h header?

Either that or prefix all SVSM handling functions with "svsm_" so that there's at least some visibility which is which.

Quoted 4 lines

arch/x86/kernel/sev.c +3 -0

+{
+	struct ghcb_state state;
+	unsigned long flags;

arch/x86/kernel/sev.c static noinstr void __sev_put_ghcb(struct ghcb_state *state) · line 663

Quoted 10 lines

arch/x86/kernel/sev.c +10 -0

+	struct ghcb *ghcb;
+	int ret;
+
+	/*
+	 * This can be called very early in the boot, use native functions in
+	 * order to avoid paravirt issues.
+	 */
+	flags = native_save_fl();
+	if (flags & X86_EFLAGS_IF)
+		native_irq_disable();

Uff, conditional locking.

What's wrong with using local_irq_save/local_irq_restore?

Quoted 22 lines

arch/x86/kernel/sev.c +21 -0

+
+	if (sev_cfg.ghcbs_initialized)
+		ghcb = __sev_get_ghcb(&state);
+	else if (boot_ghcb)
+		ghcb = boot_ghcb;
+	else
+		ghcb = NULL;
+
+	do {
+		ret = ghcb ? __svsm_ghcb_protocol(ghcb, call)
+			   : __svsm_msr_protocol(call);
+	} while (ret == SVSM_ERR_BUSY);
+
+	if (sev_cfg.ghcbs_initialized)
+		__sev_put_ghcb(&state);
+
+	if (flags & X86_EFLAGS_IF)
+		native_irq_enable();
+
+	return ret;
+}

...

arch/x86/kernel/sev.c static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp) · line 2191

Quoted 5 lines

arch/x86/kernel/sev.c +1 -0

@@ -2095,6 +2188,50 @@ static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp)
  	return cc_info;
  }
 
+static __head void setup_svsm(struct cc_blob_sev_info *cc_info)

svsm_setup

Quoted 10 lines

arch/x86/kernel/sev.c +9 -0

+{
+	struct svsm_call call = {};
+	int ret;
+	u64 pa;
+
+	/*
+	 * Record the SVSM Calling Area address (CAA) if the guest is not
+	 * running at VMPL0. The CA will be used to communicate with the
+	 * SVSM to perform the SVSM services.

arch/x86/kernel/sev.c static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp) · line 2206

Quoted 6 lines

arch/x86/kernel/sev.c +6 -0

+	 */
+	setup_svsm_ca(cc_info);
+
+	/* Nothing to do if not running under an SVSM. */
+	if (!vmpl)
+		return;

You set up stuff above and now you bail out?

Judging by setup_svsm_ca() you don't really need that vmpl var but you can check

	if (!boot_svsm_caa)
		return;

to determine whether a SVSM was detected.

arch/x86/kernel/sev.c static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp) · line 2211

Quoted 6 lines

arch/x86/kernel/sev.c +5 -0

+
+	/*
+	 * It is very early in the boot and the kernel is running identity
+	 * mapped but without having adjusted the pagetables to where the
+	 * kernel was loaded (physbase), so the get the CA address using

s/the //

Quoted 16 lines

arch/x86/kernel/sev.c +15 -0

+	 * RIP-relative addressing.
+	 */
+	pa = (u64)&RIP_REL_REF(boot_svsm_ca_page);
+
+	/*
+	 * Switch over to the boot SVSM CA while the current CA is still
+	 * addressable. There is no GHCB at this point so use the MSR protocol.
+	 *
+	 * SVSM_CORE_REMAP_CA call:
+	 *   RAX = 0 (Protocol=0, CallID=0)
+	 *   RCX = New CA GPA
+	 */
+	call.caa = __svsm_get_caa();
+	call.rax = SVSM_CORE_CALL(SVSM_CORE_REMAP_CA);
+	call.rcx = pa;

arch/x86/kernel/sev.c static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp) · line 2232

Quoted 6 lines

arch/x86/kernel/sev.c +6 -0

+	ret = svsm_protocol(&call);
+	if (ret != SVSM_SUCCESS)
+		panic("Can't remap the SVSM CA, ret=%#x (%d)\n", ret, ret);
+
+	boot_svsm_caa = (struct svsm_ca *)pa;
+	boot_svsm_caa_pa = pa;

Huh, setup_svsm_ca() already assigned those...

Quoted 35 lines

arch/x86/kernel/sev.c +10 -6

  bool __head snp_init(struct boot_params *bp)
  {
  	struct cc_blob_sev_info *cc_info;

@@ -2108,12 +2245,7 @@ bool __head snp_init(struct boot_params *bp)
 
  	setup_cpuid_table(cc_info);
 
-	/*
-	 * Record the SVSM Calling Area address (CAA) if the guest is not
-	 * running at VMPL0. The CA will be used to communicate with the
-	 * SVSM to perform the SVSM services.
-	 */
-	setup_svsm_ca(cc_info);
+	setup_svsm(cc_info);
 
  	/*
  	 * The CC blob will be used later to access the secrets page. Cache

@@ -2306,3 +2438,12 @@ void sev_show_status(void)
  	}
  	pr_cont("\n");
  }
+
+void __init snp_remap_svsm_ca(void)
+{
+	if (!vmpl)
+		return;
+
+	/* Update the CAA to a proper kernel address */
+	boot_svsm_caa = &boot_svsm_ca_page;
+}

arch/x86/mm/mem_encrypt_amd.c +0 -0

index 422602f6039b..6155020e4d2d 100644

arch/x86/mm/mem_encrypt_amd.c line 5

Quoted 6 lines

arch/x86/mm/mem_encrypt_amd.c +1 -1

@@ -2,7 +2,7 @@
  /*
   * AMD Memory Encryption Support
   *
- * Copyright (C) 2016 Advanced Micro Devices, Inc.
+ * Copyright (C) 2016-2024 Advanced Micro Devices, Inc.

Are we doing that now?

Quoted 14 lines · Borislav Petkov

arch/x86/mm/mem_encrypt_amd.c line 5

Quoted 10 lines · Tom Lendacky

arch/x86/mm/mem_encrypt_amd.c +1 -1

index 422602f6039b..6155020e4d2d 100644
@@ -2,7 +2,7 @@
   /*
    * AMD Memory Encryption Support
    *
- * Copyright (C) 2016 Advanced Micro Devices, Inc.
+ * Copyright (C) 2016-2024 Advanced Micro Devices, Inc.

Are we doing that now?

Looks like a leftover change... will remove it.

Thanks,
Tom

Quoted 1 line

ok, maybe __perform_svsm_msr_protocol or such.

There's quite a bit of interaction so I'll make sure to prefix everything.

The paravirt versions of local_irq_save and local_irq_restore can't be used as early as this routine is called.

Quoted 8 lines

Quoted 8 lines

arch/x86/kernel/sev.c +8 -0

+	struct svsm_call call = {};
+	int ret;
+	u64 pa;
+
+	/*
+	 * Record the SVSM Calling Area address (CAA) if the guest is not
+	 * running at VMPL0. The CA will be used to communicate with the
+	 * SVSM to perform the SVSM services.

Quoted 8 lines

arch/x86/kernel/sev.c static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp) · line 2206

Quoted 6 lines

arch/x86/kernel/sev.c +6 -0

+	 */
+	setup_svsm_ca(cc_info);
+
+	/* Nothing to do if not running under an SVSM. */
+	if (!vmpl)
+		return;

You set up stuff above and now you bail out?

setup_svsm_ca() is what sets the vmpl variable. So nothing will have been setup if the VMPL is zero, in which case we don't continue on.

Quoted 7 lines

Judging by setup_svsm_ca() you don't really need that vmpl var but you can check

	if (!boot_svsm_caa)
		return;

to determine whether a SVSM was detected.

Yes, but the vmpl var will be used for attestation requests, sysfs, etc.

Quoted 1 line

Huh, setup_svsm_ca() already assigned those...

setup_svsm_ca() assigned the ones from the secrets page. The kernel now switches to using its own CA.

Quoted 1 line · Tom Lendacky

ok, maybe __perform_svsm_msr_protocol or such.

We'll bikeshed it in the coming weeks.

Quoted 2 lines

There's quite a bit of interaction so I'll make sure to prefix everything.

Ack.

Quoted 2 lines

The paravirt versions of local_irq_save and local_irq_restore can't be used as early as this routine is called.

tglx says you should do native_local_irq_save()/.._restore() helpers just like the arch_local_irq_save()/..._restore() ones but use only native_ functions without the paravirt gunk.

In a prepatch pls.

Quoted 18 lines

Quoted 17 lines

Quoted 5 lines

arch/x86/kernel/sev.c +4 -0

+	struct svsm_call call = {};
+	int ret;
+	u64 pa;
+

arch/x86/kernel/sev.c static __head struct cc_blob_sev_info *find_cc_blob(struct boot_params *bp) · line 2206

Quoted 10 lines

arch/x86/kernel/sev.c +10 -0

+	/*
+	 * Record the SVSM Calling Area address (CAA) if the guest is not
+	 * running at VMPL0. The CA will be used to communicate with the
+	 * SVSM to perform the SVSM services.
+	 */
+	setup_svsm_ca(cc_info);
+
+	/* Nothing to do if not running under an SVSM. */
+	if (!vmpl)
+		return;

You set up stuff above and now you bail out?

Quoted 2 lines

setup_svsm_ca() is what sets the vmpl variable. So nothing will have been setup if the VMPL is zero, in which case we don't continue on.

You still assign

         /*
          * The CA is identity mapped when this routine is called, both by the
          * decompressor code and the early kernel code.
          */
         boot_svsm_caa = (struct svsm_ca *)caa;
         boot_svsm_caa_pa = caa;

regardless of vmpl.

I think you should assign those only when vmpl != 0.

Otherwise the code is confusing.

Quoted 9 lines

Quoted 7 lines

Judging by setup_svsm_ca() you don't really need that vmpl var but you can check

	if (!boot_svsm_caa)
		return;

to determine whether a SVSM was detected.

Yes, but the vmpl var will be used for attestation requests, sysfs, etc.

I guess that comes later in the patchset...

Quoted 4 lines

Quoted 1 line

Huh, setup_svsm_ca() already assigned those...

setup_svsm_ca() assigned the ones from the secrets page. The kernel now switches to using its own CA.

Comment pls.

Thx.

If we're not running at VMPL0 (based on the RMPADJUST check) and if the SVSM doesn't advertise a non-zero VMPL value, we will self-terminate. So those values are only set if we are not running at VMPL0 and the SVSM has provided a non-zero value to us.

I'm going to turn the function into a bool function so that the call becomes:

	if (!svsm_setup_caa(cc_info))
		return;

I do. I think you're missing the RMPADJUST check that causes the function to return early if we're running at VMPL0.

The PVALIDATE instruction can only be performed at VMPL0. An SVSM will be present when running at VMPL1 or a lower privilege level.

When an SVSM is present, use the SVSM_CORE_PVALIDATE call to perform memory validation instead of issuing the PVALIDATE instruction directly.

The validation of a single 4K page is now explicitly identified as such in the function name, pvalidate_4k_page(). The pvalidate_pages() function is used for validating 1 or more pages at either 4K or 2M in size. Each function, however, determines whether it can issue the PVALIDATE directly or whether the SVSM needs to be invoked.

Signed-off-by Tom Lendacky <[email protected]>

---
 arch/x86/boot/compressed/sev.c |  45 ++++++++-
 arch/x86/include/asm/sev.h     |  22 ++++
 arch/x86/kernel/sev-shared.c   | 179 ++++++++++++++++++++++++++++++++-
 arch/x86/kernel/sev.c          |  25 +++--
 4 files changed, 253 insertions(+), 18 deletions(-)

arch/x86/boot/compressed/sev.c +1 -0

index cb771b380a6b..32a1e98ffaa9 100644
@@ -130,6 +130,34 @@ static bool fault_in_kernel_space(unsigned long address)
 /* Include code for early handlers */
 #include "../../kernel/sev-shared.c"
 
+static struct svsm_ca *__svsm_get_caa(void)

arch/x86/boot/compressed/sev.c +17 -0

+{
+	struct ghcb *ghcb;
+	int ret;
+
+	if (boot_ghcb)
+		ghcb = boot_ghcb;
+	else
+		ghcb = NULL;
+
+	do {
+		ret = ghcb ? __svsm_ghcb_protocol(ghcb, call)
+			   : __svsm_msr_protocol(call);
+	} while (ret == SVSM_ERR_BUSY);
+
+	return ret;
+}
+
 bool sev_snp_enabled(void)
 {
 	return sev_status & MSR_AMD64_SEV_SNP_ENABLED;

arch/x86/boot/compressed/sev.c +7 -0

+	 * protocol or the GHCB shared page to perform a GHCB request. Since the
+	 * GHCB page is being changed to encrypted, it can't be used to perform
+	 * GHCB requests. Clear the boot_ghcb variable so that the GHCB MSR
+	 * protocol is used to change the GHCB page over to an encrypted page.
+	 */
+	boot_ghcb = NULL;
+
 	/*
 	 * GHCB Page must be flushed from the cache and mapped encrypted again.
 	 * Otherwise the running kernel will see strange cache effects when

arch/x86/include/asm/sev.h +22 -0

index 8f180fd3cbf0..e6f1ed3f6ce3 100644
@@ -187,6 +187,27 @@ struct svsm_ca {
 #define SVSM_ERR_INVALID_PARAMETER		0x80000005
 #define SVSM_ERR_INVALID_REQUEST		0x80000006
 #define SVSM_ERR_BUSY				0x80000007
+#define SVSM_PVALIDATE_FAIL_SIZEMISMATCH	0x80001006
+
+/*
+ * The SVSM PVALIDATE related structures
+ */
+struct svsm_pvalidate_entry {
+	u64 page_size		: 2,
+	    action		: 1,
+	    ignore_cf		: 1,
+	    rsvd		: 8,
+	    pfn			: 52;
+};
+
+struct svsm_pvalidate_call {
+	u16 entries;
+	u16 next;
+
+	u8 rsvd1[4];
+
+	struct svsm_pvalidate_entry entry[];
+};
 
 /*
  * SVSM protocol structure

@@ -207,6 +228,7 @@ struct svsm_call {
 
 #define SVSM_CORE_CALL(x)		((0ULL << 32) | (x))
 #define SVSM_CORE_REMAP_CA		0
+#define SVSM_CORE_PVALIDATE		1
 
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 extern void __sev_es_ist_enter(struct pt_regs *regs);

arch/x86/kernel/sev-shared.c +0 -0

index 6f57eb804e70..b415b10a0823 100644

arch/x86/kernel/sev-shared.c +17 -0

+	pvalidate_call_pa = __svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
+
+	pvalidate_call->entries = 1;
+	pvalidate_call->next    = 0;
+	pvalidate_call->entry[0].page_size = RMP_PG_SIZE_4K;
+	pvalidate_call->entry[0].action    = validate;
+	pvalidate_call->entry[0].ignore_cf = 0;
+	pvalidate_call->entry[0].pfn       = paddr >> PAGE_SHIFT;
+
+	/* Protocol 0, Call ID 1 */
+	call.rax = SVSM_CORE_CALL(SVSM_CORE_PVALIDATE);
+	call.rcx = pvalidate_call_pa;
+
+	ret = svsm_protocol(&call);
+
+	if (flags & X86_EFLAGS_IF)
+		native_irq_enable();

arch/x86/kernel/sev-shared.c +8 -0

+
+	if (ret) {
+		WARN(1, "Failed to validate address 0x%lx ret %d", vaddr, ret);
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+	}
+}
+
+static void base_pvalidate_pages(struct snp_psc_desc *desc)
 {
 	struct psc_entry *e;
 	unsigned long vaddr;

arch/x86/kernel/sev-shared.c +11 -0

+	unsigned int call_count, i;
+	struct svsm_call call = {};
+	u64 pvalidate_call_pa;
+	struct psc_entry *e;
+	unsigned long flags;
+	unsigned long vaddr;
+	bool action;
+	int ret;
+
+	/*
+	 * This can be called very early in the boot, use native functions in

arch/x86/kernel/sev-shared.c +23 -0

+	for (i = 0; i <= desc->hdr.end_entry; i++) {
+		e = &desc->entries[i];
+		pe = &pvalidate_call->entry[pvalidate_call->entries];
+
+		pe->page_size = e->pagesize ? RMP_PG_SIZE_2M : RMP_PG_SIZE_4K;
+		pe->action    = e->operation == SNP_PAGE_STATE_PRIVATE;
+		pe->ignore_cf = 0;
+		pe->pfn       = e->gfn;
+
+		pvalidate_call->entries++;
+		if (pvalidate_call->entries < call_count && i != desc->hdr.end_entry)
+			continue;
+
+		ret = svsm_protocol(&call);
+		if (ret == SVSM_PVALIDATE_FAIL_SIZEMISMATCH &&
+		    pvalidate_call->entry[pvalidate_call->next].page_size == RMP_PG_SIZE_2M) {
+			u64 pfn, pfn_end;
+
+			/*
+			 * The "next" field is the index of the failed entry. Calculate the
+			 * index of the entry after the failed entry before the fields are
+			 * cleared so that processing can continue on from that point (take
+			 * into account the for loop adding 1 to the entry).

arch/x86/kernel/sev-shared.c +10 -0

+			for (; pfn <= pfn_end; pfn++) {
+				pe = &pvalidate_call->entry[pvalidate_call->entries];
+
+				pe->page_size = RMP_PG_SIZE_4K;
+				pe->action    = action;
+				pe->ignore_cf = 0;
+				pe->pfn       = pfn;
+
+				pvalidate_call->entries++;
+				if (pvalidate_call->entries < call_count && pfn != pfn_end)

Quoted 2 lines · Tom Lendacky

The PVALIDATE instruction can only be performed at VMPL0. An SVSM will be present when running at VMPL1 or a lower privilege level.

s/when running/when the guest itself is running/

Quoted 3 lines

When an SVSM is present, use the SVSM_CORE_PVALIDATE call to perform memory validation instead of issuing the PVALIDATE instruction directly.

Quoted 5 lines

The validation of a single 4K page is now explicitly identified as such in the function name, pvalidate_4k_page(). The pvalidate_pages() function is used for validating 1 or more pages at either 4K or 2M in size. Each function, however, determines whether it can issue the PVALIDATE directly or whether the SVSM needs to be invoked.

This all talks more about what this is doing and I'm missing the "why".

It sounds like when you're running a SVSM under the guest, you cannot use PVALIDATE but the SVSM should do it for you. And you should start with that.

The rest/details should be visible from the diff.

Quoted 16 lines

Signed-off-by Tom Lendacky <[email protected]>

---
  arch/x86/boot/compressed/sev.c |  45 ++++++++-
  arch/x86/include/asm/sev.h     |  22 ++++
  arch/x86/kernel/sev-shared.c   | 179 ++++++++++++++++++++++++++++++++-
  arch/x86/kernel/sev.c          |  25 +++--
  4 files changed, 253 insertions(+), 18 deletions(-)

arch/x86/boot/compressed/sev.c +0 -0

index cb771b380a6b..32a1e98ffaa9 100644
@@ -130,6 +130,34 @@ static bool fault_in_kernel_space(unsigned long address)
  /* Include code for early handlers */
  #include "../../kernel/sev-shared.c"
 

arch/x86/boot/compressed/sev.c static bool fault_in_kernel_space(unsigned long address) · line 143

Quoted 11 lines

arch/x86/boot/compressed/sev.c +11 -0

+static struct svsm_ca *__svsm_get_caa(void)
+{
+	return boot_svsm_caa;
+}
+
+static u64 __svsm_get_caa_pa(void)
+{
+	return boot_svsm_caa_pa;
+}
+
+static int svsm_protocol(struct svsm_call *call)

Function name needs a verb.

Quoted 23 lines

arch/x86/boot/compressed/sev.c +17 -0

+{
+	struct ghcb *ghcb;
+	int ret;
+
+	if (boot_ghcb)
+		ghcb = boot_ghcb;
+	else
+		ghcb = NULL;
+
+	do {
+		ret = ghcb ? __svsm_ghcb_protocol(ghcb, call)
+			   : __svsm_msr_protocol(call);
+	} while (ret == SVSM_ERR_BUSY);
+
+	return ret;
+}
+
  bool sev_snp_enabled(void)
  {
  	return sev_status & MSR_AMD64_SEV_SNP_ENABLED;

@@ -146,8 +174,8 @@ static void __page_state_change(unsigned long paddr, enum psc_op op)
  	 * If private -> shared then invalidate the page before requesting the

arch/x86/boot/compressed/sev.c static void __page_state_change(unsigned long paddr, enum psc_op op) · line 178

Quoted 6 lines

arch/x86/boot/compressed/sev.c +2 -2

  	 * state change in the RMP table.
  	 */
-	if (op == SNP_PAGE_STATE_SHARED && pvalidate(paddr, RMP_PG_SIZE_4K, 0))
-		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+	if (op == SNP_PAGE_STATE_SHARED)
+		pvalidate_4k_page(paddr, paddr, 0);

That is a bool so put a "false" in there.

arch/x86/boot/compressed/sev.c static void __page_state_change(unsigned long paddr, enum psc_op op) · line 194

Quoted 11 lines

arch/x86/boot/compressed/sev.c +2 -2

  	/* Issue VMGEXIT to change the page state in RMP table. */
  	sev_es_wr_ghcb_msr(GHCB_MSR_PSC_REQ_GFN(paddr >> PAGE_SHIFT, op));

@@ -162,8 +190,8 @@ static void __page_state_change(unsigned long paddr, enum psc_op op)
  	 * Now that page state is changed in the RMP table, validate it so that it is
  	 * consistent with the RMP entry.
  	 */
-	if (op == SNP_PAGE_STATE_PRIVATE && pvalidate(paddr, RMP_PG_SIZE_4K, 1))
-		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+	if (op == SNP_PAGE_STATE_PRIVATE)
+		pvalidate_4k_page(paddr, paddr, 1);

Ditto, but "true".

arch/x86/boot/compressed/sev.c void sev_es_shutdown_ghcb(void) · line 288

Quoted 8 lines

arch/x86/boot/compressed/sev.c +2 -0

  void snp_set_page_private(unsigned long paddr)

@@ -256,6 +284,15 @@ void sev_es_shutdown_ghcb(void)
  	if (!sev_es_check_cpu_features())
  		error("SEV-ES CPU Features missing.");
 
+	/*
+	 * The boot_ghcb value is used to determine whether to use the GHCB MSR

s/The boot_ghcb value /This is used/

Quoted 56 lines

arch/x86/boot/compressed/sev.c +7 -0

+	 * protocol or the GHCB shared page to perform a GHCB request. Since the
+	 * GHCB page is being changed to encrypted, it can't be used to perform
+	 * GHCB requests. Clear the boot_ghcb variable so that the GHCB MSR
+	 * protocol is used to change the GHCB page over to an encrypted page.
+	 */
+	boot_ghcb = NULL;
+
  	/*
  	 * GHCB Page must be flushed from the cache and mapped encrypted again.
  	 * Otherwise the running kernel will see strange cache effects when

arch/x86/include/asm/sev.h +22 -0

index 8f180fd3cbf0..e6f1ed3f6ce3 100644
@@ -187,6 +187,27 @@ struct svsm_ca {
  #define SVSM_ERR_INVALID_PARAMETER		0x80000005
  #define SVSM_ERR_INVALID_REQUEST		0x80000006
  #define SVSM_ERR_BUSY				0x80000007
+#define SVSM_PVALIDATE_FAIL_SIZEMISMATCH	0x80001006
+
+/*
+ * The SVSM PVALIDATE related structures
+ */
+struct svsm_pvalidate_entry {
+	u64 page_size		: 2,
+	    action		: 1,
+	    ignore_cf		: 1,
+	    rsvd		: 8,
+	    pfn			: 52;
+};
+
+struct svsm_pvalidate_call {
+	u16 entries;
+	u16 next;
+
+	u8 rsvd1[4];
+
+	struct svsm_pvalidate_entry entry[];
+};
 
  /*
   * SVSM protocol structure

@@ -207,6 +228,7 @@ struct svsm_call {
 
  #define SVSM_CORE_CALL(x)		((0ULL << 32) | (x))
  #define SVSM_CORE_REMAP_CA		0
+#define SVSM_CORE_PVALIDATE		1
 
  #ifdef CONFIG_AMD_MEM_ENCRYPT
  extern void __sev_es_ist_enter(struct pt_regs *regs);

arch/x86/kernel/sev-shared.c +0 -0

index 6f57eb804e70..b415b10a0823 100644

arch/x86/kernel/sev-shared.c static u8 vmpl __ro_after_init; · line 44

Quoted 5 lines

arch/x86/kernel/sev-shared.c +2 -0

@@ -40,6 +40,9 @@ static u8 vmpl __ro_after_init;
  static struct svsm_ca *boot_svsm_caa __ro_after_init;
  static u64 boot_svsm_caa_pa __ro_after_init;
 
+static struct svsm_ca *__svsm_get_caa(void);
+static u64 __svsm_get_caa_pa(void);

Are we being lazy again? :)

So I know the below is bigger than two silly forward declarations but forward declarations are a sign that our source code placement is lacking and if we keep piling on that, it'll become a mess soon. And guess who gets to mop up after y'all who don't have time to do it because you have to enable features? :-\

So in order to avoid that, we'll re-position it to where we think it'll be better going forward.

Btw, this is the second time, at least, where I think that that sev-shared.c thing is starting to become more of a nuisance than a code savings thing but I don't have a better idea for it yet.

So let's extend that ifdeffery at the top which provides things which are called the same but defined differently depending on whether we're in the decompressor or kernel proper.

IOW, something like this, ontop:

arch/x86/boot/compressed/sev.c +0 -10

index 32a1e98ffaa9..9d89fc67574b 100644
@@ -130,16 +130,6 @@ static bool fault_in_kernel_space(unsigned long address)
  /* Include code for early handlers */
  #include "../../kernel/sev-shared.c"
 
-static struct svsm_ca *__svsm_get_caa(void)
-{
-	return boot_svsm_caa;
-}
-
-static u64 __svsm_get_caa_pa(void)
-{
-	return boot_svsm_caa_pa;
-}
-
  static int svsm_protocol(struct svsm_call *call)
  {
  	struct ghcb *ghcb;

arch/x86/kernel/sev-shared.c +41 -16

index b415b10a0823..b4f1fd780925 100644
@@ -11,20 +11,6 @@
 
  #include <asm/setup_data.h>
 
-#ifndef __BOOT_COMPRESSED
-#define error(v)			pr_err(v)
-#define has_cpuflag(f)			boot_cpu_has(f)
-#define sev_printk(fmt, ...)		printk(fmt, ##__VA_ARGS__)
-#define sev_printk_rtl(fmt, ...)	printk_ratelimited(fmt, ##__VA_ARGS__)
-#else
-#undef WARN
-#define WARN(condition, format...)	(!!(condition))
-#define sev_printk(fmt, ...)
-#define sev_printk_rtl(fmt, ...)
-#undef vc_forward_exception
-#define vc_forward_exception(c)		panic("SNP: Hypervisor requested exception\n")
-#endif
-
  /*
   * SVSM related information:
   *   When running under an SVSM, the VMPL that Linux is executing at must be

@@ -40,8 +26,47 @@ static u8 vmpl __ro_after_init;
  static struct svsm_ca *boot_svsm_caa __ro_after_init;
  static u64 boot_svsm_caa_pa __ro_after_init;
 
-static struct svsm_ca *__svsm_get_caa(void);
-static u64 __svsm_get_caa_pa(void);
+#ifdef __BOOT_COMPRESSED
+
+#undef WARN
+#define WARN(condition, format...)	(!!(condition))
+#define sev_printk(fmt, ...)
+#define sev_printk_rtl(fmt, ...)
+#undef vc_forward_exception
+#define vc_forward_exception(c)		panic("SNP: Hypervisor requested exception\n")
+
+static struct svsm_ca *__svsm_get_caa(void)
+{
+	return boot_svsm_caa;
+}
+
+static u64 __svsm_get_caa_pa(void)
+{
+	return boot_svsm_caa_pa;
+}
+
+#else /* __BOOT_COMPRESSED */
+
+#define error(v)			pr_err(v)
+#define has_cpuflag(f)			boot_cpu_has(f)
+#define sev_printk(fmt, ...)		printk(fmt, ##__VA_ARGS__)
+#define sev_printk_rtl(fmt, ...)	printk_ratelimited(fmt, ##__VA_ARGS__)
+
+static DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
+
+static struct svsm_ca *__svsm_get_caa(void)
+{
+	return sev_cfg.cas_initialized ? this_cpu_read(svsm_caa)
+				       : boot_svsm_caa;
+}
+
+static u64 __svsm_get_caa_pa(void)
+{
+	return sev_cfg.cas_initialized ? this_cpu_read(svsm_caa_pa)
+				       : boot_svsm_caa_pa;
+}
+
+#endif /* __BOOT_COMPRESSED */
 
  /* I/O parameters for CPUID-related helpers */
  struct cpuid_leaf {

arch/x86/kernel/sev.c +0 -13

index bb6455ff45a2..db895a7a9401 100644
@@ -138,7 +138,6 @@ static struct svsm_ca boot_svsm_ca_page __aligned(PAGE_SIZE);
 
  static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
  static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
-static DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
  static DEFINE_PER_CPU(u64, svsm_caa_pa);
 
  struct sev_config {

@@ -616,18 +615,6 @@ static __always_inline void vc_forward_exception(struct es_em_ctxt *ctxt)
  /* Include code shared with pre-decompression boot stage */
  #include "sev-shared.c"
 
-static struct svsm_ca *__svsm_get_caa(void)
-{
-	return sev_cfg.cas_initialized ? this_cpu_read(svsm_caa)
-				       : boot_svsm_caa;
-}
-
-static u64 __svsm_get_caa_pa(void)
-{
-	return sev_cfg.cas_initialized ? this_cpu_read(svsm_caa_pa)
-				       : boot_svsm_caa_pa;
-}
-
  static noinstr void __sev_put_ghcb(struct ghcb_state *state)
  {
  	struct sev_es_runtime_data *data;

Quoted 9 lines

arch/x86/kernel/sev-shared.c +1 -0

+
  /* I/O parameters for CPUID-related helpers */
  struct cpuid_leaf {
  	u32 fn;

@@ -102,6 +105,8 @@ static u32 cpuid_std_range_max __ro_after_init;
  static u32 cpuid_hyp_range_max __ro_after_init;
  static u32 cpuid_ext_range_max __ro_after_init;