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

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

[Jason Andryuk]

For v3, Add newline to warning.  R-b Yazen.

For v2, swap the patch order.

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: Avoid divide by zero on virtualized systems
  x86/amd_node: Remove smn_exclusive

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

-- 
2.55.0

[PATCH v3 1/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)

The issue is seen with Xen, but it could affect other systems.

Change roots_per_node == 0 to 1 to allow the system to boot.  Print a
warning when this is performed for non-virtualized systems.

Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
Cc: stable@vger.kernel.org
Suggested-by: Borislav Petkov <bp@alien8.de>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
v3:
Add newline to warning.
R-b: Yazen

v2:
Print as a warning instead of error
Use "!" instead of "== 0".

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 0be01725a2a4..408b9fd48349 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -287,6 +287,11 @@ static int __init amd_smn_init(void)
 		return -ENOMEM;
 
 	roots_per_node = num_roots / num_nodes;
+	if (!roots_per_node) {
+		if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
+			pr_warn(FW_BUG "Error detecting roots per node.\n");
+		roots_per_node = 1;
+	}
 
 	count = 0;
 	node = 0;
-- 
2.55.0

[PATCH v3 2/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 to __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.

commit 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region
access") added smn_exclusive which indicated the calls to
pci_request_config_region_exclusive() succeeded to prevent userspace
access.

commit 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
re-ordered initialization so pci_request_config_region_exclusive() is
called earlier and a failure exits amd_smn_init() before allocating
amd_roots.  Setting smn_exclusive moved to the end of amd_smn_init(),
after amd_roots is allocated.  smn_exclusive became redundant to
amd_roots and can be removed.

Add a comment stating how amd_roots replaces smn_exclusive's purpose.

Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB")
Cc: stable@vger.kernel.org
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
Fixes is the introduction of amd_roots, which could be a NULL deref.

v3:
No change

v2:
R-b Yazen and Mario
Add comment about amd_root taking smn_exclusive's purpose to indicate
userspace access is prevented.
Expand commit message
---
 arch/x86/kernel/amd_node.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index 408b9fd48349..833e5eda5d91 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,15 @@ 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)
+	/*
+	 * non-NULL amd_roots indicates pci_request_config_region_exclusive()
+	 * succeeded and userspace cannot access the registers.
+	 */
+	if (!amd_roots)
 		return err;
 
-	if (!smn_exclusive)
+	root = amd_roots[node];
+	if (!root)
 		return err;
 
 	guard(mutex)(&smn_mutex);
@@ -313,8 +316,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

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

[Borislav Petkov]

On Tue, Aug 25, 2026 at 05:48:02PM -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)
> 
> The issue is seen with Xen, but it could affect other systems.
> 
> Change roots_per_node == 0 to 1 to allow the system to boot.  Print a
> warning when this is performed for non-virtualized systems.
> 
> Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
> Cc: stable@vger.kernel.org
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> v3:
> Add newline to warning.
> R-b: Yazen
> 
> v2:
> Print as a warning instead of error
> Use "!" instead of "== 0".
> 
> 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 0be01725a2a4..408b9fd48349 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -287,6 +287,11 @@ static int __init amd_smn_init(void)
>  		return -ENOMEM;
>  
>  	roots_per_node = num_roots / num_nodes;

Hold on - the only way this is fixing something on Xen - and you say that
roots_per_node becomes 0 - which means that num_roots is 0.

But a couple of lines above, we have

        if (!num_roots)
                return -ENODEV;

So we would've exited already.

num_nodes can't be 0 because you'd explode even with this patch.

What am I missing?

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Jason Andryuk]

On 2026-08-26 00:45, Borislav Petkov wrote:
> On Tue, Aug 25, 2026 at 05:48:02PM -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)
>>
>> The issue is seen with Xen, but it could affect other systems.
>>
>> Change roots_per_node == 0 to 1 to allow the system to boot.  Print a
>> warning when this is performed for non-virtualized systems.
>>
>> Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
>> Cc: stable@vger.kernel.org
>> Suggested-by: Borislav Petkov <bp@alien8.de>
>> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> v3:
>> Add newline to warning.
>> R-b: Yazen
>>
>> v2:
>> Print as a warning instead of error
>> Use "!" instead of "== 0".
>>
>> 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 0be01725a2a4..408b9fd48349 100644
>> --- a/arch/x86/kernel/amd_node.c
>> +++ b/arch/x86/kernel/amd_node.c
>> @@ -287,6 +287,11 @@ static int __init amd_smn_init(void)
>>   		return -ENOMEM;
>>   
>>   	roots_per_node = num_roots / num_nodes;
> 
> Hold on - the only way this is fixing something on Xen - and you say that
> roots_per_node becomes 0 - which means that num_roots is 0.
> 
> But a couple of lines above, we have
> 
>          if (!num_roots)
>                  return -ENODEV;
> 
> So we would've exited already.
> 
> num_nodes can't be 0 because you'd explode even with this patch.
> 
> What am I missing?
num_roots is 1, but num_nodes is 2.  Integer division gives 0 for 
roots_per_node.

