Re: [PATCH 7/7] Documentation/arch/x86/amd_hsmp: Document the SMN and MSR IOCTLs

On 7/30/2026 3:11 AM, Mario Limonciello wrote:
> 
> 
> On 7/29/26 11:40, Muralidhara M K wrote:
>> Describe HSMP_IOCTL_SMN_CMD and HSMP_IOCTL_MSR_CMD: the payload structs,
>> the CAP_SYS_RAWIO and open-mode requirements, and the rule that both are
>> read-only, so a request with @write set is refused with -EPERM even for a
>> fully privileged caller on an O_RDWR handle.
>>
>> Also note that reading the protocol_version attribute can fail on the
>> Fam1Ah client platforms, because some client SMU builds reject the
>> interface-version query, which is why the telemetry ioctl rather than
>> that attribute is the way to reach the client telemetry layout.
> 
> I'm happy that these are (at least) defaulting to read-only, but I don't 
> understand their purpose.
> 
> We already have /dev/cpu/CPUNUM/msr, and we also have amd-node which 
> provides access to SMN via /sys/kernel/debug/x86/.
> 
> Can you better describe the needs of why these new messages must exist 
> and we can't have structured data for them?
> 
Thanks for the input. Yes, I will describe more on why we need them.

> Even if amd_hsmp needs to use SMN or MSR to fetch specific data data, 
> this feels a lot safer to have a structured specific fetch method than 
> such a wide (albeit permissioned) pipe.
> 
>>
>> Signed-off-by: Muralidhara M K <[email protected]>
>> ---
>>   Documentation/arch/x86/amd_hsmp.rst | 44 +++++++++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>
>> diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/ 
>> x86/amd_hsmp.rst
>> index 8a9d6fe7d4cf..0f03201a9f68 100644
>> --- a/Documentation/arch/x86/amd_hsmp.rst
>> +++ b/Documentation/arch/x86/amd_hsmp.rst
>> @@ -229,6 +229,50 @@ The following IOCTLs are defined:
>>     ``sizeof(struct hsmp_metric_table)``.  Later version metrics table
>>     layout is documented in the Public PPR.
>> +  On the Fam1Ah client platforms reading ``protocol_version`` can fail,
>> +  because some client SMU builds reject the interface-version query.
>> +  Those parts have their own telemetry layout and this ioctl is the way
>> +  to read it.
> 
> It seems to me you don't actually describe the telemetry layout or what 
> should be done with SMN.
> 
>> +
>> +``ioctl(file, HSMP_IOCTL_SMN_CMD, struct hsmp_smn_message *smn)``
>> +  Read a 32-bit SMN register on the socket given by ``sock_ind``.  This
>> +  reaches registers outside the HSMP message ABI, so it requires
>> +  ``CAP_SYS_RAWIO`` as well as a handle opened for reading, the latter
>> +  because the value read is written back into the request struct.
>> +
>> +  The ioctl is read-only.  ``write`` must be zero, and a request that
>> +  sets it is rejected with ``-EPERM`` even with ``CAP_SYS_RAWIO`` and an
>> +  ``O_RDWR`` handle; the field exists only so that write support 
>> could be
>> +  added later without changing the layout.  ``reserved`` must be zero.
>> +  Argument::
>> +
>> +    struct hsmp_smn_message {
>> +        __u32 smn_address; /* SMN address to read */
>> +        __u32 value;       /* Value read, filled in by the kernel */
>> +        __u16 sock_ind;    /* Socket index */
>> +        __u8  write;       /* Must be zero */
>> +        __u8  reserved;    /* Reserved, must be zero */
>> +    };
>> +
>> +``ioctl(file, HSMP_IOCTL_MSR_CMD, struct hsmp_msr_message *msr)``
>> +  Read a 64-bit MSR on the logical CPU given by ``core_id``, or on
>> +  whichever CPU the ioctl runs on when ``core_id`` is
>> +  ``HSMP_MSR_ANY_CPU`` (``(__u32)-1``).  Any other ``core_id`` must 
>> refer
>> +  to an online CPU, otherwise the call fails with ``-EINVAL``.  Requires
>> +  ``CAP_SYS_RAWIO`` and a handle opened for reading.
>> +
>> +  Read-only on the same terms as ``HSMP_IOCTL_SMN_CMD``: a non-zero
>> +  ``write`` is rejected with ``-EPERM``, and every byte of ``reserved``
>> +  must be zero.  Argument::
>> +
>> +    struct hsmp_msr_message {
>> +        __u32 msr_address; /* MSR index to read */
>> +        __u32 core_id;     /* Logical CPU, or HSMP_MSR_ANY_CPU */
>> +        __u64 value;       /* Value read, filled in by the kernel */
>> +        __u8  write;       /* Must be zero */
>> +        __u8  reserved[3]; /* Reserved, must be zero */
>> +    };
>> +
>>   The ioctl would return a non-zero on failure; you can read errno to see
>>   what happened. The transaction returns 0 on success.
>