[PATCH v3 3/7] x86/snp: create snp_x86_shutdown()

Tycho Andersen posted 7 patches 2 weeks, 6 days ago
There is a newer version of this series
[PATCH v3 3/7] x86/snp: create snp_x86_shutdown()
Posted by Tycho Andersen 2 weeks, 6 days ago
From: "Tycho Andersen (AMD)" <tycho@kernel.org>

After SNP_SHUTDOWN, two architecture-level things should be done:

1. clear the RMP table
2. disable MFDM to prevent the FW_WARN in k8_check_syscfg_dram_mod_en() in
   the event of a kexec

Create and export to the CCP driver a function that does them.

Also change the MFDM helper to allow for disabling the bit, since the SNP
x86 shutdown path needs to disable MFDM. The comment for
k8_check_syscfg_dram_mod_en() notes, the "BIOS" is supposed clear it, or
the kernel in the case of module unload and shutdown followed by kexec.

Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/include/asm/sev.h |  2 ++
 arch/x86/virt/svm/sev.c    | 23 ++++++++++++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 0bcd89d4fe90..36d2b1ea19c0 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -662,6 +662,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
 	__snp_leak_pages(pfn, pages, true);
 }
 void snp_prepare_for_snp_init(void);
+void snp_x86_shutdown(void);
 #else
 static inline bool snp_probe_rmptable_info(void) { return false; }
 static inline int snp_rmptable_init(void) { return -ENOSYS; }
@@ -679,6 +680,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
 static inline void kdump_sev_callback(void) { }
 static inline void snp_fixup_e820_tables(void) {}
 static inline void snp_prepare_for_snp_init(void) {}
+static inline void snp_x86_shutdown(void) {}
 #endif
 
 #endif
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 88cb4a548701..85091d663f18 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -132,12 +132,15 @@ static unsigned long snp_nr_leaked_pages;
 #undef pr_fmt
 #define pr_fmt(fmt)	"SEV-SNP: " fmt
 
-static void mfd_enable(void *arg)
+static void mfd_reconfigure(void *arg)
 {
 	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
 		return;
 
-	msr_set_bit(MSR_AMD64_SYSCFG, MSR_AMD64_SYSCFG_MFDM_BIT);
+	if (arg)
+		msr_set_bit(MSR_AMD64_SYSCFG, MSR_AMD64_SYSCFG_MFDM_BIT);
+	else
+		msr_clear_bit(MSR_AMD64_SYSCFG, MSR_AMD64_SYSCFG_MFDM_BIT);
 }
 
 static void snp_enable(void *arg)
@@ -521,12 +524,26 @@ void snp_prepare_for_snp_init(void)
 	 * MtrrFixDramModEn is not shared between threads on a core,
 	 * therefore it must be set on all CPUs prior to enabling SNP.
 	 */
-	on_each_cpu(mfd_enable, NULL, 1);
+	on_each_cpu(mfd_reconfigure, (void *)1, 1);
 
 	on_each_cpu(snp_enable, NULL, 1);
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_prepare_for_snp_init, "ccp");
 
+void snp_x86_shutdown(void)
+{
+	u64 syscfg;
+
+	rdmsrq(MSR_AMD64_SYSCFG, syscfg);
+
+	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
+		return;
+
+	clear_rmp();
+	on_each_cpu(mfd_reconfigure, 0, 1);
+}
+EXPORT_SYMBOL_FOR_MODULES(snp_x86_shutdown, "ccp");
+
 /*
  * Do the necessary preparations which are verified by the firmware as
  * described in the SNP_INIT_EX firmware command description in the SNP
-- 
2.53.0
Re: [PATCH v3 3/7] x86/snp: create snp_x86_shutdown()
Posted by Borislav Petkov 2 weeks, 2 days ago
On Tue, Mar 17, 2026 at 10:21:53AM -0600, Tycho Andersen wrote:
> Subject: Re: [PATCH v3 3/7] x86/snp: create snp_x86_shutdown()

"x86/sev: ..."

The tip tree preferred format for patch subject prefixes is
'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:',
'genirq/core:'. Please do not use file names or complete file paths as
prefix. 'git log path/to/file' should give you a reasonable hint in most
cases.

The condensed patch description in the subject line should start with a
uppercase letter and should be written in imperative tone.

Check your whole set pls.

> From: "Tycho Andersen (AMD)" <tycho@kernel.org>
> 
> After SNP_SHUTDOWN, two architecture-level things should be done:

"architecture-level things"?

> 1. clear the RMP table
> 2. disable MFDM to prevent the FW_WARN in k8_check_syscfg_dram_mod_en() in
>    the event of a kexec
> 
> Create and export to the CCP driver a function that does them.
> 
> Also change the MFDM helper to allow for disabling the bit, since the SNP
> x86 shutdown path needs to disable MFDM. The comment for
> k8_check_syscfg_dram_mod_en() notes, the "BIOS" is supposed clear it, or
> the kernel in the case of module unload and shutdown followed by kexec.
> 
> Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/include/asm/sev.h |  2 ++
>  arch/x86/virt/svm/sev.c    | 23 ++++++++++++++++++++---
>  2 files changed, 22 insertions(+), 3 deletions(-)

...

> @@ -521,12 +524,26 @@ void snp_prepare_for_snp_init(void)
>  	 * MtrrFixDramModEn is not shared between threads on a core,
>  	 * therefore it must be set on all CPUs prior to enabling SNP.
>  	 */
> -	on_each_cpu(mfd_enable, NULL, 1);
> +	on_each_cpu(mfd_reconfigure, (void *)1, 1);
				     ^^^^^^^^^
ew.

>  	on_each_cpu(snp_enable, NULL, 1);
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare_for_snp_init, "ccp");
>  
> +void snp_x86_shutdown(void)

"snp" and "x86" prefixes?

> +{
> +	u64 syscfg;
> +
> +	rdmsrq(MSR_AMD64_SYSCFG, syscfg);
> +

^ Superfluous newline.

> +	if (syscfg & MSR_AMD64_SYSCFG_SNP_EN)
> +		return;
> +
> +	clear_rmp();
> +	on_each_cpu(mfd_reconfigure, 0, 1);

s/0/NULL/

> +}
> +EXPORT_SYMBOL_FOR_MODULES(snp_x86_shutdown, "ccp");
> +
>  /*
>   * Do the necessary preparations which are verified by the firmware as
>   * described in the SNP_INIT_EX firmware command description in the SNP
> -- 
> 2.53.0
> 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 3/7] x86/snp: create snp_x86_shutdown()
Posted by Tycho Andersen 2 weeks ago
Hi Boris,

On Sat, Mar 21, 2026 at 06:05:34PM +0100, Borislav Petkov wrote:
> On Tue, Mar 17, 2026 at 10:21:53AM -0600, Tycho Andersen wrote:
> > Subject: Re: [PATCH v3 3/7] x86/snp: create snp_x86_shutdown()
> 
> "x86/sev: ..."
> 
> The tip tree preferred format for patch subject prefixes is
> 'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:',
> 'genirq/core:'. Please do not use file names or complete file paths as
> prefix. 'git log path/to/file' should give you a reasonable hint in most
> cases.
> 
> The condensed patch description in the subject line should start with a
> uppercase letter and should be written in imperative tone.
> 
> Check your whole set pls.

Will do.

> > From: "Tycho Andersen (AMD)" <tycho@kernel.org>
> > 
> > After SNP_SHUTDOWN, two architecture-level things should be done:
> 
> "architecture-level things"?

I'll just drop this entirely, i.e. "two things should be done:"

> 
> > 1. clear the RMP table
> > 2. disable MFDM to prevent the FW_WARN in k8_check_syscfg_dram_mod_en() in
> >    the event of a kexec
> > 
> > Create and export to the CCP driver a function that does them.
> > 
> > Also change the MFDM helper to allow for disabling the bit, since the SNP
> > x86 shutdown path needs to disable MFDM. The comment for
> > k8_check_syscfg_dram_mod_en() notes, the "BIOS" is supposed clear it, or
> > the kernel in the case of module unload and shutdown followed by kexec.
> > 
> > Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > ---
> >  arch/x86/include/asm/sev.h |  2 ++
> >  arch/x86/virt/svm/sev.c    | 23 ++++++++++++++++++++---
> >  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> ...
> 
> > @@ -521,12 +524,26 @@ void snp_prepare_for_snp_init(void)
> >  	 * MtrrFixDramModEn is not shared between threads on a core,
> >  	 * therefore it must be set on all CPUs prior to enabling SNP.
> >  	 */
> > -	on_each_cpu(mfd_enable, NULL, 1);
> > +	on_each_cpu(mfd_reconfigure, (void *)1, 1);
> 				     ^^^^^^^^^
> ew.

:) I can add a macro for this. Let me know if you want a full args
struct instead.

> >  	on_each_cpu(snp_enable, NULL, 1);
> >  }
> >  EXPORT_SYMBOL_FOR_MODULES(snp_prepare_for_snp_init, "ccp");
> >  
> > +void snp_x86_shutdown(void)
> 
> "snp" and "x86" prefixes?

My intent here was to indicate that it's doing the x86 (i.e.
non-firmware) parts of the SNP shutdown process. I will change it to
just snp_shutdown(), but that will still have a prefix. Just
shutdown() seemed to generic...

Will fix the rest.

Thanks,

Tycho
Re: [PATCH v3 3/7] x86/snp: create snp_x86_shutdown()
Posted by Borislav Petkov 2 weeks ago
On Mon, Mar 23, 2026 at 09:31:25AM -0600, Tycho Andersen wrote:
> :) I can add a macro for this. Let me know if you want a full args
> struct instead.

"ew" meant: this is not nice but I don't have a better idea...

A full args struct doesn't really change anything AFAICT. Maybe play with it
a bit and see what the resulting asm is but I'm sceptical it would be any
different...

> My intent here was to indicate that it's doing the x86 (i.e.
> non-firmware) parts of the SNP shutdown process. I will change it to
> just snp_shutdown(), but that will still have a prefix. Just
> shutdown() seemed to generic...

Yeah, snp_shutdown() - just one prefix :) - is enough.

Thx.

-- 
Regards/Gruss,
    Boris.

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