Re: [PATCH v7 3/4] platform/x86/amd/hsmp: Hide server-only attributes on client platforms

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

On Thu, 24 Sep 2026, Muralidhara M K wrote:

> Most ACPI sysfs device attributes are wired to hardcoded server
> message IDs (HSMP_GET_SOCKET_POWER and friends); hwmon power
> attributes registered from init_acpi() have the same problem. On a
> Family 1Ah client platform these numeric IDs resolve against enum
> hsmp_client_message_ids instead, and mostly collide with unrelated
> client commands, e.g. HSMP_GET_SOCKET_POWER (4) is
> HSMP_CLIENT_GET_METRICS_TABLE_VER on the client side.
> 
> Hide the affected ACPI sysfs attributes on client platforms except
> smu_fw_version and protocol_version, which report the same data in
> both tables. Forward-declare hattr_smu_fw_version and
> hattr_protocol_version ahead of their HSMP_DEV_ATTR() definitions.
> Skip hsmp_create_sensor() in init_acpi() on client platforms for the
> same reason.
> 
> The metrics_bin sysfs binary attribute has the same visibility problem:
> hsmp_is_sock_attr_visible() gates it on hsmp_pdev->proto_ver against
> HSMP_PROTO_VER6 with no is_client_platform() check. Exclude client
> platforms outright there too, matching the ACPI sysfs attributes
> above: the client set has its own telemetry table, read only through
> HSMP_IOCTL_GET_TELEMETRY_DATA.
> 
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
>  drivers/platform/x86/amd/hsmp/acpi.c | 29 ++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index 8b4bf57c3485..d775ea8ba603 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -300,16 +300,31 @@ static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>  	 * so that userspace which expects the file to exist gets a clear
>  	 * -EOPNOTSUPP from the read handler instead of -ENOENT, and is
>  	 * pointed at HSMP_IOCTL_GET_TELEMETRY_DATA as the supported path.
> +	 * The client set has its own telemetry table, read only through
> +	 * HSMP_IOCTL_GET_TELEMETRY_DATA; never show this file there.
>  	 */
> -	if (hsmp_pdev->proto_ver >= HSMP_PROTO_VER6)
> +	if (!is_client_platform() && hsmp_pdev->proto_ver >= HSMP_PROTO_VER6)
>  		return battr->attr.mode;
>  
>  	return 0;
>  }
>  
> +/* Defined below by HSMP_DEV_ATTR(); same message ID in both message tables */
> +static struct hsmp_sys_attr hattr_smu_fw_version;
> +static struct hsmp_sys_attr hattr_protocol_version;
> +
>  static umode_t hsmp_is_sock_dev_attr_visible(struct kobject *kobj,
>  					     struct attribute *attr, int id)
>  {
> +	/*
> +	 * smu_fw_version and protocol_version map to the same data in both
> +	 * message tables. Every other attribute here uses server-only
> +	 * message IDs that resolve to unrelated client commands.
> +	 */
> +	if (is_client_platform() && attr != &hattr_smu_fw_version.dattr.attr &&
> +	    attr != &hattr_protocol_version.dattr.attr)
> +		return 0;
> +
>  	return attr->mode;
>  }
>  
> @@ -564,9 +579,15 @@ 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");
> +	/*
> +	 * hwmon attributes use server-only message IDs, which resolve to
> +	 * unrelated client commands on client platforms.
> +	 */
> +	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]);

This series still feels misordered.

Can we like introduce is_client_platform() first, then add all these 
checks before adding the client support in the first place? This 
same ordering problem applies to the client/server check in patch 2 as 
well.

And would structs in patch 4 also needed earlier? This is not as bad 
problem as the above ordering one but I'd tend to think we'd want to 
introduce the struct before HSMP_CLIENT_GET_METRICS_TABLE actually works 
which is after patch 1, right?

-- 
 i.