Skip to contentSkip to conversation

x86/sev: Fully map the #VC exception stacks

PATCH In review Tom Lendacky 6 participants 14 messages Oct 1, 2021 – Oct 6, 2021

Subsystems (2)
x86 x86 MM
lore
Share & tools
lore

Tom Lendacky

author
#0 lore raw

PATCH 1/1 x86/sev: Fully map the #VC exception stacks

The size of the exception stacks was recently increased, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Update setup_vc_stacks() to map all the pages of both the IST stack area and the fallback stack area.

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")
Signed-off-by Tom Lendacky <[email protected]>
---
 arch/x86/kernel/sev.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)
arch/x86/kernel/sev.c +16 -8
index a6895e440bc3..33e4704164cc 100644
@@ -99,25 +99,33 @@ DEFINE_STATIC_KEY_FALSE(sev_es_enable_key);
 /* Needed in vc_early_forward_exception */
 void do_early_exception(struct pt_regs *regs, int trapnr);
 
+static void __init map_vc_stack(unsigned long bot, unsigned long top,
+				phys_addr_t pa)
+{
+	while (bot < top) {
+		cea_set_pte((void *)bot, pa, PAGE_KERNEL);
+		bot += PAGE_SIZE;
+		pa += PAGE_SIZE;
+	}
+}
+
 static void __init setup_vc_stacks(int cpu)
 {
 	struct sev_es_runtime_data *data;
 	struct cpu_entry_area *cea;
-	unsigned long vaddr;
-	phys_addr_t pa;
 
 	data = per_cpu(runtime_data, cpu);
 	cea  = get_cpu_entry_area(cpu);
 
 	/* Map #VC IST stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC);
-	pa    = __pa(data->ist_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
+	map_vc_stack(CEA_ESTACK_BOT(&cea->estacks, VC),
+		     CEA_ESTACK_TOP(&cea->estacks, VC),
+		     __pa(data->ist_stack));
 
 	/* Map VC fall-back stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC2);
-	pa    = __pa(data->fallback_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
+	map_vc_stack(CEA_ESTACK_BOT(&cea->estacks, VC2),
+		     CEA_ESTACK_TOP(&cea->estacks, VC2),
+		     __pa(data->fallback_stack));
 }
 
 static __always_inline bool on_vc_stack(struct pt_regs *regs)
Signature · 1 line
2.33.0
author
#1 lore raw
Quoted 9 lines · Tom Lendacky

The size of the exception stacks was recently increased, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Update setup_vc_stacks() to map all the pages of both the IST stack area and the fallback stack area.

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")

Arguably the Fixes: tag may not be completely accurate, since it was an issue within setup_vc_stacks(). But, this is more for if someone was to pull the patch identified by the Fixes: tag, then they would definitely need this patch to run SEV-ES.

Thanks,
Tom

Quoted 4 lines
Signed-off-by Tom Lendacky <[email protected]>
---
  arch/x86/kernel/sev.c | 24 ++++++++++++++++--------
  1 file changed, 16 insertions(+), 8 deletions(-)
maintainer
#2 lore raw
In reply to Tom Lendacky
Quoted 39 lines · Tom Lendacky

The size of the exception stacks was recently increased, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Update setup_vc_stacks() to map all the pages of both the IST stack area and the fallback stack area.

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")
Signed-off-by Tom Lendacky <[email protected]>
---
 arch/x86/kernel/sev.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)
arch/x86/kernel/sev.c +10 -2
index a6895e440bc3..33e4704164cc 100644
@@ -99,25 +99,33 @@ DEFINE_STATIC_KEY_FALSE(sev_es_enable_key);
 /* Needed in vc_early_forward_exception */
 void do_early_exception(struct pt_regs *regs, int trapnr);
 
+static void __init map_vc_stack(unsigned long bot, unsigned long top,
+				phys_addr_t pa)
+{
+	while (bot < top) {
+		cea_set_pte((void *)bot, pa, PAGE_KERNEL);
+		bot += PAGE_SIZE;
+		pa += PAGE_SIZE;
+	}
+}
+
 static void __init setup_vc_stacks(int cpu)
 {
 	struct sev_es_runtime_data *data;
 	struct cpu_entry_area *cea;
-	unsigned long vaddr;
-	phys_addr_t pa;

arch/x86/kernel/sev.c DEFINE_STATIC_KEY_FALSE(sev_es_enable_key); · line 123

Quoted 10 lines
arch/x86/kernel/sev.c +3 -3
 	data = per_cpu(runtime_data, cpu);
 	cea  = get_cpu_entry_area(cpu);
 
 	/* Map #VC IST stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC);
-	pa    = __pa(data->ist_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
+	map_vc_stack(CEA_ESTACK_BOT(&cea->estacks, VC),
+		     CEA_ESTACK_TOP(&cea->estacks, VC),
+		     __pa(data->ist_stack));

So this would not have broken if it would've used EXCEPTION_STKSZ or EXCEPTION_STACK_ORDER rather since we're mapping pages.

Please use those defines so that this keeps working when someone mad decides to increase those exception stack sizes again because everything *and* the kitchen sink wants to instrument the damn kernel. Nothing to see here people...

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
#3 lore raw
Quoted 4 lines · Borislav Petkov

Please use those defines so that this keeps working when someone mad decides to increase those exception stack sizes again because everything *and* the kitchen sink wants to instrument the damn kernel. Nothing to see here people...

Yeah, I think the right fix is to export cea_map_percpu_pages() and move the cea_map_stack() macro to a header and use it to map the VC stacks.

Regards,

Joerg

maintainer
#4 lore raw
Quoted 2 lines · Joerg Roedel

Yeah, I think the right fix is to export cea_map_percpu_pages() and move the cea_map_stack() macro to a header and use it to map the VC stacks.

I'll do you one better: Put the #VC stack mapping into percpu_setup_exception_stacks(), where it naturally belongs and where the other stacks are being mapped instead of doing everything "by hand" like now and exporting random helpers.

Thx.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
#5 lore raw
Quoted 8 lines · Borislav Petkov
Quoted 2 lines · Joerg Roedel

Yeah, I think the right fix is to export cea_map_percpu_pages() and move the cea_map_stack() macro to a header and use it to map the VC stacks.

I'll do you one better: Put the #VC stack mapping into percpu_setup_exception_stacks(), where it naturally belongs and where the other stacks are being mapped instead of doing everything "by hand" like now and exporting random helpers.

The VC stack is only allocated and mapped when SEV-ES is detected, so they can't always be mapped by generic code.

Regards,

Joerg

maintainer
#6 lore raw
Quoted 5 lines · Joerg Roedel
Quoted 4 lines · Borislav Petkov
Quoted 2 lines · Joerg Roedel

Yeah, I think the right fix is to export cea_map_percpu_pages() and move the cea_map_stack() macro to a header and use it to map the VC stacks.

Quoted 7 lines
Quoted 4 lines

I'll do you one better: Put the #VC stack mapping into percpu_setup_exception_stacks(), where it naturally belongs and where the other stacks are being mapped instead of doing everything "by hand" like now and exporting random helpers.

The VC stack is only allocated and mapped when SEV-ES is detected, so they can't always be mapped by generic code.

It's just a few pages per cpu, is that worth it? Why not have it unconditinoally allocated in the right place when it finds the CPU is SEV capable?

maintainer
#7 lore raw
In reply to Joerg Roedel
Quoted 2 lines · Joerg Roedel

The VC stack is only allocated and mapped when SEV-ES is detected, so they can't always be mapped by generic code.

And? I am assuming you do know how to check whether SEV-ES is enabled.

:-)

I also assumed it is implicitly clear that the mapping should not happen unconditionally but of course behind a check.

Thx.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
maintainer
#8 lore raw

It doesn't get any more straight-forward than this.

We can ifdef around the ESTACKS_MEMBERS VC and VC2 arrays so that those things do get allocated only on a CONFIG_AMD_MEM_ENCRYPT kernel so that we don't waste 4 pages per CPU on machines which don't do SEV but meh.

Thoughts?

---

arch/x86/include/asm/cpu_entry_area.h +2 -2
index 3d52b094850a..13a3e8510c33 100644
@@ -21,9 +21,9 @@
 	char	MCE_stack_guard[guardsize];			\
 	char	MCE_stack[EXCEPTION_STKSZ];			\
 	char	VC_stack_guard[guardsize];			\
-	char	VC_stack[optional_stack_size];			\
+	char	VC_stack[EXCEPTION_STKSZ];			\
 	char	VC2_stack_guard[guardsize];			\
-	char	VC2_stack[optional_stack_size];			\
+	char	VC2_stack[EXCEPTION_STKSZ];			\
 	char	IST_top_guard[guardsize];			\
 
 /* The exception stacks' physical storage. No guard pages required */
arch/x86/kernel/sev.c +0 -32
index a6895e440bc3..88401675dabb 100644
@@ -46,16 +46,6 @@ static struct ghcb __initdata *boot_ghcb;
 struct sev_es_runtime_data {
 	struct ghcb ghcb_page;
 
-	/* Physical storage for the per-CPU IST stack of the #VC handler */
-	char ist_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
-	/*
-	 * Physical storage for the per-CPU fall-back stack of the #VC handler.
-	 * The fall-back stack is used when it is not safe to switch back to the
-	 * interrupted stack in the #VC entry code.
-	 */
-	char fallback_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
 	/*
 	 * Reserve one page per CPU as backup storage for the unencrypted GHCB.
 	 * It is needed when an NMI happens while the #VC handler uses the real

@@ -99,27 +89,6 @@ DEFINE_STATIC_KEY_FALSE(sev_es_enable_key);
 /* Needed in vc_early_forward_exception */
 void do_early_exception(struct pt_regs *regs, int trapnr);
 
-static void __init setup_vc_stacks(int cpu)
-{
-	struct sev_es_runtime_data *data;
-	struct cpu_entry_area *cea;
-	unsigned long vaddr;
-	phys_addr_t pa;
-
-	data = per_cpu(runtime_data, cpu);
-	cea  = get_cpu_entry_area(cpu);
-
-	/* Map #VC IST stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC);
-	pa    = __pa(data->ist_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-
-	/* Map VC fall-back stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC2);
-	pa    = __pa(data->fallback_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-}
-
 static __always_inline bool on_vc_stack(struct pt_regs *regs)
 {
 	unsigned long sp = regs->sp;

@@ -787,7 +756,6 @@ void __init sev_es_init_vc_handling(void)
 	for_each_possible_cpu(cpu) {
 		alloc_runtime_data(cpu);
 		init_ghcb(cpu);
-		setup_vc_stacks(cpu);
 	}
 
 	sev_es_setup_play_dead();
arch/x86/mm/cpu_entry_area.c +7 -0
index f5e1e60c9095..82d062414f19 100644
@@ -110,6 +110,13 @@ static void __init percpu_setup_exception_stacks(unsigned int cpu)
 	cea_map_stack(NMI);
 	cea_map_stack(DB);
 	cea_map_stack(MCE);
+
+	if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
+		if (sev_es_active()) {
+			cea_map_stack(VC);
+			cea_map_stack(VC2);
+		}
+	}
 }
 #else
 static inline void percpu_setup_exception_stacks(unsigned int cpu)
Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
maintainer
#9 lore raw

PATCH 1/1 x86/sev: Make the #VC exception stacks part of the default stacks storage

---
From: Borislav Petkov <[email protected]>
Date: Fri, 1 Oct 2021 21:41:20 +0200

The size of the exception stacks was increased by the commit in Fixes, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Make the #VC exception stacks part of the default exception stacks storage and allocate them with a CONFIG_AMD_MEM_ENCRYPT=y .config. Map them only when a SEV-ES guest has been detected.

Rip out the custom VC stacks mapping and storage code.

[ bp: Steal and adapt Tom's commit message. ]

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")
Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/include/asm/cpu_entry_area.h | 16 +++++++++-----
 arch/x86/kernel/sev.c                 | 32 ---------------------------
 arch/x86/mm/cpu_entry_area.c          |  7 ++++++
 3 files changed, 18 insertions(+), 37 deletions(-)
arch/x86/include/asm/cpu_entry_area.h +11 -5
index 3d52b094850a..2512e1f5ac02 100644
@@ -10,8 +10,14 @@
 
 #ifdef CONFIG_X86_64
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+#define VC_EXCEPTION_STKSZ	EXCEPTION_STKSZ
+#else
+#define VC_EXCEPTION_STKSZ	0
+#endif
+
 /* Macro to enforce the same ordering and stack sizes */
-#define ESTACKS_MEMBERS(guardsize, optional_stack_size)		\
+#define ESTACKS_MEMBERS(guardsize)				\
 	char	DF_stack_guard[guardsize];			\
 	char	DF_stack[EXCEPTION_STKSZ];			\
 	char	NMI_stack_guard[guardsize];			\

@@ -21,19 +27,19 @@
 	char	MCE_stack_guard[guardsize];			\
 	char	MCE_stack[EXCEPTION_STKSZ];			\
 	char	VC_stack_guard[guardsize];			\
-	char	VC_stack[optional_stack_size];			\
+	char	VC_stack[VC_EXCEPTION_STKSZ];			\
 	char	VC2_stack_guard[guardsize];			\
-	char	VC2_stack[optional_stack_size];			\
+	char	VC2_stack[VC_EXCEPTION_STKSZ];			\
 	char	IST_top_guard[guardsize];			\
 
 /* The exception stacks' physical storage. No guard pages required */
 struct exception_stacks {
-	ESTACKS_MEMBERS(0, 0)
+	ESTACKS_MEMBERS(0)
 };
 
 /* The effective cpu entry area mapping with guard pages. */
 struct cea_exception_stacks {
-	ESTACKS_MEMBERS(PAGE_SIZE, EXCEPTION_STKSZ)
+	ESTACKS_MEMBERS(PAGE_SIZE)
 };
 
 /*
arch/x86/kernel/sev.c +0 -32
index a6895e440bc3..88401675dabb 100644
@@ -46,16 +46,6 @@ static struct ghcb __initdata *boot_ghcb;
 struct sev_es_runtime_data {
 	struct ghcb ghcb_page;
 
-	/* Physical storage for the per-CPU IST stack of the #VC handler */
-	char ist_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
-	/*
-	 * Physical storage for the per-CPU fall-back stack of the #VC handler.
-	 * The fall-back stack is used when it is not safe to switch back to the
-	 * interrupted stack in the #VC entry code.
-	 */
-	char fallback_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
 	/*
 	 * Reserve one page per CPU as backup storage for the unencrypted GHCB.
 	 * It is needed when an NMI happens while the #VC handler uses the real

@@ -99,27 +89,6 @@ DEFINE_STATIC_KEY_FALSE(sev_es_enable_key);
 /* Needed in vc_early_forward_exception */
 void do_early_exception(struct pt_regs *regs, int trapnr);
 
-static void __init setup_vc_stacks(int cpu)
-{
-	struct sev_es_runtime_data *data;
-	struct cpu_entry_area *cea;
-	unsigned long vaddr;
-	phys_addr_t pa;
-
-	data = per_cpu(runtime_data, cpu);
-	cea  = get_cpu_entry_area(cpu);
-
-	/* Map #VC IST stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC);
-	pa    = __pa(data->ist_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-
-	/* Map VC fall-back stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC2);
-	pa    = __pa(data->fallback_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-}
-
 static __always_inline bool on_vc_stack(struct pt_regs *regs)
 {
 	unsigned long sp = regs->sp;

@@ -787,7 +756,6 @@ void __init sev_es_init_vc_handling(void)
 	for_each_possible_cpu(cpu) {
 		alloc_runtime_data(cpu);
 		init_ghcb(cpu);
-		setup_vc_stacks(cpu);
 	}
 
 	sev_es_setup_play_dead();
arch/x86/mm/cpu_entry_area.c +7 -0
index f5e1e60c9095..82d062414f19 100644
@@ -110,6 +110,13 @@ static void __init percpu_setup_exception_stacks(unsigned int cpu)
 	cea_map_stack(NMI);
 	cea_map_stack(DB);
 	cea_map_stack(MCE);
+
+	if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
+		if (sev_es_active()) {
+			cea_map_stack(VC);
+			cea_map_stack(VC2);
+		}
+	}
 }
 #else
 static inline void percpu_setup_exception_stacks(unsigned int cpu)
Signature · 8 lines
2.29.2


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
maintainer
#10 lore raw

PATCH 1/1 x86/sev: Make the #VC exception stacks part of the default stacks storage

Yap,

here's v2, now tested. It seems we do need that optional_stack_size second arg to ESTACKS_MEMBERS(), thx Tom.

---
From: Borislav Petkov <[email protected]>

The size of the exception stacks was increased by the commit in Fixes, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Make the #VC exception stacks part of the default exception stacks storage and allocate them with a CONFIG_AMD_MEM_ENCRYPT=y .config. Map them only when a SEV-ES guest has been detected.

Rip out the custom VC stacks mapping and storage code.

[ bp: Steal and adapt Tom's commit message. ]

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")
Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/include/asm/cpu_entry_area.h |  8 ++++++-
 arch/x86/kernel/sev.c                 | 32 ---------------------------
 arch/x86/mm/cpu_entry_area.c          |  7 ++++++
 3 files changed, 14 insertions(+), 33 deletions(-)
arch/x86/include/asm/cpu_entry_area.h +7 -1
index 3d52b094850a..dd5ea1bdf04c 100644
@@ -10,6 +10,12 @@
 
 #ifdef CONFIG_X86_64
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+#define VC_EXCEPTION_STKSZ	EXCEPTION_STKSZ
+#else
+#define VC_EXCEPTION_STKSZ	0
+#endif
+
 /* Macro to enforce the same ordering and stack sizes */
 #define ESTACKS_MEMBERS(guardsize, optional_stack_size)		\
 	char	DF_stack_guard[guardsize];			\

@@ -28,7 +34,7 @@
 
 /* The exception stacks' physical storage. No guard pages required */
 struct exception_stacks {
-	ESTACKS_MEMBERS(0, 0)
+	ESTACKS_MEMBERS(0, VC_EXCEPTION_STKSZ)
 };
 
 /* The effective cpu entry area mapping with guard pages. */
arch/x86/kernel/sev.c +0 -32
index 53a6837d354b..4d0d1c2b65e1 100644
@@ -46,16 +46,6 @@ static struct ghcb __initdata *boot_ghcb;
 struct sev_es_runtime_data {
 	struct ghcb ghcb_page;
 
-	/* Physical storage for the per-CPU IST stack of the #VC handler */
-	char ist_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
-	/*
-	 * Physical storage for the per-CPU fall-back stack of the #VC handler.
-	 * The fall-back stack is used when it is not safe to switch back to the
-	 * interrupted stack in the #VC entry code.
-	 */
-	char fallback_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
 	/*
 	 * Reserve one page per CPU as backup storage for the unencrypted GHCB.
 	 * It is needed when an NMI happens while the #VC handler uses the real

@@ -99,27 +89,6 @@ DEFINE_STATIC_KEY_FALSE(sev_es_enable_key);
 /* Needed in vc_early_forward_exception */
 void do_early_exception(struct pt_regs *regs, int trapnr);
 
-static void __init setup_vc_stacks(int cpu)
-{
-	struct sev_es_runtime_data *data;
-	struct cpu_entry_area *cea;
-	unsigned long vaddr;
-	phys_addr_t pa;
-
-	data = per_cpu(runtime_data, cpu);
-	cea  = get_cpu_entry_area(cpu);
-
-	/* Map #VC IST stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC);
-	pa    = __pa(data->ist_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-
-	/* Map VC fall-back stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC2);
-	pa    = __pa(data->fallback_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-}
-
 static __always_inline bool on_vc_stack(struct pt_regs *regs)
 {
 	unsigned long sp = regs->sp;

@@ -787,7 +756,6 @@ void __init sev_es_init_vc_handling(void)
 	for_each_possible_cpu(cpu) {
 		alloc_runtime_data(cpu);
 		init_ghcb(cpu);
-		setup_vc_stacks(cpu);
 	}
 
 	sev_es_setup_play_dead();
arch/x86/mm/cpu_entry_area.c +7 -0
index f5e1e60c9095..6c2f1b76a0b6 100644
@@ -110,6 +110,13 @@ static void __init percpu_setup_exception_stacks(unsigned int cpu)
 	cea_map_stack(NMI);
 	cea_map_stack(DB);
 	cea_map_stack(MCE);
+
+	if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
+		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
+			cea_map_stack(VC);
+			cea_map_stack(VC2);
+		}
+	}
 }
 #else
 static inline void percpu_setup_exception_stacks(unsigned int cpu)
Signature · 8 lines
2.29.2


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author Tested-by on v1 1/1
#11 lore raw

Re: [PATCH -v2] x86/sev: Make the #VC exception stacks part of the default stacks storage

Quoted 7 lines · Borislav Petkov

Yap,

here's v2, now tested. It seems we do need that optional_stack_size second arg to ESTACKS_MEMBERS(), thx Tom.

---
From: Borislav Petkov <[email protected]>

Tested-by Tom Lendacky <[email protected]>
Quoted 16 lines

The size of the exception stacks was increased by the commit in Fixes, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Make the #VC exception stacks part of the default exception stacks storage and allocate them with a CONFIG_AMD_MEM_ENCRYPT=y .config. Map them only when a SEV-ES guest has been detected.

Rip out the custom VC stacks mapping and storage code.

[ bp: Steal and adapt Tom's commit message. ]

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")
Signed-off-by Borislav Petkov <[email protected]>
Tested-by on v1 1/1
#12 lore raw

Re: [PATCH -v2] x86/sev: Make the #VC exception stacks part of the default stacks storage

In reply to Borislav Petkov
Quoted 17 lines · Borislav Petkov

Yap,

here's v2, now tested. It seems we do need that optional_stack_size second arg to ESTACKS_MEMBERS(), thx Tom.

---
From: Borislav Petkov <[email protected]>

The size of the exception stacks was increased by the commit in Fixes, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Make the #VC exception stacks part of the default exception stacks storage and allocate them with a CONFIG_AMD_MEM_ENCRYPT=y .config. Map them only when a SEV-ES guest has been detected.

Quoted 7 lines

Rip out the custom VC stacks mapping and storage code.

[ bp: Steal and adapt Tom's commit message. ]

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")
Signed-off-by Borislav Petkov <[email protected]>

---

Tested-by Brijesh Singh <[email protected]>

thanks

merge bot Applied
#13 lore raw

[tip: x86/core] x86/sev: Make the #VC exception stacks part of the default stacks storage

In reply to Borislav Petkov
The following commit has been merged into the x86/core branch of tip:

The following commit has been merged into the x86/core branch of tip:

Commit-ID:     541ac97186d9ea88491961a46284de3603c914fd
Gitweb:        https://git.kernel.org/tip/541ac97186d9ea88491961a46284de3603c914fd
Author:        Borislav Petkov <[email protected]>
AuthorDate:    Fri, 01 Oct 2021 21:41:20 +02:00
Committer:     Borislav Petkov <[email protected]>
CommitterDate: Wed, 06 Oct 2021 21:48:27 +02:00

x86/sev: Make the #VC exception stacks part of the default stacks storage

The size of the exception stacks was increased by the commit in Fixes, resulting in stack sizes greater than a page in size. The #VC exception handling was only mapping the first (bottom) page, resulting in an SEV-ES guest failing to boot.

Make the #VC exception stacks part of the default exception stacks storage and allocate them with a CONFIG_AMD_MEM_ENCRYPT=y .config. Map them only when a SEV-ES guest has been detected.

Rip out the custom VC stacks mapping and storage code.

[ bp: Steal and adapt Tom's commit message. ]

Fixes 7fae4c24a2b8 ("x86: Increase exception stack sizes")
Signed-off-by Borislav Petkov <[email protected]>
Tested-by Tom Lendacky <[email protected]>
Tested-by Brijesh Singh <[email protected]>
Link https://lkml.kernel.org/r/[email protected] open here
---
 arch/x86/include/asm/cpu_entry_area.h |  8 ++++++-
 arch/x86/kernel/sev.c                 | 32 +--------------------------
 arch/x86/mm/cpu_entry_area.c          |  7 ++++++-
 3 files changed, 14 insertions(+), 33 deletions(-)
arch/x86/include/asm/cpu_entry_area.h +7 -1
index 3d52b09..dd5ea1b 100644
@@ -10,6 +10,12 @@
 
 #ifdef CONFIG_X86_64
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+#define VC_EXCEPTION_STKSZ	EXCEPTION_STKSZ
+#else
+#define VC_EXCEPTION_STKSZ	0
+#endif
+
 /* Macro to enforce the same ordering and stack sizes */
 #define ESTACKS_MEMBERS(guardsize, optional_stack_size)		\
 	char	DF_stack_guard[guardsize];			\

@@ -28,7 +34,7 @@
 
 /* The exception stacks' physical storage. No guard pages required */
 struct exception_stacks {
-	ESTACKS_MEMBERS(0, 0)
+	ESTACKS_MEMBERS(0, VC_EXCEPTION_STKSZ)
 };
 
 /* The effective cpu entry area mapping with guard pages. */
arch/x86/kernel/sev.c +0 -32
index 53a6837..4d0d1c2 100644
@@ -46,16 +46,6 @@ static struct ghcb __initdata *boot_ghcb;
 struct sev_es_runtime_data {
 	struct ghcb ghcb_page;
 
-	/* Physical storage for the per-CPU IST stack of the #VC handler */
-	char ist_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
-	/*
-	 * Physical storage for the per-CPU fall-back stack of the #VC handler.
-	 * The fall-back stack is used when it is not safe to switch back to the
-	 * interrupted stack in the #VC entry code.
-	 */
-	char fallback_stack[EXCEPTION_STKSZ] __aligned(PAGE_SIZE);
-
 	/*
 	 * Reserve one page per CPU as backup storage for the unencrypted GHCB.
 	 * It is needed when an NMI happens while the #VC handler uses the real

@@ -99,27 +89,6 @@ DEFINE_STATIC_KEY_FALSE(sev_es_enable_key);
 /* Needed in vc_early_forward_exception */
 void do_early_exception(struct pt_regs *regs, int trapnr);
 
-static void __init setup_vc_stacks(int cpu)
-{
-	struct sev_es_runtime_data *data;
-	struct cpu_entry_area *cea;
-	unsigned long vaddr;
-	phys_addr_t pa;
-
-	data = per_cpu(runtime_data, cpu);
-	cea  = get_cpu_entry_area(cpu);
-
-	/* Map #VC IST stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC);
-	pa    = __pa(data->ist_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-
-	/* Map VC fall-back stack */
-	vaddr = CEA_ESTACK_BOT(&cea->estacks, VC2);
-	pa    = __pa(data->fallback_stack);
-	cea_set_pte((void *)vaddr, pa, PAGE_KERNEL);
-}
-
 static __always_inline bool on_vc_stack(struct pt_regs *regs)
 {
 	unsigned long sp = regs->sp;

@@ -787,7 +756,6 @@ void __init sev_es_init_vc_handling(void)
 	for_each_possible_cpu(cpu) {
 		alloc_runtime_data(cpu);
 		init_ghcb(cpu);
-		setup_vc_stacks(cpu);
 	}
 
 	sev_es_setup_play_dead();
arch/x86/mm/cpu_entry_area.c +7 -0
index f5e1e60..6c2f1b7 100644
@@ -110,6 +110,13 @@ static void __init percpu_setup_exception_stacks(unsigned int cpu)
 	cea_map_stack(NMI);
 	cea_map_stack(DB);
 	cea_map_stack(MCE);
+
+	if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
+		if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
+			cea_map_stack(VC);
+			cea_map_stack(VC2);
+		}
+	}
 }
 #else
 static inline void percpu_setup_exception_stacks(unsigned int cpu)