Re: [PATCH v11 3/6] x86/sev: Initialize RMPOPT configuration MSRs

[Tom Lendacky]

On 7/27/26 14:04, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@xxxxxxx>
> 
> The new RMPOPT instruction helps manage per-CPU RMP optimization
> structures inside the CPU. It takes a 1GB-aligned physical address
> and either returns the status of the optimizations or tries to enable
> the optimizations.
> 
> Per-CPU RMPOPT tables support at most 2 TB of addressable memory for
> RMP optimizations.
> 
> Initialize the per-CPU RMPOPT table base to the starting physical
> address. This enables RMP optimization for up to 2 TB of system RAM on
> all CPUs.
> 
> Additionally, add support to setup and enable RMPOPT once SNP is
> enabled and initialized.
> 
> Suggested-by: Thomas Lendacky <thomas.lendacky@xxxxxxx>
> Suggested-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Suggested-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
> Suggested-by: K Prateek Nayak <kprateek.nayak@xxxxxxx>
> Reviewed-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx>
> ---
>  arch/x86/include/asm/msr-index.h |  3 ++
>  arch/x86/include/asm/sev.h       |  2 +
>  arch/x86/virt/svm/sev.c          | 67 +++++++++++++++++++++++++++++---
>  drivers/crypto/ccp/sev-dev.c     |  3 ++
>  4 files changed, 70 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 18c4be75e927..d2cb0a7cd0a2 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -761,6 +761,9 @@
>  #define MSR_AMD64_SEG_RMP_ENABLED_BIT	0
>  #define MSR_AMD64_SEG_RMP_ENABLED	BIT_ULL(MSR_AMD64_SEG_RMP_ENABLED_BIT)
>  #define MSR_AMD64_RMP_SEGMENT_SHIFT(x)	(((x) & GENMASK_ULL(13, 8)) >> 8)
> +#define MSR_AMD64_RMPOPT_BASE		0xc0010139
> +#define MSR_AMD64_RMPOPT_ENABLE_BIT	0
> +#define MSR_AMD64_RMPOPT_ENABLE		BIT_ULL(MSR_AMD64_RMPOPT_ENABLE_BIT)
>  
>  #define MSR_SVSM_CAA			0xc001f000
>  
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 594cfa19cbd4..6fd72a44a51e 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);
>  }
>  int snp_prepare(void);
> +void snp_setup_rmpopt(void);
>  void snp_shutdown(void);
>  #else
>  static inline bool snp_probe_rmptable_info(void) { return false; }
> @@ -680,6 +681,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 int snp_prepare(void) { return -ENODEV; }
> +static inline void snp_setup_rmpopt(void) {}
>  static inline void snp_shutdown(void) {}
>  #endif
>  
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index e2f69fba0938..8bfd80284836 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -124,6 +124,9 @@ static void *rmp_bookkeeping __ro_after_init;
>  
>  static u64 probed_rmp_base, probed_rmp_size;
>  
> +static cpumask_var_t rmpopt_cpumask;
> +static phys_addr_t rmpopt_pa_start;
> +
>  static LIST_HEAD(snp_leaked_pages_list);
>  static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
>  
> @@ -558,6 +561,17 @@ int snp_prepare(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>  
> +static void snp_cleanup_rmpopt(void)
> +{
> +	int cpu;
> +
> +	for_each_cpu(cpu, rmpopt_cpumask)
> +		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, 0);
> +
> +	free_cpumask_var(rmpopt_cpumask);
> +	rmpopt_pa_start = 0;
> +}
> +
>  void snp_shutdown(void)
>  {
>  	u64 syscfg;
> @@ -567,10 +581,11 @@ void snp_shutdown(void)
>  		return;
>  
>  	/*
> -	 * The firmware has disabled SNP (SnpEn is clear), so re-enable CPU
> -	 * hotplug.  A legacy SNP shutdown returns above with SnpEn still set and
> -	 * leaves hotplug disabled.
> +	 * Clear the RMPOPT_BASE MSRs while CPU hotplug is still disabled, then
> +	 * re-enable hotplug now that the firmware has disabled SNP.  A legacy SNP
> +	 * shutdown returns above with SnpEn still set and leaves hotplug disabled.
>  	 */
> +	snp_cleanup_rmpopt();

Why not leave everything unchanged comment wise and just add the
snp_cleanup_rmpopt() before the comment with a simple comment above the
call to snp_cleanup_rmpopt(), e.g.:

	/* Disable RMPOPT while CPU hotplug is disabled */
	snp_cleanup_rmpopt();

	/*
	 * Original comment block
	 */
	cpu_hotplug_enable();

Thanks,
Tom

>  	cpu_hotplug_enable();
>  
>  	clear_rmp();
> @@ -578,6 +593,46 @@ void snp_shutdown(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_shutdown, "ccp");
>  
> +static bool rmpopt_capable(void)
> +{
> +	return cpu_feature_enabled(X86_FEATURE_RMPOPT) &&
> +	       cc_platform_has(CC_ATTR_HOST_SEV_SNP);
> +}
> +
> +void snp_setup_rmpopt(void)
> +{
> +	u64 rmpopt_base;
> +	int cpu;
> +
> +	if (!rmpopt_capable())
> +		return;
> +
> +	if (!zalloc_cpumask_var(&rmpopt_cpumask, GFP_KERNEL)) {
> +		pr_err("Failed to allocate RMPOPT cpumask\n");
> +		return;
> +	}
> +
> +	/*
> +	 * The RMPOPT_BASE MSR is per-core, so only one thread per core needs
> +	 * to set up the RMPOPT_BASE MSR.  All primary threads are online,
> +	 * otherwise SNP would not have been enabled.
> +	 */
> +	for_each_online_cpu(cpu)
> +		if (topology_is_primary_thread(cpu))
> +			cpumask_set_cpu(cpu, rmpopt_cpumask);
> +
> +	rmpopt_pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
> +	rmpopt_base = rmpopt_pa_start | MSR_AMD64_RMPOPT_ENABLE;
> +
> +	/*
> +	 * Per-CPU RMPOPT tables cover at most 2 TB.  Program each core's
> +	 * RMPOPT_BASE with the start of RAM to optimize up to 2 TB.
> +	 */
> +	for_each_cpu(cpu, rmpopt_cpumask)
> +		wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
> +}
> +EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
> +
>  /*
>   * Do the necessary preparations which are verified by the firmware as
>   * described in the SNP_INIT_EX firmware command description in the SNP
> @@ -705,10 +760,12 @@ bool snp_probe_rmptable_info(void)
>  	if (cpu_feature_enabled(X86_FEATURE_SEGMENTED_RMP))
>  		rdmsrq(MSR_AMD64_RMP_CFG, rmp_cfg);
>  
> -	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)
> +	if (rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED) {
>  		return probe_segmented_rmptable_info();
> -	else
> +	} else {
> +		setup_clear_cpu_cap(X86_FEATURE_RMPOPT);
>  		return probe_contiguous_rmptable_info();
> +	}
>  }
>  
>  /*
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index ca473ca198b8..c002a7ca26a8 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1477,6 +1477,9 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>  	}
>  
>  	snp_hv_fixed_pages_state_update(sev, HV_FIXED);
> +
> +	snp_setup_rmpopt();
> +
>  	sev->snp_initialized = true;
>  	dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
>  		data.tio_en ? "enabled" : "disabled");