Regards,
Jason

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

[Borislav Petkov]

On Wed, Aug 26, 2026 at 08:15:49AM -0400, Jason Andryuk wrote:
> num_roots is 1, but num_nodes is 2.  Integer division gives 0 for
> roots_per_node.

Aha, Xen has some topology knowledge and it manages to produce 2 nodes.

I'll add that to the commit message.

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Borislav Petkov]

On Tue, Aug 25, 2026 at 05:48:03PM -0400, Jason Andryuk wrote:
> -	root = amd_roots[node];
> -	if (!root)
> +	/*
> +	 * non-NULL amd_roots indicates pci_request_config_region_exclusive()
> +	 * succeeded and userspace cannot access the registers.
> +	 */
> +	if (!amd_roots)

So then this should be

	if (amd_roots)
		return err;

?

-- 
Regards/Gruss,
    Boris.

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

[tip: x86/urgent] x86/amd_node: Avoid divide by zero on virtualized systems

[tip-bot2 for Jason Andryuk]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     72bd92bd8190d7869ecb462649ca40f297822a33
Gitweb:        https://git.kernel.org/tip/72bd92bd8190d7869ecb462649ca40f297822a33
Author:        Jason Andryuk <jason.andryuk@amd.com>
AuthorDate:    Tue, 25 Aug 2026 17:48:02 -04:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Sun, 30 Aug 2026 18:25:42 -07:00

x86/amd_node: Avoid divide by zero on virtualized systems

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)

because the underlying topology code on Xen ends up making num_nodes
2 and num_roots 1 and the integer division result is 0.

The issue is seen with Xen, but it could affect other systems.

Set roots_per_node to 1 in this case. Print a firmware bug when this is
performed for non-virtualized systems.

  [ bp: Massage commit message. ]

Fixes: 0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260825214805.39148-2-jason.andryuk@amd.com
---
 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 0be0172..408b9fd 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -287,6 +287,11 @@ static int __init amd_smn_init(void)
 		return -ENOMEM;
 
 	roots_per_node = num_roots / num_nodes;
+	if (!roots_per_node) {
+		if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
+			pr_warn(FW_BUG "Error detecting roots per node.\n");
+		roots_per_node = 1;
+	}
 
 	count = 0;
 	node = 0;

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

[Jason Andryuk]

On 2026-08-30 22:30, Borislav Petkov wrote:
> On Tue, Aug 25, 2026 at 05:48:03PM -0400, Jason Andryuk wrote:
>> -	root = amd_roots[node];
>> -	if (!root)
>> +	/*
>> +	 * non-NULL amd_roots indicates pci_request_config_region_exclusive()
>> +	 * succeeded and userspace cannot access the registers.
>> +	 */
>> +	if (!amd_roots)
> 
> So then this should be
> 
> 	if (amd_roots)
> 		return err;
> 
> ?
No, the code and the comment are both correct as posted.  It's just that 
I wrote the comment reversed from the code to state the positive 
property we want.

Flipping it would be:
"NULL amd_roots means pci_request_config_region_exclusive() failed and 
userspace can access the registers."

But I'd like to reconsider this comment.  There is an existing comment 
before pci_request_config_region_exclusive() in amd_smn_init() stating 
its purpose.  The commit message explains how smn_exclusive is no longer 
needed.  That seems enough to make the code change without adding a 
comment.  I wasn't sure how to phrase the comment because it feels out 
of place to me.

Because the other side is that it is a valid NULL check to prevent 
dereferencing amd_roots.  That is the important part.  Maybe the commit 
messages should have been "Avoid NULL deref in __amd_smn_rw()" to 
highlight that?  When you look at the code as a whole, smn_exclusive 
isn't doing anything, which is why I removed it.

I'll change this whichever way you want.  It's not a big deal, which is 
why I added the comment like you requested originally.

Regards,
Jason

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

[Borislav Petkov]

On Mon, Aug 31, 2026 at 01:52:08PM -0400, Jason Andryuk wrote:
> No, the code and the comment are both correct as posted.  It's just that I
> wrote the comment reversed from the code to state the positive property we
> want.

So if amd_smn_init() succeeds, that means we have registered the PCI config
regions of the PCI roots, including the SMN ones, as IORESOURCE_EXCLUSIVE and
they won't be exported to userspace through /dev/mem and what not.

So, any caller of __amd_smn_rw() should fail because it is coming from kernel
space and it can expect that userspace could interfere with the hw and thus
not a good idea.

