Re: [PATCH v4 4/7] platform/x86/amd/hsmp: Source metric-table size from firmware

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

On 6/11/2026 10:14 PM, M K, Muralidhara wrote:
> 
> 
> On 6/11/2026 1:50 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, 11 Jun 2026, M K, Muralidhara wrote:
>>> On 6/10/2026 4:35 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, 28 May 2026, Muralidhara M K wrote:
>>>>
>>>>> The driver hard-codes the metric-table region size to
>>>>> sizeof(struct hsmp_metric_table). That is correct for HSMP protocol
>>>>> version 6 but mis-sizes the ioremap of the SMU DRAM region on newer
>>>>> platforms: Family 1Ah Model 50h-5Fh exposes a ~13 KB
>>>>> hsmp_metric_table_zen6, and the table is expected to keep growing
>>>>> on future firmware. The same hard-coded value also forces
>>>>> hsmp_metric_tbl_read() to reject any read that follows the actual
>>>>> firmware layout.
>>>>>
>>>>> Pick up the table size from firmware instead. SMU on Family 1Ah
>>>>> Model 50h and later populates HSMP_GET_METRIC_TABLE_DRAM_ADDR's
>>>>> args[2] with the DRAM region size in bytes; older firmware leaves
>>>>> it 0. Bump the descriptor's response_sz to 3 so the field is read,
>>>>> store the value in hsmp_pdev.hsmp_table_size at probe time, and
>>>>> fall back to sizeof(struct hsmp_metric_table) when firmware reports
>>>>> 0. Use hsmp_table_size both for the devm_ioremap() of the region
>>>>> and as the upper bound for hsmp_metric_tbl_read().
>>>>>
>>>>> Behaviour on existing protocol-version-6 hardware is unchanged:
>>>>> firmware returns 0, the fallback yields the same value as the
>>>>> previous hard-coded one, and both the ioremap and the size check
>>>>> produce the same result as before.
>>>>>
>>>>> The ioctl interface added later in this series uses the same
>>>>> hsmp_table_size to validate the userspace request and to copy out
>>>>> the full firmware-reported region.
>>>>>
>>>>> Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
>>>>> Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
>>>>> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
>>>>> ---
>>>>>    arch/x86/include/uapi/asm/amd_hsmp.h |  5 +++--
>>>>>    drivers/platform/x86/amd/hsmp/hsmp.c | 11 ++++++++---
>>>>>    drivers/platform/x86/amd/hsmp/hsmp.h |  1 +
>>>>>    3 files changed, 12 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h
>>>>> b/arch/x86/include/uapi/asm/amd_hsmp.h
>>>>> index fce271282348..438ac38d0dc8 100644
>>>>> --- a/arch/x86/include/uapi/asm/amd_hsmp.h
>>>>> +++ b/arch/x86/include/uapi/asm/amd_hsmp.h
>>>>> @@ -365,11 +365,12 @@ static const struct hsmp_msg_desc
>>>>> hsmp_msg_desc_table[]
>>>>>         {0, 0, HSMP_GET},
>>>>>
>>>>>         /*
>>>>> -      * HSMP_GET_METRIC_TABLE_DRAM_ADDR, num_args = 0, response_sz 
>>>>> = 2
>>>>> +      * HSMP_GET_METRIC_TABLE_DRAM_ADDR, num_args = 0, response_sz 
>>>>> = 3
>>>>>          * output: args[0] = lower 32 bits of the address
>>>>>          * output: args[1] = upper 32 bits of the address
>>>>> +      * output: args[2] = DRAM region size in bytes
>>>>>          */
>>>>> -     {0, 2, HSMP_GET},
>>>>> +     {0, 3, HSMP_GET},
>>>>>
>>>>>         /*
>>>>>          * HSMP_SET_XGMI_PSTATE_RANGE, num_args = 1, response_sz = 0
>>>>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c
>>>>> b/drivers/platform/x86/amd/hsmp/hsmp.c
>>>>> index 9bad58fef304..cf9392f99298 100644
>>>>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
>>>>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
>>>>> @@ -356,8 +356,7 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket 
>>>>> *sock,
>>>>> char *buf, size_t size)
>>>>>                 return -ENOMEM;
>>>>>         }
>>>>>
>>>>> -     /* Do not support lseek(), also don't allow more than the 
>>>>> size of
>>>>> metric table */
>>>>> -     if (size != sizeof(struct hsmp_metric_table)) {
>>>>> +     if (size != hsmp_pdev.hsmp_table_size) {
>>>>>                 dev_err(sock->dev, "Wrong buffer size\n");
>>>>>                 return -EINVAL;
>>>>>         }
>>>>> @@ -398,8 +397,14 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
>>>>>                 dev_err(sock->dev, "Invalid DRAM address for metric
>>>>> table\n");
>>>>>                 return -ENOMEM;
>>>>>         }
>>>>> +     /* SMU returns table size from Family 1Ah Model 50h and 
>>>>> forward */
>>>>> +     if (msg.args[2])
>>>>> +             hsmp_pdev.hsmp_table_size = msg.args[2];
>>>>> +     else
>>>>> +             hsmp_pdev.hsmp_table_size = sizeof(struct
>>>>> hsmp_metric_table);
>>>>> +
>>>>>         sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
>>>>> -                                          sizeof(struct
>>>>> hsmp_metric_table));
>>>>> +                                          hsmp_pdev.hsmp_table_size);
>>>>>         if (!sock->metric_tbl_addr) {
>>>>>                 dev_err(sock->dev, "Failed to ioremap metric table
>>>>> addr\n");
>>>>>                 return -ENOMEM;
>>>>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h
>>>>> b/drivers/platform/x86/amd/hsmp/hsmp.h
>>>>> index b153527e0a0d..e7f051475728 100644
>>>>> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
>>>>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
>>>>> @@ -55,6 +55,7 @@ struct hsmp_plat_device {
>>>>>         u32 proto_ver;
>>>>>         u16 num_sockets;
>>>>>         bool is_probed;
>>>>> +     size_t hsmp_table_size;
>>>>
>>>> Please add types.h include.
>>>>
>>> In hsmp.h, size_t becomes visible without adding anything new, 
>>> because the
>>> file already includes linux/device.h, and that pulls in linux/types.h
>>
>> Please don't rely on indirect include paths. The rule for includes is
>> Include What You Use.
>>
>> There are some exceptions to the general rule such as linux/xx.h
>> including asm/xx.h, in such case you only need to add linux/xx.h.
>>
> Understood. I will add <include/types.h>

typo mistake. I will add #include <linux/types.h>
>> -- 
>>   i.
>