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 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>
> --
>   i.