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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5782 bytes --]

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.

-- 
 i.