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

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

On 10/8/2026 5:28 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> 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?
> 
Thanks Ilpo, it's a series reordering, not a content fix. I will make 
changes and submit next series.
> --
>   i.
>