Re: [PATCH v3 7/7] platform/x86/amd/hsmp: Make metric table read locking use guard(mutex)

[Ilpo Järvinen]

On Sun, 17 May 2026, Muralidhara M K wrote:

> Add a per-socket mutex (metric_tbl_lock) to serialize concurrent reads
> on the metric table. Without serialization, two simultaneous
> readers could interleave the SMU refresh command and the
> memcpy_fromio(), producing a torn (mixed old/new) snapshot.
> 
> Use scoped guard(mutex) in hsmp_metric_tbl_read() so the lock is
> automatically released on all return paths. Initialize the mutex with
> devm_mutex_init() in hsmp_get_tbl_dram_base() and return an error if
> initialization fails, avoiding manual mutex_destroy() cleanup paths.
> 
> Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@xxxxxxx>
> Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@xxxxxxx>
> Signed-off-by: Muralidhara M K <muralidhara.mk@xxxxxxx>
> ---
> Changes:
> v1->v2: Add lock
> v2->v3: Replace mutex_init to devm_mutex_init call
> 
>  drivers/platform/x86/amd/hsmp/hsmp.c | 8 ++++++++
>  drivers/platform/x86/amd/hsmp/hsmp.h | 3 +++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 3a02d683dea0..2fec897a95be 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -447,6 +447,7 @@ 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;
>  
> +	guard(mutex)(&sock->metric_tbl_lock);
>  	ret = hsmp_send_message(&msg);
>  	if (ret)
>  		return ret;
> @@ -492,6 +493,13 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
>  		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
>  		return -ENOMEM;
>  	}
> +
> +	ret = devm_mutex_init(sock->dev, &sock->metric_tbl_lock);
> +	if (ret) {
> +		dev_err(sock->dev, "Failed to initialize metric table lock\n");
> +		return ret;
> +	}

Sashiko notes a potential problem with this failing and that not resulting 
in a probe fail, which leaves the mutex uninitialized.

But the mutex could be initialized earlier to avoid the problem I think.

> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_NS_GPL(hsmp_get_tbl_dram_base, "AMD_HSMP");
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index e7f051475728..f7b1cbf19932 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>
> @@ -41,6 +42,8 @@ struct hsmp_socket {
>  	struct bin_attribute hsmp_attr;
>  	struct hsmp_mbaddr_info mbinfo;
>  	void __iomem *metric_tbl_addr;
> +	/* Serializes concurrent metric table refreshes from the sysfs path */
> +	struct mutex metric_tbl_lock;
>  	void __iomem *virt_base_addr;
>  	struct semaphore hsmp_sem;
>  	char name[HSMP_ATTR_GRP_NAME_SIZE];
> 

-- 
 i.