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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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.

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