[PATCH v3 0/2] Support for SEV-ES guest shadow stack

public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v3 0/2] Support for SEV-ES guest shadow stack
@ 2025-09-24 20:08 John Allen
  2025-09-24 20:08 ` [PATCH v3 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h John Allen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: John Allen @ 2025-09-24 20:08 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.
---
v3:
  - Only CPUID.0xD.1 consumes XSS. Limit including XSS in GHCB for this
    case.
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] 6+ messages in thread

* [PATCH v3 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h
  2025-09-24 20:08 [PATCH v3 0/2] Support for SEV-ES guest shadow stack John Allen
@ 2025-09-24 20:08 ` John Allen
  2025-09-24 20:08 ` [PATCH v3 2/2] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
  2025-10-31 12:21 ` [PATCH] x86/coco/sev: Convert has_cpuflag() to use cpu_feature_enabled() Borislav Petkov
  2 siblings, 0 replies; 6+ messages in thread
From: John Allen @ 2025-09-24 20:08 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] 6+ messages in thread

* [PATCH v3 2/2] x86/sev-es: Include XSS value in GHCB CPUID request
  2025-09-24 20:08 [PATCH v3 0/2] Support for SEV-ES guest shadow stack John Allen
  2025-09-24 20:08 ` [PATCH v3 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h John Allen
@ 2025-09-24 20:08 ` John Allen
  2025-09-24 23:02   ` Dave Hansen
  2025-10-31 12:21 ` [PATCH] x86/coco/sev: Convert has_cpuflag() to use cpu_feature_enabled() Borislav Petkov
  2 siblings, 1 reply; 6+ messages in thread
From: John Allen @ 2025-09-24 20:08 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_x01, the hypervisor may
be intercepting the CPUID instruction and 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>
---
v3:
  - Only CPUID.0xD.1 consumes XSS. Limit including XSS in GHCB for this
    case.
---
 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..b281feeda32b 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] 6+ messages in thread

* Re: [PATCH v3 2/2] x86/sev-es: Include XSS value in GHCB CPUID request
  2025-09-24 20:08 ` [PATCH v3 2/2] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
@ 2025-09-24 23:02   ` Dave Hansen
  2025-09-26 19:22     ` John Allen
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Hansen @ 2025-09-24 23:02 UTC (permalink / raw)
  To: John Allen, kvm, linux-kernel, x86, seanjc, pbonzini
  Cc: rick.p.edgecombe, mlevitsk, weijiang.yang, chao.gao, bp,
	dave.hansen, hpa, mingo, tglx, thomas.lendacky

On 9/24/25 13:08, John Allen wrote:
> +	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);
> +	}

Is there a reason this is open-coding CPUID_LEAF_XSTATE?

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

* Re: [PATCH v3 2/2] x86/sev-es: Include XSS value in GHCB CPUID request
  2025-09-24 23:02   ` Dave Hansen
