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

[Tycho Andersen]

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@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> > Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> > ---
> >  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