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

[M K, Muralidhara]

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@xxxxxxx>
> > ---
> >   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.