Re: [PATCH 3/7] platform/x86/amd/hsmp: Add ACPI client support via the SMNR method
From: M K, Muralidhara
Date: Thu Jul 30 2026 - 06:21:42 EST
On 7/30/2026 3:00 AM, Mario Limonciello wrote:
Mechanically it can - I built that variant and the mailbox does come up
On 7/29/26 11:40, Muralidhara M K wrote:
On the Family 1Ah client platforms (Models 80h-8Fh and E0h-E3h) the ACPI
HSMP device (HID AMDI0097, ACPI path \_SB_.TELD) is present but, unlike
server platforms, exposes neither a _CRS memory window nor a _DSD
mailbox-offset package, and names its socket differently. The probe
therefore failed in three places:
- hsmp_get_uid() requires the server "IDXX" _UID form and rejects
anything shorter than three characters. The client device carries a
bare socket number ("0"), so the probe aborted with -EINVAL before
it reached any of the code below, and without logging a reason.
- hsmp_read_acpi_crs()/_dsd() found no _CRS/_DSD and bailed out.
- hsmp_cache_proto_ver() issued HSMP_CLIENT_GET_INTERFACE_VER
(msg 0x3), which this client SMU rejects with "invalid message"
(0xFE) even though the mailbox is otherwise functional.
Handle the client explicitly:
- hsmp_get_uid(): strip the "ID" prefix only when it is present, so
both the server "IDXX" form and a bare socket number are accepted.
A _UID that is neither still fails, as kstrtou16() rejects it.
- hsmp_parse_acpi_table(): for is_client_platform(), skip _CRS/_DSD
and take the fixed client SMN mailbox addresses from the platform
descriptor, accessed through firmware. Reads go via the read-only
SMNR ACPI method
(TELD.SMNR -> CpmReadSmnRegister, the same accessor used by the
AMDHSMP Windows driver's amdtelemetry_Acpi_SMNR / 'RNMS'); writes
go via the kernel SMN helper (amd_smn_hsmp_rdwr), as this device
has no ACPI SMN write method.
- init_acpi(): treat the interface-version query as non-fatal on the
client, and call hsmp_get_tbl_dram_base() whatever version was
reported so the metric table is fetched with
HSMP_CLIENT_GET_METRICS_TABLE_DRAM_ADDR, mirroring plat.c.
As on the server path, the client branch publishes sock->dev last with
smp_store_release(). sock->dev is the readiness gate the data plane
tests with smp_load_acquire(), so it must not become visible before
the accessor and mailbox offsets that this socket's mailbox access
depends on.
is_client_platform() covers every Family 1Ah client platform (Medusa1,
Olympic Ridge and Medusa2), which all share the same client SMN
mailbox, so one branch handles them. Server behaviour (pure
_DSD/_CRS/MMIO) is unchanged: the _UID handling still strips the "ID"
prefix those platforms use.
Why even bother with the ACPI one? Couldn't amd_smn_hsmp_rdwr() handle both read and write?
on the Model 80h system here. But it drops the one thing firmware still
provides on this part, so I would like to keep SMNR for reads. v1 did not explain why it was there, which is a fair reason for the question; v2's commit message now does.
The reason is the missing _CRS. On the server the mailbox is an MMIO
window: firmware describes it in _CRS, the driver maps it, and every
access lands inside a region firmware has vouched for. The client
supports no MMIO window at all - there is no _CRS to read and nothing to
map - so the mailbox has to be reached in SMN register space at fixed
addresses that no firmware object describes.
SMNR is what closes that gap. It is the BIOS method that validates the
offset it is handed before returning the register, so it is the client
counterpart of the _CRS window the server path relies on. Going straight
to amd_smn_hsmp_rdwr() for reads would mean the client is the one path
that reaches its mailbox with no firmware validation anywhere in it,
against hardcoded addresses. Keeping the read side on SMNR also keeps us
consistent with the accessor AMD firmware exposes for this device and
that the Windows driver uses, so a BIOS change to the telemetry window is picked up on both.
The asymmetry you would be right to point at is writes. The device
exposes no ACPI write method, so the message ID, argument and status
writes in the handshake do go through amd_smn_hsmp_rdwr(). I am not
claiming the write side is validated - it cannot be until firmware offers a method for it. What SMNR buys today is that the addresses we read are confirmed by BIOS rather than only by our own table, and if a future BIOS adds a write counterpart the accessor is already routed through firmware.
If you would still rather not carry the ACPI round trip and the error
path for that, say so and I will switch both directions to
amd_smn_hsmp_rdwr(); the code is tested and it is a smaller patch. I did
not want to make that call unilaterally, because it changes the client
from firmware-mediated access to raw SMN.
Signed-off-by: Muralidhara M K <muralidhara.mk@xxxxxxx>
---
drivers/platform/x86/amd/hsmp/acpi.c | 125 +++++++++++++++++++++++++--
1 file changed, 116 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/ x86/amd/hsmp/acpi.c
index 8257cd1da48e..bbf2b9a8a408 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -10,6 +10,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <asm/amd/hsmp.h>
+#include <asm/amd/node.h>
#include <linux/acpi.h>
#include <linux/array_size.h>
@@ -71,6 +72,68 @@ static int amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
return 0;
}
+/*
+ * Family 1Ah client platforms (Models 80h-8Fh and E0h-E3h) expose the HSMP
+ * ACPI device (HID AMDI0097, ACPI path \_SB_.TELD) but, unlike server
+ * platforms, the device provides neither a _CRS memory window nor a _DSD
+ * mailbox-offset package. The mailbox lives in SMN register space and is
+ * reached through firmware:
+ *
+ * - Reads : the read-only ACPI method SMNR(addr) returns the 32-bit
+ * value of the SMN register at absolute SMN address 'addr'
+ * (TELD.SMNR -> CpmReadSmnRegister). This is the ACPI BIOS
+ * telemetry accessor referenced by the AMDHSMP Windows driver
+ * (amdtelemetry_Acpi_SMNR, method 'RNMS').
+ * - Writes : there is no corresponding ACPI write method, so SMN writes
+ * are issued through the kernel SMN helper (amd_smn_hsmp_rdwr).
+ *
+ * The SMN mailbox addresses themselves cannot be discovered from firmware
+ * on the client (no _DSD), so they are fixed; the SMN_HSMP_*_RM
+ * addresses are defined in hsmp.h and shared with the platform driver
+ * (plat.c).
+ */
+
+/* ACPI method on \_SB_.TELD that reads one SMN register: SMNR(addr) */
+#define HSMP_ACPI_SMNR_METHOD "SMNR"
+
+static int hsmp_acpi_smnr_read(struct hsmp_socket *sock, u32 smn_addr, u32 *value)
+{
+ struct acpi_object_list arg_list;
+ union acpi_object arg;
+ unsigned long long out;
+ acpi_status status;
+
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = smn_addr;
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+
+ status = acpi_evaluate_integer(ACPI_HANDLE(sock->dev),
+ HSMP_ACPI_SMNR_METHOD, &arg_list, &out);
+ if (ACPI_FAILURE(status)) {
+ dev_err_ratelimited(sock->dev,
+ "SMNR(0x%08x) ACPI method failed: %s\n",
+ smn_addr, acpi_format_exception(status));
+ return -EIO;
+ }
+
+ *value = (u32)out;
+ return 0;
+}
+
+static int amd_hsmp_acpi_smn_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write)
+{
+ u32 smn_addr = sock->mbinfo.base_addr + offset;
+
+ /* Read path uses the firmware SMNR ACPI BIOS method. */
+ if (!write)
+ return hsmp_acpi_smnr_read(sock, smn_addr, value);
+
+ /* No ACPI write method exists; drive SMN writes through the kernel. */
+ return amd_smn_hsmp_rdwr(sock->sock_ind, smn_addr, value, true);
+}
+
/* This is the UUID used for HSMP */
static const guid_t acpi_hsmp_uuid = GUID_INIT(0xb74d619d, 0x5707, 0x48bd,
0xa6, 0x9f, 0x4e, 0xa2,
@@ -89,15 +152,19 @@ static inline int hsmp_get_uid(struct device *dev, u16 *sock_ind)
char *uid;
/*
- * UID (ID00, ID01..IDXX) is used for differentiating sockets,
- * read it and strip the "ID" part of it and convert the remaining
- * bytes to integer.
+ * Server firmware differentiates the sockets with "ID00", "ID01"..
+ * "IDXX", so strip the "ID" before converting the rest. The client
+ * device carries a bare socket number with no prefix to strip, so
+ * only skip one when it is actually there.
*/
uid = acpi_device_uid(ACPI_COMPANION(dev));
- if (!uid || strlen(uid) < 3)
+ if (!uid)
return -EINVAL;
- return kstrtou16(uid + 2, 10, sock_ind);
+ if (!strncmp(uid, "ID", 2))
+ uid += 2;
+
+ return kstrtou16(uid, 10, sock_ind);
}
static acpi_status hsmp_resource(struct acpi_resource *res, void *data)
@@ -240,12 +307,35 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
int ret;
sock->sock_ind = sock_ind;
- sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr;
sema_init(&sock->hsmp_sem, 1);
dev_set_drvdata(dev, sock);
+ /*
+ * On the Family 1Ah client platforms the bound ACPI device
+ * (\_SB_.TELD) has no _CRS/_DSD, so use the fixed client SMN mailbox
+ * addresses and access them via firmware - reads through the SMNR
+ * ACPI BIOS method, writes through the kernel SMN helper.
+ */
+ if (is_client_platform()) {
+ sock->amd_hsmp_rdwr = amd_hsmp_acpi_smn_rdwr;
+ sock->mbinfo = *hsmp_pdev->desc->mbinfo;
+ dev_info(dev,
+ "Client platform: SMN mailbox via SMNR ACPI method (reads) + kernel SMN (writes)\n");
+ /*
+ * Publish sock->dev last, for the same reason as the server
+ * path below: it is the readiness gate for the data plane, so
+ * it must not become visible before the accessor and the
+ * mailbox offsets this socket needs.
+ */
+ smp_store_release(&sock->dev, dev);
+
+ return 0;
+ }
+
+ sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr;
+
/* Read MP1 base address from CRS method */
ret = hsmp_read_acpi_crs(dev, sock);
if (ret)
@@ -553,11 +643,28 @@ static int init_acpi(struct device *dev)
ret = hsmp_cache_proto_ver(sock_ind);
if (ret) {
- dev_err(dev, "Failed to read HSMP protocol version\n");
- return ret;
+ /*
+ * Some client SMU builds reject the interface-version query
+ * with "invalid message" even though the mailbox is functional
+ * (the preceding test message succeeds). The client does not
+ * need the version to reach its metric table, so treat this as
+ * non-fatal there, matching the platform driver.
+ */
+ if (is_client_platform()) {
+ dev_warn(dev,
+ "Interface version query unsupported on client SMU; continuing\n");
+ } else {
+ dev_err(dev, "Failed to read HSMP protocol version\n");
+ return ret;
+ }
}
- if (hsmp_pdev->proto_ver >= HSMP_PROTO_VER6) {
+ /*
+ * On client parts the metric table is fetched via
+ * HSMP_CLIENT_GET_METRICS_TABLE_DRAM_ADDR whatever interface version
+ * was reported, since that query may be unsupported, mirroring plat.c.
+ */
+ if (is_client_platform() || 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");