public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Support for SEV-ES guest shadow stack
@ 2025-09-08 20:20 John Allen
  2025-09-08 20:20 ` [PATCH v2 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h John Allen
  2025-09-08 20:20 ` [PATCH v2 2/2] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
  0 siblings, 2 replies; 5+ messages in thread
From: John Allen @ 2025-09-08 20:20 UTC (permalink / raw)
  To: kvm, linux-kernel, x86, seanjc, pbonzini, dave.hansen
  Cc: rick.p.edgecombe, mlevitsk, weijiang.yang, chao.gao, bp,
	dave.hansen, hpa, mingo, tglx, thomas.lendacky, John Allen

For shadow stack support in SVM when using SEV-ES, the guest kernel
needs to save XSS to the GHCB in order for the hypervisor to determine
the XSAVES save area size.

This series can be applied independently of the hypervisor series in
order to support non-KVM hypervisors.
---
v2:
  - Update changelog for patch 2/2

John Allen (2):
  x86/boot: Move boot_*msr helpers to asm/shared/msr.h
  x86/sev-es: Include XSS value in GHCB CPUID request

 arch/x86/boot/compressed/sev.c    |  7 ++++---
 arch/x86/boot/compressed/sev.h    |  6 +++---
 arch/x86/boot/cpucheck.c          | 16 ++++++++--------
 arch/x86/boot/msr.h               | 26 --------------------------
 arch/x86/coco/sev/vc-shared.c     | 11 +++++++++++
 arch/x86/include/asm/shared/msr.h | 15 +++++++++++++++
 arch/x86/include/asm/svm.h        |  1 +
 7 files changed, 42 insertions(+), 40 deletions(-)
 delete mode 100644 arch/x86/boot/msr.h

-- 
2.47.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h
  2025-09-08 20:20 [PATCH v2 0/2] Support for SEV-ES guest shadow stack John Allen
@ 2025-09-08 20:20 ` John Allen
  2025-09-08 20:20 ` [PATCH v2 2/2] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
  1 sibling, 0 replies; 5+ messages in thread
From: John Allen @ 2025-09-08 20:20 UTC (permalink / raw)
  To: kvm, linux-kernel, x86, seanjc, pbonzini, dave.hansen
  Cc: rick.p.edgecombe, mlevitsk, weijiang.yang, chao.gao, bp,
	dave.hansen, hpa, mingo, tglx, thomas.lendacky, John Allen

The boot_rdmsr and boot_wrmsr helpers used to reduce the need for inline
assembly in the boot kernel can also be useful in code shared by boot
and run-time kernel code. Move these helpers to asm/shared/msr.h and
rename to raw_rdmsr and raw_wrmsr to indicate that these may also be
used outside of the boot kernel.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: John Allen <john.allen@amd.com>
---
 arch/x86/boot/compressed/sev.c    |  7 ++++---
 arch/x86/boot/compressed/sev.h    |  6 +++---
 arch/x86/boot/cpucheck.c          | 16 ++++++++--------
 arch/x86/boot/msr.h               | 26 --------------------------
 arch/x86/include/asm/shared/msr.h | 15 +++++++++++++++
 5 files changed, 30 insertions(+), 40 deletions(-)
 delete mode 100644 arch/x86/boot/msr.h

diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index fd1b67dfea22..250b7156bd0f 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -14,6 +14,7 @@
 
 #include <asm/bootparam.h>
 #include <asm/pgtable_types.h>
+#include <asm/shared/msr.h>
 #include <asm/sev.h>
 #include <asm/trapnr.h>
 #include <asm/trap_pf.h>
@@ -436,7 +437,7 @@ void sev_enable(struct boot_params *bp)
 	}
 
 	/* Set the SME mask if this is an SEV guest. */
-	boot_rdmsr(MSR_AMD64_SEV, &m);
+	raw_rdmsr(MSR_AMD64_SEV, &m);
 	sev_status = m.q;
 	if (!(sev_status & MSR_AMD64_SEV_ENABLED))
 		return;
