[PATCH 0/2] x86/amd_node: Fixes for virtualized systems

[PATCH 0/2] x86/amd_node: Fixes for virtualized systems

[Jason Andryuk]

On my test system, I see SMN accesses from:
amd_pmf_probe() - read
amd_pmc_probe() - read
acp_acp70_audio_probe() - write

Jason Andryuk (2):
  x86/amd_node: Remove smn_exclusive
  x86/amd_node: Avoid divide by zero on virtualized systems

 arch/x86/kernel/amd_node.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.55.0

[PATCH 1/2] x86/amd_node: Remove smn_exclusive

[Jason Andryuk]

amd_smn_read/write() are exported functions around __amd_smn_rw(), so
they are always available even if amd_smn_init() fails.  smn_exclusive
would prevent access __amd_smn_rw(), but it is placed too late.  If
amd_smn_init() failed, amd_roots is NULL and __amd_smn_rw() will fault
over it.  Replace smn_exclusive with directly checking amd_roots to
avoid the NULL pointer dereference.

Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB")
Cc: stable@vger.kernel.org
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
Fixes is the introduction of amd_roots
---
 arch/x86/kernel/amd_node.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index 0be01725a2a4..ea553267e5fa 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -38,7 +38,6 @@ static struct pci_dev **amd_roots;
 
 /* Protect the PCI config register pairs used for SMN. */
 static DEFINE_MUTEX(smn_mutex);
-static bool smn_exclusive;
 
 #define SMN_INDEX_OFFSET	0x60
 #define SMN_DATA_OFFSET		0x64
@@ -91,11 +90,11 @@ static int __amd_smn_rw(u8 i_off, u8 d_off, u16 node, u32 address, u32 *value, b
 	if (node >= amd_num_nodes())
 		return err;
 
-	root = amd_roots[node];
-	if (!root)
+	if (!amd_roots)
 		return err;
 
-	if (!smn_exclusive)
+	root = amd_roots[node];
+	if (!root)
 		return err;
 
 	guard(mutex)(&smn_mutex);
@@ -308,8 +307,6 @@ static int __init amd_smn_init(void)
 		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
 	}
 
-	smn_exclusive = true;
-
 	return 0;
 }
 
-- 
2.55.0

[PATCH 2/2] x86/amd_node: Avoid divide by zero on virtualized systems

[Jason Andryuk]

On a virtualized system, the number of nodes does not have a
relationship to the number of roots.  A Xen PVH dom0 can calculate
roots_per_node as 0, which crashes with a divide by zero in:

    if (count++ % roots_per_node)

On a virtualized system, default the value to 1.  The issue is seen with
Xen, but it could affect other systems.

Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
Cc: stable@vger.kernel.org
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
X86_FEATURE_XENPV is only for PV, but this is observed with a PVH dom0.
---
 arch/x86/kernel/amd_node.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index ea553267e5fa..c5025e5291b6 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -286,6 +286,11 @@ static int __init amd_smn_init(void)
 		return -ENOMEM;
 
 	roots_per_node = num_roots / num_nodes;
+	if (roots_per_node == 0) {
+		if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
+			pr_err(FW_BUG "Error detecting roots per node.\n");
+		roots_per_node = 1;
+	}
 
 	count = 0;
 	node = 0;
-- 
2.55.0

Re: [PATCH 1/2] x86/amd_node: Remove smn_exclusive

[Yazen Ghannam]

On Thu, Aug 06, 2026 at 12:01:56PM -0400, Jason Andryuk wrote:
> amd_smn_read/write() are exported functions around __amd_smn_rw(), so
> they are always available even if amd_smn_init() fails.  smn_exclusive
> would prevent access __amd_smn_rw(), but it is placed too late.  If
> amd_smn_init() failed, amd_roots is NULL and __amd_smn_rw() will fault
> over it.  Replace smn_exclusive with directly checking amd_roots to
> avoid the NULL pointer dereference.
> 
> Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Fixes is the introduction of amd_roots
> ---
>  arch/x86/kernel/amd_node.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index 0be01725a2a4..ea553267e5fa 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -38,7 +38,6 @@ static struct pci_dev **amd_roots;
>  
>  /* Protect the PCI config register pairs used for SMN. */
>  static DEFINE_MUTEX(smn_mutex);
> -static bool smn_exclusive;
>  
>  #define SMN_INDEX_OFFSET	0x60
>  #define SMN_DATA_OFFSET		0x64
> @@ -91,11 +90,11 @@ static int __amd_smn_rw(u8 i_off, u8 d_off, u16 node, u32 address, u32 *value, b
>  	if (node >= amd_num_nodes())
>  		return err;
>  
> -	root = amd_roots[node];
> -	if (!root)
> +	if (!amd_roots)
>  		return err;
>  
> -	if (!smn_exclusive)
> +	root = amd_roots[node];
> +	if (!root)
>  		return err;
>  
>  	guard(mutex)(&smn_mutex);
> @@ -308,8 +307,6 @@ static int __init amd_smn_init(void)
>  		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
>  	}
>  
> -	smn_exclusive = true;
> -
>  	return 0;
>  }
>  

