* [PATCH v4 1/6] platform/x86/amd/hsmp: Serialize ACPI HSMP is_probed with a probe mutex
2026-07-08 4:23 [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
@ 2026-07-08 4:24 ` Muralidhara M K
2026-07-08 4:24 ` [PATCH v4 2/6] platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap it explicitly Muralidhara M K
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Muralidhara M K @ 2026-07-08 4:24 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: platform-driver-x86, linux-kernel, Muralidhara M K
Add hsmp_acpi_probe_mutex and hold it across ACPI probe, remove and
init_acpi() so concurrent platform probes cannot race the is_probed
handshake or the one-time socket allocation. Use lockdep_assert_held() in
init_acpi() to catch incorrect locking under lockdep.
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 72f68cef1297..eef799ce1fb8 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -18,8 +18,11 @@
#include <linux/device.h>
#include <linux/dev_printk.h>
#include <linux/ioport.h>
+#include <linux/cleanup.h>
+#include <linux/lockdep.h>
#include <linux/kstrtox.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/string.h>
#include <linux/sysfs.h>
@@ -39,6 +42,12 @@
static struct hsmp_plat_device *hsmp_pdev;
+/*
+ * Serializes ACPI probe, remove and init_acpi() so concurrent socket probes
+ * cannot race the is_probed handshake or the one-time socket-array allocation.
+ */
+static DEFINE_MUTEX(hsmp_acpi_probe_mutex);
+
struct hsmp_sys_attr {
struct device_attribute dattr;
u32 msg_id;
@@ -482,11 +491,17 @@ static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_att
return len;
}
+/**
+ * init_acpi() - Parse ACPI mailbox resources for one socket and validate HSMP.
+ * @dev: ACPI companion device for this socket.
+ */
static int init_acpi(struct device *dev)
{
u16 sock_ind;
int ret;
+ lockdep_assert_held(&hsmp_acpi_probe_mutex);
+
ret = hsmp_get_uid(dev, &sock_ind);
if (ret)
return ret;
@@ -607,6 +622,8 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
if (!hsmp_pdev)
return -ENOMEM;
+ guard(mutex)(&hsmp_acpi_probe_mutex);
+
if (!hsmp_pdev->is_probed) {
hsmp_pdev->num_sockets = topology_max_packages();
if (!hsmp_pdev->num_sockets) {
@@ -642,6 +659,8 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
static void hsmp_acpi_remove(struct platform_device *pdev)
{
+ guard(mutex)(&hsmp_acpi_probe_mutex);
+
/*
* We register only one misc_device even on multi-socket system.
* So, deregister should happen only once.
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 2/6] platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap it explicitly
2026-07-08 4:23 [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
2026-07-08 4:24 ` [PATCH v4 1/6] platform/x86/amd/hsmp: Serialize ACPI HSMP is_probed with a probe mutex Muralidhara M K
@ 2026-07-08 4:24 ` Muralidhara M K
2026-07-08 10:17 ` Ilpo Järvinen
2026-07-08 4:24 ` [PATCH v4 3/6] platform/x86/amd/hsmp: Serialize per-socket metric table reads with a mutex Muralidhara M K
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Muralidhara M K @ 2026-07-08 4:24 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: platform-driver-x86, linux-kernel, Muralidhara M K
The metric table DRAM region is mapped with devm_ioremap(), tying its
lifetime to the socket device. An upcoming change lets the ACPI front-end
share the socket array across sockets and coordinate its own teardown, so
the mapping can no longer be pinned to a single per-socket devres scope.
Switch hsmp_get_tbl_dram_base() to plain ioremap() and add
hsmp_unmap_metric_tbls(), which drops every socket's metric_tbl_addr
mapping. On the platform driver register it with devm_add_action_or_reset()
so the mapping is released on both remove and probe failure; the socket
array itself stays devm-managed.
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/hsmp.c | 19 +++++++++++++++++--
drivers/platform/x86/amd/hsmp/hsmp.h | 1 +
drivers/platform/x86/amd/hsmp/plat.c | 14 ++++++++++++++
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 1a87931136fd..4586d86f72a3 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -12,6 +12,7 @@
#include <linux/acpi.h>
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/io.h>
#include <linux/semaphore.h>
#include <linux/sysfs.h>
@@ -414,6 +415,21 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
}
EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
+void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev)
+{
+ u16 i;
+
+ for (i = 0; i < pdev->num_sockets; i++) {
+ struct hsmp_socket *s = &pdev->sock[i];
+
+ if (s->metric_tbl_addr) {
+ iounmap(s->metric_tbl_addr);
+ s->metric_tbl_addr = NULL;
+ }
+ }
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_unmap_metric_tbls, "AMD_HSMP");
+
int hsmp_get_tbl_dram_base(u16 sock_ind)
{
struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
@@ -438,8 +454,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
dev_err(sock->dev, "Invalid DRAM address for metric table\n");
return -ENOMEM;
}
- sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
- sizeof(struct hsmp_metric_table));
+ sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
if (!sock->metric_tbl_addr) {
dev_err(sock->dev, "Failed to ioremap metric table addr\n");
return -ENOMEM;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index 0509a442eaae..98c82a4c0cc3 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -64,6 +64,7 @@ void hsmp_misc_deregister(void);
int hsmp_misc_register(struct device *dev);
int hsmp_get_tbl_dram_base(u16 sock_ind);
ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
+void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev);
struct hsmp_plat_device *get_hsmp_pdev(void);
#if IS_ENABLED(CONFIG_HWMON)
int hsmp_create_sensor(struct device *dev, u16 sock_ind);
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index e07f68575055..e7ee6e701e5a 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -201,6 +201,16 @@ static int init_platform_device(struct device *dev)
return 0;
}
+/*
+ * The metric tables are mapped with ioremap() rather than devm, so release
+ * them explicitly. Registered as a devres action so it also runs on a probe
+ * failure after init_platform_device() has mapped some of them.
+ */
+static void hsmp_pltdrv_release(void *data)
+{
+ hsmp_unmap_metric_tbls(hsmp_pdev);
+}
+
static int hsmp_pltdrv_probe(struct platform_device *pdev)
{
int ret;
@@ -211,6 +221,10 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
if (!hsmp_pdev->sock)
return -ENOMEM;
+ ret = devm_add_action_or_reset(&pdev->dev, hsmp_pltdrv_release, NULL);
+ if (ret)
+ return ret;
+
ret = init_platform_device(&pdev->dev);
if (ret) {
dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/6] platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap it explicitly
2026-07-08 4:24 ` [PATCH v4 2/6] platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap it explicitly Muralidhara M K
@ 2026-07-08 10:17 ` Ilpo Järvinen
2026-07-09 5:40 ` M K, Muralidhara
0 siblings, 1 reply; 12+ messages in thread
From: Ilpo Järvinen @ 2026-07-08 10:17 UTC (permalink / raw)
To: Muralidhara M K; +Cc: platform-driver-x86, LKML
On Wed, 8 Jul 2026, Muralidhara M K wrote:
> The metric table DRAM region is mapped with devm_ioremap(), tying its
> lifetime to the socket device. An upcoming change lets the ACPI front-end
> share the socket array across sockets and coordinate its own teardown, so
> the mapping can no longer be pinned to a single per-socket devres scope.
>
> Switch hsmp_get_tbl_dram_base() to plain ioremap() and add
> hsmp_unmap_metric_tbls(), which drops every socket's metric_tbl_addr
> mapping. On the platform driver register it with devm_add_action_or_reset()
> so the mapping is released on both remove and probe failure;
I feel the explanation is quite basic level description of devres
functionality. I'd say something like this instead:
The unmapping for all sockets as whole is still devres managed using
devm_add_action_or_reset().
> the socket array itself stays devm-managed.
devres managed is probably the better term in textual form. "devm" is C
codish way to say it in a short way.
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/hsmp.c | 19 +++++++++++++++++--
> drivers/platform/x86/amd/hsmp/hsmp.h | 1 +
> drivers/platform/x86/amd/hsmp/plat.c | 14 ++++++++++++++
> 3 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 1a87931136fd..4586d86f72a3 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -12,6 +12,7 @@
> #include <linux/acpi.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> +#include <linux/io.h>
> #include <linux/semaphore.h>
> #include <linux/sysfs.h>
>
> @@ -414,6 +415,21 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
> }
> EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
>
> +void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev)
> +{
> + u16 i;
> +
> + for (i = 0; i < pdev->num_sockets; i++) {
> + struct hsmp_socket *s = &pdev->sock[i];
> +
> + if (s->metric_tbl_addr) {
> + iounmap(s->metric_tbl_addr);
> + s->metric_tbl_addr = NULL;
> + }
> + }
> +}
> +EXPORT_SYMBOL_NS_GPL(hsmp_unmap_metric_tbls, "AMD_HSMP");
> +
> int hsmp_get_tbl_dram_base(u16 sock_ind)
> {
> struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
> @@ -438,8 +454,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
> dev_err(sock->dev, "Invalid DRAM address for metric table\n");
> return -ENOMEM;
> }
> - sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
> - sizeof(struct hsmp_metric_table));
> + sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
> if (!sock->metric_tbl_addr) {
> dev_err(sock->dev, "Failed to ioremap metric table addr\n");
> return -ENOMEM;
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index 0509a442eaae..98c82a4c0cc3 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -64,6 +64,7 @@ void hsmp_misc_deregister(void);
> int hsmp_misc_register(struct device *dev);
> int hsmp_get_tbl_dram_base(u16 sock_ind);
> ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
> +void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev);
> struct hsmp_plat_device *get_hsmp_pdev(void);
> #if IS_ENABLED(CONFIG_HWMON)
> int hsmp_create_sensor(struct device *dev, u16 sock_ind);
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index e07f68575055..e7ee6e701e5a 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -201,6 +201,16 @@ static int init_platform_device(struct device *dev)
> return 0;
> }
>
> +/*
> + * The metric tables are mapped with ioremap() rather than devm, so release
> + * them explicitly. Registered as a devres action so it also runs on a probe
> + * failure after init_platform_device() has mapped some of them.
Please put "Registered..." into own paragraph. It's sort of different
thing it's talking about that the first sentence, and you'll be adding to
the first paragraph in the other patch as well.
/*
* Paragraph 1.
*
* Paragraph 2.
> + */
> +static void hsmp_pltdrv_release(void *data)
> +{
> + hsmp_unmap_metric_tbls(hsmp_pdev);
> +}
> +
> static int hsmp_pltdrv_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -211,6 +221,10 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
> if (!hsmp_pdev->sock)
> return -ENOMEM;
>
> + ret = devm_add_action_or_reset(&pdev->dev, hsmp_pltdrv_release, NULL);
> + if (ret)
> + return ret;
> +
> ret = init_platform_device(&pdev->dev);
> if (ret) {
> dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
>
--
i.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/6] platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap it explicitly
2026-07-08 10:17 ` Ilpo Järvinen
@ 2026-07-09 5:40 ` M K, Muralidhara
0 siblings, 0 replies; 12+ messages in thread
From: M K, Muralidhara @ 2026-07-09 5:40 UTC (permalink / raw)
To: Ilpo Järvinen, Muralidhara M K; +Cc: platform-driver-x86, LKML
On 7/8/2026 3:47 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:
>
>> The metric table DRAM region is mapped with devm_ioremap(), tying its
>> lifetime to the socket device. An upcoming change lets the ACPI front-end
>> share the socket array across sockets and coordinate its own teardown, so
>> the mapping can no longer be pinned to a single per-socket devres scope.
>>
>> Switch hsmp_get_tbl_dram_base() to plain ioremap() and add
>> hsmp_unmap_metric_tbls(), which drops every socket's metric_tbl_addr
>> mapping. On the platform driver register it with devm_add_action_or_reset()
>> so the mapping is released on both remove and probe failure;
>
> I feel the explanation is quite basic level description of devres
> functionality. I'd say something like this instead:
>
> The unmapping for all sockets as whole is still devres managed using
> devm_add_action_or_reset().
>
>> the socket array itself stays devm-managed.
>
> devres managed is probably the better term in textual form. "devm" is C
> codish way to say it in a short way.
>
Agreed, I'll reword to:
Switch hsmp_get_tbl_dram_base() to plain ioremap() and add
hsmp_unmap_metric_tbls(). The unmapping of all sockets is still devres
managed, via devm_add_action_or_reset(); the socket array itself stays
devres managed too.
>> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
>> ---
>> drivers/platform/x86/amd/hsmp/hsmp.c | 19 +++++++++++++++++--
>> drivers/platform/x86/amd/hsmp/hsmp.h | 1 +
>> drivers/platform/x86/amd/hsmp/plat.c | 14 ++++++++++++++
>> 3 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
>> index 1a87931136fd..4586d86f72a3 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
>> @@ -12,6 +12,7 @@
>> #include <linux/acpi.h>
>> #include <linux/delay.h>
>> #include <linux/device.h>
>> +#include <linux/io.h>
>> #include <linux/semaphore.h>
>> #include <linux/sysfs.h>
>>
>> @@ -414,6 +415,21 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
>> }
>> EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
>>
>> +void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev)
>> +{
>> + u16 i;
>> +
>> + for (i = 0; i < pdev->num_sockets; i++) {
>> + struct hsmp_socket *s = &pdev->sock[i];
>> +
>> + if (s->metric_tbl_addr) {
>> + iounmap(s->metric_tbl_addr);
>> + s->metric_tbl_addr = NULL;
>> + }
>> + }
>> +}
>> +EXPORT_SYMBOL_NS_GPL(hsmp_unmap_metric_tbls, "AMD_HSMP");
>> +
>> int hsmp_get_tbl_dram_base(u16 sock_ind)
>> {
>> struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
>> @@ -438,8 +454,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
>> dev_err(sock->dev, "Invalid DRAM address for metric table\n");
>> return -ENOMEM;
>> }
>> - sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
>> - sizeof(struct hsmp_metric_table));
>> + sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
>> if (!sock->metric_tbl_addr) {
>> dev_err(sock->dev, "Failed to ioremap metric table addr\n");
>> return -ENOMEM;
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
>> index 0509a442eaae..98c82a4c0cc3 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
>> @@ -64,6 +64,7 @@ void hsmp_misc_deregister(void);
>> int hsmp_misc_register(struct device *dev);
>> int hsmp_get_tbl_dram_base(u16 sock_ind);
>> ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
>> +void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev);
>> struct hsmp_plat_device *get_hsmp_pdev(void);
>> #if IS_ENABLED(CONFIG_HWMON)
>> int hsmp_create_sensor(struct device *dev, u16 sock_ind);
>> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
>> index e07f68575055..e7ee6e701e5a 100644
>> --- a/drivers/platform/x86/amd/hsmp/plat.c
>> +++ b/drivers/platform/x86/amd/hsmp/plat.c
>> @@ -201,6 +201,16 @@ static int init_platform_device(struct device *dev)
>> return 0;
>> }
>>
>> +/*
>> + * The metric tables are mapped with ioremap() rather than devm, so release
>> + * them explicitly. Registered as a devres action so it also runs on a probe
>> + * failure after init_platform_device() has mapped some of them.
>
> Please put "Registered..." into own paragraph. It's sort of different
> thing it's talking about that the first sentence, and you'll be adding to
> the first paragraph in the other patch as well.
>
> /*
> * Paragraph 1.
> *
> * Paragraph 2.
>
Will do.
>> + */
>> +static void hsmp_pltdrv_release(void *data)
>> +{
>> + hsmp_unmap_metric_tbls(hsmp_pdev);
>> +}
>> +
>> static int hsmp_pltdrv_probe(struct platform_device *pdev)
>> {
>> int ret;
>> @@ -211,6 +221,10 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>> if (!hsmp_pdev->sock)
>> return -ENOMEM;
>>
>> + ret = devm_add_action_or_reset(&pdev->dev, hsmp_pltdrv_release, NULL);
>> + if (ret)
>> + return ret;
>> +
>> ret = init_platform_device(&pdev->dev);
>> if (ret) {
>> dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
>>
>
> --
> i.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 3/6] platform/x86/amd/hsmp: Serialize per-socket metric table reads with a mutex
2026-07-08 4:23 [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
2026-07-08 4:24 ` [PATCH v4 1/6] platform/x86/amd/hsmp: Serialize ACPI HSMP is_probed with a probe mutex Muralidhara M K
2026-07-08 4:24 ` [PATCH v4 2/6] platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap it explicitly Muralidhara M K
@ 2026-07-08 4:24 ` Muralidhara M K
2026-07-08 4:24 ` [PATCH v4 4/6] platform/x86/amd/hsmp: Clear mdev.this_device on deregister Muralidhara M K
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Muralidhara M K @ 2026-07-08 4:24 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: platform-driver-x86, linux-kernel, Muralidhara M K
HSMP_GET_METRIC_TABLE makes the firmware refill a shared per-socket metric
DRAM region, which hsmp_metric_tbl_read() then copies out with
memcpy_fromio(). Two concurrent readers of the metrics_bin sysfs attribute
on the same socket can race: one can trigger a fresh fill while the other
is mid-copy and return a torn snapshot. (The hwmon path does not touch
this region; it only issues power messages via hsmp_send_message().)
Embed a struct mutex metric_read_lock in each hsmp_socket and hold it
across the fill-and-copy in hsmp_metric_tbl_read(). Add
hsmp_init_metric_read_locks() and hsmp_destroy_metric_read_locks(), which
take only struct hsmp_plat_device and iterate pdev->sock[] over
pdev->num_sockets so the caller cannot pass a count that disagrees with the
array.
Wire them into both front-ends' probe and teardown paths so the mutex is
always initialized before metrics_bin is exposed: the platform driver and
the ACPI driver both drive hsmp_metric_tbl_read() through the same 0444
metrics_bin attribute. Doing this in one patch avoids a bisection point
where an ACPI read would lock an uninitialized mutex.
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 3 +++
drivers/platform/x86/amd/hsmp/hsmp.c | 29 ++++++++++++++++++++++++++++
drivers/platform/x86/amd/hsmp/hsmp.h | 5 +++++
drivers/platform/x86/amd/hsmp/plat.c | 10 +++++++---
4 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index eef799ce1fb8..095289ddb7e7 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -636,6 +636,8 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
GFP_KERNEL);
if (!hsmp_pdev->sock)
return -ENOMEM;
+
+ hsmp_init_metric_read_locks(hsmp_pdev);
}
ret = init_acpi(&pdev->dev);
@@ -667,6 +669,7 @@ static void hsmp_acpi_remove(struct platform_device *pdev)
*/
if (hsmp_pdev->is_probed) {
hsmp_misc_deregister();
+ hsmp_destroy_metric_read_locks(hsmp_pdev);
hsmp_pdev->is_probed = false;
}
}
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 4586d86f72a3..10fc21887308 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -10,9 +10,11 @@
#include <asm/amd/hsmp.h>
#include <linux/acpi.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/io.h>
+#include <linux/mutex.h>
#include <linux/semaphore.h>
#include <linux/sysfs.h>
@@ -406,6 +408,15 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
msg.msg_id = HSMP_GET_METRIC_TABLE;
msg.sock_ind = sock->sock_ind;
+ /*
+ * HSMP_GET_METRIC_TABLE makes the firmware refill the shared metric
+ * DRAM region, then the snapshot is copied out of it. Serialize the
+ * fill-and-copy per socket so two concurrent readers cannot have one
+ * trigger a fresh fill while the other is mid-copy and return a torn
+ * snapshot.
+ */
+ guard(mutex)(&sock->metric_read_lock);
+
ret = hsmp_send_message(&msg);
if (ret)
return ret;
@@ -415,6 +426,24 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
}
EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
+void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev)
+{
+ u16 i;
+
+ for (i = 0; i < pdev->num_sockets; i++)
+ mutex_init(&pdev->sock[i].metric_read_lock);
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_init_metric_read_locks, "AMD_HSMP");
+
+void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev)
+{
+ u16 i;
+
+ for (i = 0; i < pdev->num_sockets; i++)
+ mutex_destroy(&pdev->sock[i].metric_read_lock);
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_destroy_metric_read_locks, "AMD_HSMP");
+
void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev)
{
u16 i;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index 98c82a4c0cc3..a43a8043a6cb 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -15,6 +15,7 @@
#include <linux/hwmon.h>
#include <linux/kconfig.h>
#include <linux/miscdevice.h>
+#include <linux/mutex.h>
#include <linux/pci.h>
#include <linux/semaphore.h>
#include <linux/sysfs.h>
@@ -43,6 +44,8 @@ struct hsmp_socket {
void __iomem *metric_tbl_addr;
void __iomem *virt_base_addr;
struct semaphore hsmp_sem;
+ /* Serializes HSMP_GET_METRIC_TABLE fill-and-copy for this socket */
+ struct mutex metric_read_lock;
char name[HSMP_ATTR_GRP_NAME_SIZE];
struct device *dev;
u16 sock_ind;
@@ -64,6 +67,8 @@ void hsmp_misc_deregister(void);
int hsmp_misc_register(struct device *dev);
int hsmp_get_tbl_dram_base(u16 sock_ind);
ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
+void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev);
+void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev);
void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev);
struct hsmp_plat_device *get_hsmp_pdev(void);
#if IS_ENABLED(CONFIG_HWMON)
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index e7ee6e701e5a..d46dcb8c7f03 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -202,13 +202,15 @@ static int init_platform_device(struct device *dev)
}
/*
- * The metric tables are mapped with ioremap() rather than devm, so release
- * them explicitly. Registered as a devres action so it also runs on a probe
- * failure after init_platform_device() has mapped some of them.
+ * The metric tables are mapped with ioremap() rather than devm and the
+ * per-socket mutexes need an explicit mutex_destroy(), so tear both down
+ * here. Registered as a devres action so it also runs on a probe failure
+ * after init_platform_device() has mapped some tables.
*/
static void hsmp_pltdrv_release(void *data)
{
hsmp_unmap_metric_tbls(hsmp_pdev);
+ hsmp_destroy_metric_read_locks(hsmp_pdev);
}
static int hsmp_pltdrv_probe(struct platform_device *pdev)
@@ -221,6 +223,8 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
if (!hsmp_pdev->sock)
return -ENOMEM;
+ hsmp_init_metric_read_locks(hsmp_pdev);
+
ret = devm_add_action_or_reset(&pdev->dev, hsmp_pltdrv_release, NULL);
if (ret)
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 4/6] platform/x86/amd/hsmp: Clear mdev.this_device on deregister
2026-07-08 4:23 [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
` (2 preceding siblings ...)
2026-07-08 4:24 ` [PATCH v4 3/6] platform/x86/amd/hsmp: Serialize per-socket metric table reads with a mutex Muralidhara M K
@ 2026-07-08 4:24 ` Muralidhara M K
2026-07-08 4:24 ` [PATCH v4 5/6] platform/x86/amd/hsmp: ACPI HSMP refcounted sockets and coordinated release Muralidhara M K
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Muralidhara M K @ 2026-07-08 4:24 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: platform-driver-x86, linux-kernel, Muralidhara M K
misc_deregister() destroys the device but leaves miscdevice.this_device
pointing at the freed struct device. Clear it so any later check of
this_device, and a subsequent re-register, does not observe a stale
pointer. An upcoming change uses this_device to track whether /dev/hsmp is
registered across the shared ACPI sockets and relies on it being NULL after
deregister.
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/hsmp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 10fc21887308..ccd19c7bf115 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -531,6 +531,12 @@ EXPORT_SYMBOL_NS_GPL(hsmp_misc_register, "AMD_HSMP");
void hsmp_misc_deregister(void)
{
misc_deregister(&hsmp_pdev.mdev);
+ /*
+ * misc_deregister() leaves mdev.this_device pointing at the freed
+ * device. Clear it so a later re-register is not skipped on a stale
+ * pointer.
+ */
+ hsmp_pdev.mdev.this_device = NULL;
}
EXPORT_SYMBOL_NS_GPL(hsmp_misc_deregister, "AMD_HSMP");
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 5/6] platform/x86/amd/hsmp: ACPI HSMP refcounted sockets and coordinated release
2026-07-08 4:23 [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
` (3 preceding siblings ...)
2026-07-08 4:24 ` [PATCH v4 4/6] platform/x86/amd/hsmp: Clear mdev.this_device on deregister Muralidhara M K
@ 2026-07-08 4:24 ` Muralidhara M K
2026-07-08 4:24 ` [PATCH v4 6/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
2026-07-08 10:07 ` [PATCH v4 0/6] " Ilpo Järvinen
6 siblings, 0 replies; 12+ messages in thread
From: Muralidhara M K @ 2026-07-08 4:24 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: platform-driver-x86, linux-kernel, Muralidhara M K
The ACPI driver binds one platform device per socket but shares a single
socket array and a single /dev/hsmp misc device across them. Replace the
is_probed flag with state that tracks this shared ownership:
- miscdevice.this_device tells whether /dev/hsmp is registered, so the
misc device is registered on the first socket and torn down last. A
preceding change clears mdev.this_device on deregister so this gate
stays reliable across a re-probe.
- hsmp_acpi_sock_refs counts the sockets that have probed successfully.
It is guarded by hsmp_acpi_probe_mutex, so a plain counter is enough and
no atomic refcount is needed. The shared socket array is allocated with
kcalloc() on the first probe and freed by hsmp_acpi_sock_release() when
the count drops back to zero.
hsmp_acpi_sock_release() is the single teardown helper: it deregisters
/dev/hsmp if registered, unmaps any metric-table DRAM, destroys the
per-socket mutexes and frees the array. Both the remove path and the
probe-failure path call it once they are the last owner, so the teardown
lives in one place.
Both paths also clear this socket's dev, so a message issued after a
non-final unbind (or to a socket that failed to probe on a multi-socket
system, whose array stays alive and whose remove() is never called) cannot
reach the mailbox that devres is about to unmap.
Two lifetime fixes fall out of the array persisting across a non-final
unbind:
- hsmp_get_tbl_dram_base() iounmap()s any stale metric_tbl_addr before
remapping, so a rebind does not leak one mapping per cycle. It runs
during (re)probe before the metric sysfs attribute is exposed, so no
reader can be using the old mapping.
- /dev/hsmp is no longer parented to a per-socket device. Sockets can be
unbound individually and out of order, and the misc device outlives all
but the last of them, so parenting it to one socket's device would leave
a dangling parent. Register it unparented instead.
An upcoming change wires both teardown paths into a data-plane rwsem so
they are drained against the lock-free data plane, nested inside
hsmp_acpi_probe_mutex.
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 102 ++++++++++++++++++++++-----
drivers/platform/x86/amd/hsmp/hsmp.c | 21 +++++-
drivers/platform/x86/amd/hsmp/hsmp.h | 1 -
3 files changed, 106 insertions(+), 18 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 095289ddb7e7..d35465079eea 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -43,11 +43,20 @@
static struct hsmp_plat_device *hsmp_pdev;
/*
- * Serializes ACPI probe, remove and init_acpi() so concurrent socket probes
- * cannot race the is_probed handshake or the one-time socket-array allocation.
+ * Serializes ACPI probe, remove and init_acpi() and guards the shared socket
+ * state they manage: the one-time socket-array allocation, /dev/hsmp
+ * registration and hsmp_acpi_sock_refs. A plain counter is therefore enough
+ * for the refcount; no atomic is needed.
*/
static DEFINE_MUTEX(hsmp_acpi_probe_mutex);
+/*
+ * Number of ACPI socket platform devices that have probed successfully. The
+ * shared socket array is allocated on the first probe and freed once this
+ * drops back to zero.
+ */
+static unsigned int hsmp_acpi_sock_refs;
+
struct hsmp_sys_attr {
struct device_attribute dattr;
u32 msg_id;
@@ -614,6 +623,56 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
};
MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
+/*
+ * Tear down the shared ACPI socket state once the last socket is gone:
+ * deregister /dev/hsmp if it was registered, unmap any metric-table DRAM,
+ * destroy the per-socket mutexes and free the socket array.
+ *
+ * Called with hsmp_acpi_probe_mutex held, serializing it against a concurrent
+ * probe.
+ */
+static void hsmp_acpi_sock_release(void)
+{
+ lockdep_assert_held(&hsmp_acpi_probe_mutex);
+
+ if (!IS_ERR_OR_NULL(hsmp_pdev->mdev.this_device))
+ hsmp_misc_deregister();
+ hsmp_unmap_metric_tbls(hsmp_pdev);
+ hsmp_destroy_metric_read_locks(hsmp_pdev);
+ kfree(hsmp_pdev->sock);
+ hsmp_pdev->sock = NULL;
+ hsmp_pdev->num_sockets = 0;
+ hsmp_pdev->proto_ver = 0;
+}
+
+/**
+ * hsmp_acpi_probe_failure_cleanup() - Undo a failed ACPI socket probe.
+ * @dev: ACPI companion device whose probe failed.
+ *
+ * This device never incremented hsmp_acpi_sock_refs, so clear its sock->dev
+ * and, if it was the only socket in play, release the shared state.
+ *
+ * Clearing sock->dev matters on multi-socket systems: when a non-first socket
+ * fails, the array stays alive (owned by an already-probed socket) and
+ * remove() is never called for this device, yet devres unmaps its mailbox once
+ * probe() returns. Without clearing dev, a later message to this index would
+ * pass every gate in hsmp_send_message() and reach the unmapped mailbox.
+ *
+ * sock is NULL if probe failed before hsmp_parse_acpi_table() set the drvdata.
+ */
+static void hsmp_acpi_probe_failure_cleanup(struct device *dev)
+{
+ struct hsmp_socket *sock = dev_get_drvdata(dev);
+
+ lockdep_assert_held(&hsmp_acpi_probe_mutex);
+
+ if (sock)
+ sock->dev = NULL;
+
+ if (!hsmp_acpi_sock_refs)
+ hsmp_acpi_sock_release();
+}
+
static int hsmp_acpi_probe(struct platform_device *pdev)
{
int ret;
@@ -624,16 +683,16 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
guard(mutex)(&hsmp_acpi_probe_mutex);
- if (!hsmp_pdev->is_probed) {
+ if (!hsmp_pdev->sock) {
hsmp_pdev->num_sockets = topology_max_packages();
if (!hsmp_pdev->num_sockets) {
dev_err(&pdev->dev, "No CPU sockets detected\n");
return -ENODEV;
}
- hsmp_pdev->sock = devm_kcalloc(&pdev->dev, hsmp_pdev->num_sockets,
- sizeof(*hsmp_pdev->sock),
- GFP_KERNEL);
+ hsmp_pdev->sock = kcalloc(hsmp_pdev->num_sockets,
+ sizeof(*hsmp_pdev->sock),
+ GFP_KERNEL);
if (!hsmp_pdev->sock)
return -ENOMEM;
@@ -643,35 +702,46 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
ret = init_acpi(&pdev->dev);
if (ret) {
dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
+ hsmp_acpi_probe_failure_cleanup(&pdev->dev);
return ret;
}
- if (!hsmp_pdev->is_probed) {
+ if (IS_ERR_OR_NULL(hsmp_pdev->mdev.this_device)) {
ret = hsmp_misc_register(&pdev->dev);
if (ret) {
dev_err(&pdev->dev, "Failed to register misc device\n");
+ hsmp_acpi_probe_failure_cleanup(&pdev->dev);
return ret;
}
- hsmp_pdev->is_probed = true;
- dev_dbg(&pdev->dev, "AMD HSMP ACPI is probed successfully\n");
+ dev_dbg(&pdev->dev, "AMD HSMP ACPI misc device registered\n");
}
+ hsmp_acpi_sock_refs++;
+
return 0;
}
static void hsmp_acpi_remove(struct platform_device *pdev)
{
+ struct hsmp_socket *sock = dev_get_drvdata(&pdev->dev);
+
+ /*
+ * Serialize the decrement and any release it triggers against a
+ * concurrent probe so the count cannot be revived from zero.
+ */
guard(mutex)(&hsmp_acpi_probe_mutex);
/*
- * We register only one misc_device even on multi-socket system.
- * So, deregister should happen only once.
+ * Clear this socket's dev so hsmp_send_message() rejects it before
+ * devres unmaps the mailbox. On a non-final unbind the socket array
+ * stays alive, so without this a later message to this index would
+ * reach an unmapped iomem region.
*/
- if (hsmp_pdev->is_probed) {
- hsmp_misc_deregister();
- hsmp_destroy_metric_read_locks(hsmp_pdev);
- hsmp_pdev->is_probed = false;
- }
+ sock->dev = NULL;
+
+ hsmp_acpi_sock_refs--;
+ if (!hsmp_acpi_sock_refs)
+ hsmp_acpi_sock_release();
}
static struct platform_driver amd_hsmp_driver = {
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index ccd19c7bf115..e47e86116f16 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -483,6 +483,18 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
dev_err(sock->dev, "Invalid DRAM address for metric table\n");
return -ENOMEM;
}
+ /*
+ * The ACPI socket array is shared across sockets and outlives a
+ * per-socket unbind, so metric_tbl_addr may hold a mapping from an
+ * earlier bind of this socket. Unmap it before remapping so an
+ * unbind/rebind cycle does not leak a metric-table mapping. This runs
+ * during probe before the metric sysfs attribute is exposed, so no
+ * reader can be using it.
+ */
+ if (sock->metric_tbl_addr) {
+ iounmap(sock->metric_tbl_addr);
+ sock->metric_tbl_addr = NULL;
+ }
sock->metric_tbl_addr = ioremap(dram_addr, sizeof(struct hsmp_metric_table));
if (!sock->metric_tbl_addr) {
dev_err(sock->dev, "Failed to ioremap metric table addr\n");
n
@@ -520,7 +532,14 @@ int hsmp_misc_register(struct device *dev)
hsmp_pdev.mdev.name = HSMP_CDEV_NAME;
hsmp_pdev.mdev.minor = MISC_DYNAMIC_MINOR;
hsmp_pdev.mdev.fops = &hsmp_fops;
- hsmp_pdev.mdev.parent = dev;
+ /*
+ * /dev/hsmp is a singleton shared by all sockets and is torn down on
+ * the last socket's unbind. Do not parent it to a per-socket device:
+ * those can be unbound individually and out of order, which would leave
+ * the misc device parented to an already-removed device. Leave it
+ * unparented so its lifetime is independent.
+ */
+ hsmp_pdev.mdev.parent = NULL;
hsmp_pdev.mdev.nodename = HSMP_DEVNODE_NAME;
hsmp_pdev.mdev.mode = 0644;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index a43a8043a6cb..bc2050dcfdb5 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -57,7 +57,6 @@ struct hsmp_plat_device {
struct hsmp_socket *sock;
u32 proto_ver;
u16 num_sockets;
- bool is_probed;
};
int hsmp_cache_proto_ver(u16 sock_ind);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 6/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown
2026-07-08 4:23 [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
` (4 preceding siblings ...)
2026-07-08 4:24 ` [PATCH v4 5/6] platform/x86/amd/hsmp: ACPI HSMP refcounted sockets and coordinated release Muralidhara M K
@ 2026-07-08 4:24 ` Muralidhara M K
2026-07-08 10:07 ` [PATCH v4 0/6] " Ilpo Järvinen
6 siblings, 0 replies; 12+ messages in thread
From: Muralidhara M K @ 2026-07-08 4:24 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: platform-driver-x86, linux-kernel, Muralidhara M K
The HSMP data plane is lock-free: open /dev/hsmp fds and hwmon sysfs reads
call hsmp_send_message() without any coordination with driver teardown.
misc_deregister() does not drain already-open fds, so an in-flight message
can race a concurrent unbind and touch a freed socket array or an unmapped
mailbox.
Add hsmp_sock_rwsem and export it so the data plane and the teardown paths
take it directly. hsmp_send_message() holds it for read across the whole
bounds-check + MMIO access via guard(rwsem_read), so it is dropped on every
return path without an unlock label. A teardown path holds it for write to
drain any in-flight message and keep new ones out while it tears the socket
down.
Wire both teardown paths into the drain:
- plat.c: the devres release action takes the write lock, unmaps the
metric tables, destroys the per-socket mutexes and drops the global
socket pointer before devres frees the array.
- acpi.c: hsmp_acpi_remove() and the probe-failure cleanup take the write
lock (nested inside hsmp_acpi_probe_mutex) while they clear sock->dev
and, on the last unbind, unmap the mailbox and free the socket array via
hsmp_acpi_sock_release().
hsmp_sock_rwsem is deliberately separate from acpi.c's
hsmp_acpi_probe_mutex, which serializes the ACPI control-plane handshake
(socket-array allocation, misc registration state and the sock refcount
lifecycle). The ACPI probe path cannot reuse this rwsem for that: it drives
the data plane itself through hsmp_test(), which takes the rwsem for read,
so holding it for write across probe would deadlock. The two nest as
probe_mutex -> rwsem write.
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 19 ++++++++++++++++-
drivers/platform/x86/amd/hsmp/hsmp.c | 32 +++++++++++++++++++++++++++-
drivers/platform/x86/amd/hsmp/hsmp.h | 8 +++++++
drivers/platform/x86/amd/hsmp/plat.c | 8 +++++++
4 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index d35465079eea..814f0a812a85 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -24,6 +24,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/rwsem.h>
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/topology.h>
@@ -629,7 +630,9 @@ MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
* destroy the per-socket mutexes and free the socket array.
*
* Called with hsmp_acpi_probe_mutex held, serializing it against a concurrent
- * probe.
+ * probe, and with hsmp_sock_rwsem held for write, which has drained any
+ * in-flight hsmp_send_message() so unmapping the mailbox and freeing the array
+ * cannot race the lock-free data plane.
*/
static void hsmp_acpi_sock_release(void)
{
@@ -659,6 +662,10 @@ static void hsmp_acpi_sock_release(void)
* pass every gate in hsmp_send_message() and reach the unmapped mailbox.
*
* sock is NULL if probe failed before hsmp_parse_acpi_table() set the drvdata.
+ *
+ * Takes hsmp_sock_rwsem for write to serialize against the lock-free data
+ * plane: init_acpi() drives hsmp_test() and a previously probed socket may
+ * already have exposed /dev/hsmp.
*/
static void hsmp_acpi_probe_failure_cleanup(struct device *dev)
{
@@ -666,6 +673,8 @@ static void hsmp_acpi_probe_failure_cleanup(struct device *dev)
lockdep_assert_held(&hsmp_acpi_probe_mutex);
+ guard(rwsem_write)(&hsmp_sock_rwsem);
+
if (sock)
sock->dev = NULL;
@@ -731,6 +740,14 @@ static void hsmp_acpi_remove(struct platform_device *pdev)
*/
guard(mutex)(&hsmp_acpi_probe_mutex);
+ /*
+ * Drain the lock-free data plane and keep it out for the whole
+ * teardown. This covers the per-socket unbind, whose mailbox devres
+ * unmaps once we return, and the last unbind that frees the socket
+ * array in hsmp_acpi_sock_release().
+ */
+ guard(rwsem_write)(&hsmp_sock_rwsem);
+
/*
* Clear this socket's dev so hsmp_send_message() rejects it before
* devres unmaps the mailbox. On a non-final unbind the socket array
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index e47e86116f16..ab77a4996ebb 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -15,6 +15,7 @@
#include <linux/device.h>
#include <linux/io.h>
#include <linux/mutex.h>
+#include <linux/rwsem.h>
#include <linux/semaphore.h>
#include <linux/sysfs.h>
@@ -43,6 +44,25 @@
static struct hsmp_plat_device hsmp_pdev;
+/*
+ * Serializes the lock-free data plane (hsmp_send_message() and the per-socket
+ * MMIO access it performs) against socket teardown. The data plane takes it
+ * for read so multiple sockets can be driven concurrently; a teardown path
+ * takes it for write while it clears sock->dev, frees the socket array or
+ * unmaps the mailbox, so a reader can never observe a half-torn-down or freed
+ * socket.
+ *
+ * This is distinct from acpi.c's hsmp_acpi_probe_mutex, which serializes the
+ * ACPI control-plane handshake: the one-time socket-array allocation, misc
+ * device registration state and the socket refcount lifecycle. The ACPI probe
+ * path cannot use this rwsem for that: it drives the data plane itself through
+ * hsmp_test(), which takes the rwsem for read, so holding it for write across
+ * probe would deadlock. A teardown path instead nests the rwsem write side
+ * inside that mutex.
+ */
+DECLARE_RWSEM(hsmp_sock_rwsem);
+EXPORT_SYMBOL_NS_GPL(hsmp_sock_rwsem, "AMD_HSMP");
+
/*
* Send a message to the HSMP port via PCI-e config space registers
* or by writing to MMIO space.
@@ -214,6 +234,15 @@ int hsmp_send_message(struct hsmp_message *msg)
if (ret)
return ret;
+ /*
+ * Hold the teardown rwsem for read across the whole MMIO access. A
+ * teardown path takes it for write before clearing sock->dev, freeing
+ * the socket array or unmapping the mailbox, so the lock-free data
+ * plane can never dereference a freed socket or touch an unmapped
+ * mailbox. guard() drops it on every return path below.
+ */
+ guard(rwsem_read)(&hsmp_sock_rwsem);
+
if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets)
return -ENODEV;
@@ -234,7 +263,8 @@ int hsmp_send_message(struct hsmp_message *msg)
* non-NULL dev also guarantees virt_base_addr, the mailbox offsets and
* the semaphore are visible.
*
- * Pairs with smp_store_release(&sock->dev) in hsmp_parse_acpi_table().
+ * Held under hsmp_sock_rwsem; pairs with smp_store_release(&sock->dev)
+ * in hsmp_parse_acpi_table().
*/
if (!smp_load_acquire(&sock->dev))
return -ENODEV;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index bc2050dcfdb5..9c36a9252c73 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -17,6 +17,7 @@
#include <linux/miscdevice.h>
#include <linux/mutex.h>
#include <linux/pci.h>
+#include <linux/rwsem.h>
#include <linux/semaphore.h>
#include <linux/sysfs.h>
@@ -70,6 +71,13 @@ void hsmp_init_metric_read_locks(struct hsmp_plat_device *pdev);
void hsmp_destroy_metric_read_locks(struct hsmp_plat_device *pdev);
void hsmp_unmap_metric_tbls(struct hsmp_plat_device *pdev);
struct hsmp_plat_device *get_hsmp_pdev(void);
+
+/*
+ * Data-plane teardown rwsem. hsmp_send_message() holds it for read; a teardown
+ * path in plat.c/acpi.c holds it for write to drain the data plane while it
+ * clears sock->dev, frees the socket array or unmaps the mailbox.
+ */
+extern struct rw_semaphore hsmp_sock_rwsem;
#if IS_ENABLED(CONFIG_HWMON)
int hsmp_create_sensor(struct device *dev, u16 sock_ind);
#else
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index d46dcb8c7f03..f704095eeeb0 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -13,12 +13,14 @@
#include <linux/acpi.h>
#include <linux/build_bug.h>
+#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/dev_printk.h>
#include <linux/kconfig.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/rwsem.h>
#include <linux/sysfs.h>
#include <asm/amd/node.h>
@@ -206,11 +208,17 @@ static int init_platform_device(struct device *dev)
* per-socket mutexes need an explicit mutex_destroy(), so tear both down
* here. Registered as a devres action so it also runs on a probe failure
* after init_platform_device() has mapped some tables.
+ *
+ * Take the data-plane rwsem for write to drain any in-flight
+ * hsmp_send_message() and drop the global socket pointer before devres frees
+ * the array, so a late message bails out at its first check.
*/
static void hsmp_pltdrv_release(void *data)
{
+ guard(rwsem_write)(&hsmp_sock_rwsem);
hsmp_unmap_metric_tbls(hsmp_pdev);
hsmp_destroy_metric_read_locks(hsmp_pdev);
+ hsmp_pdev->sock = NULL;
}
static int hsmp_pltdrv_probe(struct platform_device *pdev)
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown
2026-07-08 4:23 [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
` (5 preceding siblings ...)
2026-07-08 4:24 ` [PATCH v4 6/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown Muralidhara M K
@ 2026-07-08 10:07 ` Ilpo Järvinen
2026-07-09 5:33 ` M K, Muralidhara
6 siblings, 1 reply; 12+ messages in thread
From: Ilpo Järvinen @ 2026-07-08 10:07 UTC (permalink / raw)
To: Muralidhara M K; +Cc: platform-driver-x86, LKML
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?
> - 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.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown
2026-07-08 10:07 ` [PATCH v4 0/6] " Ilpo Järvinen
@ 2026-07-09 5:33 ` M K, Muralidhara
2026-07-09 9:15 ` Ilpo Järvinen
0 siblings, 1 reply; 12+ messages in thread
From: M K, Muralidhara @ 2026-07-09 5:33 UTC (permalink / raw)
To: Ilpo Järvinen, Muralidhara M K; +Cc: platform-driver-x86, LKML
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.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/6] platform/x86/amd/hsmp: Serialize the data plane against socket teardown
2026-07-09 5:33 ` M K, Muralidhara
@ 2026-07-09 9:15 ` Ilpo Järvinen
0 siblings, 0 replies; 12+ messages in thread
From: Ilpo Järvinen @ 2026-07-09 9:15 UTC (permalink / raw)
To: M K, Muralidhara; +Cc: Muralidhara M K, platform-driver-x86, LKML
[-- Attachment #1: Type: text/plain, Size: 6028 bytes --]
On Thu, 9 Jul 2026, M K, Muralidhara wrote:
> On 7/8/2026 3:37 PM, Ilpo Järvinen wrote:
> > 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.
Yes, if that's the case.
> 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.
Yes, I think it's okay to reuse an existing rwsem for probe/remove. It
just keeps things much simpler.
I'm actually expecting it to save us some trouble in future over separate
locks exactly because we can now know the other sockets cannot race with
the probe.
> I'll add
> lockdep_assert_held_write() in the locked core to keep the "rwsem already
> held" contract explicit.
Good.
> 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.
> >
>
--
i.
^ permalink raw reply [flat|nested] 12+ messages in thread