Re: [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown

["M K, Muralidhara" <muralimk@amd.com>]

On 7/8/2026 3:37 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Wed, 8 Jul 2026, Muralidhara M K wrote:
> 
>> This series makes the AMD HSMP driver safe against concurrent probe/remove
>> of its per-socket devices and against the lock-free data plane (open
>> /dev/hsmp fds and hwmon/sysfs reads) racing socket teardown.
>>
>> The ACPI front-end binds one platform device per socket but shares a single
>> socket array and a single /dev/hsmp misc device across them, while the data
>> plane issues mailbox messages with no coordination with driver teardown.
>> misc_deregister() does not drain already-open fds, so an in-flight message
>> can touch a freed socket array or an unmapped mailbox on unbind.
>>
>> The fix is built up in small, bisectable steps:
>>
>>   1. Serialize the ACPI probe/remove handshake with a dedicated mutex.
>>   2. Map the metric table with ioremap() and release it via a devres action,
>>      so its lifetime is no longer pinned to a single per-socket devres scope.
>>   3. Serialize the per-socket metric-table fill-and-copy with a mutex.
>>   4. Clear mdev.this_device on deregister (independent hygiene fix that the
>>      next patch relies on to track /dev/hsmp registration).
>>   5. Track shared socket ownership with a refcount and a single coordinated
>>      release helper, drop the is_probed flag and unparent /dev/hsmp.
>>   6. Add hsmp_sock_rwsem: the data plane holds it for read, a teardown path
>>      holds it for write to drain in-flight messages before freeing the socket
>>      array or unmapping the mailbox.
>>
>> Each patch builds on its own and the series is checkpatch --strict clean.
>>
>> Changes since v3:
>>   - Use guard()/scoped locking consistently for both the probe mutex and the
>>     data-plane rwsem, from the first patch that introduces each lock.
>>   - Platform teardown now uses devm_add_action_or_reset(): the metric-table
>>     unmap and the per-socket mutex destroy run from a single devres action
>>     instead of explicit remove()/probe-failure code. The ACPI array is shared
>>     across per-socket devices and must outlive an individual unbind, so it
>>     stays on explicit teardown.
>>   - Split the mdev.this_device clear into its own patch (new patch 4).
>>   - Drop dead defensive checks: the !sock guard in hsmp_unmap_metric_tbls()
>>     and the if (sock) in hsmp_acpi_remove(). Keep and document the one in the
>>     probe-failure path, where sock can legitimately be NULL.
>>   - Replace the !--refs construct with a plain decrement-then-test.
>>   - Explain why the probe path uses a separate mutex rather than the rwsem
>>     write side: the probe path itself drives the data plane via hsmp_test(),
>>     which takes the rwsem for read, so holding it for write across probe
>>     would deadlock.
> 
> Hi,
> 
> Could we have xx_locked() variant function for hsmp_test() to use so the
> recursive locking problem is avoided?
> 
Yes, that works and let me drop hsmp_acpi_probe_mutex entirely and 
serialize the ACPI control plane on hsmp_sock_rwsem alone.

One clarification I am thinking of is: the recursion isn't only through 
hsmp_test(). init_acpi()/init_platform_device() also reach the data 
plane via hsmp_cache_proto_ver(), hsmp_get_tbl_dram_base() and the hwmon 
setup, all through hsmp_send_message(). So rather than a one-off 
hsmp_test_locked(), I'll split hsmp_send_message() into a 
hsmp_send_message_locked() core (bounds/nospec/dev check + per-socket 
semaphore + MMIO, no rwsem) and a thin hsmp_send_message() wrapper that 
takes the rwsem for read. The probe-only senders call the _locked core, 
the probe path takes the rwsem for write, and hsmp_acpi_probe_mutex goes 
away.

The one behavioral change is the probe then holds the rwsem for write 
across the whole mailbox handshake (a few messages, up to 
HSMP_MSG_TIMEOUT each), so the data plane on already-probed sockets is 
blocked for that window. Probe is a boot/rare-rebind event so I think 
that's fine. I'll add lockdep_assert_held_write() in the locked core to 
keep the "rwsem already held" contract explicit.

or Let me know your inputs ?

>>   - Comment cleanups: drop history and "patch N" references, drop
>>     parenthetical function asides, single space after periods.
>>
>> Muralidhara M K (6):
>>    platform/x86/amd/hsmp: Serialize ACPI HSMP is_probed with a probe
>>      mutex
>>    platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap
>>      it explicitly
>>    platform/x86/amd/hsmp: Serialize per-socket metric table reads with a
>>      mutex
>>    platform/x86/amd/hsmp: Clear mdev.this_device on deregister
>>    platform/x86/amd/hsmp: ACPI HSMP refcounted sockets and coordinated
>>      release
>>    platform/x86/amd/hsmp: Serialize the data plane against socket
>>      teardown
>>
>>   drivers/platform/x86/amd/hsmp/acpi.c | 135 ++++++++++++++++++++++++---
>>   drivers/platform/x86/amd/hsmp/hsmp.c | 107 ++++++++++++++++++++-
>>   drivers/platform/x86/amd/hsmp/hsmp.h |  15 ++-
>>   drivers/platform/x86/amd/hsmp/plat.c |  26 ++++++
>>   4 files changed, 265 insertions(+), 18 deletions(-)
>>
>>
>> base-commit: ff7836fa850c2f815bc219f1e48f6ec8699f4ae7
>>
> 
> --
>   i.
>