[PATCH v4 0/7] platform/x86/amd/hsmp: Support Family 1Ah Model 50h-5Fh telemetry

[PATCH v4 0/7] platform/x86/amd/hsmp: Support Family 1Ah Model 50h-5Fh telemetry

[Muralidhara M K]

Hi,

This series adds HSMP protocol version 7 support to the AMD HSMP driver
for Family 1Ah Model 50h-5Fh systems, and addresses a small set of
related issues that surfaced while wiring up the new platform.

The bulk of the work is exposing the new firmware-defined metric table
layout to userspace. HSMP_PROTO_VER7 firmware delivers a per-IOD /
per-CCD blob (struct hsmp_metric_table_zen6, ~13 KB) that does not fit
the existing per-socket struct hsmp_metric_table and does not fit the
existing sysfs metrics_bin transport at all, since binary sysfs
attributes are bounded by PAGE_SIZE. metrics_bin also has a long-
standing tearing problem: userspace can read it in multiple chunks and
observe an inconsistent snapshot if SMU refreshes the table between
read() calls.

To address both, the series:

  - Adds a stable UAPI description of the v7 metric-table layout
    (struct hsmp_metric_table_zen6{,_iod,_ccd}) that mirrors the
    firmware blob 1:1.

  - Stops hard-coding the metric-table region size in the driver and
    sources it from firmware (HSMP_GET_METRIC_TABLE_DRAM_ADDR args[2]),
    falling back to the old size on v6 firmware that leaves the field
    zero. The same size is used for ioremap() and for the ioctl
    bounds check.

  - Adds a new HSMP_IOCTL_GET_TELEMETRY_DATA on the existing HSMP
    character device that always copies the full firmware-reported
    table in one shot, removing both the PAGE_SIZE cap and the
    chunked-read tearing window. The trailing reserved field in the
    request is rejected if non-zero so future kernels can repurpose
    it without breaking deployed userspace, and user-supplied indices
    (sock_ind, msg_id) are clamped with array_index_nospec() to
    mitigate Spectre v1.

  - Wires the ACPI driver to populate the DRAM mapping for v7
    (currently gated on '== HSMP_PROTO_VER6'), so the new ioctl path
    is actually reachable on Family 1Ah Model 50h-5Fh hardware. The
    legacy metrics_bin attribute is preserved and continues to work
    on v6; on v7 it returns -EOPNOTSUPP pointing at the ioctl.

Around that, the series also:

  - Adds the new HSMP message IDs (0x29-0x2A, 0x33-0x3A) introduced by
    Family 1Ah Model 50h-5Fh firmware (PC6/CC6 control, CCD power /
    thermal monitoring, DIMM sideband access, floor- and SDPS-limit
    control, command-enable discovery) and converts three SET-only
    messages (HSMP_SET_XGMI_LINK_WIDTH, HSMP_SET_DF_PSTATE,
    HSMP_SET_PSTATE_MAX_MIN) to HSMP_SET_GET so userspace can read
    back the currently programmed value via bit[31] of args[0].

  - Relaxes validate_message() to an upper-bound check for every
    message type (HSMP_SET_GET already did this). Existing userspace
    that asks for fewer response words than firmware now provides is
    no longer rejected with -EINVAL.

  - Tightens metric-table read locking with a per-socket guard(mutex)
    around the SMU-side refresh + memcpy_fromio() sequence, and
    reorders devm_mutex_init() before devm_ioremap() so the
    "metric_tbl_addr != NULL implies metric_tbl_lock is initialized"
    invariant holds on every probe path.

Documentation/arch/x86/amd_hsmp.rst is updated to describe the
metrics_bin / IOCTL_GET_TELEMETRY_DATA split and the protocol-version
gating.

The series is bisect-safe: every commit was built individually and
checkpatch.pl --strict is clean across the series.

v3:
https://lore.kernel.org/platform-driver-x86/20260517151211.415627-1-muralidhara.mk@amd.com/T/#t

Changes since v3:

  - 1/7 (new HSMP messages): Tightened the inline bitfield comments
    on HSMP_SET_POWER_MODE / HSMP_PC6_ENABLE / HSMP_CC6_ENABLE /
    HSMP_DIMM_SB_RD / HSMP_DIMM_SB_WR / HSMP_FLOOR_LIMIT /
    HSMP_SDPS_LIMIT. Rewrote the commit log to spell out the
    backward-compatibility analysis for bit[31] of args[0] on
    the three SET->SET_GET conversions.

  - 2/7 (UAPI structs): Added per-field comments explaining the
    num_active_ccds placement (in the IOD block, matching the
    firmware-side aggregator) and the ccd[] sizing
    (HSMP_F1A_M50_M5F_MAX_CCDS = 8, hardware maximum, userspace
    iterates [0 .. num_active_ccds - 1]).

  - 3/7 (response_sz): Rewrote the commit log to motivate the change
    in terms of forward compatibility of older userspace against
    newer firmware/descriptor tables.

  - 4/7 (firmware-reported table size): Rewrote the commit log to
    document args[2] semantics and the 0-fallback path that keeps
    behaviour identical on v6 firmware.

  - 5/7 (HSMP_IOCTL_GET_TELEMETRY_DATA):
      * Added array_index_nospec() on req.sock_ind and msg.msg_id to
        mitigate Spectre v1 on the user-controlled ioctl indices.
      * Rejected non-zero req.reserved with -EINVAL so the field
        stays repurposable.
      * Tightened the UAPI struct layout reasoning under the
        surrounding #pragma pack(4) and the size-mismatch handling.
      * Trimmed the long implementation-detail paragraphs from the
        commit log.

  - 6/7 (enable v7 on the ACPI driver):
      * Restored the legacy sysfs metrics_bin attribute (it was
        dropped in v3) and gated reads by protocol version so v6
        userspace is unaffected and v7 userspace gets -EOPNOTSUPP
        pointing at HSMP_IOCTL_GET_TELEMETRY_DATA.
      * Removed the ABI-break paragraph from the commit log now that
        metrics_bin is preserved.

  - 7/7 (guard(mutex)):
      * Reordered devm_mutex_init() before devm_ioremap() so that
        sock->metric_tbl_addr is never published with an
        uninitialized metric_tbl_lock on the non-fatal init_acpi() /
        init_platform_device() error paths.
      * Rewrote the commit log to document that ordering invariant.

  - Documentation/arch/x86/amd_hsmp.rst: Documented the metrics_bin
    PAGE_SIZE / protocol-version constraints and the new
    HSMP_IOCTL_GET_TELEMETRY_DATA path, and cleaned up whitespace in
    the embedded struct snippet.

Thanks,
Murali

Muralidhara M K (7):
  platform/x86/amd/hsmp: Add new HSMP messages for Family 1Ah, Model
    50h-5Fh
  platform/x86/amd/hsmp: Add UAPI structures for Family 1Ah Model
    50h-5Fh metrics table
  platform/x86/amd/hsmp: Unify response_sz validation to an upper-bound
    check
  platform/x86/amd/hsmp: Source metric-table size from firmware
  platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table
    reads
  platform/x86/amd/hsmp: Enable HSMP_PROTO_VER7 metric tables on the
    ACPI driver via the IOCTL
  platform/x86/amd/hsmp: Make metric table read locking use guard(mutex)

 Documentation/arch/x86/amd_hsmp.rst  |  30 ++-
 arch/x86/include/uapi/asm/amd_hsmp.h | 294 +++++++++++++++++++++++++--
 drivers/platform/x86/amd/hsmp/acpi.c |  23 ++-
 drivers/platform/x86/amd/hsmp/hsmp.c | 169 +++++++++++++--
 drivers/platform/x86/amd/hsmp/hsmp.h |   6 +-
 5 files changed, 485 insertions(+), 37 deletions(-)

-- 
2.34.1

[PATCH v4 1/7] platform/x86/amd/hsmp: Add new HSMP messages for Family 1Ah, Model 50h-5Fh

[Muralidhara M K]

From: Suma Hegde <suma.hegde@amd.com>

Family 1Ah Model 50h-5Fh firmware exposes new HSMP messages
(0x29-0x2A, 0x33-0x3A) for PC6/CC6 control, CCD power/thermal
monitoring, DIMM sideband access, floor- and SDPS-limit control,
and command-enable discovery. The driver currently marks those IDs
as HSMP_RSVD and validate_message() rejects them with -ENOMSG, so
userspace has no way to reach the new functionality.

The same firmware also extends three existing SET-only messages
(HSMP_SET_XGMI_LINK_WIDTH 0x0C, HSMP_SET_DF_PSTATE 0x0D,
HSMP_SET_PSTATE_MAX_MIN 0x22) with a read-back path selected by
bit[31] of args[0] (0 = set, 1 = get). Without converting these
entries to HSMP_SET_GET, userspace cannot query the currently
programmed value even though firmware now supports it.

Add the new IDs, convert the three messages to HSMP_SET_GET, and
bump the driver version to 2.6.

Backward compatibility is preserved on prior platforms:

- The new IDs previously occupied HSMP_RSVD slots and were rejected
  with -ENOMSG, so no existing caller could have been issuing them.

- Existing userspace that leaves bit[31] = 0 continues to take a
  pure SET path (is_get_msg() returns false); the lax upper-bound
  validation already used for HSMP_SET_GET accepts the historical
  response_sz = 0 callers unchanged.

- A new-userspace build that issues a GET against older firmware
  fails cleanly with a firmware error, because that firmware rejects
  the non-zero Reserved bit it now sees in bit[31].

Reviewed-by: Suma Hegde <suma.hegde@amd.com>
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 | 153 ++++++++++++++++++++++++---
 drivers/platform/x86/amd/hsmp/hsmp.h |   2 +-
 2 files changed, 140 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
index 92d8f256d096..52e53ec5877f 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -53,9 +53,19 @@ enum hsmp_message_ids {
 	HSMP_SET_XGMI_PSTATE_RANGE,	/* 26h Set xGMI P-state range */
 	HSMP_CPU_RAIL_ISO_FREQ_POLICY,	/* 27h Get/Set Cpu Iso frequency policy */
 	HSMP_DFC_ENABLE_CTRL,		/* 28h Enable/Disable DF C-state */
+	HSMP_PC6_ENABLE,		/* 29h Get/Set PC6 enable/disable status */
+	HSMP_CC6_ENABLE,		/* 2Ah Get/Set CC6 enable/disable status */
 	HSMP_GET_RAPL_UNITS = 0x30,	/* 30h Get scaling factor for energy */
 	HSMP_GET_RAPL_CORE_COUNTER,	/* 31h Get core energy counter value */
 	HSMP_GET_RAPL_PACKAGE_COUNTER,	/* 32h Get package energy counter value */
+	HSMP_DIMM_SB_RD,		/* 33h Get DIMM sideband data */
+	HSMP_READ_CCD_POWER,		/* 34h Get average CCD power */
+	HSMP_READ_TDELTA,		/* 35h Get thermal behaviour */
+	HSMP_GET_SVI3_VR_CTRL_TEMP,	/* 36h Get SVI3 VR controller rail temp */
+	HSMP_GET_ENABLED_HSMP_CMDS,	/* 37h Get supported HSMP commands */
+	HSMP_SET_GET_FLOOR_LIMIT,	/* 38h Get/Set core floor frequency limit */
+	HSMP_DIMM_SB_WR,		/* 39h Set DIMM sideband data */
+	HSMP_SDPS_LIMIT,		/* 3Ah Get/Set SDPS limit */
 	HSMP_MSG_ID_MAX,
 };
 
@@ -170,16 +180,27 @@ static const struct hsmp_msg_desc hsmp_msg_desc_table[]
 	{0, 1, HSMP_GET},
 
 	/*
-	 * HSMP_SET_XGMI_LINK_WIDTH, num_args = 1, response_sz = 0
-	 * input: args[0] = min link width[15:8] + max link width[7:0]
+	 * HSMP_SET_XGMI_LINK_WIDTH, num_args = 1, response_sz = 0/1
+	 * input: args[0] = set/get XGMI Link width[31] (0 = set, 1 = get) +
+	 *                  min link width[15:8] + max link width[7:0]
+	 *        Link width encoding: 0 = x4, 1 = x8, 2 = x16.
+	 *        On SET, max must be >= min.  On GET, [15:0] are reserved.
+	 * output: args[0] = reserved[31:16] + min link width[15:8] +
+	 *                   max link width[7:0]
 	 */