'smn_exclusive' wasn't originally intended to be used for bounds
checking the array. But the original use is no longer needed, so it can
be removed.

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>

Thanks,
Yazen

Re: [PATCH 2/2] x86/amd_node: Avoid divide by zero on virtualized systems

[Yazen Ghannam]

On Thu, Aug 06, 2026 at 12:01:57PM -0400, Jason Andryuk wrote:
> On a virtualized system, the number of nodes does not have a
> relationship to the number of roots.  A Xen PVH dom0 can calculate
> roots_per_node as 0, which crashes with a divide by zero in:
> 
>     if (count++ % roots_per_node)
> 
> On a virtualized system, default the value to 1.  The issue is seen with
> Xen, but it could affect other systems.
> 
> Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

I agree with the idea with some minor feedback below.

> ---
> X86_FEATURE_XENPV is only for PV, but this is observed with a PVH dom0.
> ---
>  arch/x86/kernel/amd_node.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index ea553267e5fa..c5025e5291b6 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -286,6 +286,11 @@ static int __init amd_smn_init(void)
>  		return -ENOMEM;
>  
>  	roots_per_node = num_roots / num_nodes;
> +	if (roots_per_node == 0) {

Can be '!roots_per_node'.

> +		if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
> +			pr_err(FW_BUG "Error detecting roots per node.\n");
> +		roots_per_node = 1;
> +	}

I don't think this is necessarily a Firmware bug.

Likewise, it can be a 'warning' rather than 'error'. I interpret 'error'
as something known to be incorrect. This was my position before on this
topic. But the various contrary reports changed my mind (even if they
come from virtualization).

Furthermore, I think 'warning' is more appropriate. It says "We found
something unexpected. We're letting you (the user) know about it. And
we'll mitigate it to avoid an error."

Anyways, I just wanted to write out some thoughts since this topic has
come up a few times.

