Re: [PATCH 1/2] x86/amd_node: Remove smn_exclusive
From: Jason Andryuk
Date: Fri Aug 14 2026 - 09:38:37 EST
On 2026-08-13 19:58, Borislav Petkov wrote:
On Thu, Aug 13, 2026 at 09:40:33AM -0400, Jason Andryuk wrote:
On 2026-08-12 15:28, Borislav Petkov wrote:
On Mon, Aug 10, 2026 at 10:07:47AM -0400, Yazen Ghannam wrote:83518453074d set smn_exclusive after successfully calling
'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.
Why is it no longer needed? Apparently I didn't get that memo yet...
83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access") is
talking about the potential concurrent access. Why is that ok now all of
a sudden?
pci_request_config_region_exclusive(). smn_exclusive gated access in
__amd_smn_rw().
Things moved around, and now pci_request_config_region_exclusive() is called
earlier. At the end of amd_smn_init(), either amd_roots == NULL and
smn_exclusive == false or amd_roots != NULL and smn_exclusive == true.
smn_exclusive is redundant, and amd_roots can just be used directly.
So you want to use amd_roots to *also* *denote* whether exclusive SMN access
is enabled. Then you need to at least put a comment in __amd_smn_rw() where
you're testing it, what exactly that test means.
I'm not sure adding a comment is helpful. smn_exclusive lost its usefulness between its introduction and now.
Then, your patch has a Fixes: tag and CC:stable which means, it needs to go to
all kernels but then I don't know what you're fixing.
In my original commit message I stated this removes a potential NULL deref:
"""
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 deref - writeerence.
"""
I will add something about how smn_exclusive became redundant.
Looking at it more, you're basically fixing one more aspect of running the
kernel as a Xen guest and it not having AMD roots exported.
If amd_smn_init() fails for any reason, __amd_smn_rw() will fault. It was seen with Xen, but it is not a property of running on Xen.
But what you really wanna do, IMNSVHO, you want to make your patch 2 be patchRe-ordering is fine. With the divide by zero fix, then amd_smn_init() succeeds in my test with Xen. So the other patch isn't needed in that case, but it looked like a worthwhile fix. In the cover letter, I noted accesses from amd_pmf_probe(), amd_pmc_probe(), and acp_acp70_audio_probe() to __amd_smn_rw() on my test system. These are called even if amd_smn_init() fails.
1 and it can go to stable.
Your patch 1 then becomes a simplification/cleanup patch which goes as patch
2 and doesn't need to be Cc:stable but it can be queued later, during the
normal cycle.
Regards,
Jason