@@ -499,7 +500,7 @@ u64 sev_get_status(void)
 	if (sev_check_cpu_support() < 0)
 		return 0;
 
-	boot_rdmsr(MSR_AMD64_SEV, &m);
+	raw_rdmsr(MSR_AMD64_SEV, &m);
 	return m.q;
 }
 
@@ -549,7 +550,7 @@ bool early_is_sevsnp_guest(void)
 			struct msr m;
 
 			/* Obtain the address of the calling area to use */
-			boot_rdmsr(MSR_SVSM_CAA, &m);
+			raw_rdmsr(MSR_SVSM_CAA, &m);
 			boot_svsm_caa = (void *)m.q;
 			boot_svsm_caa_pa = m.q;
 
diff --git a/arch/x86/boot/compressed/sev.h b/arch/x86/boot/compressed/sev.h
index 92f79c21939c..81766d002c0a 100644
--- a/arch/x86/boot/compressed/sev.h
+++ b/arch/x86/boot/compressed/sev.h
@@ -10,7 +10,7 @@
 
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 
-#include "../msr.h"
+#include "asm/shared/msr.h"
 
 void snp_accept_memory(phys_addr_t start, phys_addr_t end);
 u64 sev_get_status(void);
@@ -20,7 +20,7 @@ static inline u64 sev_es_rd_ghcb_msr(void)
 {
 	struct msr m;
 
-	boot_rdmsr(MSR_AMD64_SEV_ES_GHCB, &m);
+	raw_rdmsr(MSR_AMD64_SEV_ES_GHCB, &m);
 
 	return m.q;
 }
@@ -30,7 +30,7 @@ static inline void sev_es_wr_ghcb_msr(u64 val)
 	struct msr m;
 
 	m.q = val;
-	boot_wrmsr(MSR_AMD64_SEV_ES_GHCB, &m);
+	raw_wrmsr(MSR_AMD64_SEV_ES_GHCB, &m);
 }
 
 #else
diff --git a/arch/x86/boot/cpucheck.c b/arch/x86/boot/cpucheck.c
index f82de8de5dc6..2e1bb936cba2 100644
--- a/arch/x86/boot/cpucheck.c
+++ b/arch/x86/boot/cpucheck.c
@@ -26,9 +26,9 @@
 #include <asm/intel-family.h>
 #include <asm/processor-flags.h>
 #include <asm/msr-index.h>
+#include <asm/shared/msr.h>
 
 #include "string.h"
-#include "msr.h"
 
 static u32 err_flags[NCAPINTS];
 
@@ -134,9 +134,9 @@ int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr)
 
 		struct msr m;
 
-		boot_rdmsr(MSR_K7_HWCR, &m);
+		raw_rdmsr(MSR_K7_HWCR, &m);
 		m.l &= ~(1 << 15);
-		boot_wrmsr(MSR_K7_HWCR, &m);
+		raw_wrmsr(MSR_K7_HWCR, &m);
 
 		get_cpuflags();	/* Make sure it really did something */
 		err = check_cpuflags();
@@ -148,9 +148,9 @@ int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr)
 
 		struct msr m;
 
-		boot_rdmsr(MSR_VIA_FCR, &m);
+		raw_rdmsr(MSR_VIA_FCR, &m);
 		m.l |= (1 << 1) | (1 << 7);
-		boot_wrmsr(MSR_VIA_FCR, &m);
+		raw_wrmsr(MSR_VIA_FCR, &m);
 
 		set_bit(X86_FEATURE_CX8, cpu.flags);
 		err = check_cpuflags();
@@ -160,14 +160,14 @@ int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr)
 		struct msr m, m_tmp;
 		u32 level = 1;
 
-		boot_rdmsr(0x80860004, &m);
+		raw_rdmsr(0x80860004, &m);
 		m_tmp = m;
 		m_tmp.l = ~0;
