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@xxxxxxxxxxxxxxx
> > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > ---
> > 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@xxxxxxx>
>
> Thanks,
> Yazen

Reviewed-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>