Re: [PATCH v3 2/5] platform/x86/amd/hsmp: Map the metric table with ioremap() and unmap it explicitly

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

On 7/7/2026 5:14 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Tue, 7 Jul 2026, Muralidhara M K wrote:
> 
>> The metric table DRAM region is mapped with devm_ioremap(), tying its
>> lifetime to the socket device.  The ACPI driver (a later patch) needs to
>> 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.  Wire it into the platform driver's probe-failure and remove
>> paths so the non-devm mapping is released explicitly; the socket array
>> itself stays devm-managed.
>>
>> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
>> ---
>>   drivers/platform/x86/amd/hsmp/hsmp.c | 22 ++++++++++++++++++++--
>>   drivers/platform/x86/amd/hsmp/hsmp.h |  1 +
>>   drivers/platform/x86/amd/hsmp/plat.c | 14 ++++++++++++--
>>   3 files changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
>> index 1a87931136fd..ccf21f2d230d 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,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_unmap_metric_tbls(struct hsmp_plat_device *pdev)
>> +{
>> +     u16 i;
>> +
>> +     if (!pdev->sock)
>> +             return;
>> +
>> +     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 +457,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..e0a99e8f2be8 100644
>> --- a/drivers/platform/x86/amd/hsmp/plat.c
>> +++ b/drivers/platform/x86/amd/hsmp/plat.c
>> @@ -214,22 +214,32 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>>        ret = init_platform_device(&pdev->dev);
>>        if (ret) {
>>                dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
>> -             return ret;
>> +             goto err_unmap;
>>        }
>>
>>        ret = hsmp_misc_register(&pdev->dev);
>>        if (ret) {
>>                dev_err(&pdev->dev, "Failed to register misc device\n");
>> -             return ret;
>> +             goto err_unmap;
>>        }
>>
>>        dev_dbg(&pdev->dev, "AMD HSMP is probed successfully\n");
>>        return 0;
>> +
>> +err_unmap:
>> +     /*
>> +      * init_platform_device() may have ioremap()ed metric tables before
>> +      * failing.  They are no longer devm-managed, so drop them explicitly;
>> +      * the socket array itself is devm-managed.
> 
> One additional thought... While you cannot use devm for the ioremap(), you
> probably could still use devm_add_action_or_reset() to make the
> hsmp_unmap_metric_tbls() call using devres.
> 
> ... And the same thing with the cleanup added in patch 3.
> 
Thanks for the suggestion. Sure will Adopt devm_add_action_or_reset(). 
So platform teardown now runs via a hsmp_pltdrv_release() devres action 
and the goto and the explicit unmap in remove will go.
>> +      */
>> +     hsmp_unmap_metric_tbls(hsmp_pdev);
>> +     return ret;
>>   }
>>
>>   static void hsmp_pltdrv_remove(struct platform_device *pdev)
>>   {
>>        hsmp_misc_deregister();
>> +     hsmp_unmap_metric_tbls(hsmp_pdev);
>>   }
>>
>>   static struct platform_driver amd_hsmp_driver = {
>>
> 
> --
>   i.
>