So yes, this is what got me confused and the (!amd_roots) check is correct.

> I'll change this whichever way you want.  It's not a big deal, which is why
> I added the comment like you requested originally.

I think I already "intercepted" your suggestion, see here:

From 1a1c4addc5a14e9ef2bb7d351a62ff4563d0f10a Mon Sep 17 00:00:00 2001
From: Jason Andryuk <jason.andryuk@amd.com>
Date: Tue, 25 Aug 2026 17:48:03 -0400
Subject: [PATCH] x86/amd_node: Prevent potential NULL pointer dereference

amd_smn_read/write() are exported functions around __amd_smn_rw(), so
they are always available even if amd_smn_init() fails. In that case,
amd_roots is NULL and __amd_smn_rw() will access uninitialized memory.

Then, commit

  83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")

added smn_exclusive which indicated the calls to
pci_request_config_region_exclusive() succeeded to prevent concurrent
userspace access.

Commit

  0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")

re-ordered initialization so pci_request_config_region_exclusive() is called
earlier and a failure exits amd_smn_init() before allocating amd_roots.
Setting smn_exclusive moved to the end of amd_smn_init(), after amd_roots is
allocated. It became redundant and can be removed.

Replace smn_exclusive with directly checking amd_roots to avoid a potential
NULL pointer dereference.

  [ bp: Reorg commit message, touchup comment. ]

Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB")
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260825214805.39148-3-jason.andryuk@amd.com
---
 arch/x86/kernel/amd_node.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index 408b9fd48349..762585775b5a 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,16 @@ 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)
+	/*
+	 * Uninitialized amd_roots indicates pci_request_config_region_exclusive()
+	 * didn't run or failed and thus the kernel cannot rely on having
+	 * exclusive access to SMN registers so prevent that.
+	 */
+	if (!amd_roots)
 		return err;
 
-	if (!smn_exclusive)
+	root = amd_roots[node];
+	if (!root)
 		return err;
 
 	guard(mutex)(&smn_mutex);
@@ -313,8 +317,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.53.0


-- 
Regards/Gruss,
    Boris.

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

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

[Jason Andryuk]

On 2026-08-31 20:27, Borislav Petkov wrote:
> On Mon, Aug 31, 2026 at 01:52:08PM -0400, Jason Andryuk wrote:
>> No, the code and the comment are both correct as posted.  It's just that I
>> wrote the comment reversed from the code to state the positive property we
>> want.
> 
> So if amd_smn_init() succeeds, that means we have registered the PCI config
> regions of the PCI roots, including the SMN ones, as IORESOURCE_EXCLUSIVE and
> they won't be exported to userspace through /dev/mem and what not.
> 
> So, any caller of __amd_smn_rw() should fail because it is coming from kernel
> space and it can expect that userspace could interfere with the hw and thus
> not a good idea.
> 
> So yes, this is what got me confused and the (!amd_roots) check is correct.
> 
>> I'll change this whichever way you want.  It's not a big deal, which is why
>> I added the comment like you requested originally.
> 
> I think I already "intercepted" your suggestion, see here:
> 
>  From 1a1c4addc5a14e9ef2bb7d351a62ff4563d0f10a Mon Sep 17 00:00:00 2001
> From: Jason Andryuk <jason.andryuk@amd.com>
> Date: Tue, 25 Aug 2026 17:48:03 -0400
> Subject: [PATCH] x86/amd_node: Prevent potential NULL pointer dereference
> 
> amd_smn_read/write() are exported functions around __amd_smn_rw(), so
> they are always available even if amd_smn_init() fails. In that case,
> amd_roots is NULL and __amd_smn_rw() will access uninitialized memory.
> 
> Then, commit
> 
>    83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
> 
> added smn_exclusive which indicated the calls to
> pci_request_config_region_exclusive() succeeded to prevent concurrent
> userspace access.
> 
> Commit
> 
>    0a4b61d9c2e4 ("x86/amd_node: Fix AMD root device caching")
> 
> re-ordered initialization so pci_request_config_region_exclusive() is called
> earlier and a failure exits amd_smn_init() before allocating amd_roots.
> Setting smn_exclusive moved to the end of amd_smn_init(), after amd_roots is
> allocated. It became redundant and can be removed.
> 
> Replace smn_exclusive with directly checking amd_roots to avoid a potential
> NULL pointer dereference.
> 
>    [ bp: Reorg commit message, touchup comment. ]
> 
> Fixes: 77466b798d59 ("x86/amd_node: Remove dependency on AMD_NB")
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Cc: stable@vger.kernel.org
> Link: https://patch.msgid.link/20260825214805.39148-3-jason.andryuk@amd.com
Looks good to me.

Thanks,
Jason