-		boot_wrmsr(0x80860004, &m_tmp);
+		raw_wrmsr(0x80860004, &m_tmp);
 		asm("cpuid"
 		    : "+a" (level), "=d" (cpu.flags[0])
 		    : : "ecx", "ebx");
-		boot_wrmsr(0x80860004, &m);
+		raw_wrmsr(0x80860004, &m);
 
 		err = check_cpuflags();
 	} else if (err == 0x01 &&
diff --git a/arch/x86/boot/msr.h b/arch/x86/boot/msr.h
deleted file mode 100644
index aed66f7ae199..000000000000
--- a/arch/x86/boot/msr.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Helpers/definitions related to MSR access.
- */
-
-#ifndef BOOT_MSR_H
-#define BOOT_MSR_H
-
-#include <asm/shared/msr.h>
-
-/*
- * The kernel proper already defines rdmsr()/wrmsr(), but they are not for the
- * boot kernel since they rely on tracepoint/exception handling infrastructure
- * that's not available here.
- */
-static inline void boot_rdmsr(unsigned int reg, struct msr *m)
-{
-	asm volatile("rdmsr" : "=a" (m->l), "=d" (m->h) : "c" (reg));
-}
-
-static inline void boot_wrmsr(unsigned int reg, const struct msr *m)
-{
-	asm volatile("wrmsr" : : "c" (reg), "a"(m->l), "d" (m->h) : "memory");
-}
-
-#endif /* BOOT_MSR_H */
diff --git a/arch/x86/include/asm/shared/msr.h b/arch/x86/include/asm/shared/msr.h
index 1e6ec10b3a15..a20b1c08c99f 100644
--- a/arch/x86/include/asm/shared/msr.h
+++ b/arch/x86/include/asm/shared/msr.h
@@ -12,4 +12,19 @@ struct msr {
 	};
 };
 
+/*
+ * The kernel proper already defines rdmsr()/wrmsr(), but they are not for the
+ * boot kernel since they rely on tracepoint/exception handling infrastructure
+ * that's not available here.
+ */
+static inline void raw_rdmsr(unsigned int reg, struct msr *m)
+{
+	asm volatile("rdmsr" : "=a" (m->l), "=d" (m->h) : "c" (reg));
+}
+
+static inline void raw_wrmsr(unsigned int reg, const struct msr *m)
+{
+	asm volatile("wrmsr" : : "c" (reg), "a"(m->l), "d" (m->h) : "memory");
+}
+
 #endif /* _ASM_X86_SHARED_MSR_H */
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] x86/sev-es: Include XSS value in GHCB CPUID request
  2025-09-08 20:20 [PATCH v2 0/2] Support for SEV-ES guest shadow stack John Allen
  2025-09-08 20:20 ` [PATCH v2 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h John Allen
@ 2025-09-08 20:20 ` John Allen
  2025-09-09 20:04   ` Tom Lendacky
  1 sibling, 1 reply; 5+ messages in thread
From: John Allen @ 2025-09-08 20:20 UTC (permalink / raw)
  To: kvm, linux-kernel, x86, seanjc, pbonzini, dave.hansen
  Cc: rick.p.edgecombe, mlevitsk, weijiang.yang, chao.gao, bp,
	dave.hansen, hpa, mingo, tglx, thomas.lendacky, John Allen

When a guest issues a cpuid instruction for Fn0000000D_{x00,x01}, the
hypervisor will be intercepting the CPUID instruction and will need to access
the guest XSS value. For SEV-ES, the XSS value is encrypted and needs to be
included in the GHCB to be visible to the hypervisor.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: John Allen <john.allen@amd.com>
---
 arch/x86/coco/sev/vc-shared.c | 11 +++++++++++
 arch/x86/include/asm/svm.h    |  1 +
 2 files changed, 12 insertions(+)

diff --git a/arch/x86/coco/sev/vc-shared.c b/arch/x86/coco/sev/vc-shared.c
index 2c0ab0fdc060..079fffdb12c0 100644
--- a/arch/x86/coco/sev/vc-shared.c
+++ b/arch/x86/coco/sev/vc-shared.c
@@ -1,5 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#ifndef __BOOT_COMPRESSED
+#define has_cpuflag(f)                  boot_cpu_has(f)
+#endif
+
 static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
 					    unsigned long exit_code)
 {
@@ -452,6 +456,13 @@ static enum es_result vc_handle_cpuid(struct ghcb *ghcb,
 		/* xgetbv will cause #GP - use reset value for xcr0 */
 		ghcb_set_xcr0(ghcb, 1);
 
+	if (has_cpuflag(X86_FEATURE_SHSTK) && regs->ax == 0xd && regs->cx <= 1) {
+		struct msr m;
+
+		raw_rdmsr(MSR_IA32_XSS, &m);
+		ghcb_set_xss(ghcb, m.q);
+	}
+
 	ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0);
 	if (ret != ES_OK)
 		return ret;
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 17f6c3fedeee..0581c477d466 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -701,5 +701,6 @@ DEFINE_GHCB_ACCESSORS(sw_exit_info_1)
 DEFINE_GHCB_ACCESSORS(sw_exit_info_2)
 DEFINE_GHCB_ACCESSORS(sw_scratch)
 DEFINE_GHCB_ACCESSORS(xcr0)
+DEFINE_GHCB_ACCESSORS(xss)
 
 #endif
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] x86/sev-es: Include XSS value in GHCB CPUID request
  2025-09-08 20:20 ` [PATCH v2 2/2] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
