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

[Jason Andryuk <jason.andryuk@amd.com>]

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