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

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

On 5/22/2026 4:25 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> 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@amd.com>
>> Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
>> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
>> ---
>> 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.
> 
In the v2 series, you suggested using devm_mutex_init(). However, I will 
test it and initialize it earlier if required.

>> +
>>        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.
>