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

[M K, Muralidhara]

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.