During early boot phases, check for the presence of an SVSM when running
as an SEV-SNP guest.
An SVSM is present if 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/boot/compressed/sev.c | 35 ++++++++---------
arch/x86/include/asm/sev-common.h | 4 ++
arch/x86/include/asm/sev.h | 25 +++++++++++-
arch/x86/kernel/sev-shared.c | 64 +++++++++++++++++++++++++++++++
arch/x86/kernel/sev.c | 16 ++++++++
5 files changed, 125 insertions(+), 19 deletions(-)
diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 49dc9661176d..fe61ff630c7e 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -12,6 +12,7 @@
*/
#include "misc.h"
+#include <linux/mm.h>
#include <asm/bootparam.h>
#include <asm/pgtable_types.h>
#include <asm/sev.h>
@@ -29,6 +30,15 @@
static struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
struct ghcb *boot_ghcb;
+/*
+ * SVSM related information:
+ * When running under an SVSM, the VMPL that Linux is executing at must be
+ * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
+ */
+static u8 vmpl __section(".data");
+static u64 boot_svsm_caa_pa __section(".data");
+static struct svsm_ca *boot_svsm_caa __section(".data");
+
/*
* Copy a version of this function here - insn-eval.c can't be used in
* pre-decompression code.
@@ -335,24 +345,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_GEN_REQ);
}
-static bool running_at_vmpl0(void *va)
-{
- u64 attrs;
-
- /*
- * 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.
- */
- attrs = 1;
- return !rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
-}
-
/*
* SNP_FEATURES_IMPL_REQ is the mask of SNP features that will need
* guest side implementation for proper functioning of the guest. If any
@@ -480,6 +472,13 @@ static bool early_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);
+
/*
* Pass run-time kernel a pointer to CC info via boot_params so EFI
* config table doesn't need to be searched again during early startup
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index b463fcbd4b90..68a8cdf6fd6a 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -159,6 +159,10 @@ struct snp_psc_desc {
#define GHCB_TERM_NOT_VMPL0 3 /* SNP guest is not running at VMPL-0 */
#define GHCB_TERM_CPUID 4 /* CPUID-validation failure */
#define GHCB_TERM_CPUID_HV 5 /* CPUID failure during hypervisor fallback */
+#define GHCB_TERM_SECRETS_PAGE 6 /* Secrets page failure */
+#define GHCB_TERM_NO_SVSM 7 /* SVSM is not advertised in the secrets page */
+#define GHCB_TERM_SVSM_VMPL0 8 /* SVSM is present but has set VMPL to 0 */
+#define GHCB_TERM_SVSM_CAA 9 /* SVSM is present but the CA is not page aligned */
#define GHCB_RESP_CODE(v) ((v) & GHCB_MSR_INFO_MASK)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 9477b4053bce..891e7d9a1f66 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -152,9 +152,32 @@ struct snp_secrets_page_layout {
u8 vmpck2[VMPCK_KEY_LEN];
u8 vmpck3[VMPCK_KEY_LEN];
struct secrets_os_area os_area;
- u8 rsvd3[3840];
+
+ u8 vmsa_tweak_bitmap[64];
+
+ /* SVSM fields */
+ u64 svsm_base;
+ u64 svsm_size;
+ u64 svsm_caa;
+ u32 svsm_max_version;
+ u8 svsm_guest_vmpl;
+ u8 rsvd3[3];
+
+ /* Remainder of page */
+ u8 rsvd4[3744];
} __packed;
+/*
+ * The SVSM Calling Area (CA) related structures.
+ */
+struct svsm_ca {
+ u8 call_pending;
+ u8 mem_available;
+ u8 rsvd1[6];
+
+ u8 svsm_buffer[PAGE_SIZE - 8];
+};
+
#ifdef CONFIG_AMD_MEM_ENCRYPT
extern void __sev_es_ist_enter(struct pt_regs *regs);
extern void __sev_es_ist_exit(void);
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 8b04958da5e7..66d33292eb78 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -111,6 +111,24 @@ sev_es_terminate(unsigned int set, unsigned int reason)
asm volatile("hlt\n" : : : "memory");
}
+static bool running_at_vmpl0(void *va)
+{
+ u64 attrs;
+
+ /*
+ * 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.
+ */
+ attrs = 1;
+ return !rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
+}
+
/*
* The hypervisor features are available from GHCB version 2 onward.
*/
@@ -1267,3 +1285,49 @@ static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
return ES_UNSUPPORTED;
}
+
+/*
+ * Maintain the GPA of the SVSM Calling Area (CA) in order to utilize the SVSM
+ * services needed when not running in VMPL0.
+ */
+static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
+{
+ struct snp_secrets_page_layout *secrets_page;
+ u64 caa;
+
+ BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
+
+ /*
+ * Use __pa() since this routine is running identity mapped when
+ * called, both by the decompressor code and the early kernel code.
+ */
+ if (running_at_vmpl0((void *)__pa(&boot_ghcb_page)))
+ return;
+
+ /*
+ * Not running at VMPL0, ensure everything has been properly supplied
+ * for running under an SVSM.
+ */
+ 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_layout *)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);
+
+ vmpl = secrets_page->svsm_guest_vmpl;
+
+ caa = secrets_page->svsm_caa;
+ if (!PAGE_ALIGNED(caa))
+ sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_CAA);
+
+ /*
+ * 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;
+}
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index b59b09c2f284..64799a04feb4 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -135,6 +135,15 @@ struct ghcb_state {
static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
+/*
+ * SVSM related information:
+ * When running under an SVSM, the VMPL that Linux is executing at must be
+ * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
+ */
+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;
+
struct sev_config {
__u64 debug : 1,
@@ -2122,6 +2131,13 @@ 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);
+
/*
* The CC blob will be used later to access the secrets page. Cache
* it here like the boot kernel does.
--
2.43.2
On 3/25/2024 11:26 PM, Tom Lendacky wrote:
> During early boot phases, check for the presence of an SVSM when running
> as an SEV-SNP guest.
>
> An SVSM is present if 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]>
Looks good.
Reviewed-by: Pankaj Gupta <[email protected]>
> ---
> arch/x86/boot/compressed/sev.c | 35 ++++++++---------
> arch/x86/include/asm/sev-common.h | 4 ++
> arch/x86/include/asm/sev.h | 25 +++++++++++-
> arch/x86/kernel/sev-shared.c | 64 +++++++++++++++++++++++++++++++
> arch/x86/kernel/sev.c | 16 ++++++++
> 5 files changed, 125 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 49dc9661176d..fe61ff630c7e 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -12,6 +12,7 @@
> */
> #include "misc.h"
>
> +#include <linux/mm.h>
> #include <asm/bootparam.h>
> #include <asm/pgtable_types.h>
> #include <asm/sev.h>
> @@ -29,6 +30,15 @@
> static struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
> struct ghcb *boot_ghcb;
>
> +/*
> + * SVSM related information:
> + * When running under an SVSM, the VMPL that Linux is executing at must be
> + * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
> + */
> +static u8 vmpl __section(".data");
> +static u64 boot_svsm_caa_pa __section(".data");
> +static struct svsm_ca *boot_svsm_caa __section(".data");
> +
> /*
> * Copy a version of this function here - insn-eval.c can't be used in
> * pre-decompression code.
> @@ -335,24 +345,6 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code)
> sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_GEN_REQ);
> }
>
> -static bool running_at_vmpl0(void *va)
> -{
> - u64 attrs;
> -
> - /*
> - * 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.
> - */
> - attrs = 1;
> - return !rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
> -}
> -
> /*
> * SNP_FEATURES_IMPL_REQ is the mask of SNP features that will need
> * guest side implementation for proper functioning of the guest. If any
> @@ -480,6 +472,13 @@ static bool early_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);
> +
> /*
> * Pass run-time kernel a pointer to CC info via boot_params so EFI
> * config table doesn't need to be searched again during early startup
> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index b463fcbd4b90..68a8cdf6fd6a 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -159,6 +159,10 @@ struct snp_psc_desc {
> #define GHCB_TERM_NOT_VMPL0 3 /* SNP guest is not running at VMPL-0 */
> #define GHCB_TERM_CPUID 4 /* CPUID-validation failure */
> #define GHCB_TERM_CPUID_HV 5 /* CPUID failure during hypervisor fallback */
> +#define GHCB_TERM_SECRETS_PAGE 6 /* Secrets page failure */
> +#define GHCB_TERM_NO_SVSM 7 /* SVSM is not advertised in the secrets page */
> +#define GHCB_TERM_SVSM_VMPL0 8 /* SVSM is present but has set VMPL to 0 */
> +#define GHCB_TERM_SVSM_CAA 9 /* SVSM is present but the CA is not page aligned */
>
> #define GHCB_RESP_CODE(v) ((v) & GHCB_MSR_INFO_MASK)
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 9477b4053bce..891e7d9a1f66 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -152,9 +152,32 @@ struct snp_secrets_page_layout {
> u8 vmpck2[VMPCK_KEY_LEN];
> u8 vmpck3[VMPCK_KEY_LEN];
> struct secrets_os_area os_area;
> - u8 rsvd3[3840];
> +
> + u8 vmsa_tweak_bitmap[64];
> +
> + /* SVSM fields */
> + u64 svsm_base;
> + u64 svsm_size;
> + u64 svsm_caa;
> + u32 svsm_max_version;
> + u8 svsm_guest_vmpl;
> + u8 rsvd3[3];
> +
> + /* Remainder of page */
> + u8 rsvd4[3744];
> } __packed;
>
> +/*
> + * The SVSM Calling Area (CA) related structures.
> + */
> +struct svsm_ca {
> + u8 call_pending;
> + u8 mem_available;
> + u8 rsvd1[6];
> +
> + u8 svsm_buffer[PAGE_SIZE - 8];
> +};
> +
> #ifdef CONFIG_AMD_MEM_ENCRYPT
> extern void __sev_es_ist_enter(struct pt_regs *regs);
> extern void __sev_es_ist_exit(void);
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index 8b04958da5e7..66d33292eb78 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -111,6 +111,24 @@ sev_es_terminate(unsigned int set, unsigned int reason)
> asm volatile("hlt\n" : : : "memory");
> }
>
> +static bool running_at_vmpl0(void *va)
> +{
> + u64 attrs;
> +
> + /*
> + * 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.
> + */
> + attrs = 1;
> + return !rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
> +}
> +
> /*
> * The hypervisor features are available from GHCB version 2 onward.
> */
> @@ -1267,3 +1285,49 @@ static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
>
> return ES_UNSUPPORTED;
> }
> +
> +/*
> + * Maintain the GPA of the SVSM Calling Area (CA) in order to utilize the SVSM
> + * services needed when not running in VMPL0.
> + */
> +static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
> +{
> + struct snp_secrets_page_layout *secrets_page;
> + u64 caa;
> +
> + BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
> +
> + /*
> + * Use __pa() since this routine is running identity mapped when
> + * called, both by the decompressor code and the early kernel code.
> + */
> + if (running_at_vmpl0((void *)__pa(&boot_ghcb_page)))
> + return;
> +
> + /*
> + * Not running at VMPL0, ensure everything has been properly supplied
> + * for running under an SVSM.
> + */
> + 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_layout *)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);
> +
> + vmpl = secrets_page->svsm_guest_vmpl;
> +
> + caa = secrets_page->svsm_caa;
> + if (!PAGE_ALIGNED(caa))
> + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_CAA);
> +
> + /*
> + * 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;
> +}
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index b59b09c2f284..64799a04feb4 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -135,6 +135,15 @@ struct ghcb_state {
> static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
> static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
>
> +/*
> + * SVSM related information:
> + * When running under an SVSM, the VMPL that Linux is executing at must be
> + * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
> + */
> +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;
> +
> struct sev_config {
> __u64 debug : 1,
>
> @@ -2122,6 +2131,13 @@ 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);
> +
> /*
> * The CC blob will be used later to access the secrets page. Cache
> * it here like the boot kernel does.
On Mon, Mar 25, 2024 at 05:26:22PM -0500, Tom Lendacky wrote:
> During early boot phases, check for the presence of an SVSM when running
> as an SEV-SNP guest.
>
> An SVSM is present if 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.
Where are we pointing to the SVSM spec?
This is in the 0th message
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
but pls add it to our documentation here:
Documentation/arch/x86/amd-memory-encryption.rst
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> arch/x86/boot/compressed/sev.c | 35 ++++++++---------
> arch/x86/include/asm/sev-common.h | 4 ++
> arch/x86/include/asm/sev.h | 25 +++++++++++-
> arch/x86/kernel/sev-shared.c | 64 +++++++++++++++++++++++++++++++
> arch/x86/kernel/sev.c | 16 ++++++++
> 5 files changed, 125 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 49dc9661176d..fe61ff630c7e 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -12,6 +12,7 @@
> */
> #include "misc.h"
>
> +#include <linux/mm.h>
> #include <asm/bootparam.h>
> #include <asm/pgtable_types.h>
> #include <asm/sev.h>
> @@ -29,6 +30,15 @@
> static struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
> struct ghcb *boot_ghcb;
>
> +/*
> + * SVSM related information:
> + * When running under an SVSM, the VMPL that Linux is executing at must be
> + * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
> + */
> +static u8 vmpl __section(".data");
> +static u64 boot_svsm_caa_pa __section(".data");
> +static struct svsm_ca *boot_svsm_caa __section(".data");
Explain what those last 2 are in comments above it pls.
> /*
> * SNP_FEATURES_IMPL_REQ is the mask of SNP features that will need
> * guest side implementation for proper functioning of the guest. If any
> @@ -480,6 +472,13 @@ static bool early_snp_init(struct boot_params *bp)
> */
> setup_cpuid_table(cc_info);
>
> + /*
> + * Record the SVSM Calling Area address (CAA) if the guest is not
Calling Area (CA) address
> + * running at VMPL0. The CA will be used to communicate with the
and then you can use "CA" here.
> + * SVSM to perform the SVSM services.
> + */
> + setup_svsm_ca(cc_info);
> +
> /*
> * Pass run-time kernel a pointer to CC info via boot_params so EFI
> * config table doesn't need to be searched again during early startup
> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index b463fcbd4b90..68a8cdf6fd6a 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -159,6 +159,10 @@ struct snp_psc_desc {
> #define GHCB_TERM_NOT_VMPL0 3 /* SNP guest is not running at VMPL-0 */
> #define GHCB_TERM_CPUID 4 /* CPUID-validation failure */
> #define GHCB_TERM_CPUID_HV 5 /* CPUID failure during hypervisor fallback */
> +#define GHCB_TERM_SECRETS_PAGE 6 /* Secrets page failure */
> +#define GHCB_TERM_NO_SVSM 7 /* SVSM is not advertised in the secrets page */
> +#define GHCB_TERM_SVSM_VMPL0 8 /* SVSM is present but has set VMPL to 0 */
> +#define GHCB_TERM_SVSM_CAA 9 /* SVSM is present but the CA is not page aligned */
"CAA" in the comment I guess. :)
> +/*
> + * Maintain the GPA of the SVSM Calling Area (CA) in order to utilize the SVSM
> + * services needed when not running in VMPL0.
> + */
> +static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
> +{
> + struct snp_secrets_page_layout *secrets_page;
Why was that thing ever called "_layout" and not simply
snp_secrets_page?
Fix it?
> + u64 caa;
> +
> + BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
Put it in the header under the struct definition I guess.
> + /*
> + * Use __pa() since this routine is running identity mapped when
> + * called, both by the decompressor code and the early kernel code.
> + */
> + if (running_at_vmpl0((void *)__pa(&boot_ghcb_page)))
> + return;
> +
> + /*
> + * Not running at VMPL0, ensure everything has been properly supplied
> + * for running under an SVSM.
> + */
> + 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_layout *)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);
> +
> + vmpl = secrets_page->svsm_guest_vmpl;
> +
> + caa = secrets_page->svsm_caa;
> + if (!PAGE_ALIGNED(caa))
> + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_CAA);
> +
> + /*
> + * 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;
> +}
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index b59b09c2f284..64799a04feb4 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -135,6 +135,15 @@ struct ghcb_state {
> static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
> static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
>
> +/*
> + * SVSM related information:
> + * When running under an SVSM, the VMPL that Linux is executing at must be
> + * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
> + */
> +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;
Uff, duplication.
Let's put them in sev-shared.c pls and avoid that.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 4/17/24 15:40, Borislav Petkov wrote:
> On Mon, Mar 25, 2024 at 05:26:22PM -0500, Tom Lendacky wrote:
>> During early boot phases, check for the presence of an SVSM when running
>> as an SEV-SNP guest.
>>
>> An SVSM is present if 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.
>
> Where are we pointing to the SVSM spec?
>
> This is in the 0th message
>
> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58019.pdf
>
> but pls add it to our documentation here:
>
> Documentation/arch/x86/amd-memory-encryption.rst
Do you want it added as a in this patch or in a documentation patch at the
end of the series?
>
>> Signed-off-by: Tom Lendacky <[email protected]>
>> ---
>> arch/x86/boot/compressed/sev.c | 35 ++++++++---------
>> arch/x86/include/asm/sev-common.h | 4 ++
>> arch/x86/include/asm/sev.h | 25 +++++++++++-
>> arch/x86/kernel/sev-shared.c | 64 +++++++++++++++++++++++++++++++
>> arch/x86/kernel/sev.c | 16 ++++++++
>> 5 files changed, 125 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
>> index 49dc9661176d..fe61ff630c7e 100644
>> --- a/arch/x86/boot/compressed/sev.c
>> +++ b/arch/x86/boot/compressed/sev.c
>> @@ -12,6 +12,7 @@
>> */
>> #include "misc.h"
>>
>> +#include <linux/mm.h>
>> #include <asm/bootparam.h>
>> #include <asm/pgtable_types.h>
>> #include <asm/sev.h>
>> @@ -29,6 +30,15 @@
>> static struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
>> struct ghcb *boot_ghcb;
>>
>> +/*
>> + * SVSM related information:
>> + * When running under an SVSM, the VMPL that Linux is executing at must be
>> + * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
>> + */
>> +static u8 vmpl __section(".data");
>> +static u64 boot_svsm_caa_pa __section(".data");
>> +static struct svsm_ca *boot_svsm_caa __section(".data");
>
> Explain what those last 2 are in comments above it pls.
Will do.
>
>> /*
>> * SNP_FEATURES_IMPL_REQ is the mask of SNP features that will need
>> * guest side implementation for proper functioning of the guest. If any
>> @@ -480,6 +472,13 @@ static bool early_snp_init(struct boot_params *bp)
>> */
>> setup_cpuid_table(cc_info);
>>
>> + /*
>> + * Record the SVSM Calling Area address (CAA) if the guest is not
>
> Calling Area (CA) address
>
>> + * running at VMPL0. The CA will be used to communicate with the
>
> and then you can use "CA" here.
Will do.
>
>> + * SVSM to perform the SVSM services.
>> + */
>> + setup_svsm_ca(cc_info);
>> +
>> /*
>> * Pass run-time kernel a pointer to CC info via boot_params so EFI
>> * config table doesn't need to be searched again during early startup
>> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
>> index b463fcbd4b90..68a8cdf6fd6a 100644
>> --- a/arch/x86/include/asm/sev-common.h
>> +++ b/arch/x86/include/asm/sev-common.h
>> @@ -159,6 +159,10 @@ struct snp_psc_desc {
>> #define GHCB_TERM_NOT_VMPL0 3 /* SNP guest is not running at VMPL-0 */
>> #define GHCB_TERM_CPUID 4 /* CPUID-validation failure */
>> #define GHCB_TERM_CPUID_HV 5 /* CPUID failure during hypervisor fallback */
>> +#define GHCB_TERM_SECRETS_PAGE 6 /* Secrets page failure */
>> +#define GHCB_TERM_NO_SVSM 7 /* SVSM is not advertised in the secrets page */
>> +#define GHCB_TERM_SVSM_VMPL0 8 /* SVSM is present but has set VMPL to 0 */
>> +#define GHCB_TERM_SVSM_CAA 9 /* SVSM is present but the CA is not page aligned */
>
> "CAA" in the comment I guess. :)
Will do.
>
>> +/*
>> + * Maintain the GPA of the SVSM Calling Area (CA) in order to utilize the SVSM
>> + * services needed when not running in VMPL0.
>> + */
>> +static void __head setup_svsm_ca(const struct cc_blob_sev_info *cc_info)
>> +{
>> + struct snp_secrets_page_layout *secrets_page;
>
> Why was that thing ever called "_layout" and not simply
> snp_secrets_page?
>
> Fix it?
Sure, I can change that as a pre-patch to the series.
>
>> + u64 caa;
>> +
>> + BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
>
> Put it in the header under the struct definition I guess.
It can't stand on it's own in the header file. I'd have to put it in a
#define or an inline function and then use that in some code. So it's
probably best to keep it here.
>
>> + /*
>> + * Use __pa() since this routine is running identity mapped when
>> + * called, both by the decompressor code and the early kernel code.
>> + */
>> + if (running_at_vmpl0((void *)__pa(&boot_ghcb_page)))
>> + return;
>> +
>> + /*
>> + * Not running at VMPL0, ensure everything has been properly supplied
>> + * for running under an SVSM.
>> + */
>> + 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_layout *)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);
>> +
>> + vmpl = secrets_page->svsm_guest_vmpl;
>> +
>> + caa = secrets_page->svsm_caa;
>> + if (!PAGE_ALIGNED(caa))
>> + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_CAA);
>> +
>> + /*
>> + * 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;
>> +}
>> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
>> index b59b09c2f284..64799a04feb4 100644
>> --- a/arch/x86/kernel/sev.c
>> +++ b/arch/x86/kernel/sev.c
>> @@ -135,6 +135,15 @@ struct ghcb_state {
>> static DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
>> static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
>>
>> +/*
>> + * SVSM related information:
>> + * When running under an SVSM, the VMPL that Linux is executing at must be
>> + * non-zero. The VMPL is therefore used to indicate the presence of an SVSM.
>> + */
>> +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;
>
> Uff, duplication.
>
> Let's put them in sev-shared.c pls and avoid that.
Ok, but it will require moving some functions after the inclusion of
sev-shared.c and then (later) adding some advance function declarations.
Thanks,
Tom
>
> Thx.
>
On Thu, Apr 18, 2024 at 04:17:36PM -0500, Tom Lendacky wrote:
> Do you want it added as a in this patch or in a documentation patch at the
> end of the series?
Either way's fine.
> > Why was that thing ever called "_layout" and not simply
> > snp_secrets_page?
> >
> > Fix it?
>
> Sure, I can change that as a pre-patch to the series.
Ack.
>
> >
> > > + u64 caa;
> > > +
> > > + BUILD_BUG_ON(sizeof(*secrets_page) != PAGE_SIZE);
> >
> > Put it in the header under the struct definition I guess.
>
> It can't stand on it's own in the header file. I'd have to put it in a
> #define or an inline function and then use that in some code. So it's
> probably best to keep it here.
You can always put it in an inline function in the header to move this
macro out of the way but ok, one macro is not too nasty yet. :-)
> > Uff, duplication.
> >
> > Let's put them in sev-shared.c pls and avoid that.
>
> Ok, but it will require moving some functions after the inclusion of
> sev-shared.c and then (later) adding some advance function declarations.
I guess I'll have to see it.
I get the feeling that this sev-shared.c is starting to get problematic
and we have to do some dancing to get it all to work nicely.
In this particular case, those decompressor and kernel proper variables
should probably be passed explicitly to the shared function or returned
from it so that there's no "magic" fitting of the shared function
touching external variables of the same name and thus those names are
kept the same and it all becomes fragile.
IOW:
svsm_ca = setup_svsm_ca(...);
svsm_ca_pa = (unsigned long)svsm_ca;
or whatever needs to happen. But you get the idea...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette