Re: [PATCH 4/7] platform/x86/amd/hsmp: Leave the hwmon power sensors off on client platforms

[Mario Limonciello <mario.limonciello@amd.com>]

On 7/29/26 11:40, Muralidhara M K wrote:
> hwmon.c issues the server power message IDs unconditionally, so on the
> Family 1Ah client platforms the sensors address whatever the Ryzen
> Master SMC set puts at those numbers:
> 
> 	power1_input	04h	HSMP_CLIENT_GET_METRICS_TABLE_VER
> 	power1_cap	06h	HSMP_CLIENT_GET_METRICS_TABLE_DRAM_ADDR
> 
> Both match the num_args and response_sz that the sensors ask for, so
> validate_message() accepts them and the two files report a metrics-table
> version and the low half of a DRAM address as microwatts.
> 
> The other two accesses are already rejected, as they land on a client
> message taking a different num_args: power1_cap_max on 07h
> (HSMP_CLIENT_SET_CORE_PSM_MARGIN) and the power1_cap write path, which
> sends HSMP_SET_SOCKET_POWER_LIMIT on 05h
> (HSMP_CLIENT_GET_METRICS_TABLE). No unintended firmware write is
> possible either way.
> 
> The client message set has no power telemetry to report instead

Is that right?  I thought the metrics table has accumulator values.

Which brings me to my next question - where is the format of the metrics 
table declared?

It's probably (but not guaranteed) the same metrics table format used in 
PMF.  IE this:

https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=review-ilpo-next&id=641b41a7a12537f8898b1e14c62e0d85a8b872c2

But if userspace does and HSMP message to get the metrics table I think 
we need a kernel header that shows them how to decode the structure.

> , so gate
> the registration on the platform in both drivers rather than teach
> hwmon.c two message sets. Server behaviour is unchanged.
> 
> The ACPI sysfs attributes are left alone. They are hardcoded to server
> message IDs too, but on the client each one is either meaningful or
> rejected: 02h and 03h are the SMU and interface version queries in both
> message sets, and the num_args and response_sz checks in
> validate_message() reject the rest, including every ID that lands on a
> client SET.
> 
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
>   drivers/platform/x86/amd/hsmp/acpi.c | 13 ++++++++++---
>   drivers/platform/x86/amd/hsmp/plat.c | 15 +++++++++++----
>   2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index bbf2b9a8a408..57758df9d353 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -670,9 +670,16 @@ static int init_acpi(struct device *dev)
>   			dev_info(dev, "Failed to init metric table\n");
>   	}
>   
> -	ret = hsmp_create_sensor(dev, sock_ind);
> -	if (ret)
> -		dev_info(dev, "Failed to register HSMP sensors with hwmon\n");
> +	/*
> +	 * The hwmon power messages are numbered as metric table queries in the
> +	 * client message set, so the sensors would report those responses as
> +	 * power readings.  Leave them off there, as plat.c does.
> +	 */
> +	if (!is_client_platform()) {
> +		ret = hsmp_create_sensor(dev, sock_ind);
> +		if (ret)
> +			dev_info(dev, "Failed to register HSMP sensors with hwmon\n");
> +	}
>   
>   	dev_set_drvdata(dev, &hsmp_pdev->sock[sock_ind]);
>   
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index bc29b0ec18f7..17877e25f43e 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -196,10 +196,17 @@ static int init_platform_device(struct device *dev)
>   				dev_info(dev, "Failed to init metric table\n");
>   		}
>   
> -		/* Register with hwmon interface for reporting power */
> -		ret = hsmp_create_sensor(dev, i);
> -		if (ret)
> -			dev_info(dev, "Failed to register HSMP sensors with hwmon\n");
> +		/*
> +		 * Register with hwmon interface for reporting power.  Skipped
> +		 * on the client, where the power messages the sensors issue
> +		 * are numbered as metric table queries and would report those
> +		 * responses as power readings.
> +		 */
> +		if (!is_client_platform()) {
> +			ret = hsmp_create_sensor(dev, i);
> +			if (ret)
> +				dev_info(dev, "Failed to register HSMP sensors with hwmon\n");
> +		}
>   	}
>   
>   	return 0;