* [PATCH v6 1/8] platform/x86/amd/hsmp: Add HSMP messages for Family 1Ah, Model 50h-5Fh
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 2/8] platform/x86/amd/hsmp: Add UAPI structures for Family 1Ah Model 50h-5Fh metrics table Muralidhara M K
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K, Suma Hegde,
Muthusamy Ramalingam
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 same firmware 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). Add the new IDs, convert the three
messages to HSMP_SET_GET, and bump the driver version to 2.6.
Also add PQoS-related HSMP messages HSMP_PQOS_TRAFFIC_PRIORITY (0x3B)
and HSMP_PQOS_FLOATING_BW (0x3C) with matching hsmp_msg_desc_table[]
descriptors so userspace can reach the new functionality.
Backward compatibility is preserved on prior platforms: new IDs
previously occupied HSMP_RSVD slots, and existing userspace that leaves
bit[31] = 0 continues to take a pure SET path.
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 | 169 ++++++++++++++++++++++++---
drivers/platform/x86/amd/hsmp/hsmp.h | 2 +-
2 files changed, 156 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..ff8b6defec1d 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -53,9 +53,21 @@ 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_PQOS_TRAFFIC_PRIORITY, /* 3Bh Get/Set traffic priority */
+ HSMP_PQOS_FLOATING_BW, /* 3Ch Get/Set max floating bandwidth */
HSMP_MSG_ID_MAX,
};
@@ -170,16 +182,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 +328,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 +394,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 +446,86 @@ 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},
+
+ /*
+ * HSMP_PQOS_TRAFFIC_PRIORITY, num_args = 1, response_sz = 1
+ * input: args[0] = Op[31:30] + Priority sel[27:26] + Priority val[21:20] + Input[19:0]
+ * output: args[0] = Supported priorities or Priority val[1:0]
+ */
+ {1, 1, HSMP_SET_GET},
+
+ /*
+ * HSMP_PQOS_FLOATING_BW, num_args = 1, response_sz = 2
+ * input: args[0] = Op[31] + Sub-op[30:29] + params[28:0]
+ * output: args[0] = Discovery bits or Floating/Global memory BW (Gbps)
+ * output: args[1] = Reserved or config (drop adj, sampling delay, hysteresis)
+ */
+ {1, 2, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 2/8] platform/x86/amd/hsmp: Add UAPI structures for Family 1Ah Model 50h-5Fh metrics table
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 1/8] platform/x86/amd/hsmp: Add HSMP messages for Family 1Ah, Model 50h-5Fh Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 3/8] platform/x86/amd/hsmp: Unify response_sz validation to an upper-bound check Muralidhara M K
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K, Suma Hegde,
Muthusamy Ramalingam
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 ff8b6defec1d..34e25d9048d5 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -608,6 +608,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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 3/8] platform/x86/amd/hsmp: Unify response_sz validation to an upper-bound check
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 1/8] platform/x86/amd/hsmp: Add HSMP messages for Family 1Ah, Model 50h-5Fh Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 2/8] platform/x86/amd/hsmp: Add UAPI structures for Family 1Ah Model 50h-5Fh metrics table Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 4/8] platform/x86/amd/hsmp: Source metric-table size from firmware Muralidhara M K
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K, Suma Hegde,
Muthusamy Ramalingam
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 4/8] platform/x86/amd/hsmp: Source metric-table size from firmware
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
` (2 preceding siblings ...)
2026-06-12 4:26 ` [PATCH v6 3/8] platform/x86/amd/hsmp: Unify response_sz validation to an upper-bound check Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 5/8] platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table reads Muralidhara M K
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K,
Muthusamy Ramalingam
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 | 2 ++
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
index 34e25d9048d5..77bea5ab7ade 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -367,11 +367,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..7343f8a1681f 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -18,6 +18,7 @@
#include <linux/pci.h>
#include <linux/semaphore.h>
#include <linux/sysfs.h>
+#include <linux/types.h>
#define HSMP_METRICS_TABLE_NAME "metrics_bin"
@@ -55,6 +56,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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 5/8] platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table reads
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
` (3 preceding siblings ...)
2026-06-12 4:26 ` [PATCH v6 4/8] platform/x86/amd/hsmp: Source metric-table size from firmware Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 6/8] platform/x86/amd/hsmp: Clamp ioctl/send_message indices (Spectre v1) Muralidhara M K
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K,
Muthusamy Ramalingam
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 socket index in HSMP_IOCTL_GET_TELEMETRY_DATA is
clamped with array_index_nospec() before indexing hsmp_pdev.sock[],
mitigating Spectre v1 (CVE-2017-5753).
Reviewed-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 | 104 ++++++++++++++++++++++++++-
3 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
index 8bb411f0d70d..301c0091f5bf 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 77bea5ab7ade..9b246fd724ef 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -702,6 +702,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()
@@ -709,4 +743,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..36ff83744684 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 };
@@ -343,6 +346,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 equal to 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 6/8] platform/x86/amd/hsmp: Clamp ioctl/send_message indices (Spectre v1)
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
` (4 preceding siblings ...)
2026-06-12 4:26 ` [PATCH v6 5/8] platform/x86/amd/hsmp: Add IOCTL_GET_TELEMETRY_DATA for metric table reads Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 7/8] platform/x86/amd/hsmp: Enable HSMP_PROTO_VER7 metric tables on the ACPI driver via the IOCTL Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 8/8] platform/x86/amd/hsmp: Serialize metric table access and ACPI HSMP teardown Muralidhara M K
7 siblings, 0 replies; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K,
Muthusamy Ramalingam
Although validate_message() checks msg_id, a mispredicted branch can
still allow speculative indexing into hsmp_msg_desc_table[]. Clamp
msg.msg_id with array_index_nospec() at entry to hsmp_ioctl_msg() so
downstream dereferences (including via is_get_msg() and
hsmp_send_message()) see a bounded index.
Similarly, hsmp_send_message() bounds-checks msg->sock_ind before
indexing hsmp_pdev.sock[], but a mispredicted branch can still
speculatively use the raw index (Spectre v1, CVE-2017-5753). Apply
array_index_nospec() after the check so every caller that reaches
hsmp_pdev.sock[] through this helper sees a clamped socket
index—including hsmp_ioctl_msg() and any other path that hands a
user-derived struct hsmp_message to hsmp_send_message().
Reviewed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/hsmp.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 36ff83744684..9f7a5fb67728 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -200,6 +200,7 @@ static int validate_message(struct hsmp_message *msg)
int hsmp_send_message(struct hsmp_message *msg)
{
struct hsmp_socket *sock;
+ unsigned int sock_ind;
int ret;
if (!msg)
@@ -210,7 +211,15 @@ int hsmp_send_message(struct hsmp_message *msg)
if (!hsmp_pdev.sock || msg->sock_ind >= hsmp_pdev.num_sockets)
return -ENODEV;
- sock = &hsmp_pdev.sock[msg->sock_ind];
+
+ /*
+ * Sanitize sock_ind after the bounds check. A mispredicted branch can
+ * still let the CPU speculatively use msg->sock_ind as an index into
+ * hsmp_pdev.sock[] (Spectre v1, CVE-2017-5753), including for callers
+ * other than hsmp_ioctl_msg() that pass a user-derived socket index.
+ */
+ sock_ind = array_index_nospec(msg->sock_ind, hsmp_pdev.num_sockets);
+ sock = &hsmp_pdev.sock[sock_ind];
ret = down_interruptible(&sock->hsmp_sem);
if (ret < 0)
@@ -306,6 +315,19 @@ static long hsmp_ioctl_msg(struct file *fp, 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:
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 7/8] platform/x86/amd/hsmp: Enable HSMP_PROTO_VER7 metric tables on the ACPI driver via the IOCTL
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
` (5 preceding siblings ...)
2026-06-12 4:26 ` [PATCH v6 6/8] platform/x86/amd/hsmp: Clamp ioctl/send_message indices (Spectre v1) Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 4:26 ` [PATCH v6 8/8] platform/x86/amd/hsmp: Serialize metric table access and ACPI HSMP teardown Muralidhara M K
7 siblings, 0 replies; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K,
Muthusamy Ramalingam
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.
Reviewed-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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 8/8] platform/x86/amd/hsmp: Serialize metric table access and ACPI HSMP teardown
2026-06-12 4:26 [PATCH v6 0/8] platform/x86/amd/hsmp: Family 1Ah Model 50h-5Fh HSMP and metrics Muralidhara M K
` (6 preceding siblings ...)
2026-06-12 4:26 ` [PATCH v6 7/8] platform/x86/amd/hsmp: Enable HSMP_PROTO_VER7 metric tables on the ACPI driver via the IOCTL Muralidhara M K
@ 2026-06-12 4:26 ` Muralidhara M K
2026-06-12 12:57 ` Ilpo Järvinen
7 siblings, 1 reply; 10+ messages in thread
From: Muralidhara M K @ 2026-06-12 4:26 UTC (permalink / raw)
To: ilpo.jarvinen
Cc: platform-driver-x86, linux-kernel, Muralidhara M K,
Muthusamy Ramalingam
Each ACPI HSMP socket bumps acpi_sock_refcnt after a successful probe;
the last socket to remove runs refcount_dec_and_test() and tears down
shared state once. If init_acpi() or misc_register() fails before
refcount_inc(), call hsmp_acpi_free_sock_alloc_if_idle() to free
hsmp_pdev->sock so a concurrent probe cannot follow a dangling pointer.
1) Metric table sysfs reads ask the SMU to refresh the table, then
memcpy_fromio() from the mmio mapping. Concurrent readers could
interleave those steps and return inconsistent data. Serialize with
per-socket metric_tbl_lock and guard(mutex) around
hsmp_send_message() and memcpy_fromio() so each read() observes one
coherent snapshot.
2) Every ACPI AMDI0097 device shares hsmp_pdev->sock[]. The driver used
devm_kcalloc() from whichever device probed first, an unlocked probe
flag, and misc_register() from the first successful path. Parallel
probes could race, and devm tied the array lifetime to one device
while others (and devm hwmon paths) still called hsmp_send_message(),
which could use-after-free the array on remove.
3) Metric DRAM used devm_ioremap() and devm_mutex_init() per ACPI device
while the socket table is global, so devm teardown order did not
match shared storage. Switch to ioremap(), one-time mutex_init()
(metric_lock_inited), and hsmp_metric_tbl_unmap_all() on the last
ACPI remove together with the rest of global teardown.
4) After kcalloc(sock) but before refcount_inc(), one failing probe
could kfree() the array while another probe still ran init_acpi() on
the same pointer. Hold hsmp_acpi_probe_mutex across init_acpi(),
misc_register(), and refcount_inc() so setup does not overlap
teardown or a concurrent probe.
5) On the legacy platform driver, call hsmp_misc_deregister() before
hsmp_metric_tbl_unmap_all() so /dev/hsmp is gone before metric mmio
is unmapped, matching the ACPI remove path.
Tested-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 58 +++++++++++++++++++++-------
drivers/platform/x86/amd/hsmp/hsmp.c | 56 +++++++++++++++++++++++++--
drivers/platform/x86/amd/hsmp/hsmp.h | 9 ++++-
drivers/platform/x86/amd/hsmp/plat.c | 1 +
4 files changed, 106 insertions(+), 18 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 8dc6b4a8bd27..e526e2cea3e2 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -20,7 +20,10 @@
#include <linux/ioport.h>
#include <linux/kstrtox.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/refcount.h>
+#include <linux/slab.h>
#include <linux/sysfs.h>
#include <linux/topology.h>
#include <linux/uuid.h>
@@ -595,6 +598,19 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
};
MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
+static DEFINE_MUTEX(hsmp_acpi_probe_mutex);
+
+/* Caller holds hsmp_acpi_probe_mutex. */
+static void hsmp_acpi_free_sock_alloc_if_idle(void)
+{
+ if (refcount_read(&hsmp_pdev->acpi_sock_refcnt) == 0 &&
+ !hsmp_pdev->acpi_misc_registered && hsmp_pdev->sock) {
+ kfree(hsmp_pdev->sock);
+ hsmp_pdev->sock = NULL;
+ hsmp_pdev->num_sockets = 0;
+ }
+}
+
static int hsmp_acpi_probe(struct platform_device *pdev)
{
int ret;
@@ -603,49 +619,63 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
if (!hsmp_pdev)
return -ENOMEM;
- if (!hsmp_pdev->is_probed) {
+ mutex_lock(&hsmp_acpi_probe_mutex);
+ if (!hsmp_pdev->sock) {
hsmp_pdev->num_sockets = topology_max_packages();
if (!hsmp_pdev->num_sockets) {
dev_err(&pdev->dev, "No CPU sockets detected\n");
+ mutex_unlock(&hsmp_acpi_probe_mutex);
return -ENODEV;
}
- hsmp_pdev->sock = devm_kcalloc(&pdev->dev, hsmp_pdev->num_sockets,
- sizeof(*hsmp_pdev->sock),
- GFP_KERNEL);
- if (!hsmp_pdev->sock)
+ hsmp_pdev->sock = kcalloc(hsmp_pdev->num_sockets,
+ sizeof(*hsmp_pdev->sock),
+ GFP_KERNEL);
+ if (!hsmp_pdev->sock) {
+ mutex_unlock(&hsmp_acpi_probe_mutex);
return -ENOMEM;
+ }
}
ret = init_acpi(&pdev->dev);
if (ret) {
dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
+ hsmp_acpi_free_sock_alloc_if_idle();
+ mutex_unlock(&hsmp_acpi_probe_mutex);
return ret;
}
- if (!hsmp_pdev->is_probed) {
+ if (!hsmp_pdev->acpi_misc_registered) {
ret = hsmp_misc_register(&pdev->dev);
if (ret) {
dev_err(&pdev->dev, "Failed to register misc device\n");
+ hsmp_acpi_free_sock_alloc_if_idle();
+ mutex_unlock(&hsmp_acpi_probe_mutex);
return ret;
}
- hsmp_pdev->is_probed = true;
- dev_dbg(&pdev->dev, "AMD HSMP ACPI is probed successfully\n");
+ hsmp_pdev->acpi_misc_registered = true;
+ dev_dbg(&pdev->dev, "AMD HSMP ACPI misc device registered\n");
}
+ refcount_inc(&hsmp_pdev->acpi_sock_refcnt);
+ mutex_unlock(&hsmp_acpi_probe_mutex);
return 0;
}
static void hsmp_acpi_remove(struct platform_device *pdev)
{
- /*
- * We register only one misc_device even on multi-socket system.
- * So, deregister should happen only once.
- */
- if (hsmp_pdev->is_probed) {
+ mutex_lock(&hsmp_acpi_probe_mutex);
+ if (refcount_dec_and_test(&hsmp_pdev->acpi_sock_refcnt)) {
hsmp_misc_deregister();
- hsmp_pdev->is_probed = false;
+ hsmp_pdev->acpi_misc_registered = false;
+ hsmp_metric_tbl_unmap_all(hsmp_pdev, hsmp_pdev->num_sockets);
+ kfree(hsmp_pdev->sock);
+ hsmp_pdev->sock = NULL;
+ hsmp_pdev->num_sockets = 0;
+ hsmp_pdev->proto_ver = 0;
+ hsmp_pdev->hsmp_table_size = 0;
}
+ mutex_unlock(&hsmp_acpi_probe_mutex);
}
static struct platform_driver amd_hsmp_driver = {
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 9f7a5fb67728..4dc3826a0d57 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -12,6 +12,7 @@
#include <linux/acpi.h>
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/io.h>
#include <linux/nospec.h>
#include <linux/semaphore.h>
#include <linux/slab.h>
@@ -41,7 +42,9 @@
*/
#define CHECK_GET_BIT BIT(31)
-static struct hsmp_plat_device hsmp_pdev;
+static struct hsmp_plat_device hsmp_pdev = {
+ .acpi_sock_refcnt = REFCOUNT_INIT(0),
+};
/*
* Send a message to the HSMP port via PCI-e config space registers
@@ -488,6 +491,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;
@@ -497,6 +501,28 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
}
EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
+void hsmp_metric_tbl_unmap_all(struct hsmp_plat_device *pdev, u16 num_sockets)
+{
+ u16 i;
+
+ if (!pdev->sock || !num_sockets)
+ return;
+
+ for (i = 0; i < num_sockets; i++) {
+ struct hsmp_socket *s = &pdev->sock[i];
+
+ if (s->metric_tbl_addr) {
+ iounmap(s->metric_tbl_addr);
+ s->metric_tbl_addr = NULL;
+ }
+ if (s->metric_lock_inited) {
+ mutex_destroy(&s->metric_tbl_lock);
+ s->metric_lock_inited = false;
+ }
+ }
+}
+EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_unmap_all, "AMD_HSMP");
+
int hsmp_get_tbl_dram_base(u16 sock_ind)
{
struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
@@ -504,6 +530,31 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
phys_addr_t dram_addr;
int ret;
+ if (sock->metric_tbl_addr)
+ return 0;
+
+ /*
+ * 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 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.
+ *
+ * Use non-devm mutex + ioremap so the shared sock[] array (ACPI
+ * allocates once for all sockets) can be torn down from a single
+ * refcounted release path without devm lifetime being tied to the
+ * wrong struct device.
+ */
+ if (!sock->metric_lock_inited) {
+ mutex_init(&sock->metric_tbl_lock);
+ sock->metric_lock_inited = true;
+ }
+
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;
@@ -527,8 +578,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
else
hsmp_pdev.hsmp_table_size = sizeof(struct hsmp_metric_table);
- sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
- hsmp_pdev.hsmp_table_size);
+ sock->metric_tbl_addr = ioremap(dram_addr, 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 7343f8a1681f..5f5ad2885c22 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -15,7 +15,9 @@
#include <linux/hwmon.h>
#include <linux/kconfig.h>
#include <linux/miscdevice.h>
+#include <linux/mutex.h>
#include <linux/pci.h>
+#include <linux/refcount.h>
#include <linux/semaphore.h>
#include <linux/sysfs.h>
#include <linux/types.h>
@@ -42,11 +44,14 @@ 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];
struct device *dev;
u16 sock_ind;
+ bool metric_lock_inited;
int (*amd_hsmp_rdwr)(struct hsmp_socket *sock, u32 off, u32 *val, bool rw);
};
@@ -55,7 +60,8 @@ struct hsmp_plat_device {
struct hsmp_socket *sock;
u32 proto_ver;
u16 num_sockets;
- bool is_probed;
+ refcount_t acpi_sock_refcnt;
+ bool acpi_misc_registered;
size_t hsmp_table_size;
};
@@ -66,6 +72,7 @@ void hsmp_misc_deregister(void);
int hsmp_misc_register(struct device *dev);
int hsmp_get_tbl_dram_base(u16 sock_ind);
ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
+void hsmp_metric_tbl_unmap_all(struct hsmp_plat_device *pdev, u16 num_sockets);
struct hsmp_plat_device *get_hsmp_pdev(void);
#if IS_ENABLED(CONFIG_HWMON)
int hsmp_create_sensor(struct device *dev, u16 sock_ind);
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index e07f68575055..2ef2102d0af3 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -230,6 +230,7 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
static void hsmp_pltdrv_remove(struct platform_device *pdev)
{
hsmp_misc_deregister();
+ hsmp_metric_tbl_unmap_all(hsmp_pdev, hsmp_pdev->num_sockets);
}
static struct platform_driver amd_hsmp_driver = {
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v6 8/8] platform/x86/amd/hsmp: Serialize metric table access and ACPI HSMP teardown
2026-06-12 4:26 ` [PATCH v6 8/8] platform/x86/amd/hsmp: Serialize metric table access and ACPI HSMP teardown Muralidhara M K
@ 2026-06-12 12:57 ` Ilpo Järvinen
0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2026-06-12 12:57 UTC (permalink / raw)
To: Muralidhara M K; +Cc: platform-driver-x86, LKML, Muthusamy Ramalingam
On Fri, 12 Jun 2026, Muralidhara M K wrote:
> Each ACPI HSMP socket bumps acpi_sock_refcnt after a successful probe;
> the last socket to remove runs refcount_dec_and_test() and tears down
> shared state once. If init_acpi() or misc_register() fails before
> refcount_inc(), call hsmp_acpi_free_sock_alloc_if_idle() to free
> hsmp_pdev->sock so a concurrent probe cannot follow a dangling pointer.
>
> 1) Metric table sysfs reads ask the SMU to refresh the table, then
> memcpy_fromio() from the mmio mapping. Concurrent readers could
> interleave those steps and return inconsistent data. Serialize with
> per-socket metric_tbl_lock and guard(mutex) around
> hsmp_send_message() and memcpy_fromio() so each read() observes one
> coherent snapshot.
> 2) Every ACPI AMDI0097 device shares hsmp_pdev->sock[]. The driver used
> devm_kcalloc() from whichever device probed first, an unlocked probe
> flag, and misc_register() from the first successful path. Parallel
> probes could race, and devm tied the array lifetime to one device
> while others (and devm hwmon paths) still called hsmp_send_message(),
> which could use-after-free the array on remove.
> 3) Metric DRAM used devm_ioremap() and devm_mutex_init() per ACPI device
> while the socket table is global, so devm teardown order did not
> match shared storage. Switch to ioremap(), one-time mutex_init()
> (metric_lock_inited), and hsmp_metric_tbl_unmap_all() on the last
> ACPI remove together with the rest of global teardown.
> 4) After kcalloc(sock) but before refcount_inc(), one failing probe
> could kfree() the array while another probe still ran init_acpi() on
> the same pointer. Hold hsmp_acpi_probe_mutex across init_acpi(),
> misc_register(), and refcount_inc() so setup does not overlap
> teardown or a concurrent probe.
> 5) On the legacy platform driver, call hsmp_misc_deregister() before
> hsmp_metric_tbl_unmap_all() so /dev/hsmp is gone before metric mmio
> is unmapped, matching the ACPI remove path.
Thanks. To me it would look better to have the concurrency fixes done
first. You could actually send them in own series and once that work is
done, I'll process the ver7 series on top of that.
I took patch 6 nospec change into the review-ilpo-next already as it seems
independent of the ver7 changes.
Many comments below.
> Tested-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/acpi.c | 58 +++++++++++++++++++++-------
> drivers/platform/x86/amd/hsmp/hsmp.c | 56 +++++++++++++++++++++++++--
> drivers/platform/x86/amd/hsmp/hsmp.h | 9 ++++-
> drivers/platform/x86/amd/hsmp/plat.c | 1 +
> 4 files changed, 106 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index 8dc6b4a8bd27..e526e2cea3e2 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -20,7 +20,10 @@
> #include <linux/ioport.h>
> #include <linux/kstrtox.h>
> #include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/platform_device.h>
> +#include <linux/refcount.h>
> +#include <linux/slab.h>
> #include <linux/sysfs.h>
> #include <linux/topology.h>
> #include <linux/uuid.h>
> @@ -595,6 +598,19 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
> };
> MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
>
> +static DEFINE_MUTEX(hsmp_acpi_probe_mutex);
> +
> +/* Caller holds hsmp_acpi_probe_mutex. */
Please use lockdep_assert() to enforce any context requirement.
For comments, the kerneldoc compatible formatting should be used (even if
not writing a full kerneldoc comment):
* Context: caller must hold hsmp_acpi_probe_mutex.
> +static void hsmp_acpi_free_sock_alloc_if_idle(void)
> +{
> + if (refcount_read(&hsmp_pdev->acpi_sock_refcnt) == 0 &&
> + !hsmp_pdev->acpi_misc_registered && hsmp_pdev->sock) {
> + kfree(hsmp_pdev->sock);
> + hsmp_pdev->sock = NULL;
> + hsmp_pdev->num_sockets = 0;
> + }
> +}
> +
> static int hsmp_acpi_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -603,49 +619,63 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
> if (!hsmp_pdev)
> return -ENOMEM;
>
> - if (!hsmp_pdev->is_probed) {
> + mutex_lock(&hsmp_acpi_probe_mutex);
You should generally use cleanup.h when taking the mutex, especially, if
you need to duplicate the unlock to many places.
> + if (!hsmp_pdev->sock) {
> hsmp_pdev->num_sockets = topology_max_packages();
> if (!hsmp_pdev->num_sockets) {
> dev_err(&pdev->dev, "No CPU sockets detected\n");
> + mutex_unlock(&hsmp_acpi_probe_mutex);
> return -ENODEV;
> }
>
> - hsmp_pdev->sock = devm_kcalloc(&pdev->dev, hsmp_pdev->num_sockets,
> - sizeof(*hsmp_pdev->sock),
> - GFP_KERNEL);
> - if (!hsmp_pdev->sock)
> + hsmp_pdev->sock = kcalloc(hsmp_pdev->num_sockets,
> + sizeof(*hsmp_pdev->sock),
> + GFP_KERNEL);
> + if (!hsmp_pdev->sock) {
> + mutex_unlock(&hsmp_acpi_probe_mutex);
> return -ENOMEM;
> + }
> }
>
> ret = init_acpi(&pdev->dev);
> if (ret) {
> dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
> + hsmp_acpi_free_sock_alloc_if_idle();
> + mutex_unlock(&hsmp_acpi_probe_mutex);
> return ret;
> }
>
> - if (!hsmp_pdev->is_probed) {
> + if (!hsmp_pdev->acpi_misc_registered) {
> ret = hsmp_misc_register(&pdev->dev);
> if (ret) {
> dev_err(&pdev->dev, "Failed to register misc device\n");
> + hsmp_acpi_free_sock_alloc_if_idle();
> + mutex_unlock(&hsmp_acpi_probe_mutex);
> return ret;
> }
> - hsmp_pdev->is_probed = true;
> - dev_dbg(&pdev->dev, "AMD HSMP ACPI is probed successfully\n");
> + hsmp_pdev->acpi_misc_registered = true;
> + dev_dbg(&pdev->dev, "AMD HSMP ACPI misc device registered\n");
> }
> + refcount_inc(&hsmp_pdev->acpi_sock_refcnt);
> + mutex_unlock(&hsmp_acpi_probe_mutex);
>
> return 0;
> }
>
> static void hsmp_acpi_remove(struct platform_device *pdev)
> {
> - /*
> - * We register only one misc_device even on multi-socket system.
> - * So, deregister should happen only once.
> - */
> - if (hsmp_pdev->is_probed) {
> + mutex_lock(&hsmp_acpi_probe_mutex);
> + if (refcount_dec_and_test(&hsmp_pdev->acpi_sock_refcnt)) {
> hsmp_misc_deregister();
> - hsmp_pdev->is_probed = false;
> + hsmp_pdev->acpi_misc_registered = false;
> + hsmp_metric_tbl_unmap_all(hsmp_pdev, hsmp_pdev->num_sockets);
> + kfree(hsmp_pdev->sock);
> + hsmp_pdev->sock = NULL;
> + hsmp_pdev->num_sockets = 0;
> + hsmp_pdev->proto_ver = 0;
> + hsmp_pdev->hsmp_table_size = 0;
Using kref.h, you can give a release function for the put call and
get/put will be cleaner interface anyway than handling the refcount
directly.
> }
> + mutex_unlock(&hsmp_acpi_probe_mutex);
> }
>
> static struct platform_driver amd_hsmp_driver = {
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 9f7a5fb67728..4dc3826a0d57 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -12,6 +12,7 @@
> #include <linux/acpi.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> +#include <linux/io.h>
> #include <linux/nospec.h>
> #include <linux/semaphore.h>
> #include <linux/slab.h>
> @@ -41,7 +42,9 @@
> */
> #define CHECK_GET_BIT BIT(31)
>
> -static struct hsmp_plat_device hsmp_pdev;
> +static struct hsmp_plat_device hsmp_pdev = {
> + .acpi_sock_refcnt = REFCOUNT_INIT(0),
> +};
>
> /*
> * Send a message to the HSMP port via PCI-e config space registers
> @@ -488,6 +491,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;
> @@ -497,6 +501,28 @@ ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
> }
> EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
>
> +void hsmp_metric_tbl_unmap_all(struct hsmp_plat_device *pdev, u16 num_sockets)
> +{
> + u16 i;
> +
> + if (!pdev->sock || !num_sockets)
> + return;
> +
> + for (i = 0; i < num_sockets; i++) {
> + struct hsmp_socket *s = &pdev->sock[i];
> +
> + if (s->metric_tbl_addr) {
> + iounmap(s->metric_tbl_addr);
> + s->metric_tbl_addr = NULL;
> + }
> + if (s->metric_lock_inited) {
> + mutex_destroy(&s->metric_tbl_lock);
> + s->metric_lock_inited = false;
> + }
> + }
> +}
> +EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_unmap_all, "AMD_HSMP");
> +
> int hsmp_get_tbl_dram_base(u16 sock_ind)
> {
> struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
> @@ -504,6 +530,31 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
> phys_addr_t dram_addr;
> int ret;
>
> + if (sock->metric_tbl_addr)
> + return 0;
> +
> + /*
> + * 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 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.
> + *
> + * Use non-devm mutex + ioremap so the shared sock[] array (ACPI
> + * allocates once for all sockets) can be torn down from a single
> + * refcounted release path without devm lifetime being tied to the
> + * wrong struct device.
> + */
> + if (!sock->metric_lock_inited) {
> + mutex_init(&sock->metric_tbl_lock);
> + sock->metric_lock_inited = true;
Perhaps it would be possible to arrange code such that booleans like this
are not required.
This problem may in part stem from using global array to store sock
instead of sock[] being just pointers to memory allocated dynamically
on probe and freed on remove, and in part from the init order.
> + }
This patch should probably be split into series so it's easier to
differentiate what belongs to which fix (or if something strictly belongs
to the ver7 ioctl additions and not the existing code, it should be sent
with them once the concurrency changes are made to come first).
> +
> 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;
> @@ -527,8 +578,7 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
> else
> hsmp_pdev.hsmp_table_size = sizeof(struct hsmp_metric_table);
>
> - sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
> - hsmp_pdev.hsmp_table_size);
> + sock->metric_tbl_addr = ioremap(dram_addr, 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 7343f8a1681f..5f5ad2885c22 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -15,7 +15,9 @@
> #include <linux/hwmon.h>
> #include <linux/kconfig.h>
> #include <linux/miscdevice.h>
> +#include <linux/mutex.h>
> #include <linux/pci.h>
> +#include <linux/refcount.h>
> #include <linux/semaphore.h>
> #include <linux/sysfs.h>
> #include <linux/types.h>
> @@ -42,11 +44,14 @@ 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];
> struct device *dev;
> u16 sock_ind;
> + bool metric_lock_inited;
> int (*amd_hsmp_rdwr)(struct hsmp_socket *sock, u32 off, u32 *val, bool rw);
> };
>
> @@ -55,7 +60,8 @@ struct hsmp_plat_device {
> struct hsmp_socket *sock;
> u32 proto_ver;
> u16 num_sockets;
> - bool is_probed;
> + refcount_t acpi_sock_refcnt;
> + bool acpi_misc_registered;
Don't you know this based on the reference count?
> size_t hsmp_table_size;
> };
>
> @@ -66,6 +72,7 @@ void hsmp_misc_deregister(void);
> int hsmp_misc_register(struct device *dev);
> int hsmp_get_tbl_dram_base(u16 sock_ind);
> ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size);
> +void hsmp_metric_tbl_unmap_all(struct hsmp_plat_device *pdev, u16 num_sockets);
> struct hsmp_plat_device *get_hsmp_pdev(void);
> #if IS_ENABLED(CONFIG_HWMON)
> int hsmp_create_sensor(struct device *dev, u16 sock_ind);
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index e07f68575055..2ef2102d0af3 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -230,6 +230,7 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
> static void hsmp_pltdrv_remove(struct platform_device *pdev)
> {
> hsmp_misc_deregister();
> + hsmp_metric_tbl_unmap_all(hsmp_pdev, hsmp_pdev->num_sockets);
> }
>
> static struct platform_driver amd_hsmp_driver = {
>
--
i.
^ permalink raw reply [flat|nested] 10+ messages in thread