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

[M K, Muralidhara]

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