@ 2025-09-09 20:04   ` Tom Lendacky
  2025-09-16 19:44     ` Sean Christopherson
  0 siblings, 1 reply; 5+ messages in thread
From: Tom Lendacky @ 2025-09-09 20:04 UTC (permalink / raw)
  To: John Allen, kvm, linux-kernel, x86, seanjc, pbonzini, dave.hansen
  Cc: rick.p.edgecombe, mlevitsk, weijiang.yang, chao.gao, bp,
	dave.hansen, hpa, mingo, tglx

On 9/8/25 15:20, John Allen wrote:
> When a guest issues a cpuid instruction for Fn0000000D_{x00,x01}, the
> hypervisor will be intercepting the CPUID instruction and will need to access
> the guest XSS value. For SEV-ES, the XSS value is encrypted and needs to be
> included in the GHCB to be visible to the hypervisor.
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: John Allen <john.allen@amd.com>
> ---
>  arch/x86/coco/sev/vc-shared.c | 11 +++++++++++
>  arch/x86/include/asm/svm.h    |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/x86/coco/sev/vc-shared.c b/arch/x86/coco/sev/vc-shared.c
> index 2c0ab0fdc060..079fffdb12c0 100644
> --- a/arch/x86/coco/sev/vc-shared.c
> +++ b/arch/x86/coco/sev/vc-shared.c
> @@ -1,5 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +#ifndef __BOOT_COMPRESSED
> +#define has_cpuflag(f)                  boot_cpu_has(f)
> +#endif
> +
>  static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
>  					    unsigned long exit_code)
>  {
> @@ -452,6 +456,13 @@ static enum es_result vc_handle_cpuid(struct ghcb *ghcb,
>  		/* xgetbv will cause #GP - use reset value for xcr0 */
>  		ghcb_set_xcr0(ghcb, 1);
>  
> +	if (has_cpuflag(X86_FEATURE_SHSTK) && regs->ax == 0xd && regs->cx <= 1) {

Just a nit, but I wonder if we should be generic here and just do
has_cpuflag(X86_FEATURE_XSAVES) since that should be set if shadow stack
is enabled, right? And when X86_FEATURE_XSAVES is set, we don't
intercept XSS access (see sev_es_recalc_msr_intercepts()).

Thoughts?

Thanks,
Tom

> +		struct msr m;
> +
> +		raw_rdmsr(MSR_IA32_XSS, &m);
> +		ghcb_set_xss(ghcb, m.q);
> +	}
> +
>  	ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0);
>  	if (ret != ES_OK)
>  		return ret;
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 17f6c3fedeee..0581c477d466 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -701,5 +701,6 @@ DEFINE_GHCB_ACCESSORS(sw_exit_info_1)
>  DEFINE_GHCB_ACCESSORS(sw_exit_info_2)
>  DEFINE_GHCB_ACCESSORS(sw_scratch)
>  DEFINE_GHCB_ACCESSORS(xcr0)
> +DEFINE_GHCB_ACCESSORS(xss)
>  
>  #endif

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] x86/sev-es: Include XSS value in GHCB CPUID request
  2025-09-09 20:04   ` Tom Lendacky
@ 2025-09-16 19:44     ` Sean Christopherson
  0 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2025-09-16 19:44 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: John Allen, kvm, linux-kernel, x86, pbonzini, dave.hansen,
	rick.p.edgecombe, mlevitsk, weijiang.yang, chao.gao, bp,
	dave.hansen, hpa, mingo, tglx