Possible rework for the above change:

	if (!roots_per_node && !cpu_feature_enabled(X86_FEATURE_HYPERVISOR)
		pr_warn("Expected at least 1 root per AMD node.\n");

	roots_per_node = max(roots_per_node, 1);

In any case, we should just go with a simple fix for the virt cases.

I think we could even do away with caching the 'root' devices. But
that'll be another rework/cleanup.

Thanks,
Yazen

Re: [PATCH 2/2] x86/amd_node: Avoid divide by zero on virtualized systems

[Jason Andryuk]

On 2026-08-10 10:47, Yazen Ghannam wrote:
> On Thu, Aug 06, 2026 at 12:01:57PM -0400, Jason Andryuk wrote:
>> On a virtualized system, the number of nodes does not have a
>> relationship to the number of roots.  A Xen PVH dom0 can calculate
>> roots_per_node as 0, which crashes with a divide by zero in:
>>
>>      if (count++ % roots_per_node)
>>
>> On a virtualized system, default the value to 1.  The issue is seen with
>> Xen, but it could affect other systems.
>>
>> Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> 
> I agree with the idea with some minor feedback below.
> 
>> ---
>> X86_FEATURE_XENPV is only for PV, but this is observed with a PVH dom0.
>> ---
>>   arch/x86/kernel/amd_node.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
>> index ea553267e5fa..c5025e5291b6 100644
>> --- a/arch/x86/kernel/amd_node.c
>> +++ b/arch/x86/kernel/amd_node.c
>> @@ -286,6 +286,11 @@ static int __init amd_smn_init(void)
>>   		return -ENOMEM;
>>   
>>   	roots_per_node = num_roots / num_nodes;
>> +	if (roots_per_node == 0) {
> 
> Can be '!roots_per_node'.

Sure.
>> +		if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
>> +			pr_err(FW_BUG "Error detecting roots per node.\n");
>> +		roots_per_node = 1;
>> +	}
> 
> I don't think this is necessarily a Firmware bug.
> 
> Likewise, it can be a 'warning' rather than 'error'. I interpret 'error'
> as something known to be incorrect. This was my position before on this
> topic. But the various contrary reports changed my mind (even if they
> come from virtualization).
> 
> Furthermore, I think 'warning' is more appropriate. It says "We found
> something unexpected. We're letting you (the user) know about it. And
> we'll mitigate it to avoid an error."

Makes sense to me.  Boris suggested error in 
https://lore.kernel.org/lkml/20260624192530.GGajwvKuhxbnHe0s42@fat_crate.local/

(I probably should have used Suggested-by.)

> Anyways, I just wanted to write out some thoughts since this topic has
> come up a few times.
> 
> Possible rework for the above change:
> 
> 	if (!roots_per_node && !cpu_feature_enabled(X86_FEATURE_HYPERVISOR)
> 		pr_warn("Expected at least 1 root per AMD node.\n");
> 
> 	roots_per_node = max(roots_per_node, 1);
> 
> In any case, we should just go with a simple fix for the virt cases.

I'm okay with this.  I'll wait to see if anyone chimes in further.  If 
not, I'll go with your approach and Suggested-by.

Thanks,
Jason

Re: [PATCH 2/2] x86/amd_node: Avoid divide by zero on virtualized systems

[David Laight]

On Mon, 10 Aug 2026 10:47:06 -0400
Yazen Ghannam <yazen.ghannam@amd.com> wrote:

> On Thu, Aug 06, 2026 at 12:01:57PM -0400, Jason Andryuk wrote:
> > On a virtualized system, the number of nodes does not have a
> > relationship to the number of roots.  A Xen PVH dom0 can calculate
> > roots_per_node as 0, which crashes with a divide by zero in:
> > 
> >     if (count++ % roots_per_node)
> > 
> > On a virtualized system, default the value to 1.  The issue is seen with
> > Xen, but it could affect other systems.
> > 
> > Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>  
> 
> I agree with the idea with some minor feedback below.
> 
> > ---
> > X86_FEATURE_XENPV is only for PV, but this is observed with a PVH dom0.
> > ---
> >  arch/x86/kernel/amd_node.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> > index ea553267e5fa..c5025e5291b6 100644
> > --- a/arch/x86/kernel/amd_node.c
> > +++ b/arch/x86/kernel/amd_node.c
> > @@ -286,6 +286,11 @@ static int __init amd_smn_init(void)
> >  		return -ENOMEM;
> >  
> >  	roots_per_node = num_roots / num_nodes;
> > +	if (roots_per_node == 0) {  
> 
> Can be '!roots_per_node'.
> 
> > +		if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
> > +			pr_err(FW_BUG "Error detecting roots per node.\n");
> > +		roots_per_node = 1;
> > +	}  
> 
> I don't think this is necessarily a Firmware bug.
> 
> Likewise, it can be a 'warning' rather than 'error'. I interpret 'error'
> as something known to be incorrect. This was my position before on this
> topic. But the various contrary reports changed my mind (even if they
> come from virtualization).
> 
> Furthermore, I think 'warning' is more appropriate. It says "We found
> something unexpected. We're letting you (the user) know about it. And
> we'll mitigate it to avoid an error."
> 
> Anyways, I just wanted to write out some thoughts since this topic has
> come up a few times.
> 
> Possible rework for the above change:
> 
> 	if (!roots_per_node && !cpu_feature_enabled(X86_FEATURE_HYPERVISOR)
> 		pr_warn("Expected at least 1 root per AMD node.\n");
> 
> 	roots_per_node = max(roots_per_node, 1);

I'd not add the max(), just;
	if (!roots_per_node) {
		roots_per_node = 1;
		if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR)
			pr_warn("Expected at least 1 root per AMD node.\n");
	}

David

> 
> In any case, we should just go with a simple fix for the virt cases.
> 
> I think we could even do away with caching the 'root' devices. But
> that'll be another rework/cleanup.
> 
> Thanks,
> Yazen
> 

Re: [PATCH 1/2] x86/amd_node: Remove smn_exclusive

[Mario Limonciello]

On 8/10/26 09:07, Yazen Ghannam wrote:
> On Thu, Aug 06, 2026 at 12:01:56PM -0400, Jason Andryuk wrote:
>> amd_smn_read/write() are exported functions around __amd_smn_rw(), so
>> they are always available even if amd_smn_init() fails.  smn_exclusive
>> would prevent access __amd_smn_rw(), but it is placed too late.  If
>> amd_smn_init() failed, amd_roots is NULL and __amd_smn_rw() will fault
>> over it.  Replace smn_exclusive with directly checking amd_roots to
>> avoid the NULL pointer dereference.
>>
>> Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> Fixes is the introduction of amd_roots
>> ---
>>   arch/x86/kernel/amd_node.c | 9 +++------
>>   1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
>> index 0be01725a2a4..ea553267e5fa 100644
>> --- a/arch/x86/kernel/amd_node.c
>> +++ b/arch/x86/kernel/amd_node.c
>> @@ -38,7 +38,6 @@ static struct pci_dev **amd_roots;
>>   
>>   /* Protect the PCI config register pairs used for SMN. */
>>   static DEFINE_MUTEX(smn_mutex);
>> -static bool smn_exclusive;
>>   
>>   #define SMN_INDEX_OFFSET	0x60
>>   #define SMN_DATA_OFFSET		0x64
>> @@ -91,11 +90,11 @@ static int __amd_smn_rw(u8 i_off, u8 d_off, u16 node, u32 address, u32 *value, b
>>   	if (node >= amd_num_nodes())
>>   		return err;
>>   
>> -	root = amd_roots[node];
>> -	if (!root)
>> +	if (!amd_roots)
>>   		return err;
>>   
>> -	if (!smn_exclusive)
>> +	root = amd_roots[node];
>> +	if (!root)
>>   		return err;
>>   
>>   	guard(mutex)(&smn_mutex);
>> @@ -308,8 +307,6 @@ static int __init amd_smn_init(void)
>>   		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
>>   	}
>>   
>> -	smn_exclusive = true;
>> -
>>   	return 0;
>>   }
>>   
> 
> 'smn_exclusive' wasn't originally intended to be used for bounds
> checking the array. But the original use is no longer needed, so it can
> be removed.
> 
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> 
> Thanks,
> Yazen

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>