-	{1, 0, HSMP_SET},
+	{1, 1, HSMP_SET_GET},
 
 	/*
-	 * HSMP_SET_DF_PSTATE, num_args = 1, response_sz = 0
-	 * input: args[0] = df pstate[7:0]
+	 * HSMP_SET_DF_PSTATE (APBDisable), num_args = 1, response_sz = 0/1
+	 * input: args[0] = set APB_DISABLE / get APB state[31]
+	 *                  (0 = set & lock DF P-state, 1 = get) +
+	 *                  reserved[30:8] +
+	 *                  DF P-state[7:0] (0..2; reserved on GET)
+	 * output: args[0] = reserved[31:9] +
+	 *                   APB state[8] (1 = disabled, 0 = enabled) +
+	 *                   locked DF P-state[7:0] if [8] = 1, else reserved
 	 */
-	{1, 0, HSMP_SET},
+	{1, 1, HSMP_SET_GET},
 
 	/* HSMP_SET_AUTO_DF_PSTATE, num_args = 0, response_sz = 0 */
 	{0, 0, HSMP_SET},
@@ -305,16 +326,32 @@ static const struct hsmp_msg_desc hsmp_msg_desc_table[]
 	{1, 1, HSMP_SET},
 
 	/*
-	 * HSMP_SET_POWER_MODE, num_args = 1, response_sz = 0
-	 * input: args[0] = power efficiency mode[2:0]
+	 * HSMP_SET_POWER_MODE (PwrEfficiencyModeSelection),
+	 *	num_args = 1, response_sz = 1
+	 * input: args[0] = set/get policy[31] (0 = set, 1 = get) +
+	 *                  high util point[30:24] +
+	 *                  low util point[23:17] +
+	 *                  PPT limit[16:5] +
+	 *                  reserved[4:3] + mode selection[2:0]
+	 *        [30:5] are valid only when [2:0] is a balanced core mode
+	 *        (4 or 5). [2:0] is reserved when getting (bit[31] = 1).
+	 * output: args[0] same layout, [31] reserved, [2:0] = arbitrated
+	 *         current efficiency mode.
 	 */
 	{1, 1, HSMP_SET_GET},
 
 	/*
-	 * HSMP_SET_PSTATE_MAX_MIN, num_args = 1, response_sz = 0
-	 * input: args[0] = min df pstate[15:8] + max df pstate[7:0]
+	 * HSMP_SET_PSTATE_MAX_MIN (DfPstateRange), num_args = 1, response_sz = 0/1
+	 * input: args[0] = set/get DF P-state range[31] (0 = set, 1 = get) +
+	 *                  reserved[30:16] +
+	 *                  min DF P-state[15:8] + max DF P-state[7:0]
+	 *        DF P-state encoding: 0 = DFP0 (high performance),
+	 *                             1 = DFP1, 2 = DFP2 (low performance).
+	 *        [15:0] are reserved when getting (args[0] bit[31] = 1).
+	 * output: args[0] = reserved[31:16] + min DF P-state[15:8] +
+	 *                   max DF P-state[7:0]
 	 */
-	{1, 0, HSMP_SET},
+	{1, 1, HSMP_SET_GET},
 
 	/*
 	 * HSMP_GET_METRIC_TABLE_VER, num_args = 0, response_sz = 1
@@ -355,9 +392,31 @@ static const struct hsmp_msg_desc hsmp_msg_desc_table[]
 	 */
 	{1, 1, HSMP_SET_GET},
 
-	/* RESERVED(0x29-0x2f) */
-	{0, 0, HSMP_RSVD},
-	{0, 0, HSMP_RSVD},
+	/*
+	 * HSMP_PC6_ENABLE (Pc6Enable), num_args = 1, response_sz = 0/1
+	 * input: args[0] = set/get PC6 control[31] (0 = set, 1 = get) +
+	 *                  reserved[30:1] +
+	 *                  enable PC6[0] (0 = disable, 1 = enable;
+	 *                  reserved on GET)
+	 * output: args[0] = reserved[31:1] + current PC6 control[0]
+	 *                   (last value configured via HSMP or APML)
+	 */
+	{1, 1, HSMP_SET_GET},
+
+	/*
+	 * HSMP_CC6_ENABLE (CC6Enable), num_args = 1, response_sz = 0/1
+	 * Configures CC6 enable for all cores; changing the setting does
+	 * not by itself transition cores in or out of CC6.
+	 * input: args[0] = set/get CC6 control[31] (0 = set, 1 = get) +
+	 *                  reserved[30:1] +
+	 *                  enable CC6[0] (0 = disable, 1 = enable;
+	 *                  reserved on GET)
+	 * output: args[0] = reserved[31:1] + current CC6 control[0]
+	 *                   (last value configured via HSMP or APML)
+	 */
+	{1, 1, HSMP_SET_GET},
+
+	/* RESERVED(0x2B-0x2F) */
 	{0, 0, HSMP_RSVD},
 	{0, 0, HSMP_RSVD},
 	{0, 0, HSMP_RSVD},
@@ -385,6 +444,72 @@ static const struct hsmp_msg_desc hsmp_msg_desc_table[]
 	 */
 	{0, 2, HSMP_GET},
 
+	/*
+	 * HSMP_DIMM_SB_RD, num_args = 1, response_sz = 1
+	 * input: args[0] = reg space[23] + reg offset[22:12] +
+	 *                  device LID[11:8] + DIMM address[7:0]
+	 * output: args[0] = read data byte[3:0]
+	 */
+	{1, 1, HSMP_GET},
+
+	/*
+	 * HSMP_READ_CCD_POWER, num_args = 1, response_sz = 1
+	 * input: args[0]  = apic id of core[15:0]
+	 * output: args[0] = CCD power(mWatts)[31:0]
+	 */
+	{1, 1, HSMP_GET},
+
+	/*
+	 * HSMP_READ_TDELTA, num_args = 0, response_sz = 1
+	 * output: args[0] = thermal behaviour[31:0]
+	 */
+	{0, 1, HSMP_GET},
+
+	/*
+	 * HSMP_GET_SVI3_VR_CTRL_TEMP, num_args = 1, response_sz = 1
+	 * input: args[0] = SVI3 rail index[3:1] + read temperature[0]
+	 * output: args[0] = SVI3 rail index[30:28] +
+	 *                   rail temperature in degree C[27:0]
+	 */
+	{1, 1, HSMP_GET},
+
+	/*
+	 * HSMP_GET_ENABLED_HSMP_CMDS, num_args = 1, response_sz = 3
+	 * input: args[0] = HSMP command mask[0]
+	 * output: status of HSMP command = args[0], args[1], args[2]
+	 */
+	{1, 3, HSMP_GET},
+
+	/*
+	 * HSMP_SET_GET_FLOOR_LIMIT, num_args = 1, response_sz = 1
+	 * input: args[0] = op[31:30] + reserved[29:28] +
+	 *                  apic id[27:16] + floor frequency MHz[15:0]
+	 *        op encoding: 00 = set per-core floor,
+	 *                     01 = set all-cores floor (apic id reserved),
+	 *                     10 = get per-core floor,
+	 *                     11 = get per-core effective floor.
+	 *        Floor frequency field is reserved on GET (bit[31] = 1).
+	 * output: args[0] = floor frequency MHz[15:0]
+	 *                   (effective for op 11, configured for op 10;
+	 *                   reserved on SET)
+	 */
+	{1, 1, HSMP_SET_GET},
+
+	/*
+	 * HSMP_DIMM_SB_WR, num_args = 1, response_sz = 0
+	 * input: args[0] = write data[31:24] + reg space[23] +
+	 *                  reg offset[22:12] + device LID[11:8] +
+	 *                  DIMM address[7:0]
+	 */
+	{1, 0, HSMP_SET},
+
+	/*
+	 * HSMP_SDPS_LIMIT, num_args = 1, response_sz = 1
+	 * input: args[0] = Set/Get[31] + SDPS Limit[30:0]
+	 * output: args[0] = SDPS Limit[30:0]
+	 */
+	{1, 1, HSMP_SET_GET},
+
 };
 
 /* Metrics table (supported only with proto version 6) */
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index 0509a442eaae..b153527e0a0d 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -27,7 +27,7 @@
 #define HSMP_DEVNODE_NAME	"hsmp"
 #define ACPI_HSMP_DEVICE_HID    "AMDI0097"
 