@ 2025-09-26 19:22     ` John Allen
  0 siblings, 0 replies; 6+ messages in thread
From: John Allen @ 2025-09-26 19:22 UTC (permalink / raw)
  To: Dave Hansen
  Cc: kvm, linux-kernel, x86, seanjc, pbonzini, rick.p.edgecombe,
	mlevitsk, weijiang.yang, chao.gao, bp, dave.hansen, hpa, mingo,
	tglx, thomas.lendacky

On Wed, Sep 24, 2025 at 04:02:11PM -0700, Dave Hansen wrote:
> On 9/24/25 13:08, John Allen wrote:
> > +	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);
> > +	}
> 
> Is there a reason this is open-coding CPUID_LEAF_XSTATE?

That's a good question. This patch was adapted from an old SNP patch
from years ago so if I had to guess, maybe there was some issue bringing
in the old pre-split cpuid header into the early boot/shared
environment. It looks like including asm/cpuid/types.h and using
CPUID_LEAF_XSTATE works here and now so I'll include it in the next
version.

Thanks,
John

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

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

On Wed, Sep 24, 2025 at 08:08:50PM +0000, John Allen wrote:
> 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.
> ---
> v3:
>   - Only CPUID.0xD.1 consumes XSS. Limit including XSS in GHCB for this
>     case.
> 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

---

Ontop:

From: "Borislav Petkov (AMD)" <bp@alien8.de>
Date: Thu, 30 Oct 2025 17:59:11 +0100
Subject: [PATCH] x86/coco/sev: Convert has_cpuflag() to use cpu_feature_enabled()

Drop one redundant definition, while at it.

There should be no functional changes.

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 arch/x86/boot/startup/sev-shared.c | 2 +-
 arch/x86/coco/sev/vc-handle.c      | 1 -
 arch/x86/coco/sev/vc-shared.c      | 2 +-
 arch/x86/lib/kaslr.c               | 2 +-
 4 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
index 4e22ffd73516..a0fa8bb2b945 100644
--- a/arch/x86/boot/startup/sev-shared.c
+++ b/arch/x86/boot/startup/sev-shared.c
@@ -12,7 +12,7 @@
 #include <asm/setup_data.h>
 
 #ifndef __BOOT_COMPRESSED
-#define has_cpuflag(f)			boot_cpu_has(f)
+#define has_cpuflag(f)			cpu_feature_enabled(f)
 #else
 #undef WARN
 #define WARN(condition, format...) (!!(condition))
diff --git a/arch/x86/coco/sev/vc-handle.c b/arch/x86/coco/sev/vc-handle.c
index 7fc136a35334..f08c7505ed82 100644
--- a/arch/x86/coco/sev/vc-handle.c
+++ b/arch/x86/coco/sev/vc-handle.c
@@ -352,7 +352,6 @@ static enum es_result vc_read_mem(struct es_em_ctxt *ctxt,
 
 #define sev_printk(fmt, ...)		printk(fmt, ##__VA_ARGS__)
 #define error(v)
-#define has_cpuflag(f)			boot_cpu_has(f)
 
 #include "vc-shared.c"
 
diff --git a/arch/x86/coco/sev/vc-shared.c b/arch/x86/coco/sev/vc-shared.c
index e2ac95de4611..58b2f985d546 100644
--- a/arch/x86/coco/sev/vc-shared.c
+++ b/arch/x86/coco/sev/vc-shared.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #ifndef __BOOT_COMPRESSED
-#define has_cpuflag(f)                  boot_cpu_has(f)
+#define has_cpuflag(f)                  cpu_feature_enabled(f)
 #endif
 
 static enum es_result vc_check_opcode_bytes(struct es_em_ctxt *ctxt,
diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
index b5893928d55c..8c7cd115b484 100644
--- a/arch/x86/lib/kaslr.c
+++ b/arch/x86/lib/kaslr.c
@@ -22,7 +22,7 @@
 #include <asm/setup.h>
 
 #define debug_putstr(v) early_printk("%s", v)
-#define has_cpuflag(f) boot_cpu_has(f)
+#define has_cpuflag(f) cpu_feature_enabled(f)
 #define get_boot_seed() kaslr_offset()
 #endif
 
-- 
2.51.0

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

end of thread, other threads:[~2025-10-31 12:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-24 20:08 [PATCH v3 0/2] Support for SEV-ES guest shadow stack John Allen
2025-09-24 20:08 ` [PATCH v3 1/2] x86/boot: Move boot_*msr helpers to asm/shared/msr.h John Allen
2025-09-24 20:08 ` [PATCH v3 2/2] x86/sev-es: Include XSS value in GHCB CPUID request John Allen
2025-09-24 23:02   ` Dave Hansen
2025-09-26 19:22     ` John Allen
2025-10-31 12:21 ` [PATCH] x86/coco/sev: Convert has_cpuflag() to use cpu_feature_enabled() Borislav Petkov

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