Re: [PATCH 2/2] platform/x86/amd/hsmp: Add support for variable-sized metrics tables
From: Ramalingam, Muthusamy
Date: Wed Apr 01 2026 - 07:29:21 EST
Hi Ilpo,
Thank you for the suggestions. I will update version v2 shortly to address the comments listed below.
On 31-03-2026 02:13 pm, Ilpo Järvinen wrote:
On Thu, 5 Mar 2026, Muthusamy Ramalingam wrote:Sure.
Add support for the new metrics table format introduced in AMD Family 1Ah
Model 50h-5Fh processors with HSMP protocol version 7.
Use CPU family/model and protocol versions to map respective variable-sized
metric table configurations.
The exported hsmp_metric_tbl_read() function provides offset support for
variable-sized tables.
This patch should be split into multiple changes.
Thanks will replace.
Co-developed-by: Muralidhara M K <muralimk@xxxxxxx>
Signed-off-by: Muralidhara M K <muralimk@xxxxxxx>
Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@xxxxxxx>
---
arch/x86/include/uapi/asm/amd_hsmp.h | 86 ++++++++++++++++++++++++++++
drivers/platform/x86/amd/hsmp/acpi.c | 9 +--
drivers/platform/x86/amd/hsmp/hsmp.c | 83 ++++++++++++++++++++++-----
drivers/platform/x86/amd/hsmp/hsmp.h | 3 +-
drivers/platform/x86/amd/hsmp/plat.c | 3 +-
5 files changed, 164 insertions(+), 20 deletions(-)
diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h
index 603d62f8d4da..1daa7c5352f3 100644
--- a/arch/x86/include/uapi/asm/amd_hsmp.h
+++ b/arch/x86/include/uapi/asm/amd_hsmp.h
@@ -575,6 +575,92 @@ struct hsmp_metric_table {
__u32 gfxclk_frequency[8];
};
+#define F1A_M50_M5F_MAX_CORES_PER_CCD_32 32
+#define F1A_M50_M5F_MAX_FREQ_TABLE_SIZE 4
+#define F1A_M50_M5F_MAX_XGMI 8
+#define F1A_M50_M5F_MAX_PCIE 8
+#define F1A_M50_M5F_MAX_CCD 8
+
+/* Metrics table (supported only with proto version 7) */
+struct hsmp_metric_table_f1a_m50_5f_iod {
+ __u32 num_active_ccds;
+ __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[F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+
+ /* FREQUENCY RANGE */
+ __u32 fclk_frequency_table[F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+ __u32 uclk_frequency_table[F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+ __u32 ddr_rate_table[F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+ __u32 max_df_pstate_range;
+ __u32 min_df_pstate_range;
+ __u32 lclk_frequency_table[F1A_M50_M5F_MAX_FREQ_TABLE_SIZE];
+ __u32 max_lclk_dpm_range;
+ __u32 min_lclk_dpm_range;
+
+ /* XGMI */
+ __u64 xgmi_bit_rate[F1A_M50_M5F_MAX_XGMI];
+ __u64 xgmi_read_bandwidth[F1A_M50_M5F_MAX_XGMI];
+ __u64 xgmi_write_bandwidth[F1A_M50_M5F_MAX_XGMI];
+
+ /* 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[F1A_M50_M5F_MAX_PCIE];
+
+ /* 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_f1a_m50_5f_ccd {
+ __u32 core_apicid_of_thread0[F1A_M50_M5F_MAX_CORES_PER_CCD_32];
+ __u64 core_c0[F1A_M50_M5F_MAX_CORES_PER_CCD_32];
+ __u64 core_cc1[F1A_M50_M5F_MAX_CORES_PER_CCD_32];
+ __u64 core_cc6[F1A_M50_M5F_MAX_CORES_PER_CCD_32];
+ __u64 core_frequency[F1A_M50_M5F_MAX_CORES_PER_CCD_32];
+ __u64 core_frequency_effective[F1A_M50_M5F_MAX_CORES_PER_CCD_32];
+ __u64 core_power[F1A_M50_M5F_MAX_CORES_PER_CCD_32];
+};
+
+/*
+ * Future processors within the same family and model may support a
+ * variable number of CCDs and cores
+ */
+struct hsmp_metric_table_f1a_m50_5f {
+ struct hsmp_metric_table_f1a_m50_5f_iod iod;
+ struct hsmp_metric_table_f1a_m50_5f_ccd ccd[F1A_M50_M5F_MAX_CCD];
+};
+
/* Reset to default packing */
#pragma pack()
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 97ed71593bdf..c91b694bd394 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -238,13 +238,14 @@ 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);
- return hsmp_metric_tbl_read(sock, buf, count);
+ return hsmp_metric_tbl_read(sock, buf, count, off);
}
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)
+ if (hsmp_pdev->proto_ver == HSMP_PROTO_VER6 ||
+ hsmp_pdev->proto_ver == HSMP_PROTO_VER7)
return battr->attr.mode;
return 0;
@@ -491,7 +492,8 @@ 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 ||
+ hsmp_pdev->proto_ver == HSMP_PROTO_VER7) {
ret = hsmp_get_tbl_dram_base(sock_ind);
if (ret)
dev_info(dev, "Failed to init metric table\n");
@@ -509,7 +511,6 @@ static int init_acpi(struct device *dev)
static const struct bin_attribute hsmp_metric_tbl_attr = {
.attr = { .name = HSMP_METRICS_TABLE_NAME, .mode = 0444},
.read = hsmp_metric_tbl_acpi_read,
- .size = sizeof(struct hsmp_metric_table),
};
static const struct bin_attribute *hsmp_attr_list[] = {
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 19f82c1d3090..55b941f8a819 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -348,9 +348,22 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
return 0;
}
-ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size)
+/**
+ * hsmp_metric_tbl_read - Read metric table
+ *
+ * This function maintains ABI compatibility for external consumers.
+ * It reads from offset 0, which works for all metrics table formats.
+ * External modules using this function will continue to work without
+ * modification.
Please don't write history descriptions like this into comments. Only
focus on explain what it does now. The sentences like the last one belong
more into changelog text.
I will update using min() macro.+ *
+ * Return: number of bytes read or negative error code
+ */
+
+ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf,
+ size_t size, loff_t off)
{
struct hsmp_message msg = { 0 };
+ size_t var_size, remaining;
int ret;
if (!sock || !buf)
@@ -361,29 +374,37 @@ 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)) {
- dev_err(sock->dev, "Wrong buffer size\n");
+ if (off < 0 || off > hsmp_pdev.hsmp_table_size) {
+ dev_err(sock->dev, "Invalid offset\n");
return -EINVAL;
}
- msg.msg_id = HSMP_GET_METRIC_TABLE;
- msg.sock_ind = sock->sock_ind;
+ /* Compute remaining bytes using explicit cast to avoid signed/unsigned mixing */
+ remaining = hsmp_pdev.hsmp_table_size - (size_t)off;
+ var_size = min_t(size_t, size, remaining);
Both inputs are already size_t so you should use min() instead.
Sure will split size and offset seperately and update changelog accrodingly.
+ if (off == 0) {
+ msg.msg_id = HSMP_GET_METRIC_TABLE;
+ msg.sock_ind = sock->sock_ind;
- ret = hsmp_send_message(&msg);
- if (ret)
- return ret;
- memcpy_fromio(buf, sock->metric_tbl_addr, size);
+ ret = hsmp_send_message(&msg);
+ if (ret) {
+ dev_err(sock->dev, "Failed to send HSMP_GET_METRIC_TABLE, ret: %d\n", ret);
+ return ret;
+ }
+ }
+ memcpy_fromio(buf, (u8 __iomem *)sock->metric_tbl_addr + off, var_size);
This change which changes size/off handling should be made separately and
properly explain the changes in the interface in its changelog text.
I started to wonder how is concurrency control supposed to work withGood point. I'll review some scenarios and update if any locks are required.
these calls, there doesn't seem to be any locks taken?
Yes, We want this for unsupported family check.- return size;
+ return var_size;
}
EXPORT_SYMBOL_NS_GPL(hsmp_metric_tbl_read, "AMD_HSMP");
int hsmp_get_tbl_dram_base(u16 sock_ind)
{
struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
+ struct hsmp_message msg_tbl_ver = { 0 };
struct hsmp_message msg = { 0 };
phys_addr_t dram_addr;
+ u32 table_ver;
int ret;
msg.sock_ind = sock_ind;
@@ -403,8 +424,44 @@ int hsmp_get_tbl_dram_base(u16 sock_ind)
dev_err(sock->dev, "Invalid DRAM address for metric table\n");
return -ENOMEM;
}
- sock->metric_tbl_addr = devm_ioremap(sock->dev, dram_addr,
- sizeof(struct hsmp_metric_table));
+
+ /* Get metric table version */
+ msg_tbl_ver.sock_ind = sock_ind;
+ msg_tbl_ver.response_sz = hsmp_msg_desc_table[HSMP_GET_METRIC_TABLE_VER].response_sz;
+ msg_tbl_ver.msg_id = HSMP_GET_METRIC_TABLE_VER;
+
+ ret = hsmp_send_message(&msg_tbl_ver);
+ if (ret)
+ return ret;
+
+ table_ver = msg_tbl_ver.args[0];
+
+ hsmp_pdev.hsmp_table_size = 0;
Is this necessary?
Yes, since this is older support, I will split.
+ /* Determine metric table size based on CPU family/model and table version */
+ switch (boot_cpu_data.x86) {
+ case 0x1A:
+ if (boot_cpu_data.x86_model >= 0x50 &&
+ boot_cpu_data.x86_model <= 0x5F &&
+ table_ver == 0x00700000) {
+ hsmp_pdev.hsmp_table_size = sizeof(struct hsmp_metric_table_f1a_m50_5f);
+ }
+ break;
+ case 0x19:
+ if (boot_cpu_data.x86_model >= 0x90 &&
+ boot_cpu_data.x86_model <= 0x9F) {
Isn't this a new check compared with the old code? It should be added in
own patch as well.
+ hsmp_pdev.hsmp_table_size = sizeof(struct hsmp_metric_table);
+ }
+ break;
+ }
+
+ if (!hsmp_pdev.hsmp_table_size) {
+ dev_err(sock->dev,
+ "Metric table not supported for F%02Xh_M%02Xh (table version: 0x%08X)\n",
+ boot_cpu_data.x86, boot_cpu_data.x86_model, table_ver);
+ return -EOPNOTSUPP;
+ }
+
+ sock->metric_tbl_addr = devm_ioremap(sock->dev, 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 b153527e0a0d..a887eaa061e4 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);
@@ -63,7 +64,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
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);
+ssize_t hsmp_metric_tbl_read(struct hsmp_socket *sock, char *buf, size_t size, loff_t off);
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..9e37502defb2 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -59,7 +59,7 @@ static ssize_t hsmp_metric_tbl_plat_read(struct file *filp, struct kobject *kobj
sock = &hsmp_pdev->sock[sock_ind];
- return hsmp_metric_tbl_read(sock, buf, count);
+ return hsmp_metric_tbl_read(sock, buf, count, off);
}
static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
@@ -94,7 +94,6 @@ static const struct bin_attribute attr##index = { \
.attr = { .name = HSMP_METRICS_TABLE_NAME, .mode = 0444}, \
.private = (void *)index, \
.read = hsmp_metric_tbl_plat_read, \
- .size = sizeof(struct hsmp_metric_table), \
}; \
static const struct bin_attribute _list[] = { \
&attr##index, \