-#define DRIVER_VERSION		"2.5"
+#define DRIVER_VERSION		"2.6"
 
 struct hsmp_mbaddr_info {
 	u32 base_addr;
-- 
2.34.1

[PATCH v4 2/7] platform/x86/amd/hsmp: Add UAPI structures for Family 1Ah Model 50h-5Fh metrics table

[Muralidhara M K]

HSMP protocol version 7 on Family 1Ah Model 50h-5Fh delivers a new
per-IOD/per-CCD metric-table layout that is not compatible with the
existing hsmp_metric_table. Userspace tooling needs a stable UAPI
description of this layout to interpret the firmware blob carried by
the HSMP character device.

Add hsmp_metric_table_zen6_iod, hsmp_metric_table_zen6_ccd and the
top-level hsmp_metric_table_zen6 that mirror the firmware-defined
binary layout 1:1. The ccd[] array is sized to the hardware maximum
(HSMP_F1A_M50_M5F_MAX_CCDS = 8) to keep that mirror exact; userspace
iterates ccd[0 .. iod.num_active_ccds - 1] and ignores the rest.
num_active_ccds lives in the IOD block because the IOD die is the
aggregator that reports the CCD population, and keeping it there
preserves the 1:1 mapping with the firmware layout.

Driver enablement for protocol version 7 is added in a follow-up
patch.

Reviewed-by: Suma Hegde <suma.hegde@amd.com>
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 | 93 ++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
index 52e53ec5877f..fce271282348 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -592,6 +592,99 @@ struct hsmp_metric_table {
 	__u32 gfxclk_frequency[8];
 };
 
+#define HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD	32
+#define HSMP_F1A_M50_M5F_MAX_FREQ_TABLE_SIZE	4
+#define HSMP_F1A_M50_M5F_MAX_XGMI_LINKS	8
+#define HSMP_F1A_M50_M5F_MAX_PCIE_LINKS	8
+#define HSMP_F1A_M50_M5F_MAX_CCDS		8
+
+/* Metrics table (supported only with proto version 7) */
+struct hsmp_metric_table_zen6_iod {
+	__u32 num_active_ccds;	/* Number of valid entries in the sibling ccd[] array */
+	__u32 accumulation_counter;
+
+	/* TEMPERATURE */
+	__u64 max_socket_temperature_acc;
+
+	/* POWER */
+	__u32 socket_power_limit;
+	__u32 max_socket_power_limit;
+	__u64 socket_power_acc;
+	__u64 core_power_acc;
+	__u64 uncore_power_acc;
+
+	/* ENERGY */
+	__u64 timestamp;
+	__u64 socket_energy_acc;
+	__u64 core_energy_acc;
+	__u64 uncore_energy_acc;
+
+	/* FREQUENCY */
+	__u64 fclk_frequency_acc;
+	__u64 uclk_frequency_acc;
+	__u64 ddr_rate_acc;
+	__u64 lclk_frequency_acc[HSMP_F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+
+	/* FREQUENCY RANGE */
+	__u32 fclk_frequency_table[HSMP_F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+	__u32 uclk_frequency_table[HSMP_F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+	__u32 ddr_rate_table[HSMP_F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+	__u32 max_df_pstate_range;
+	__u32 min_df_pstate_range;
+	__u32 lclk_frequency_table[HSMP_F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+	__u32 max_lclk_dpm_range;
+	__u32 min_lclk_dpm_range;
+
+	/* XGMI */
+	__u64 xgmi_bit_rate[HSMP_F1A_M50_M5F_MAX_XGMI_LINKS];
+	__u64 xgmi_read_bandwidth[HSMP_F1A_M50_M5F_MAX_XGMI_LINKS];
+	__u64 xgmi_write_bandwidth[HSMP_F1A_M50_M5F_MAX_XGMI_LINKS];
+
+	/* ACTIVITY */
+	__u64 socket_c0_residency_acc;
+	__u64 socket_df_cstate_residency_acc;
+	__u64 dram_read_bandwidth_acc;
+	__u64 dram_write_bandwidth_acc;
+	__u32 max_dram_bandwidth;
+	__u64 pcie_bandwidth_acc[HSMP_F1A_M50_M5F_MAX_PCIE_LINKS];
+
+	/* THROTTLERS */
+	__u32 prochot_residency_acc;
+	__u32 ppt_residency_acc;
+	__u32 thm_residency_acc;
+	__u32 vrhot_residency_acc;
+	__u32 cpu_tdc_residency_acc;
+	__u32 soc_tdc_residency_acc;
+	__u32 io_mem_tdc_residency_acc;
+	__u32 fit_residency_acc;
+};
+
+struct hsmp_metric_table_zen6_ccd {
+	__u32 core_apicid_of_thread0[HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD];
+	__u64 core_c0[HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD];
+	__u64 core_cc1[HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD];
+	__u64 core_cc6[HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD];
+	__u64 core_frequency[HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD];
+	__u64 core_frequency_effective[HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD];
+	__u64 core_power[HSMP_F1A_M50_M5F_MAX_CORES_PER_CCD];
+};
+
+/*
+ * Metrics table for Family 0x1A, Models 0x50 to 0x5F, table version 0x00700000.
+ *
+ * The layout mirrors the SMU's binary metric table verbatim: one IOD block
+ * followed by HSMP_F1A_M50_M5F_MAX_CCDS fixed-size CCD blocks. Only the first
+ * iod.num_active_ccds entries in ccd[] carry meaningful data; the remaining
+ * entries are firmware padding and must be ignored by userspace.
+ *
+ * Future processors within the same family and model may support a
+ * variable number of CCDs and cores.
+ */
+struct hsmp_metric_table_zen6 {
+	struct hsmp_metric_table_zen6_iod iod;
+	struct hsmp_metric_table_zen6_ccd ccd[HSMP_F1A_M50_M5F_MAX_CCDS];
+};
+
 /* Reset to default packing */
 #pragma pack()
 
-- 
2.34.1

[PATCH v4 3/7] platform/x86/amd/hsmp: Unify response_sz validation to an upper-bound check

[Muralidhara M K]

As HSMP protocol versions evolve, existing message IDs sometimes
gain additional response words on newer firmware. validate_message()
currently enforces a strict equality (response_sz == table value)
for HSMP_SET and HSMP_GET, so userspace compiled against an earlier
descriptor table is rejected with -EINVAL when it asks for fewer
response words than the in-kernel table now declares - even though
that caller has no interest in the additional words. Only
HSMP_SET_GET already used a relaxed upper-bound check.

Replace the per-type branching with a single upper-bound check for
all message types. Userspace can now request fewer response words
than hardware provides, while requests that exceed the descriptor
table (and therefore the hardware capability) are still rejected.

Reviewed-by: Suma Hegde <suma.hegde@amd.com>
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>
---
 drivers/platform/x86/amd/hsmp/hsmp.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 631ffc0978d1..9bad58fef304 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -182,20 +182,15 @@ static int validate_message(struct hsmp_message *msg)
 		return -EINVAL;
 
 	/*
-	 * Some older HSMP SET messages are updated to add GET in the same message.
-	 * In these messages, GET returns the current value and SET also returns
-	 * the successfully set value. To support this GET and SET in same message
-	 * while maintaining backward compatibility for the HSMP users,
-	 * hsmp_msg_desc_table[] indicates only maximum allowed response_sz.
+	 * As the HSMP protocol evolves, newer platforms may define more
+	 * response arguments for existing messages.  Use an upper-bound
+	 * check so that older userspace callers requesting fewer response
+	 * words than what the current hsmp_msg_desc_table[] defines are
+	 * still accepted, while rejecting requests that exceed the
+	 * hardware capability.
 	 */
-	if (hsmp_msg_desc_table[msg->msg_id].type == HSMP_SET_GET) {
-		if (msg->response_sz > hsmp_msg_desc_table[msg->msg_id].response_sz)
-			return -EINVAL;
-	} else {
-		/* only HSMP_SET or HSMP_GET messages go through this strict check */
-		if (msg->response_sz != hsmp_msg_desc_table[msg->msg_id].response_sz)
-			return -EINVAL;
-	}
+	if (msg->response_sz > hsmp_msg_desc_table[msg->msg_id].response_sz)
+		return -EINVAL;
 	return 0;
 }
 
-- 
2.34.1

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

[Muralidhara M K]

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;
 };
 
 int hsmp_cache_proto_ver(u16 sock_ind);
-- 
2.34.1

[PATCH v4 5/7] platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table reads

[Muralidhara M K]

The metric table needs to be delivered to userspace as a single
atomic snapshot, but the current sysfs metrics_bin path is a file
read: userspace can read it in chunks and observe a torn snapshot
if an SMU refresh happens between read() calls. The same path is
also bounded by PAGE_SIZE, so the ~13 KB hsmp_metric_table_zen6
used on Family 1Ah Model 50h-5Fh cannot be returned at all,
regardless of how userspace reads it. Rather than extend sysfs to
lift both restrictions, expose the metric table through the
existing HSMP character device using a new ioctl that always copies
the full table in one shot.

Add struct hsmp_telemetry_data and HSMP_IOCTL_GET_TELEMETRY_DATA
to the UAPI header. Under the surrounding #pragma pack(4), placing
the __u64 user pointer first gives a tight 16-byte layout that is
identical for 32- and 64-bit callers, and the trailing __u16
reserved field is rejected with -EINVAL if non-zero so future
kernels can repurpose it without breaking already-deployed
userspace. Userspace sizes its buffer using the matching UAPI
metric-table struct for the running platform; sizes that disagree
with the firmware-reported table size are rejected so a short copy
can never silently truncate the snapshot.

Dispatch hsmp_ioctl() on the ioctl command: the existing message
handler is factored out as hsmp_ioctl_msg() for HSMP_IOCTL_CMD, and
HSMP_IOCTL_GET_TELEMETRY_DATA goes to a new
hsmp_ioctl_get_telemetry() helper.

The user-controlled indices reaching kernel arrays via these ioctl
paths are clamped with array_index_nospec() to mitigate Spectre v1
(CVE-2017-5753): req.sock_ind before indexing hsmp_pdev.sock[], and
msg.msg_id once at the entry to hsmp_ioctl_msg() so all six
downstream hsmp_msg_desc_table[] dereferences inherit the clamp
from a single mask.

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>
---
 Documentation/arch/x86/amd_hsmp.rst  |  30 ++++++-
 arch/x86/include/uapi/asm/amd_hsmp.h |  43 ++++++++++
 drivers/platform/x86/amd/hsmp/hsmp.c | 117 ++++++++++++++++++++++++++-
 3 files changed, 188 insertions(+), 2 deletions(-)

diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
index 8bb411f0d70d..74c259507888 100644
--- a/Documentation/arch/x86/amd_hsmp.rst
+++ b/Documentation/arch/x86/amd_hsmp.rst
@@ -68,6 +68,14 @@ under per socket sysfs directory created at
 
 Note: lseek() is not supported as entire metrics table is read.
 
+The sysfs metrics_bin path supports only HSMP protocol version 6 and,
+because it is a file read, can return a torn snapshot if userspace
+reads in pieces.  Protocol version 7 metric tables
+(``struct hsmp_metric_table_zen6``, ~13 KB) also exceed PAGE_SIZE, so
+a read returns ``-EOPNOTSUPP``.  For atomic reads on any protocol
+version, use the ``HSMP_IOCTL_GET_TELEMETRY_DATA`` ioctl on /dev/hsmp
+(see below).
+
 Metrics table definitions will be documented as part of Public PPR.
 The same is defined in the amd_hsmp.h header.
 
@@ -167,7 +175,7 @@ Next thing, open the device file, as follows::
     exit(1);
   }
 
-The following IOCTL is defined:
+The following IOCTLs are defined:
 
 ``ioctl(file, HSMP_IOCTL_CMD, struct hsmp_message *msg)``
   The argument is a pointer to a::
@@ -180,6 +188,26 @@ The following IOCTL is defined:
     	__u16	sock_ind;			/* socket number */
     };
 
+``ioctl(file, HSMP_IOCTL_GET_TELEMETRY_DATA, struct hsmp_telemetry_data *req)``
+  Atomically fetch the firmware metric (telemetry) table for a socket.
+  The ioctl copies the table in one shot, so unlike the metrics_bin
+  sysfs path it cannot return a torn snapshot and is not bounded by
+  PAGE_SIZE.  Required for HSMP protocol version 7+ (e.g. Family 1Ah
+  Model 50h-5Fh, ~13 KB ``struct hsmp_metric_table_zen6``).  Argument::
+
+    struct hsmp_telemetry_data {
+    	__u64 buf; /* User pointer to destination buffer */
+    	__u32 size; /* Size of @buf, must equal firmware-reported table size */
+    	__u16 sock_ind; /* Socket index */
+    	__u16 reserved; /* Reserved, must be zero */
+    };
+
+  Userspace picks the layout matching the running protocol version
+  (``struct hsmp_metric_table`` for v6, ``struct hsmp_metric_table_zen6``
+  for v7, queried via the ``protocol_version`` sysfs attribute) and
+  uses ``sizeof()`` of that struct for ``size``.  The kernel rejects
+  a mismatched ``size`` or non-zero ``reserved`` with ``-EINVAL``.
+
 The ioctl would return a non-zero on failure; you can read errno to see
 what happened. The transaction returns 0 on success.
 
diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
index 438ac38d0dc8..50b28ccc7597 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -686,6 +686,40 @@ struct hsmp_metric_table_zen6 {
 	struct hsmp_metric_table_zen6_ccd ccd[HSMP_F1A_M50_M5F_MAX_CCDS];
 };
 
+/**
+ * struct hsmp_telemetry_data - Request descriptor for HSMP telemetry IOCTL
+ * @buf:       Input. Userspace pointer (encoded as __u64 to keep the layout
+ *             stable between 32-bit and 64-bit callers) to the destination
+ *             buffer that receives the metric table.
+ * @size:      Input. Size in bytes of the buffer pointed to by @buf.  Must
+ *             match the firmware-reported metric table size for the running
+ *             HSMP protocol version (see below); any other value results in
+ *             -EINVAL.  The kernel does not write this field back.
+ * @sock_ind:  Input. Socket index from which the metric table is read.
+ * @reserved:  Reserved for future use.  Callers should set this to zero;
+ *             future kernels may begin interpreting the field, so passing
+ *             a non-zero value today is not forwards compatible.
+ *
+ * Placing @buf first lets all fields fall on their natural alignment under
+ * the surrounding #pragma pack(4), so the struct is a tight 16 bytes with
+ * the same wire layout on 32-bit and 64-bit userspace.
+ *
+ * The exact metric table layout depends on the HSMP protocol version reported
+ * by the firmware:
+ *   - Protocol version 6 -> struct hsmp_metric_table
+ *   - Protocol version 7 -> struct hsmp_metric_table_zen6
+ *
+ * Userspace queries the protocol version (e.g. via the protocol_version sysfs
+ * attribute) and uses sizeof() on the matching UAPI structure for both @size
+ * and the allocation backing @buf.
+ */
+struct hsmp_telemetry_data {
+	__u64	buf;
+	__u32	size;
+	__u16	sock_ind;
+	__u16	reserved;
+};
+
 /* Reset to default packing */
 #pragma pack()
 
@@ -693,4 +727,13 @@ struct hsmp_metric_table_zen6 {
 #define HSMP_BASE_IOCTL_NR	0xF8
 #define HSMP_IOCTL_CMD		_IOWR(HSMP_BASE_IOCTL_NR, 0, struct hsmp_message)
 
+/*
+ * Fetch the firmware metric (telemetry) table for a given socket via the
+ * HSMP character device.  This avoids the PAGE_SIZE limitation of the
+ * sysfs binary attribute path for tables larger than one page (such as the
+ * ~13 KB hsmp_metric_table_zen6 used on Family 1Ah Model 50h-5Fh).
+ */
+#define HSMP_IOCTL_GET_TELEMETRY_DATA \
+	_IOWR(HSMP_BASE_IOCTL_NR, 1, struct hsmp_telemetry_data)
+
 #endif /*_ASM_X86_AMD_HSMP_H_*/
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index cf9392f99298..67f0074bb532 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -12,8 +12,11 @@
 #include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/nospec.h>
 #include <linux/semaphore.h>
+#include <linux/slab.h>
 #include <linux/sysfs.h>
+#include <linux/uaccess.h>
 
 #include "hsmp.h"
 
@@ -287,7 +290,7 @@ static bool is_get_msg(struct hsmp_message *msg)
 	return false;
 }
 
-long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+static long hsmp_ioctl_msg(struct file *fp, unsigned long arg)
 {
 	int __user *arguser = (int  __user *)arg;
 	struct hsmp_message msg = { 0 };
@@ -303,6 +306,19 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
 	if (msg.msg_id < HSMP_TEST || msg.msg_id >= HSMP_MSG_ID_MAX)
 		return -ENOMSG;
 
+	/*
+	 * Sanitize the user-controlled msg_id against speculative
+	 * execution.  The bounds check above retires the out-of-range
+	 * case with -ENOMSG, but a mispredicted branch can still let the
+	 * CPU speculatively use msg_id as an index into
+	 * hsmp_msg_desc_table[] (here and in validate_message() /
+	 * is_get_msg() called downstream via hsmp_send_message()), and
+	 * pull arbitrary kernel memory into the cache (Spectre v1,
+	 * CVE-2017-5753).  Clamp once into msg.msg_id so every downstream
+	 * dereference sees the sanitized value.
+	 */
+	msg.msg_id = array_index_nospec(msg.msg_id, HSMP_MSG_ID_MAX);
+
 	switch (fp->f_mode & (FMODE_WRITE | FMODE_READ)) {
 	case FMODE_WRITE:
 		/*
@@ -343,6 +359,105 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
 	return 0;
 }
 
+/*
+ * Fetch the firmware metric (telemetry) table for the requested socket and
+ * copy it to the userspace buffer described by the request.
+ *
+ * The metric table size is variable across HSMP protocol versions and on
+ * Family 1Ah Model 50h-5Fh exceeds PAGE_SIZE.  Userspace must therefore
+ * supply a buffer at least the firmware-reported size in bytes.
+ */
+static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
+{
+	void __user *arguser = (void __user *)arg;
+	struct hsmp_telemetry_data req;
+	struct hsmp_socket *sock;
+	void __user *user_buf;
+	size_t tbl_size;
+	unsigned int sock_ind;
+	void *kbuf;
+	int ret;
+
+	/* Telemetry data is read-only; require read access on the fd. */
+	if (!(fp->f_mode & FMODE_READ))
+		return -EPERM;
+
+	if (copy_from_user(&req, arguser, sizeof(req)))
+		return -EFAULT;
+
+	/*
+	 * Reserved fields must be zero so future kernels can safely
+	 * repurpose them without breaking already-deployed userspace.
+	 */
+	if (req.reserved)
+		return -EINVAL;
+
+	if (!hsmp_pdev.sock || req.sock_ind >= hsmp_pdev.num_sockets)
+		return -ENODEV;
+
+	tbl_size = hsmp_pdev.hsmp_table_size;
+	if (!tbl_size)
+		return -ENODEV;
+
+	/*
+	 * Userspace must size its buffer using the appropriate UAPI metric
+	 * table struct for the running protocol version.  Reject mismatched
+	 * sizes so we never silently truncate or short-write.
+	 */
+	if (req.size != tbl_size)
+		return -EINVAL;
+
+	/*
+	 * Sanitize the user-controlled socket index against speculative
+	 * execution.  The bounds check above retires the out-of-range case
+	 * with -ENODEV, but a mispredicted branch can still let the CPU
+	 * speculatively use sock_ind as an index into hsmp_pdev.sock[] and
+	 * pull arbitrary kernel memory into the cache (Spectre v1, CVE-2017-
+	 * 5753).  array_index_nospec() turns the bounds check into a
+	 * data-flow clamp so the speculative load is in-range too.
+	 */
+	sock_ind = array_index_nospec(req.sock_ind, hsmp_pdev.num_sockets);
+	sock = &hsmp_pdev.sock[sock_ind];
+	if (!sock->metric_tbl_addr)
+		return -ENODEV;
+
+	user_buf = u64_to_user_ptr(req.buf);
+
+	/*
+	 * The bounce buffer is overwritten in full by memcpy_fromio() inside
+	 * hsmp_metric_tbl_read(); use kvmalloc() to avoid the zeroing cost of
+	 * kvzalloc() on the ~13 KB allocation done on every ioctl call.
+	 */
+	kbuf = kvmalloc(tbl_size, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	ret = hsmp_metric_tbl_read(sock, kbuf, tbl_size);
+	if (ret < 0)
+		goto out;
+
+	if (copy_to_user(user_buf, kbuf, tbl_size))
+		ret = -EFAULT;
+	else
+		ret = 0;
+
+out:
+	kvfree(kbuf);
+	return ret;
+}
+
+long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+	switch (cmd) {
+	case HSMP_IOCTL_CMD:
+		return hsmp_ioctl_msg(fp, arg);
+	case HSMP_IOCTL_GET_TELEMETRY_DATA:
+		return hsmp_ioctl_get_telemetry(fp, arg);
+	default:
+		return -ENOTTY;
+	}
+}
+
 ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
 {
 	struct hsmp_message msg = { 0 };
-- 
2.34.1

[PATCH v4 6/7] platform/x86/amd/hsmp: Enable HSMP_PROTO_VER7 metric tables on the ACPI driver via the IOCTL

[Muralidhara M K]

The ACPI driver currently prepares the per-socket metric table only
on HSMP_PROTO_VER6. With protocol version 7 in use on Family 1Ah
Model 50h-5Fh, userspace cannot reach the new ~13 KB
hsmp_metric_table_zen6: hsmp_get_tbl_dram_base() is skipped,
sock->metric_tbl_addr stays NULL, and the ioctl added earlier in
this series has nothing to read.

Widen the proto_ver gate in init_acpi() from '== HSMP_PROTO_VER6'
to '>= HSMP_PROTO_VER6' so the DRAM region is mapped and
hsmp_pdev.hsmp_table_size is populated on protocol version 7 (and
any future compatible version), making the ioctl path functional.

hsmp_metric_tbl_acpi_read() now returns -EOPNOTSUPP whenever the
running protocol version is not VER6. v7 userspace gets a clear,
actionable error and a documented pointer to
HSMP_IOCTL_GET_TELEMETRY_DATA; v6 userspace sees no change.

The non-ACPI plat.c path is intentionally left untouched: it covers
Family 1Ah Model 0h-Fh hardware fixed at protocol version 6, where
the existing metrics_bin remains the supported interface.

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>
---
 drivers/platform/x86/amd/hsmp/acpi.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 97ed71593bdf..8dc6b4a8bd27 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -238,13 +238,32 @@ static ssize_t hsmp_metric_tbl_acpi_read(struct file *filp, struct kobject *kobj
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct hsmp_socket *sock = dev_get_drvdata(dev);
 
+	/*
+	 * metrics_bin is a sysfs binary attribute and is capped at PAGE_SIZE.
+	 * It can therefore only carry the protocol version 6 metric table
+	 * (struct hsmp_metric_table).  Larger tables (HSMP_PROTO_VER7+, e.g.
+	 * struct hsmp_metric_table_zen6 at ~13 KB used on Family 1Ah Model
+	 * 50h-5Fh) do not fit; userspace on those systems must read the
+	 * snapshot through HSMP_IOCTL_GET_TELEMETRY_DATA on /dev/hsmp.
+	 * Surface the unsupported case here as -EOPNOTSUPP rather than
+	 * silently truncating the snapshot.
+	 */
+	if (hsmp_pdev->proto_ver != HSMP_PROTO_VER6)
+		return -EOPNOTSUPP;
+
 	return hsmp_metric_tbl_read(sock, buf, count);
 }
 
 static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
 					 const struct bin_attribute *battr, int id)
 {
-	if (hsmp_pdev->proto_ver == HSMP_PROTO_VER6)
+	/*
+	 * Keep metrics_bin visible for HSMP_PROTO_VER7+ as well, 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.
+	 */
+	if (hsmp_pdev->proto_ver >= HSMP_PROTO_VER6)
 		return battr->attr.mode;
 
 	return 0;
@@ -491,7 +510,7 @@ static int init_acpi(struct device *dev)
 		return ret;
 	}
 
-	if (hsmp_pdev->proto_ver == HSMP_PROTO_VER6) {
+	if (hsmp_pdev->proto_ver >= HSMP_PROTO_VER6) {
 		ret = hsmp_get_tbl_dram_base(sock_ind);
 		if (ret)
 			dev_info(dev, "Failed to init metric table\n");
-- 
2.34.1

[PATCH v4 7/7] platform/x86/amd/hsmp: Make metric table read locking use guard(mutex)

[Muralidhara M K]

hsmp_metric_tbl_read() refreshes the SMU-side metric table and then
memcpy_fromio()'s the result. Without serialization, two parallel
readers can interleave the refresh and the copy and the caller
observes a torn (mixed old/new) snapshot. Add a per-socket
metric_tbl_lock so the refresh-and-copy sequence is atomic from
userspace's point of view.

Use scoped guard(mutex) so the lock is released on every return
path without hand-written goto chains, and initialize the mutex
with devm_mutex_init() so no explicit mutex_destroy() cleanup is
required.

Initialize the mutex before devm_ioremap() so the invariant
"sock->metric_tbl_addr != NULL implies metric_tbl_lock is usable"
holds on every error exit. Both callers of hsmp_get_tbl_dram_base()
(init_acpi() and init_platform_device()) intentionally only log a
failure and continue probing, so initializing the mutex after a
successful ioremap would leave sock->metric_tbl_addr populated with
an uninitialized lock, and the next hsmp_metric_tbl_read() would
take guard(mutex)() on garbage memory. With the order swapped, a
devm_mutex_init() failure returns early before metric_tbl_addr is
ever set, and the existing NULL check in hsmp_metric_tbl_read()
keeps rejecting the read with -ENOMEM as before.

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>
---
 drivers/platform/x86/amd/hsmp/hsmp.c | 20 ++++++++++++++++++++
 drivers/platform/x86/amd/hsmp/hsmp.h |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 67f0074bb532..fda57225939c 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -479,6 +479,7 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
 	msg.msg_id	= HSMP_GET_METRIC_TABLE;
 	msg.sock_ind	= sock->sock_ind;
 
+	guard(mutex)(&sock->metric_tbl_lock);
 	ret = hsmp_send_message(&msg);
 	if (ret)
 		return ret;
@@ -495,6 +496,24 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
 	phys_addr_t dram_addr;
 	int ret;
 
+	/*
+	 * Initialize the per-socket lock before anything that can set
+	 * sock->metric_tbl_addr to a non-NULL value.  hsmp_metric_tbl_read()
+	 * gates on sock->metric_tbl_addr being non-NULL and then takes
+	 * metric_tbl_lock unconditionally; both callers of this function
+	 * (init_acpi() and init_platform_device()) intentionally only log
+	 * a failure here and continue probing, so an init order that left
+	 * metric_tbl_addr populated while devm_mutex_init() failed would
+	 * leave the read path locking an uninitialized mutex.  Doing the
+	 * mutex init first preserves the invariant "metric_tbl_addr !=
+	 * NULL implies the lock is usable" on every error exit.
+	 */
+	ret = devm_mutex_init(sock->dev, &sock->metric_tbl_lock);
+	if (ret) {
+		dev_err(sock->dev, "Failed to initialize metric table lock\n");
+		return ret;
+	}
+
 	msg.sock_ind	= sock_ind;
 	msg.response_sz	= hsmp_msg_desc_table[HSMP_GET_METRIC_TABLE_DRAM_ADDR].response_sz;
 	msg.msg_id	= HSMP_GET_METRIC_TABLE_DRAM_ADDR;
@@ -524,6 +543,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
 		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
 		return -ENOMEM;
 	}
+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(hsmp_get_tbl_dram_base, "AMD_HSMP");
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index e7f051475728..f7b1cbf19932 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -15,6 +15,7 @@
 #include <linux/hwmon.h>
 #include <linux/kconfig.h>
 #include <linux/miscdevice.h>
+#include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/semaphore.h>
 #include <linux/sysfs.h>
@@ -41,6 +42,8 @@ struct hsmp_socket {
 	struct bin_attribute hsmp_attr;
 	struct hsmp_mbaddr_info mbinfo;
 	void __iomem *metric_tbl_addr;
+	/* Serializes concurrent metric table refreshes from the sysfs path */
+	struct mutex metric_tbl_lock;
 	void __iomem *virt_base_addr;
 	struct semaphore hsmp_sem;
 	char name[HSMP_ATTR_GRP_NAME_SIZE];
-- 
2.34.1

RE: [PATCH v4 0/7] platform/x86/amd/hsmp: Support Family 1Ah Model 50h-5Fh telemetry

[M K, Muralidhara]

AMD General

Hi Ilpo,
I have addressed all the comments, Could you review this series.

-----Original Message-----
From: M K, Muralidhara <Muralidhara.MK@amd.com>
Sent: Thursday, May 28, 2026 3:10 PM
To: ilpo.jarvinen@linux.intel.com
Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org; M K, Muralidhara <Muralidhara.MK@amd.com>
Subject: [PATCH v4 0/7] platform/x86/amd/hsmp: Support Family 1Ah Model 50h-5Fh telemetry

Hi,

This series adds HSMP protocol version 7 support to the AMD HSMP driver for Family 1Ah Model 50h-5Fh systems, and addresses a small set of related issues that surfaced while wiring up the new platform.

The bulk of the work is exposing the new firmware-defined metric table layout to userspace. HSMP_PROTO_VER7 firmware delivers a per-IOD / per-CCD blob (struct hsmp_metric_table_zen6, ~13 KB) that does not fit the existing per-socket struct hsmp_metric_table and does not fit the existing sysfs metrics_bin transport at all, since binary sysfs attributes are bounded by PAGE_SIZE. metrics_bin also has a long- standing tearing problem: userspace can read it in multiple chunks and observe an inconsistent snapshot if SMU refreshes the table between
read() calls.

To address both, the series:

  - Adds a stable UAPI description of the v7 metric-table layout
    (struct hsmp_metric_table_zen6{,_iod,_ccd}) that mirrors the
    firmware blob 1:1.

  - Stops hard-coding the metric-table region size in the driver and
    sources it from firmware (HSMP_GET_METRIC_TABLE_DRAM_ADDR args[2]),
    falling back to the old size on v6 firmware that leaves the field
    zero. The same size is used for ioremap() and for the ioctl
    bounds check.

  - Adds a new HSMP_IOCTL_GET_TELEMETRY_DATA on the existing HSMP
    character device that always copies the full firmware-reported
    table in one shot, removing both the PAGE_SIZE cap and the
    chunked-read tearing window. The trailing reserved field in the
    request is rejected if non-zero so future kernels can repurpose
    it without breaking deployed userspace, and user-supplied indices
    (sock_ind, msg_id) are clamped with array_index_nospec() to
    mitigate Spectre v1.

  - Wires the ACPI driver to populate the DRAM mapping for v7
    (currently gated on '== HSMP_PROTO_VER6'), so the new ioctl path
    is actually reachable on Family 1Ah Model 50h-5Fh hardware. The
    legacy metrics_bin attribute is preserved and continues to work
    on v6; on v7 it returns -EOPNOTSUPP pointing at the ioctl.

Around that, the series also:

  - Adds the new HSMP message IDs (0x29-0x2A, 0x33-0x3A) introduced by
    Family 1Ah Model 50h-5Fh firmware (PC6/CC6 control, CCD power /
    thermal monitoring, DIMM sideband access, floor- and SDPS-limit
    control, command-enable discovery) and converts three SET-only
    messages (HSMP_SET_XGMI_LINK_WIDTH, HSMP_SET_DF_PSTATE,
    HSMP_SET_PSTATE_MAX_MIN) to HSMP_SET_GET so userspace can read
    back the currently programmed value via bit[31] of args[0].

  - Relaxes validate_message() to an upper-bound check for every
    message type (HSMP_SET_GET already did this). Existing userspace
    that asks for fewer response words than firmware now provides is
    no longer rejected with -EINVAL.

  - Tightens metric-table read locking with a per-socket guard(mutex)
    around the SMU-side refresh + memcpy_fromio() sequence, and
    reorders devm_mutex_init() before devm_ioremap() so the
    "metric_tbl_addr != NULL implies metric_tbl_lock is initialized"
    invariant holds on every probe path.

Documentation/arch/x86/amd_hsmp.rst is updated to describe the metrics_bin / IOCTL_GET_TELEMETRY_DATA split and the protocol-version gating.

The series is bisect-safe: every commit was built individually and checkpatch.pl --strict is clean across the series.

v3:
https://lore.kernel.org/platform-driver-x86/20260517151211.415627-1-muralidhara.mk@amd.com/T/#t

Changes since v3:

  - 1/7 (new HSMP messages): Tightened the inline bitfield comments
    on HSMP_SET_POWER_MODE / HSMP_PC6_ENABLE / HSMP_CC6_ENABLE /
    HSMP_DIMM_SB_RD / HSMP_DIMM_SB_WR / HSMP_FLOOR_LIMIT /
    HSMP_SDPS_LIMIT. Rewrote the commit log to spell out the
    backward-compatibility analysis for bit[31] of args[0] on
    the three SET->SET_GET conversions.

  - 2/7 (UAPI structs): Added per-field comments explaining the
    num_active_ccds placement (in the IOD block, matching the
    firmware-side aggregator) and the ccd[] sizing
    (HSMP_F1A_M50_M5F_MAX_CCDS = 8, hardware maximum, userspace
    iterates [0 .. num_active_ccds - 1]).

  - 3/7 (response_sz): Rewrote the commit log to motivate the change
    in terms of forward compatibility of older userspace against
    newer firmware/descriptor tables.

  - 4/7 (firmware-reported table size): Rewrote the commit log to
    document args[2] semantics and the 0-fallback path that keeps
    behaviour identical on v6 firmware.

  - 5/7 (HSMP_IOCTL_GET_TELEMETRY_DATA):
      * Added array_index_nospec() on req.sock_ind and msg.msg_id to
        mitigate Spectre v1 on the user-controlled ioctl indices.
      * Rejected non-zero req.reserved with -EINVAL so the field
        stays repurposable.
      * Tightened the UAPI struct layout reasoning under the
        surrounding #pragma pack(4) and the size-mismatch handling.
      * Trimmed the long implementation-detail paragraphs from the
        commit log.

  - 6/7 (enable v7 on the ACPI driver):
      * Restored the legacy sysfs metrics_bin attribute (it was
        dropped in v3) and gated reads by protocol version so v6
        userspace is unaffected and v7 userspace gets -EOPNOTSUPP
        pointing at HSMP_IOCTL_GET_TELEMETRY_DATA.
      * Removed the ABI-break paragraph from the commit log now that
        metrics_bin is preserved.

  - 7/7 (guard(mutex)):
      * Reordered devm_mutex_init() before devm_ioremap() so that
        sock->metric_tbl_addr is never published with an
        uninitialized metric_tbl_lock on the non-fatal init_acpi() /
        init_platform_device() error paths.
      * Rewrote the commit log to document that ordering invariant.

  - Documentation/arch/x86/amd_hsmp.rst: Documented the metrics_bin
    PAGE_SIZE / protocol-version constraints and the new
    HSMP_IOCTL_GET_TELEMETRY_DATA path, and cleaned up whitespace in
    the embedded struct snippet.

Thanks,
Murali

Muralidhara M K (7):
  platform/x86/amd/hsmp: Add new HSMP messages for Family 1Ah, Model
    50h-5Fh
  platform/x86/amd/hsmp: Add UAPI structures for Family 1Ah Model
    50h-5Fh metrics table
  platform/x86/amd/hsmp: Unify response_sz validation to an upper-bound
    check
  platform/x86/amd/hsmp: Source metric-table size from firmware
  platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table
    reads
  platform/x86/amd/hsmp: Enable HSMP_PROTO_VER7 metric tables on the
    ACPI driver via the IOCTL
  platform/x86/amd/hsmp: Make metric table read locking use guard(mutex)

 Documentation/arch/x86/amd_hsmp.rst  |  30 ++-  arch/x86/include/uapi/asm/amd_hsmp.h | 294 +++++++++++++++++++++++++--  drivers/platform/x86/amd/hsmp/acpi.c |  23 ++-  drivers/platform/x86/amd/hsmp/hsmp.c | 169 +++++++++++++--
 drivers/platform/x86/amd/hsmp/hsmp.h |   6 +-
 5 files changed, 485 insertions(+), 37 deletions(-)

--
2.34.1

Re: [RFC PATCH 1/8] platform/x86/amd/hsmp: Add new HSMP messages for Family 1Ah, Model 50h-5Fh

[Qinyun Tan]

On Mon, 19 Jan 2026, Muralidhara M K wrote:
> The same firmware also extends three existing SET-only messages
> (HSMP_SET_XGMI_LINK_WIDTH 0x0C, HSMP_SET_DF_PSTATE 0x0D,
> HSMP_SET_PSTATE_MAX_MIN 0x22) with a read-back path selected by
> bit[31] of args[0] (0 = set, 1 = get). Without converting these
> entries to HSMP_SET_GET, userspace cannot query the currently
> programmed value even though firmware now supports it.

I noticed that HSMP_SET_XGMI_PSTATE_RANGE (0x26) is not converted
here. The esmi_ib_library already implements a GET path for it in
esmi_xgmi_pstate_range_get() using the same bit[31] convention:

  msg.msg_id    = HSMP_SET_XGMI_PSTATE_RANGE;
  msg.args[0]   = BIT(31);
  ret = hsmp_xfer(&msg, O_RDONLY);

Since the kernel descriptor is still {1, 0, HSMP_SET}, hsmp_ioctl()
would reject the O_RDONLY open with -EPERM because is_get_msg() only
recognises HSMP_GET and HSMP_SET_GET types.

>  	/*
> -	 * HSMP_SET_XGMI_LINK_WIDTH, num_args = 1, response_sz = 0
> +	 * HSMP_SET_XGMI_LINK_WIDTH, num_args = 1, response_sz = 0/1
> +	 * input: args[0] = set/get XGMI Link width[31] (0 = set, 1 = get) +
[...]
> -	{1, 0, HSMP_SET},
> +	{1, 1, HSMP_SET_GET},

Does HSMP_SET_XGMI_PSTATE_RANGE also need the same conversion?

Thanks,
Qinyun

Re: [RFC PATCH 1/8] platform/x86/amd/hsmp: Add new HSMP messages for Family 1Ah, Model 50h-5Fh

[M K, Muralidhara]

On 6/9/2026 1:44 PM, Qinyun Tan wrote:
> [Some people who received this message don't often get email from qinyuntan@linux.alibaba.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Mon, 19 Jan 2026, Muralidhara M K wrote:
>> The same firmware also extends three existing SET-only messages
>> (HSMP_SET_XGMI_LINK_WIDTH 0x0C, HSMP_SET_DF_PSTATE 0x0D,
>> HSMP_SET_PSTATE_MAX_MIN 0x22) with a read-back path selected by
>> bit[31] of args[0] (0 = set, 1 = get). Without converting these
>> entries to HSMP_SET_GET, userspace cannot query the currently
>> programmed value even though firmware now supports it.
> 
> I noticed that HSMP_SET_XGMI_PSTATE_RANGE (0x26) is not converted
> here. The esmi_ib_library already implements a GET path for it in
> esmi_xgmi_pstate_range_get() using the same bit[31] convention:
> 
>    msg.msg_id    = HSMP_SET_XGMI_PSTATE_RANGE;
>    msg.args[0]   = BIT(31);
>    ret = hsmp_xfer(&msg, O_RDONLY);
> 
> Since the kernel descriptor is still {1, 0, HSMP_SET}, hsmp_ioctl()
> would reject the O_RDONLY open with -EPERM because is_get_msg() only
> recognises HSMP_GET and HSMP_SET_GET types.
> 
>>        /*
>> -      * HSMP_SET_XGMI_LINK_WIDTH, num_args = 1, response_sz = 0
>> +      * HSMP_SET_XGMI_LINK_WIDTH, num_args = 1, response_sz = 0/1
>> +      * input: args[0] = set/get XGMI Link width[31] (0 = set, 1 = get) +
> [...]
>> -     {1, 0, HSMP_SET},
>> +     {1, 1, HSMP_SET_GET},
> 
> Does HSMP_SET_XGMI_PSTATE_RANGE also need the same conversion?
> 
Hi Qinyun Tan,

This command is not supported on the Venice platform (Family 1A Models 
50h-5Fh). Please Refer 
https://docs.amd.com/v/u/en-US/58264_B0_Venice_PPR_026
The modifications need to be made in the esmi_ib_library, not in this patch.


> Thanks,
> Qinyun

Re: [RFC PATCH 1/8] platform/x86/amd/hsmp: Add new HSMP messages for Family 1Ah, Model 50h-5Fh

[qinyuntan]

On 6/9/26 6:06 PM, M K, Muralidhara wrote:
> 
> This command is not supported on the Venice platform (Family 1A Models 
> 50h-5Fh). Please Refer https://docs.amd.com/v/u/en- 
> US/58264_B0_Venice_PPR_026
> The modifications need to be made in the esmi_ib_library, not in this 
> patch.

Hi, Muralidhara:

Thank you for the prompt clarification and the PPR reference. That
makes sense — the kernel descriptor for 0x26 is correct as-is.

Thanks again,
Qinyun

Re: [PATCH v4 5/7] platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table reads

[Ilpo Järvinen]

On Thu, 28 May 2026, Muralidhara M K wrote:

> The metric table needs to be delivered to userspace as a single
> atomic snapshot, but the current sysfs metrics_bin path is a file
> read: userspace can read it in chunks and observe a torn snapshot
> if an SMU refresh happens between read() calls. The same path is
> also bounded by PAGE_SIZE, so the ~13 KB hsmp_metric_table_zen6
> used on Family 1Ah Model 50h-5Fh cannot be returned at all,
> regardless of how userspace reads it. Rather than extend sysfs to
> lift both restrictions, expose the metric table through the
> existing HSMP character device using a new ioctl that always copies
> the full table in one shot.
> 
> Add struct hsmp_telemetry_data and HSMP_IOCTL_GET_TELEMETRY_DATA
> to the UAPI header. Under the surrounding #pragma pack(4), placing
> the __u64 user pointer first gives a tight 16-byte layout that is
> identical for 32- and 64-bit callers, and the trailing __u16
> reserved field is rejected with -EINVAL if non-zero so future
> kernels can repurpose it without breaking already-deployed
> userspace. Userspace sizes its buffer using the matching UAPI
> metric-table struct for the running platform; sizes that disagree
> with the firmware-reported table size are rejected so a short copy
> can never silently truncate the snapshot.
> 
> Dispatch hsmp_ioctl() on the ioctl command: the existing message
> handler is factored out as hsmp_ioctl_msg() for HSMP_IOCTL_CMD, and
> HSMP_IOCTL_GET_TELEMETRY_DATA goes to a new
> hsmp_ioctl_get_telemetry() helper.
> 
> The user-controlled indices reaching kernel arrays via these ioctl
> paths are clamped with array_index_nospec() to mitigate Spectre v1
> (CVE-2017-5753): req.sock_ind before indexing hsmp_pdev.sock[], and
> msg.msg_id once at the entry to hsmp_ioctl_msg() so all six
> downstream hsmp_msg_desc_table[] dereferences inherit the clamp
> from a single mask.
> 
> 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>
> ---
>  Documentation/arch/x86/amd_hsmp.rst  |  30 ++++++-
>  arch/x86/include/uapi/asm/amd_hsmp.h |  43 ++++++++++
>  drivers/platform/x86/amd/hsmp/hsmp.c | 117 ++++++++++++++++++++++++++-
>  3 files changed, 188 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
> index 8bb411f0d70d..74c259507888 100644
> --- a/Documentation/arch/x86/amd_hsmp.rst
> +++ b/Documentation/arch/x86/amd_hsmp.rst
> @@ -68,6 +68,14 @@ under per socket sysfs directory created at
>  
>  Note: lseek() is not supported as entire metrics table is read.
>  
> +The sysfs metrics_bin path supports only HSMP protocol version 6 and,
> +because it is a file read, can return a torn snapshot if userspace
> +reads in pieces.  Protocol version 7 metric tables
> +(``struct hsmp_metric_table_zen6``, ~13 KB) also exceed PAGE_SIZE, so
> +a read returns ``-EOPNOTSUPP``.  For atomic reads on any protocol
> +version, use the ``HSMP_IOCTL_GET_TELEMETRY_DATA`` ioctl on /dev/hsmp
> +(see below).
> +
>  Metrics table definitions will be documented as part of Public PPR.
>  The same is defined in the amd_hsmp.h header.
>  
> @@ -167,7 +175,7 @@ Next thing, open the device file, as follows::
>      exit(1);
>    }
>  
> -The following IOCTL is defined:
> +The following IOCTLs are defined:
>  
>  ``ioctl(file, HSMP_IOCTL_CMD, struct hsmp_message *msg)``
>    The argument is a pointer to a::
> @@ -180,6 +188,26 @@ The following IOCTL is defined:
>      	__u16	sock_ind;			/* socket number */
>      };
>  
> +``ioctl(file, HSMP_IOCTL_GET_TELEMETRY_DATA, struct hsmp_telemetry_data *req)``
> +  Atomically fetch the firmware metric (telemetry) table for a socket.
> +  The ioctl copies the table in one shot, so unlike the metrics_bin
> +  sysfs path it cannot return a torn snapshot and is not bounded by
> +  PAGE_SIZE.  Required for HSMP protocol version 7+ (e.g. Family 1Ah
> +  Model 50h-5Fh, ~13 KB ``struct hsmp_metric_table_zen6``).  Argument::
> +
> +    struct hsmp_telemetry_data {
> +    	__u64 buf; /* User pointer to destination buffer */
> +    	__u32 size; /* Size of @buf, must equal firmware-reported table size */
> +    	__u16 sock_ind; /* Socket index */
> +    	__u16 reserved; /* Reserved, must be zero */
> +    };
> +
> +  Userspace picks the layout matching the running protocol version
> +  (``struct hsmp_metric_table`` for v6, ``struct hsmp_metric_table_zen6``
> +  for v7, queried via the ``protocol_version`` sysfs attribute) and
> +  uses ``sizeof()`` of that struct for ``size``.  The kernel rejects
> +  a mismatched ``size`` or non-zero ``reserved`` with ``-EINVAL``.
> +
>  The ioctl would return a non-zero on failure; you can read errno to see
>  what happened. The transaction returns 0 on success.
>  
> diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
> index 438ac38d0dc8..50b28ccc7597 100644
> --- a/arch/x86/include/uapi/asm/amd_hsmp.h
> +++ b/arch/x86/include/uapi/asm/amd_hsmp.h
> @@ -686,6 +686,40 @@ struct hsmp_metric_table_zen6 {
>  	struct hsmp_metric_table_zen6_ccd ccd[HSMP_F1A_M50_M5F_MAX_CCDS];
>  };
>  
> +/**
> + * struct hsmp_telemetry_data - Request descriptor for HSMP telemetry IOCTL
> + * @buf:       Input. Userspace pointer (encoded as __u64 to keep the layout
> + *             stable between 32-bit and 64-bit callers) to the destination
> + *             buffer that receives the metric table.
> + * @size:      Input. Size in bytes of the buffer pointed to by @buf.  Must
> + *             match the firmware-reported metric table size for the running
> + *             HSMP protocol version (see below); any other value results in
> + *             -EINVAL.  The kernel does not write this field back.
> + * @sock_ind:  Input. Socket index from which the metric table is read.
> + * @reserved:  Reserved for future use.  Callers should set this to zero;
> + *             future kernels may begin interpreting the field, so passing
> + *             a non-zero value today is not forwards compatible.
> + *
> + * Placing @buf first lets all fields fall on their natural alignment under
> + * the surrounding #pragma pack(4), so the struct is a tight 16 bytes with
> + * the same wire layout on 32-bit and 64-bit userspace.
> + *
> + * The exact metric table layout depends on the HSMP protocol version reported
> + * by the firmware:
> + *   - Protocol version 6 -> struct hsmp_metric_table
> + *   - Protocol version 7 -> struct hsmp_metric_table_zen6
> + *
> + * Userspace queries the protocol version (e.g. via the protocol_version sysfs
> + * attribute) and uses sizeof() on the matching UAPI structure for both @size
> + * and the allocation backing @buf.
> + */
> +struct hsmp_telemetry_data {
> +	__u64	buf;
> +	__u32	size;
> +	__u16	sock_ind;
> +	__u16	reserved;
> +};
> +
>  /* Reset to default packing */
>  #pragma pack()
>  
> @@ -693,4 +727,13 @@ struct hsmp_metric_table_zen6 {
>  #define HSMP_BASE_IOCTL_NR	0xF8
>  #define HSMP_IOCTL_CMD		_IOWR(HSMP_BASE_IOCTL_NR, 0, struct hsmp_message)
>  
> +/*
> + * Fetch the firmware metric (telemetry) table for a given socket via the
> + * HSMP character device.  This avoids the PAGE_SIZE limitation of the
> + * sysfs binary attribute path for tables larger than one page (such as the
> + * ~13 KB hsmp_metric_table_zen6 used on Family 1Ah Model 50h-5Fh).
> + */
> +#define HSMP_IOCTL_GET_TELEMETRY_DATA \
> +	_IOWR(HSMP_BASE_IOCTL_NR, 1, struct hsmp_telemetry_data)
> +
>  #endif /*_ASM_X86_AMD_HSMP_H_*/
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index cf9392f99298..67f0074bb532 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -12,8 +12,11 @@
>  #include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> +#include <linux/nospec.h>
>  #include <linux/semaphore.h>
> +#include <linux/slab.h>
>  #include <linux/sysfs.h>
> +#include <linux/uaccess.h>
>  
>  #include "hsmp.h"
>  
> @@ -287,7 +290,7 @@ static bool is_get_msg(struct hsmp_message *msg)
>  	return false;
>  }
>  
> -long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +static long hsmp_ioctl_msg(struct file *fp, unsigned long arg)
>  {
>  	int __user *arguser = (int  __user *)arg;
>  	struct hsmp_message msg = { 0 };
> @@ -303,6 +306,19 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>  	if (msg.msg_id < HSMP_TEST || msg.msg_id >= HSMP_MSG_ID_MAX)
>  		return -ENOMSG;
>  
> +	/*
> +	 * Sanitize the user-controlled msg_id against speculative
> +	 * execution.  The bounds check above retires the out-of-range
> +	 * case with -ENOMSG, but a mispredicted branch can still let the
> +	 * CPU speculatively use msg_id as an index into
> +	 * hsmp_msg_desc_table[] (here and in validate_message() /
> +	 * is_get_msg() called downstream via hsmp_send_message()), and
> +	 * pull arbitrary kernel memory into the cache (Spectre v1,
> +	 * CVE-2017-5753).  Clamp once into msg.msg_id so every downstream
> +	 * dereference sees the sanitized value.
> +	 */
> +	msg.msg_id = array_index_nospec(msg.msg_id, HSMP_MSG_ID_MAX);
> +

This is a good change, but it looks to me it's fixing an existing problem, 
that is unrelated to this patch which is supposed to be about:

"Add IOCTL_GET_TELEMETRY_DATA for metric table reads"

So I think it should be done in own patch.

>  	switch (fp->f_mode & (FMODE_WRITE | FMODE_READ)) {
>  	case FMODE_WRITE:
>  		/*
> @@ -343,6 +359,105 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>  	return 0;
>  }
>  
> +/*
> + * Fetch the firmware metric (telemetry) table for the requested socket and
> + * copy it to the userspace buffer described by the request.
> + *
> + * The metric table size is variable across HSMP protocol versions and on
> + * Family 1Ah Model 50h-5Fh exceeds PAGE_SIZE.  Userspace must therefore
> + * supply a buffer at least the firmware-reported size in bytes.

Here the comment says "at least" but you check it's exactly equal to the 
required size?

> + */
> +static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
> +{
> +	void __user *arguser = (void __user *)arg;
> +	struct hsmp_telemetry_data req;
> +	struct hsmp_socket *sock;
> +	void __user *user_buf;
> +	size_t tbl_size;
> +	unsigned int sock_ind;
> +	void *kbuf;
> +	int ret;
> +
> +	/* Telemetry data is read-only; require read access on the fd. */
> +	if (!(fp->f_mode & FMODE_READ))
> +		return -EPERM;
> +
> +	if (copy_from_user(&req, arguser, sizeof(req)))
> +		return -EFAULT;
> +
> +	/*
> +	 * Reserved fields must be zero so future kernels can safely
> +	 * repurpose them without breaking already-deployed userspace.
> +	 */
> +	if (req.reserved)
> +		return -EINVAL;
> +
> +	if (!hsmp_pdev.sock || req.sock_ind >= hsmp_pdev.num_sockets)
> +		return -ENODEV;
> +
> +	tbl_size = hsmp_pdev.hsmp_table_size;
> +	if (!tbl_size)
> +		return -ENODEV;
> +
> +	/*
> +	 * Userspace must size its buffer using the appropriate UAPI metric
> +	 * table struct for the running protocol version.  Reject mismatched
> +	 * sizes so we never silently truncate or short-write.
> +	 */
> +	if (req.size != tbl_size)
> +		return -EINVAL;
> +
> +	/*
> +	 * Sanitize the user-controlled socket index against speculative
> +	 * execution.  The bounds check above retires the out-of-range case
> +	 * with -ENODEV, but a mispredicted branch can still let the CPU
> +	 * speculatively use sock_ind as an index into hsmp_pdev.sock[] and
> +	 * pull arbitrary kernel memory into the cache (Spectre v1, CVE-2017-
> +	 * 5753).  array_index_nospec() turns the bounds check into a
> +	 * data-flow clamp so the speculative load is in-range too.
> +	 */
> +	sock_ind = array_index_nospec(req.sock_ind, hsmp_pdev.num_sockets);
> +	sock = &hsmp_pdev.sock[sock_ind];
> +	if (!sock->metric_tbl_addr)
> +		return -ENODEV;
> +
> +	user_buf = u64_to_user_ptr(req.buf);
> +
> +	/*
> +	 * The bounce buffer is overwritten in full by memcpy_fromio() inside
> +	 * hsmp_metric_tbl_read(); use kvmalloc() to avoid the zeroing cost of
> +	 * kvzalloc() on the ~13 KB allocation done on every ioctl call.
> +	 */
> +	kbuf = kvmalloc(tbl_size, GFP_KERNEL);
> +	if (!kbuf)
> +		return -ENOMEM;
> +
> +	ret = hsmp_metric_tbl_read(sock, kbuf, tbl_size);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (copy_to_user(user_buf, kbuf, tbl_size))
> +		ret = -EFAULT;
> +	else
> +		ret = 0;
> +
> +out:
> +	kvfree(kbuf);
> +	return ret;
> +}
> +
> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> +	switch (cmd) {
> +	case HSMP_IOCTL_CMD:
> +		return hsmp_ioctl_msg(fp, arg);
> +	case HSMP_IOCTL_GET_TELEMETRY_DATA:
> +		return hsmp_ioctl_get_telemetry(fp, arg);
> +	default:
> +		return -ENOTTY;
> +	}
> +}
> +
>  ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
>  {
>  	struct hsmp_message msg = { 0 };
> 

-- 
 i.

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

[Ilpo Järvinen]

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.

>  };
>  
>  int hsmp_cache_proto_ver(u16 sock_ind);
> 

-- 
 i.

Re: [PATCH v4 7/7] platform/x86/amd/hsmp: Make metric table read locking use guard(mutex)

[Ilpo Järvinen]

On Thu, 28 May 2026, Muralidhara M K wrote:

> hsmp_metric_tbl_read() refreshes the SMU-side metric table and then
> memcpy_fromio()'s the result. Without serialization, two parallel
> readers can interleave the refresh and the copy and the caller
> observes a torn (mixed old/new) snapshot. Add a per-socket
> metric_tbl_lock so the refresh-and-copy sequence is atomic from
> userspace's point of view.
> 
> Use scoped guard(mutex) so the lock is released on every return
> path without hand-written goto chains, and initialize the mutex
> with devm_mutex_init() so no explicit mutex_destroy() cleanup is
> required.
> 
> Initialize the mutex before devm_ioremap() so the invariant
> "sock->metric_tbl_addr != NULL implies metric_tbl_lock is usable"
> holds on every error exit. Both callers of hsmp_get_tbl_dram_base()
> (init_acpi() and init_platform_device()) intentionally only log a
> failure and continue probing, so initializing the mutex after a
> successful ioremap would leave sock->metric_tbl_addr populated with
> an uninitialized lock, and the next hsmp_metric_tbl_read() would
> take guard(mutex)() on garbage memory. With the order swapped, a
> devm_mutex_init() failure returns early before metric_tbl_addr is
> ever set, and the existing NULL check in hsmp_metric_tbl_read()
> keeps rejecting the read with -ENOMEM as before.
> 
> 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>
> ---
>  drivers/platform/x86/amd/hsmp/hsmp.c | 20 ++++++++++++++++++++
>  drivers/platform/x86/amd/hsmp/hsmp.h |  3 +++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 67f0074bb532..fda57225939c 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -479,6 +479,7 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
>  	msg.msg_id	= HSMP_GET_METRIC_TABLE;
>  	msg.sock_ind	= sock->sock_ind;
>  
> +	guard(mutex)(&sock->metric_tbl_lock);
>  	ret = hsmp_send_message(&msg);
>  	if (ret)
>  		return ret;
> @@ -495,6 +496,24 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
>  	phys_addr_t dram_addr;
>  	int ret;
>  
> +	/*
> +	 * Initialize the per-socket lock before anything that can set
> +	 * sock->metric_tbl_addr to a non-NULL value.  hsmp_metric_tbl_read()
> +	 * gates on sock->metric_tbl_addr being non-NULL and then takes
> +	 * metric_tbl_lock unconditionally; both callers of this function
> +	 * (init_acpi() and init_platform_device()) intentionally only log
> +	 * a failure here and continue probing, so an init order that left
> +	 * metric_tbl_addr populated while devm_mutex_init() failed would
> +	 * leave the read path locking an uninitialized mutex.  Doing the
> +	 * mutex init first preserves the invariant "metric_tbl_addr !=
> +	 * NULL implies the lock is usable" on every error exit.
> +	 */
> +	ret = devm_mutex_init(sock->dev, &sock->metric_tbl_lock);
> +	if (ret) {
> +		dev_err(sock->dev, "Failed to initialize metric table lock\n");
> +		return ret;
> +	}
> +
>  	msg.sock_ind	= sock_ind;
>  	msg.response_sz	= hsmp_msg_desc_table[HSMP_GET_METRIC_TABLE_DRAM_ADDR].response_sz;
>  	msg.msg_id	= HSMP_GET_METRIC_TABLE_DRAM_ADDR;
> @@ -524,6 +543,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
>  		dev_err(sock->dev, "Failed to ioremap metric table addr\n");
>  		return -ENOMEM;
>  	}
> +
>  	return 0;

A spurious change.

>  }
>  EXPORT_SYMBOL_NS_GPL(hsmp_get_tbl_dram_base, "AMD_HSMP");
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index e7f051475728..f7b1cbf19932 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -15,6 +15,7 @@
>  #include <linux/hwmon.h>
>  #include <linux/kconfig.h>
>  #include <linux/miscdevice.h>
> +#include <linux/mutex.h>
>  #include <linux/pci.h>
>  #include <linux/semaphore.h>
>  #include <linux/sysfs.h>
> @@ -41,6 +42,8 @@ struct hsmp_socket {
>  	struct bin_attribute hsmp_attr;
>  	struct hsmp_mbaddr_info mbinfo;
>  	void __iomem *metric_tbl_addr;
> +	/* Serializes concurrent metric table refreshes from the sysfs path */
> +	struct mutex metric_tbl_lock;
>  	void __iomem *virt_base_addr;
>  	struct semaphore hsmp_sem;
>  	char name[HSMP_ATTR_GRP_NAME_SIZE];
> 

-- 
 i.

Re: [PATCH v4 5/7] platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table reads

[M K, Muralidhara]

On 6/10/2026 4:32 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 metric table needs to be delivered to userspace as a single
>> atomic snapshot, but the current sysfs metrics_bin path is a file
>> read: userspace can read it in chunks and observe a torn snapshot
>> if an SMU refresh happens between read() calls. The same path is
>> also bounded by PAGE_SIZE, so the ~13 KB hsmp_metric_table_zen6
>> used on Family 1Ah Model 50h-5Fh cannot be returned at all,
>> regardless of how userspace reads it. Rather than extend sysfs to
>> lift both restrictions, expose the metric table through the
>> existing HSMP character device using a new ioctl that always copies
>> the full table in one shot.
>>
>> Add struct hsmp_telemetry_data and HSMP_IOCTL_GET_TELEMETRY_DATA
>> to the UAPI header. Under the surrounding #pragma pack(4), placing
>> the __u64 user pointer first gives a tight 16-byte layout that is
>> identical for 32- and 64-bit callers, and the trailing __u16
>> reserved field is rejected with -EINVAL if non-zero so future
>> kernels can repurpose it without breaking already-deployed
>> userspace. Userspace sizes its buffer using the matching UAPI
>> metric-table struct for the running platform; sizes that disagree
>> with the firmware-reported table size are rejected so a short copy
>> can never silently truncate the snapshot.
>>
>> Dispatch hsmp_ioctl() on the ioctl command: the existing message
>> handler is factored out as hsmp_ioctl_msg() for HSMP_IOCTL_CMD, and
>> HSMP_IOCTL_GET_TELEMETRY_DATA goes to a new
>> hsmp_ioctl_get_telemetry() helper.
>>
>> The user-controlled indices reaching kernel arrays via these ioctl
>> paths are clamped with array_index_nospec() to mitigate Spectre v1
>> (CVE-2017-5753): req.sock_ind before indexing hsmp_pdev.sock[], and
>> msg.msg_id once at the entry to hsmp_ioctl_msg() so all six
>> downstream hsmp_msg_desc_table[] dereferences inherit the clamp
>> from a single mask.
>>
>> 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>
>> ---
>>   Documentation/arch/x86/amd_hsmp.rst  |  30 ++++++-
>>   arch/x86/include/uapi/asm/amd_hsmp.h |  43 ++++++++++
>>   drivers/platform/x86/amd/hsmp/hsmp.c | 117 ++++++++++++++++++++++++++-
>>   3 files changed, 188 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
>> index 8bb411f0d70d..74c259507888 100644
>> --- a/Documentation/arch/x86/amd_hsmp.rst
>> +++ b/Documentation/arch/x86/amd_hsmp.rst
>> @@ -68,6 +68,14 @@ under per socket sysfs directory created at
>>
>>   Note: lseek() is not supported as entire metrics table is read.
>>
>> +The sysfs metrics_bin path supports only HSMP protocol version 6 and,
>> +because it is a file read, can return a torn snapshot if userspace
>> +reads in pieces.  Protocol version 7 metric tables
>> +(``struct hsmp_metric_table_zen6``, ~13 KB) also exceed PAGE_SIZE, so
>> +a read returns ``-EOPNOTSUPP``.  For atomic reads on any protocol
>> +version, use the ``HSMP_IOCTL_GET_TELEMETRY_DATA`` ioctl on /dev/hsmp
>> +(see below).
>> +
>>   Metrics table definitions will be documented as part of Public PPR.
>>   The same is defined in the amd_hsmp.h header.
>>
>> @@ -167,7 +175,7 @@ Next thing, open the device file, as follows::
>>       exit(1);
>>     }
>>
>> -The following IOCTL is defined:
>> +The following IOCTLs are defined:
>>
>>   ``ioctl(file, HSMP_IOCTL_CMD, struct hsmp_message *msg)``
>>     The argument is a pointer to a::
>> @@ -180,6 +188,26 @@ The following IOCTL is defined:
>>        __u16   sock_ind;                       /* socket number */
>>       };
>>
>> +``ioctl(file, HSMP_IOCTL_GET_TELEMETRY_DATA, struct hsmp_telemetry_data *req)``
>> +  Atomically fetch the firmware metric (telemetry) table for a socket.
>> +  The ioctl copies the table in one shot, so unlike the metrics_bin
>> +  sysfs path it cannot return a torn snapshot and is not bounded by
>> +  PAGE_SIZE.  Required for HSMP protocol version 7+ (e.g. Family 1Ah
>> +  Model 50h-5Fh, ~13 KB ``struct hsmp_metric_table_zen6``).  Argument::
>> +
>> +    struct hsmp_telemetry_data {
>> +     __u64 buf; /* User pointer to destination buffer */
>> +     __u32 size; /* Size of @buf, must equal firmware-reported table size */
>> +     __u16 sock_ind; /* Socket index */
>> +     __u16 reserved; /* Reserved, must be zero */
>> +    };
>> +
>> +  Userspace picks the layout matching the running protocol version
>> +  (``struct hsmp_metric_table`` for v6, ``struct hsmp_metric_table_zen6``
>> +  for v7, queried via the ``protocol_version`` sysfs attribute) and
>> +  uses ``sizeof()`` of that struct for ``size``.  The kernel rejects
>> +  a mismatched ``size`` or non-zero ``reserved`` with ``-EINVAL``.
>> +
>>   The ioctl would return a non-zero on failure; you can read errno to see
>>   what happened. The transaction returns 0 on success.
>>
>> diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
>> index 438ac38d0dc8..50b28ccc7597 100644
>> --- a/arch/x86/include/uapi/asm/amd_hsmp.h
>> +++ b/arch/x86/include/uapi/asm/amd_hsmp.h
>> @@ -686,6 +686,40 @@ struct hsmp_metric_table_zen6 {
>>        struct hsmp_metric_table_zen6_ccd ccd[HSMP_F1A_M50_M5F_MAX_CCDS];
>>   };
>>
>> +/**
>> + * struct hsmp_telemetry_data - Request descriptor for HSMP telemetry IOCTL
>> + * @buf:       Input. Userspace pointer (encoded as __u64 to keep the layout
>> + *             stable between 32-bit and 64-bit callers) to the destination
>> + *             buffer that receives the metric table.
>> + * @size:      Input. Size in bytes of the buffer pointed to by @buf.  Must
>> + *             match the firmware-reported metric table size for the running
>> + *             HSMP protocol version (see below); any other value results in
>> + *             -EINVAL.  The kernel does not write this field back.
>> + * @sock_ind:  Input. Socket index from which the metric table is read.
>> + * @reserved:  Reserved for future use.  Callers should set this to zero;
>> + *             future kernels may begin interpreting the field, so passing
>> + *             a non-zero value today is not forwards compatible.
>> + *
>> + * Placing @buf first lets all fields fall on their natural alignment under
>> + * the surrounding #pragma pack(4), so the struct is a tight 16 bytes with
>> + * the same wire layout on 32-bit and 64-bit userspace.
>> + *
>> + * The exact metric table layout depends on the HSMP protocol version reported
>> + * by the firmware:
>> + *   - Protocol version 6 -> struct hsmp_metric_table
>> + *   - Protocol version 7 -> struct hsmp_metric_table_zen6
>> + *
>> + * Userspace queries the protocol version (e.g. via the protocol_version sysfs
>> + * attribute) and uses sizeof() on the matching UAPI structure for both @size
>> + * and the allocation backing @buf.
>> + */
>> +struct hsmp_telemetry_data {
>> +     __u64   buf;
>> +     __u32   size;
>> +     __u16   sock_ind;
>> +     __u16   reserved;
>> +};
>> +
>>   /* Reset to default packing */
>>   #pragma pack()
>>
>> @@ -693,4 +727,13 @@ struct hsmp_metric_table_zen6 {
>>   #define HSMP_BASE_IOCTL_NR   0xF8
>>   #define HSMP_IOCTL_CMD               _IOWR(HSMP_BASE_IOCTL_NR, 0, struct hsmp_message)
>>
>> +/*
>> + * Fetch the firmware metric (telemetry) table for a given socket via the
>> + * HSMP character device.  This avoids the PAGE_SIZE limitation of the
>> + * sysfs binary attribute path for tables larger than one page (such as the
>> + * ~13 KB hsmp_metric_table_zen6 used on Family 1Ah Model 50h-5Fh).
>> + */
>> +#define HSMP_IOCTL_GET_TELEMETRY_DATA \
>> +     _IOWR(HSMP_BASE_IOCTL_NR, 1, struct hsmp_telemetry_data)
>> +
>>   #endif /*_ASM_X86_AMD_HSMP_H_*/
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
>> index cf9392f99298..67f0074bb532 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
>> @@ -12,8 +12,11 @@
>>   #include <linux/acpi.h>
>>   #include <linux/delay.h>
>>   #include <linux/device.h>
>> +#include <linux/nospec.h>
>>   #include <linux/semaphore.h>
>> +#include <linux/slab.h>
>>   #include <linux/sysfs.h>
>> +#include <linux/uaccess.h>
>>
>>   #include "hsmp.h"
>>
>> @@ -287,7 +290,7 @@ static bool is_get_msg(struct hsmp_message *msg)
>>        return false;
>>   }
>>
>> -long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>> +static long hsmp_ioctl_msg(struct file *fp, unsigned long arg)
>>   {
>>        int __user *arguser = (int  __user *)arg;
>>        struct hsmp_message msg = { 0 };
>> @@ -303,6 +306,19 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>>        if (msg.msg_id < HSMP_TEST || msg.msg_id >= HSMP_MSG_ID_MAX)
>>                return -ENOMSG;
>>
>> +     /*
>> +      * Sanitize the user-controlled msg_id against speculative
>> +      * execution.  The bounds check above retires the out-of-range
>> +      * case with -ENOMSG, but a mispredicted branch can still let the
>> +      * CPU speculatively use msg_id as an index into
>> +      * hsmp_msg_desc_table[] (here and in validate_message() /
>> +      * is_get_msg() called downstream via hsmp_send_message()), and
>> +      * pull arbitrary kernel memory into the cache (Spectre v1,
>> +      * CVE-2017-5753).  Clamp once into msg.msg_id so every downstream
>> +      * dereference sees the sanitized value.
>> +      */
>> +     msg.msg_id = array_index_nospec(msg.msg_id, HSMP_MSG_ID_MAX);
>> +
> 
> This is a good change, but it looks to me it's fixing an existing problem,
> that is unrelated to this patch which is supposed to be about:
> 
> "Add IOCTL_GET_TELEMETRY_DATA for metric table reads"
> 
> So I think it should be done in own patch.
> 
Sure. I will make this as a seperate patch.

>>        switch (fp->f_mode & (FMODE_WRITE | FMODE_READ)) {
>>        case FMODE_WRITE:
>>                /*
>> @@ -343,6 +359,105 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>>        return 0;
>>   }
>>
>> +/*
>> + * Fetch the firmware metric (telemetry) table for the requested socket and
>> + * copy it to the userspace buffer described by the request.
>> + *
>> + * The metric table size is variable across HSMP protocol versions and on
>> + * Family 1Ah Model 50h-5Fh exceeds PAGE_SIZE.  Userspace must therefore
>> + * supply a buffer at least the firmware-reported size in bytes.
> 
> Here the comment says "at least" but you check it's exactly equal to the
> required size?
> 
>> + */
>> +static long hsmp_ioctl_get_telemetry(struct file *fp, unsigned long arg)
>> +{
>> +     void __user *arguser = (void __user *)arg;
>> +     struct hsmp_telemetry_data req;
>> +     struct hsmp_socket *sock;
>> +     void __user *user_buf;
>> +     size_t tbl_size;
>> +     unsigned int sock_ind;
>> +     void *kbuf;
>> +     int ret;
>> +
>> +     /* Telemetry data is read-only; require read access on the fd. */
>> +     if (!(fp->f_mode & FMODE_READ))
>> +             return -EPERM;
>> +
>> +     if (copy_from_user(&req, arguser, sizeof(req)))
>> +             return -EFAULT;
>> +
>> +     /*
>> +      * Reserved fields must be zero so future kernels can safely
>> +      * repurpose them without breaking already-deployed userspace.
>> +      */
>> +     if (req.reserved)
>> +             return -EINVAL;
>> +
>> +     if (!hsmp_pdev.sock || req.sock_ind >= hsmp_pdev.num_sockets)
>> +             return -ENODEV;
>> +
>> +     tbl_size = hsmp_pdev.hsmp_table_size;
>> +     if (!tbl_size)
>> +             return -ENODEV;
>> +
>> +     /*
>> +      * Userspace must size its buffer using the appropriate UAPI metric
>> +      * table struct for the running protocol version.  Reject mismatched
>> +      * sizes so we never silently truncate or short-write.
>> +      */
>> +     if (req.size != tbl_size)
>> +             return -EINVAL;
>> +
>> +     /*
>> +      * Sanitize the user-controlled socket index against speculative
>> +      * execution.  The bounds check above retires the out-of-range case
>> +      * with -ENODEV, but a mispredicted branch can still let the CPU
>> +      * speculatively use sock_ind as an index into hsmp_pdev.sock[] and
>> +      * pull arbitrary kernel memory into the cache (Spectre v1, CVE-2017-
>> +      * 5753).  array_index_nospec() turns the bounds check into a
>> +      * data-flow clamp so the speculative load is in-range too.
>> +      */
>> +     sock_ind = array_index_nospec(req.sock_ind, hsmp_pdev.num_sockets);
>> +     sock = &hsmp_pdev.sock[sock_ind];
>> +     if (!sock->metric_tbl_addr)
>> +             return -ENODEV;
>> +
>> +     user_buf = u64_to_user_ptr(req.buf);
>> +
>> +     /*
>> +      * The bounce buffer is overwritten in full by memcpy_fromio() inside
>> +      * hsmp_metric_tbl_read(); use kvmalloc() to avoid the zeroing cost of
>> +      * kvzalloc() on the ~13 KB allocation done on every ioctl call.
>> +      */
>> +     kbuf = kvmalloc(tbl_size, GFP_KERNEL);
>> +     if (!kbuf)
>> +             return -ENOMEM;
>> +
>> +     ret = hsmp_metric_tbl_read(sock, kbuf, tbl_size);
>> +     if (ret < 0)
>> +             goto out;
>> +
>> +     if (copy_to_user(user_buf, kbuf, tbl_size))
>> +             ret = -EFAULT;
>> +     else
>> +             ret = 0;
>> +
>> +out:
>> +     kvfree(kbuf);
>> +     return ret;
>> +}
>> +
>> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>> +{
>> +     switch (cmd) {
>> +     case HSMP_IOCTL_CMD:
>> +             return hsmp_ioctl_msg(fp, arg);
>> +     case HSMP_IOCTL_GET_TELEMETRY_DATA:
>> +             return hsmp_ioctl_get_telemetry(fp, arg);
>> +     default:
>> +             return -ENOTTY;
>> +     }
>> +}
>> +
>>   ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
>>   {
>>        struct hsmp_message msg = { 0 };
>>
> 
> --
>   i.
> 

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

[M K, Muralidhara]

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

>>   };
>>
>>   int hsmp_cache_proto_ver(u16 sock_ind);
>>
> 
> --
>   i.
> 

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

[Ilpo Järvinen]

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

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

[M K, Muralidhara]

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.

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

[M K, Muralidhara]

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