On Tue, Sep 09, 2025, Tom Lendacky wrote:
> On 9/8/25 15:20, John Allen wrote:
> > When a guest issues a cpuid instruction for Fn0000000D_{x00,x01}, the
> > hypervisor will be intercepting the CPUID instruction and will need to access
> > the guest XSS value. For SEV-ES, the XSS value is encrypted and needs to be
> > included in the GHCB to be visible to the hypervisor.
> > 
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > Signed-off-by: John Allen <john.allen@amd.com>
> > ---
> >  arch/x86/coco/sev/vc-shared.c | 11 +++++++++++
> >  arch/x86/include/asm/svm.h    |  1 +
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/arch/x86/coco/sev/vc-shared.c b/arch/x86/coco/sev/vc-shared.c
> > index 2c0ab0fdc060..079fffdb12c0 100644
> > --- a/arch/x86/coco/sev/vc-shared.c
> > +++ b/arch/x86/coco/sev/vc-shared.c
> > @@ -1,5 +1,9 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  
> > +#ifndef __BOOT_COMPRESSED
> > +#define has_cpuflag(f)                  boot_cpu_has(f)
> > +#endif
> > +
> >  static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
> >  					    unsigned long exit_code)
> >  {
> > @@ -452,6 +456,13 @@ static enum es_result vc_handle_cpuid(struct ghcb *ghcb,
> >  		/* xgetbv will cause #GP - use reset value for xcr0 */
> >  		ghcb_set_xcr0(ghcb, 1);
> >  
> > +	if (has_cpuflag(X86_FEATURE_SHSTK) && regs->ax == 0xd && regs->cx <= 1) {

Only CPUID.0xD.1 consumes XSS.  CPUID.0xD.0 only consumes XCR0.  I.e. this could
be "&& regs->cx == 1".

> Just a nit, but I wonder if we should be generic here and just do
> has_cpuflag(X86_FEATURE_XSAVES) since that should be set if shadow stack
> is enabled, right? And when X86_FEATURE_XSAVES is set, we don't
> intercept XSS access (see sev_es_recalc_msr_intercepts()).

On the other hand, by exposing XSS to the host only on CPUID #VCs, you've already
"optimized" this code based on presumed usage of XSS by the hypervisor.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-16 19:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-08 20:20 [PATCH v2 0/2] Support for SEV-ES guest shadow stack John Allen
2025-09-08 20:20 ` [PATCH v2 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h John Allen
2025-09-08 20:20 ` [PATCH v2 2/2] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
2025-09-09 20:04   ` Tom Lendacky
2025-09-16 19:44     ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox