[PATCH v1 1/2] x86/sev: Do not initialize SNP if missing CPUs

[PATCH v1 1/2] x86/sev: Do not initialize SNP if missing CPUs

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

The SEV firmware checks that the SNP enable bit is set on each CPU during
SNP initialization, and will fail if it is not. If there are some CPUs
offline, they will not run the setup functions, so SNP initialization will
always fail.

Skip the IPIs in this case and return an error so that the CCP driver can
skip the SNP_INIT that will fail.

Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
---
 arch/x86/include/asm/sev.h |  4 ++--
 arch/x86/virt/svm/sev.c    | 11 +++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 09e605c85de4..594cfa19cbd4 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -661,7 +661,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
 {
 	__snp_leak_pages(pfn, pages, true);
 }
-void snp_prepare(void);
+int snp_prepare(void);
 void snp_shutdown(void);
 #else
 static inline bool snp_probe_rmptable_info(void) { return false; }
@@ -679,7 +679,7 @@ static inline void __snp_leak_pages(u64 pfn, unsigned int npages, bool dump_rmp)
 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(void) {}
+static inline int snp_prepare(void) { return -ENODEV; }
 static inline void snp_shutdown(void) {}
 #endif
 
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 41f76f15caa1..e9ded15dbe60 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -511,8 +511,9 @@ static void clear_hsave_pa(void *arg)
 	wrmsrq(MSR_VM_HSAVE_PA, 0);
 }
 
-void snp_prepare(void)
+int snp_prepare(void)
 {
+	int ret = -EOPNOTSUPP;
 	u64 val;
 
 	/*
@@ -521,12 +522,15 @@ void snp_prepare(void)
 	 */
 	rdmsrq(MSR_AMD64_SYSCFG, val);
 	if (val & MSR_AMD64_SYSCFG_SNP_EN)
-		return;
+		return 0;
 
 	clear_rmp();
 
 	cpus_read_lock();
 
+	if (!cpumask_equal(cpu_online_mask, cpu_possible_mask))
+		goto unlock;
+
 	/*
 	 * MtrrFixDramModEn is not shared between threads on a core,
 	 * therefore it must be set on all CPUs prior to enabling SNP.
@@ -537,7 +541,10 @@ void snp_prepare(void)
 	/* SNP_INIT requires MSR_VM_HSAVE_PA to be cleared on all CPUs. */
 	on_each_cpu(clear_hsave_pa, NULL, 1);
 
+	ret = 0;
+unlock:
 	cpus_read_unlock();
+	return ret;
 }
 EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
 

base-commit: cf112712c193e837225d740ec3e139774f2496f2
-- 
2.53.0

[PATCH v1 2/2] crypto/ccp: skip SNP_INIT if preparation fails

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

During SNP_INIT, the firmware checks to see that the SNP enable bit is set
on all CPUs. If snp_prepare() failed because not all CPUs were online,
SNP_INIT will fail, so skip it.

Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
---
 drivers/crypto/ccp/sev-dev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 939fa8aa155c..1fc7ee432e28 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1374,7 +1374,10 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 		return -EOPNOTSUPP;
 	}
 
-	snp_prepare();
+	if (snp_prepare() < 0) {
+		dev_dbg(sev->dev, "SNP preparation failed, are all CPUs online?\n");
+		return -EOPNOTSUPP;
+	}
 
 	/*
 	 * Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list
-- 
2.53.0

[PATCH v1 2/2] crypto/ccp: Skip SNP_INIT if preparation fails

[Tycho Andersen]

From: "Tycho Andersen (AMD)" <tycho@kernel.org>

During SNP_INIT, the firmware checks to see that the SNP enable bit is set
on all CPUs. If snp_prepare() failed because not all CPUs were online,
SNP_INIT will fail, so skip it.

Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
---
 drivers/crypto/ccp/sev-dev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 939fa8aa155c..1fc7ee432e28 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1374,7 +1374,10 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 		return -EOPNOTSUPP;
 	}
 
-	snp_prepare();
+	if (snp_prepare() < 0) {
+		dev_dbg(sev->dev, "SNP preparation failed, are all CPUs online?\n");
+		return -EOPNOTSUPP;
+	}
 
 	/*
 	 * Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list
-- 
2.53.0

Re: [PATCH v1 2/2] crypto/ccp: skip SNP_INIT if preparation fails

[Tycho Andersen]

On Wed, Apr 01, 2026 at 08:35:51AM -0600, Tycho Andersen wrote:
> From: "Tycho Andersen (AMD)" <tycho@kernel.org>
> 
> During SNP_INIT, the firmware checks to see that the SNP enable bit is set
> on all CPUs. If snp_prepare() failed because not all CPUs were online,
> SNP_INIT will fail, so skip it.

Heh, you can ignore this one. Somehow it got sent out when I fixed the
capitalization in the commit message.

Tycho

Re: [PATCH v1 1/2] x86/sev: Do not initialize SNP if missing CPUs

[Tom Lendacky]

On 4/1/26 09:35, Tycho Andersen wrote:
> From: "Tycho Andersen (AMD)" <tycho@kernel.org>
> 
> The SEV firmware checks that the SNP enable bit is set on each CPU during
> SNP initialization, and will fail if it is not. If there are some CPUs
> offline, they will not run the setup functions, so SNP initialization will
> always fail.
> 
> Skip the IPIs in this case and return an error so that the CCP driver can
> skip the SNP_INIT that will fail.
> 
> Suggested-by: Borislav Petkov (AMD) <bp@alien8.de>
> Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
> ---
>  arch/x86/include/asm/sev.h |  4 ++--
>  arch/x86/virt/svm/sev.c    | 11 +++++++++--
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 09e605c85de4..594cfa19cbd4 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -661,7 +661,7 @@ static inline void snp_leak_pages(u64 pfn, unsigned int pages)
>  {
>  	__snp_leak_pages(pfn, pages, true);
>  }
> -void snp_prepare(void);
> +int snp_prepare(void);
>  void snp_shutdown(void);
>  #else
>  static inline bool snp_probe_rmptable_info(void) { return false; }
> @@ -679,7 +679,7 @@ static inline void __snp_leak_pages(u64 pfn, unsigned int npages, bool dump_rmp)
>  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(void) {}
> +static inline int snp_prepare(void) { return -ENODEV; }
>  static inline void snp_shutdown(void) {}
>  #endif
>  
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 41f76f15caa1..e9ded15dbe60 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -511,8 +511,9 @@ static void clear_hsave_pa(void *arg)
>  	wrmsrq(MSR_VM_HSAVE_PA, 0);
>  }
>  
> -void snp_prepare(void)
> +int snp_prepare(void)
>  {
> +	int ret = -EOPNOTSUPP;
>  	u64 val;
>  
>  	/*
> @@ -521,12 +522,15 @@ void snp_prepare(void)
>  	 */
>  	rdmsrq(MSR_AMD64_SYSCFG, val);
>  	if (val & MSR_AMD64_SYSCFG_SNP_EN)
> -		return;
> +		return 0;
>  
>  	clear_rmp();
>  
>  	cpus_read_lock();
>  
> +	if (!cpumask_equal(cpu_online_mask, cpu_possible_mask))

If CONFIG_INIT_ALL_POSSIBLE is set, won't that set cpu_possible_mask to
include all CPUs up to NR_CPUS? That would result in this always failing.

Not sure if this change is worth it.

Thanks,
Tom

> +		goto unlock;
> +
>  	/*
>  	 * MtrrFixDramModEn is not shared between threads on a core,
>  	 * therefore it must be set on all CPUs prior to enabling SNP.
> @@ -537,7 +541,10 @@ void snp_prepare(void)
>  	/* SNP_INIT requires MSR_VM_HSAVE_PA to be cleared on all CPUs. */
>  	on_each_cpu(clear_hsave_pa, NULL, 1);
>  
> +	ret = 0;
> +unlock:
>  	cpus_read_unlock();
> +	return ret;
>  }
>  EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>  
> 
> base-commit: cf112712c193e837225d740ec3e139774f2496f2

Re: [PATCH v1 2/2] crypto/ccp: skip SNP_INIT if preparation fails

[Tom Lendacky]

On 4/1/26 09:35, Tycho Andersen wrote:
> From: "Tycho Andersen (AMD)" <tycho@kernel.org>
> 
> During SNP_INIT, the firmware checks to see that the SNP enable bit is set
> on all CPUs. If snp_prepare() failed because not all CPUs were online,
> SNP_INIT will fail, so skip it.
> 
> Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
> ---
>  drivers/crypto/ccp/sev-dev.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 939fa8aa155c..1fc7ee432e28 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1374,7 +1374,10 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>  		return -EOPNOTSUPP;
>  	}
>  
> -	snp_prepare();
> +	if (snp_prepare() < 0) {
> +		dev_dbg(sev->dev, "SNP preparation failed, are all CPUs online?\n");
> +		return -EOPNOTSUPP;
> +	}

Shouldn't this propagate the snp_prepare() return code:

	rc = snp_prepare();
	if (rc) {
		dev_dbg(...)
		return ret;
	}

Thanks,
Tom

>  
>  	/*
>  	 * Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list

Re: [PATCH v1 1/2] x86/sev: Do not initialize SNP if missing CPUs

[Borislav Petkov]

On Fri, Apr 03, 2026 at 08:31:24AM -0500, Tom Lendacky wrote:
> > +	if (!cpumask_equal(cpu_online_mask, cpu_possible_mask))
> 
> If CONFIG_INIT_ALL_POSSIBLE is set, won't that set cpu_possible_mask to
> include all CPUs up to NR_CPUS? That would result in this always failing.

Yah,

sashiko gave another possible situation where this can fail:

https://sashiko.dev/#/patchset/20260401143552.3038979-1-tycho%40kernel.org

> Not sure if this change is worth it.

Well, it would save us a lot of effort if we can check at module load time
whether some CPUs are offlined. A very simple use case:

echo 0 > /sys/devices/system/cpu.../online
modprobe ccp

<boom>.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v1 1/2] x86/sev: Do not initialize SNP if missing CPUs

[Tycho Andersen]

On Fri, Apr 03, 2026 at 07:18:33PM +0200, Borislav Petkov wrote:
> On Fri, Apr 03, 2026 at 08:31:24AM -0500, Tom Lendacky wrote:
> > > +	if (!cpumask_equal(cpu_online_mask, cpu_possible_mask))
> > 
> > If CONFIG_INIT_ALL_POSSIBLE is set, won't that set cpu_possible_mask to
> > include all CPUs up to NR_CPUS? That would result in this always failing.

Thanks, I didn't know about this config.

> Yah,
> 
> sashiko gave another possible situation where this can fail:
> 
> https://sashiko.dev/#/patchset/20260401143552.3038979-1-tycho%40kernel.org

> disabled ACPI MADT entries for hotplug

I suppose depends on how the firmware reasons about such things too.
But it seems like the suggestion of cpus_present_mask is right.

> > Not sure if this change is worth it.
> 
> Well, it would save us a lot of effort if we can check at module load time
> whether some CPUs are offlined. A very simple use case:
> 
> echo 0 > /sys/devices/system/cpu.../online
> modprobe ccp

This is how I was testing this, but I'll see about testing the ACPI
thing too.

Tycho

Re: [PATCH v1 1/2] x86/sev: Do not initialize SNP if missing CPUs

[Borislav Petkov]

On Fri, Apr 03, 2026 at 11:52:09AM -0600, Tycho Andersen wrote:
> I suppose depends on how the firmware reasons about such things too.
> But it seems like the suggestion of cpus_present_mask is right.

Yes, I think that should work.

And in that check you could dump both masks if they're unequal - hint *%pb or
%pbl as printk format strings:

Documentation/core-api/printk-formats.rst

so that we can get more debug info on what the mismatch actually is.

-- 
Regards/Gruss,
    Boris.

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