Re: [PATCH 1/7] platform/x86/amd/hsmp: Add HSMP client support for Family 1Ah
Mario Limonciello <[email protected]> Wed, 29 Jul 2026 16:27:25 -0500
| Newsgroups | org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 7/29/26 11:40, Muralidhara M K wrote: > Add non-ACPI HSMP mailbox support, Ryzen Master SMC client message > descriptors, and platform routing for the Family 1Ah client platforms: > > Models 80h-87h Medusa1 > Models 88h-8Fh Olympic Ridge > Models E0h-E3h Medusa2 > > All three share the same client mailbox addresses and message set, so a > single is_client_platform() predicate covers them, and the two adjacent > Model 8xh ranges are tested as one. > > The client parts drive a different mailbox from the server HSMP > interface. They expose no _DSD/_CRS to discover the mailbox, so the SMN > addresses are fixed in hsmp.h and shared by the platform and ACPI > probes, and they speak the Ryzen Master SMC (RMSMC) message set rather > than the server message IDs. Add that message set and its descriptor > table to the UAPI header. > > Two message descriptor tables now exist, so every lookup has to pick the > one matching the running platform. Rather than test the platform at each > lookup, describe the platform once: struct hsmp_plat_desc holds the > mailbox addresses, the descriptor table and its size, and the IDs of the > messages the driver issues itself. get_hsmp_pdev() resolves it, which > both drivers call before they bring any socket up and which runs on a > CPU that is fixed long before either can load. The server descriptor is > also the static initialiser, so the message plumbing cannot reach a NULL > descriptor even if a future caller arrives another way. > > get_msg_desc() then indexes the table behind that pointer, and > validate_message(), is_get_msg(), hsmp_ioctl_msg(), hsmp_test() and > hsmp_cache_proto_ver() read the bound and the message IDs from the > descriptor. get_msg_desc() returns NULL rather than indexing out of > range if a caller skipped the bounds check, and the Spectre v1 clamp in > hsmp_ioctl_msg() uses the running platform's table size. > > In init_platform_device() the mailbox setup becomes a copy of the > descriptor's addresses. The F1Ah M0h message-id offset stays behind > is_f1a_m0h() as an override, because it applies only to this non-ACPI > path while the descriptor is shared with the ACPI one. It cannot disturb > the client mailbox: it requires model <= 0x0F and the client models are > all >= 0x80. The server mailbox addresses move from plat.c to hsmp.h so > both address sets sit with the descriptors that consume them; the values > are unchanged, written to the same width as the client set they now sit > beside. > > Since this is the change that lets the platform driver match the client > models, it also has to let the client probe survive the interface-version > query. Some client SMU builds reject HSMP_CLIENT_GET_INTERFACE_VER with > "invalid message" even though the mailbox works, as the test message > issued just before it shows, so treat that failure as non-fatal on the > client and register the socket anyway. Nothing on the client depends on > the reported version: hsmp_pdev->proto_ver gates the metrics_bin > attribute and the protocol_version sysfs file, both of which describe > the server interface. The query stays fatal on server parts. > > The client descriptor table carries __attribute__((unused)), matching > hsmp_msg_desc_table[]. Both are defined in a UAPI header that several > translation units include without using the table, so without it > -Wunused-const-variable fires on plat.c, acpi.c and hwmon.c. > > Document the client platforms in Documentation/arch/x86/amd_hsmp.rst, > which until now described HSMP as available only on the EPYC server > parts and MI300A. The character device and ioctl interface are unchanged > there; what differs is the accepted message IDs, so the text points at > the HSMP_CLIENT_* set in the UAPI header. > > Signed-off-by: Muralidhara M K <[email protected]> Generic comment for the series. With such rich function comments, I think a lot of this would be really valuable to have as kdoc (/** */) and then reference in hsmp documentation. > --- > Documentation/arch/x86/amd_hsmp.rst | 18 +++- > arch/x86/include/uapi/asm/amd_hsmp.h | 137 +++++++++++++++++++++++++++ > drivers/platform/x86/amd/hsmp/hsmp.c | 125 ++++++++++++++++++++---- > drivers/platform/x86/amd/hsmp/hsmp.h | 73 ++++++++++++++ > drivers/platform/x86/amd/hsmp/plat.c | 39 ++++---- > 5 files changed, 354 insertions(+), 38 deletions(-) > > diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst > index fa1fc240e212..8a9d6fe7d4cf 100644 > --- a/Documentation/arch/x86/amd_hsmp.rst > +++ b/Documentation/arch/x86/amd_hsmp.rst > @@ -8,6 +8,21 @@ Newer Fam19h(model 0x00-0x1f, 0x30-0x3f, 0x90-0x9f, 0xa0-0xaf), > Fam1Ah(model 0x00-0x1f) EPYC server line of processors from AMD support > system management functionality via HSMP (Host System Management Port). > > +The Fam1Ah client platforms are supported as well: > + > + ============= ============= > + Model Platform > + ============= ============= > + 0x80-0x87 Medusa1 > + 0x88-0x8f Olympic Ridge > + 0xe0-0xe3 Medusa2 > + ============= ============= > + > +They share one mailbox and speak the Ryzen Master SMC message set instead > +of the server HSMP message set, so the message IDs accepted on them are > +the HSMP_CLIENT_* ones listed in arch/x86/include/uapi/asm/amd_hsmp.h. > +The character device and ioctl interface described below are the same. > + > The Host System Management Port (HSMP) is an interface to provide > OS-level software with access to system management functions via a > set of mailbox registers. > @@ -17,7 +32,8 @@ More details on the interface can be found in chapter > Eg: https://docs.amd.com/v/u/en-US/55898_B1_pub_0_50 > > > -HSMP interface is supported on EPYC line of server CPUs and MI300A (APU). > +HSMP interface is supported on EPYC line of server CPUs, MI300A (APU) and > +the Fam1Ah client platforms listed above. > > > HSMP device > diff --git a/arch/x86/include/uapi/asm/amd_hsmp.h b/arch/x86/include/uapi/asm/amd_hsmp.h > index eda336bfd3e9..b4c3ddb9d1c1 100644 > --- a/arch/x86/include/uapi/asm/amd_hsmp.h > +++ b/arch/x86/include/uapi/asm/amd_hsmp.h > @@ -664,4 +664,141 @@ struct hsmp_telemetry_data { > #define HSMP_IOCTL_GET_TELEMETRY_DATA \ > _IOW(HSMP_BASE_IOCTL_NR, 1, struct hsmp_telemetry_data) > > +/* > + * Client HSMP messages supported on the Family 1Ah client platforms: > + * Models 80h-87h (Medusa1), Models 88h-8Fh (Olympic Ridge) and > + * Models E0h-E3h (Medusa2). > + * These map to Ryzen Master SMC (RMSMC) message IDs. > + */ > +enum hsmp_client_message_ids { > + HSMP_CLIENT_TEST = 1, /* 01h Test message */ > + HSMP_CLIENT_GET_SMU_VER, /* 02h SMU FW version */ > + HSMP_CLIENT_GET_INTERFACE_VER, /* 03h Interface version */ > + HSMP_CLIENT_GET_METRICS_TABLE_VER, /* 04h Metrics table version */ > + HSMP_CLIENT_GET_METRICS_TABLE, /* 05h Metrics table log sample */ > + HSMP_CLIENT_GET_METRICS_TABLE_DRAM_ADDR,/* 06h Metrics table DRAM address */ > + HSMP_CLIENT_SET_CORE_PSM_MARGIN, /* 07h Set core PSM margin */ > + HSMP_CLIENT_SET_ALL_CORE_PSM_MARGIN, /* 08h Set all core PSM margin */ > + HSMP_CLIENT_SET_FAST_PPT_LIMIT, /* 09h Set fast PPT limit */ > + HSMP_CLIENT_SET_VRM_VDD_CURRENT_LIMIT, /* 0Ah Set VRM VDD current limit */ > + HSMP_CLIENT_SET_VRM_VDD_MAX_CURRENT_LIMIT,/* 0Bh Set VRM VDD max current limit */ > + HSMP_CLIENT_SET_TJ_MAX, /* 0Ch Set TjMax */ > + HSMP_CLIENT_SET_FIT_LIMIT_SCALAR, /* 0Dh Set FIT limit scalar */ > + HSMP_CLIENT_ENABLE_OVERCLOCKING, /* 0Eh Enable overclocking */ > + HSMP_CLIENT_DISABLE_OVERCLOCKING, /* 0Fh Disable overclocking */ > + HSMP_CLIENT_SET_OVERCLOCK_FREQ_ALL_CORES,/* 10h Set OC freq all cores */ > + HSMP_CLIENT_SET_OVERCLOCK_FREQ_PER_CORE,/* 11h Set OC freq per core */ > + HSMP_CLIENT_SET_OVERCLOCK_VID, /* 12h Set OC VID */ > + HSMP_CLIENT_SET_FCLK_OVERCLOCK_ON_THE_FLY,/* 13h Set FCLK OC on-the-fly */ > + HSMP_CLIENT_GET_CORE_PERF_ORDER, /* 14h Get core performance order */ > + HSMP_CLIENT_SET_SUSTAINED_POWER_LIMIT, /* 15h Set sustained power limit */ > + HSMP_CLIENT_SET_SLOW_PPT_LIMIT, /* 16h Set slow PPT limit */ > + HSMP_CLIENT_SET_VRM_GFX_MAX_CURRENT_LIMIT,/* 17h Set VRM GFX max current limit */ > + HSMP_CLIENT_SET_VRM_SOC_CURRENT_LIMIT, /* 18h Set VRM SOC current limit */ > + HSMP_CLIENT_SET_FAST_SPM_LIMIT, /* 19h Set fast SPM limit */ > + HSMP_CLIENT_SET_SLOW_SPM_LIMIT, /* 1Ah Set slow SPM limit */ > + HSMP_CLIENT_GET_CORE_PSM_MARGIN, /* 1Bh Get core PSM margin */ > + HSMP_CLIENT_GET_GFX_PSM_MARGIN, /* 1Ch Get GFX PSM margin */ > + HSMP_CLIENT_SPARE_0X1D, /* 1Dh Spare */ > + HSMP_CLIENT_SPARE_0X1E, /* 1Eh Spare */ > + HSMP_CLIENT_SPARE_0X1F, /* 1Fh Spare */ > + HSMP_CLIENT_SPARE_0X20, /* 20h Spare */ > + HSMP_CLIENT_SET_GFXCLK_OVERDRIVE_BY_FREQ_VID,/* 21h Set GfxClk overdrive by freq/VID */ > + HSMP_CLIENT_DISABLE_GFXCLK_OVERDRIVE, /* 22h Disable GfxClk overdrive */ > + HSMP_CLIENT_SET_GFX_PSM_MARGIN, /* 23h Set GFX PSM margin */ > + HSMP_CLIENT_SET_CCLK_FMAX_OFFSET, /* 24h Set CCLK Fmax offset */ > + HSMP_CLIENT_SET_CORE_POWER_LIMIT_OFFSET,/* 25h Set core power limit offset */ > + HSMP_CLIENT_ADD_EXTRA_PSM_GUARDBAND, /* 26h Add extra PSM guardband */ > + HSMP_CLIENT_ADD_EXTRA_PSM_GUARDBAND_GFX,/* 27h Add extra PSM guardband GFX */ > + HSMP_CLIENT_SET_GFXCLK_FMAX, /* 28h Set GfxClk Fmax */ > + HSMP_CLIENT_MSG_ID_MAX, > +}; > + > +/* Client message descriptor table (Family 1Ah Models 80h-8Fh, E0h-E3h) */ > +static const struct hsmp_msg_desc hsmp_client_msg_desc_table[] > + __attribute__((unused)) = { > + /* RESERVED */ > + {0, 0, HSMP_RSVD}, > + /* HSMP_CLIENT_TEST */ > + {1, 1, HSMP_GET}, > + /* HSMP_CLIENT_GET_SMU_VER */ > + {0, 1, HSMP_GET}, > + /* HSMP_CLIENT_GET_INTERFACE_VER */ > + {0, 1, HSMP_GET}, > + /* HSMP_CLIENT_GET_METRICS_TABLE_VER */ > + {0, 1, HSMP_GET}, > + /* HSMP_CLIENT_GET_METRICS_TABLE */ > + {0, 0, HSMP_GET}, > + /* HSMP_CLIENT_GET_METRICS_TABLE_DRAM_ADDR */ > + {0, 3, HSMP_GET}, > + /* HSMP_CLIENT_SET_CORE_PSM_MARGIN */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_ALL_CORE_PSM_MARGIN */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_FAST_PPT_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_VRM_VDD_CURRENT_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_VRM_VDD_MAX_CURRENT_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_TJ_MAX */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_FIT_LIMIT_SCALAR */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_ENABLE_OVERCLOCKING */ > + {0, 0, HSMP_SET}, > + /* HSMP_CLIENT_DISABLE_OVERCLOCKING */ > + {0, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_OVERCLOCK_FREQ_ALL_CORES */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_OVERCLOCK_FREQ_PER_CORE */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_OVERCLOCK_VID */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_FCLK_OVERCLOCK_ON_THE_FLY */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_GET_CORE_PERF_ORDER */ > + {1, 1, HSMP_GET}, > + /* HSMP_CLIENT_SET_SUSTAINED_POWER_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_SLOW_PPT_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_VRM_GFX_MAX_CURRENT_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_VRM_SOC_CURRENT_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_FAST_SPM_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_SLOW_SPM_LIMIT */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_GET_CORE_PSM_MARGIN */ > + {1, 1, HSMP_GET}, > + /* HSMP_CLIENT_GET_GFX_PSM_MARGIN */ > + {0, 1, HSMP_GET}, > + /* HSMP_CLIENT_SPARE_0X1D */ > + {0, 0, HSMP_RSVD}, > + /* HSMP_CLIENT_SPARE_0X1E */ > + {0, 0, HSMP_RSVD}, > + /* HSMP_CLIENT_SPARE_0X1F */ > + {0, 0, HSMP_RSVD}, > + /* HSMP_CLIENT_SPARE_0X20 */ > + {0, 0, HSMP_RSVD}, > + /* HSMP_CLIENT_SET_GFXCLK_OVERDRIVE_BY_FREQ_VID */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_DISABLE_GFXCLK_OVERDRIVE */ > + {0, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_GFX_PSM_MARGIN */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_CCLK_FMAX_OFFSET */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_CORE_POWER_LIMIT_OFFSET */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_ADD_EXTRA_PSM_GUARDBAND */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_ADD_EXTRA_PSM_GUARDBAND_GFX */ > + {1, 0, HSMP_SET}, > + /* HSMP_CLIENT_SET_GFXCLK_FMAX */ > + {1, 0, HSMP_SET}, > +}; How would you feel about adding a static (compile time) assert that the number of entries here matched the number of enums? This can avoid making a mistake later. > + > #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 5e123a4ecea9..9b65dbd47965 100644 > --- a/drivers/platform/x86/amd/hsmp/hsmp.c > +++ b/drivers/platform/x86/amd/hsmp/hsmp.c > @@ -45,7 +45,67 @@ > */ > #define CHECK_GET_BIT BIT(31) > > -static struct hsmp_plat_device hsmp_pdev; > +static const struct hsmp_mbaddr_info hsmp_mbinfo_server = { > + .base_addr = SMN_HSMP_BASE, > + .msg_id_off = SMN_HSMP_MSG_ID, > + .msg_resp_off = SMN_HSMP_MSG_RESP, > + .msg_arg_off = SMN_HSMP_MSG_DATA, > +}; > + > +/* RM: Ryzen Master, the SMC interface these client addresses belong to */ > +static const struct hsmp_mbaddr_info hsmp_mbinfo_client = { > + .base_addr = SMN_HSMP_BASE_RM, > + .msg_id_off = SMN_HSMP_MSG_ID_RM, > + .msg_resp_off = SMN_HSMP_MSG_RESP_RM, > + .msg_arg_off = SMN_HSMP_MSG_DATA_RM, > +}; > + > +/* > + * The descriptors carry only the message IDs the driver issues itself, which > + * are also the only ones the two message sets have in common. Messages named > + * by userspace in an ioctl need no entry here, as that path already resolves > + * the ID against the running platform's table. > + */ > +static const struct hsmp_plat_desc hsmp_desc_server = { > + .mbinfo = &hsmp_mbinfo_server, > + .msg_desc = hsmp_msg_desc_table, > + .num_msgs = HSMP_MSG_ID_MAX, > + .test_msg = HSMP_TEST, > + .proto_ver_msg = HSMP_GET_PROTO_VER, > +}; > + > +/* The client drives a different mailbox with the Ryzen Master SMC message set */ > +static const struct hsmp_plat_desc hsmp_desc_client = { > + .mbinfo = &hsmp_mbinfo_client, > + .msg_desc = hsmp_client_msg_desc_table, > + .num_msgs = HSMP_CLIENT_MSG_ID_MAX, > + .test_msg = HSMP_CLIENT_TEST, > + .proto_ver_msg = HSMP_CLIENT_GET_INTERFACE_VER, > +}; > + > +static struct hsmp_plat_device hsmp_pdev = { > + /* > + * Overridden for the client in get_hsmp_pdev(), which every driver > + * calls before it touches a socket. Defaulting it here rather than > + * leaving it NULL keeps the message plumbing safe for any caller that > + * arrives without going through there. > + */ > + .desc = &hsmp_desc_server, > +}; > + > +/* > + * Callers must bounds-check msg_id against desc->num_msgs first; this returns > + * NULL rather than indexing out of range if they did not. > + */ > +static inline const struct hsmp_msg_desc *get_msg_desc(u32 msg_id) > +{ > + const struct hsmp_plat_desc *desc = hsmp_pdev.desc; > + > + if (msg_id >= desc->num_msgs) > + return NULL; > + > + return &desc->msg_desc[msg_id]; > +} > > /* > * Gates the AMD HSMP data plane against socket bring-up and teardown. > @@ -184,30 +244,37 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms > > static int validate_message(struct hsmp_message *msg) > { > + const struct hsmp_msg_desc *desc; > + > /* msg_id against valid range of message IDs */ > - if (msg->msg_id < HSMP_TEST || msg->msg_id >= HSMP_MSG_ID_MAX) > + if (msg->msg_id < hsmp_pdev.desc->test_msg || > + msg->msg_id >= hsmp_pdev.desc->num_msgs) > + return -ENOMSG; > + > + desc = get_msg_desc(msg->msg_id); > + if (!desc) > return -ENOMSG; > > /* msg_id is a reserved message ID */ > - if (hsmp_msg_desc_table[msg->msg_id].type == HSMP_RSVD) > + if (desc->type == HSMP_RSVD) > return -ENOMSG; > > /* > * num_args passed by user should match the num_args specified in > * message description table. > */ > - if (msg->num_args != hsmp_msg_desc_table[msg->msg_id].num_args) > + if (msg->num_args != desc->num_args) > return -EINVAL; > > /* > * 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. > + * words than what the current descriptor table defines are still > + * accepted, while rejecting requests that exceed the hardware > + * capability. > */ > - if (msg->response_sz > hsmp_msg_desc_table[msg->msg_id].response_sz) > + if (msg->response_sz > desc->response_sz) > return -EINVAL; > > return 0; > @@ -316,7 +383,7 @@ int hsmp_test(u16 sock_ind, u32 value) > * Test the hsmp port by performing TEST command. The test message > * takes one argument and returns the value of that argument + 1. > */ > - msg.msg_id = HSMP_TEST; > + msg.msg_id = hsmp_pdev.desc->test_msg; > msg.num_args = 1; > msg.response_sz = 1; > msg.args[0] = value; > @@ -340,10 +407,15 @@ EXPORT_SYMBOL_NS_GPL(hsmp_test, "AMD_HSMP"); > > static bool is_get_msg(struct hsmp_message *msg) > { > - if (hsmp_msg_desc_table[msg->msg_id].type == HSMP_GET) > + const struct hsmp_msg_desc *desc = get_msg_desc(msg->msg_id); > + > + if (!desc) > + return false; > + > + if (desc->type == HSMP_GET) > return true; > > - if (hsmp_msg_desc_table[msg->msg_id].type == HSMP_SET_GET && > + if (desc->type == HSMP_SET_GET && > (msg->args[0] & CHECK_GET_BIT)) > return true; > > @@ -354,6 +426,7 @@ static long hsmp_ioctl_msg(struct file *fp, unsigned long arg) > { > int __user *arguser = (int __user *)arg; > struct hsmp_message msg = { 0 }; > + const struct hsmp_msg_desc *desc; > int ret; > > if (copy_struct_from_user(&msg, sizeof(msg), arguser, sizeof(struct hsmp_message))) > @@ -361,23 +434,28 @@ static long hsmp_ioctl_msg(struct file *fp, unsigned long arg) > > /* > * Check msg_id is within the range of supported msg ids > - * i.e within the array bounds of hsmp_msg_desc_table > + * i.e within the array bounds of the platform's descriptor table > */ > - if (msg.msg_id < HSMP_TEST || msg.msg_id >= HSMP_MSG_ID_MAX) > + if (msg.msg_id < hsmp_pdev.desc->test_msg || > + msg.msg_id >= hsmp_pdev.desc->num_msgs) > 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() / > + * CPU speculatively use msg_id as an index into the message > + * descriptor tables (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); > + msg.msg_id = array_index_nospec(msg.msg_id, hsmp_pdev.desc->num_msgs); > + > + desc = get_msg_desc(msg.msg_id); > + if (!desc || desc->type == HSMP_RSVD) > + return -ENOMSG; > > switch (fp->f_mode & (FMODE_WRITE | FMODE_READ)) { > case FMODE_WRITE: > @@ -410,7 +488,7 @@ static long hsmp_ioctl_msg(struct file *fp, unsigned long arg) > if (ret) > return ret; > > - if (hsmp_msg_desc_table[msg.msg_id].response_sz > 0) { > + if (desc->response_sz > 0) { > /* Copy results back to user for get/monitor commands */ > if (copy_to_user(arguser, &msg, sizeof(struct hsmp_message))) > return -EFAULT; > @@ -687,9 +765,9 @@ int hsmp_cache_proto_ver(u16 sock_ind) > struct hsmp_message msg = { 0 }; > int ret; > > - msg.msg_id = HSMP_GET_PROTO_VER; > + msg.msg_id = hsmp_pdev.desc->proto_ver_msg; > msg.sock_ind = sock_ind; > - msg.response_sz = hsmp_msg_desc_table[HSMP_GET_PROTO_VER].response_sz; > + msg.response_sz = get_msg_desc(msg.msg_id)->response_sz; > > ret = hsmp_send_message_locked(&msg); > if (!ret) > @@ -735,6 +813,15 @@ EXPORT_SYMBOL_NS_GPL(hsmp_misc_deregister, "AMD_HSMP"); > > struct hsmp_plat_device *get_hsmp_pdev(void) > { > + /* > + * Both drivers call this before they bring any socket up, and the CPU > + * this selects on is fixed long before either can load, so resolving > + * the platform description here is enough to have it in place for > + * every mailbox access and for the probe paths themselves. > + */ > + if (is_client_platform()) > + hsmp_pdev.desc = &hsmp_desc_client; > + > return &hsmp_pdev; > } > EXPORT_SYMBOL_NS_GPL(get_hsmp_pdev, "AMD_HSMP"); > diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h > index 8dbff16a87b1..b4bd9800dee1 100644 > --- a/drivers/platform/x86/amd/hsmp/hsmp.h > +++ b/drivers/platform/x86/amd/hsmp/hsmp.h > @@ -10,6 +10,8 @@ > #ifndef HSMP_H > #define HSMP_H > > +#include <asm/amd/hsmp.h> > + > #include <linux/compiler_types.h> > #include <linux/device.h> > #include <linux/hwmon.h> > @@ -17,6 +19,7 @@ > #include <linux/miscdevice.h> > #include <linux/mutex.h> > #include <linux/pci.h> > +#include <linux/processor.h> > #include <linux/rwsem.h> > #include <linux/semaphore.h> > #include <linux/sysfs.h> > @@ -32,6 +35,51 @@ > > #define DRIVER_VERSION "2.6" You're adding new IOCTL's in patches 5 and 6, does this version need to bump for userspace to discover? > > +/* > + * To access a specific HSMP mailbox register, s/w writes the SMN address of the > + * HSMP mailbox register into the SMN_INDEX register, and reads/writes the > + * SMN_DATA reg. Below are the SMN addresses of the HSMP mailbox registers in > + * SMU address space, as consumed by the fixed mailbox descriptors in hsmp.c. > + * > + * The server set is used by the non-ACPI path only; the ACPI path discovers the > + * equivalent values from _CRS/_DSD. The _RM set is the client one, named for > + * the Ryzen Master SMC (RMSMC) interface whose mailbox it addresses, and is > + * used by both paths because the client exposes no _CRS/_DSD to discover them > + * from. The two sets share a base and differ only in the register offsets > + * behind it, but are kept separate so each stays tied to the platform it was > + * documented for. > + */ > +#define SMN_HSMP_BASE 0x03B00000 > +#define SMN_HSMP_MSG_ID 0x00010534 > +#define SMN_HSMP_MSG_ID_F1A_M0H 0x00010934 > +#define SMN_HSMP_MSG_RESP 0x00010980 > +#define SMN_HSMP_MSG_DATA 0x000109E0 > + > +/* _RM: Ryzen Master SMC, the client mailbox */ > +#define SMN_HSMP_BASE_RM 0x03B00000 > +#define SMN_HSMP_MSG_ID_RM 0x00010A8C > +#define SMN_HSMP_MSG_RESP_RM 0x00010A90 > +#define SMN_HSMP_MSG_DATA_RM 0x00010A94 Although these are the same for these 3 platforms, they might not be the same for the follow on ones. Have you seen the work that's going into the AMD_PMF driver to build structs of register lists? Take a look at this. https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=review-ilpo-next&id=e860e561926bf75e323d43fe59a9bf25d2ef33a3 > + > +/* > + * Family 1Ah client platforms use the Ryzen Master SMC (client) mailbox and > + * message set instead of the server HSMP interface: > + * > + * Models 80h-87h Medusa1 > + * Models 88h-8Fh Olympic Ridge > + * Models E0h-E3h Medusa2 > + * > + * They all share the SMN_HSMP_*_RM mailbox addresses above and the same > + * message set, so one predicate covers them and the two adjacent Model 8xh > + * ranges are tested as one. > + */ > +static inline bool is_client_platform(void) > +{ > + return boot_cpu_data.x86 == 0x1A && > + ((boot_cpu_data.x86_model >= 0x80 && boot_cpu_data.x86_model <= 0x8F) || > + (boot_cpu_data.x86_model >= 0xE0 && boot_cpu_data.x86_model <= 0xE3)); > +} Rather than enumerating lists of CPU IDs, could you instead go off the ACPI ID? Or are these identical from server to client? > + > struct hsmp_mbaddr_info { > u32 base_addr; > u32 msg_id_off; > @@ -40,6 +88,30 @@ struct hsmp_mbaddr_info { > u32 size; > }; > > +/* > + * Per-platform description of the mailbox and the message set behind it. > + * Resolved once from the running CPU and reached through hsmp_pdev->desc, so > + * neither the message plumbing nor the probe paths have to re-derive which > + * platform they are on. > + * > + * @mbinfo: fixed SMN mailbox addresses. The ACPI path uses these > + * only where firmware exposes no _CRS/_DSD to read them > + * from, i.e. on the client. > + * @msg_desc: message descriptor table, indexed by message ID. > + * @num_msgs: number of entries in @msg_desc, and so the exclusive > + * upper bound on a valid message ID. > + * @test_msg: no-op message used to probe the mailbox. Also the > + * lowest valid message ID, as ID 0 is reserved. > + * @proto_ver_msg: returns the interface version. > + */ > +struct hsmp_plat_desc { > + const struct hsmp_mbaddr_info *mbinfo; > + const struct hsmp_msg_desc *msg_desc; > + u32 num_msgs; > + u32 test_msg; > + u32 proto_ver_msg; > +}; > + > struct hsmp_socket { > struct bin_attribute hsmp_attr; > struct hsmp_mbaddr_info mbinfo; > @@ -59,6 +131,7 @@ struct hsmp_socket { > struct hsmp_plat_device { > struct miscdevice mdev; > struct hsmp_socket *sock; > + const struct hsmp_plat_desc *desc; > u32 proto_ver; > u16 num_sockets; > }; > diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c > index e9b2b809c0f5..7423c98a7b3e 100644 > --- a/drivers/platform/x86/amd/hsmp/plat.c > +++ b/drivers/platform/x86/amd/hsmp/plat.c > @@ -29,17 +29,6 @@ > > #define DRIVER_NAME "amd_hsmp" > > -/* > - * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox > - * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg. > - * Below are required SMN address for HSMP Mailbox register offsets in SMU address space > - */ > -#define SMN_HSMP_BASE 0x3B00000 > -#define SMN_HSMP_MSG_ID 0x0010534 > -#define SMN_HSMP_MSG_ID_F1A_M0H 0x0010934 > -#define SMN_HSMP_MSG_RESP 0x0010980 > -#define SMN_HSMP_MSG_DATA 0x00109E0 > - > static struct hsmp_plat_device *hsmp_pdev; > > static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset, > @@ -157,20 +146,18 @@ static int init_platform_device(struct device *dev) > sock = &hsmp_pdev->sock[i]; > sock->sock_ind = i; > sock->dev = dev; > - sock->mbinfo.base_addr = SMN_HSMP_BASE; > sock->amd_hsmp_rdwr = amd_hsmp_pci_rdwr; > + sock->mbinfo = *hsmp_pdev->desc->mbinfo; > > /* > * This is a transitional change from non-ACPI to ACPI, only > * family 0x1A, model 0x00 platform is supported for both ACPI and non-ACPI. > + * It is the one part whose message-id register sits at a > + * different offset on this path than the descriptor gives. > */ > if (is_f1a_m0h()) > sock->mbinfo.msg_id_off = SMN_HSMP_MSG_ID_F1A_M0H; > - else > - sock->mbinfo.msg_id_off = SMN_HSMP_MSG_ID; > > - sock->mbinfo.msg_resp_off = SMN_HSMP_MSG_RESP; > - sock->mbinfo.msg_arg_off = SMN_HSMP_MSG_DATA; > sema_init(&sock->hsmp_sem, 1); > > /* Test the hsmp interface on each socket */ > @@ -184,8 +171,18 @@ static int init_platform_device(struct device *dev) > > ret = hsmp_cache_proto_ver(i); > 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, as the test message above just > + * showed. Keep going there instead of failing the > + * probe: proto_ver only gates server-side features. > + */ > + if (!is_client_platform()) { > + dev_err(dev, "Failed to read HSMP protocol version\n"); > + return ret; > + } > + dev_warn(dev, "Interface version query unsupported on client SMU; continuing\n"); Shouldn't this be fatal? Can we fix the F/W to follow the expected HSMP driver behavior? > } > > if (hsmp_pdev->proto_ver == HSMP_PROTO_VER6) { > @@ -317,6 +314,12 @@ static bool legacy_hsmp_support(void) > case 0x1A: > switch (boot_cpu_data.x86_model) { > case 0x00 ... 0x0F: > + /* > + * Client platforms: 80h-87h (Medusa1), 88h-8Fh (Olympic > + * Ridge), E0h-E3h (Medusa2) > + */ > + case 0x80 ... 0x8F: > + case 0xE0 ... 0xE3: > return true; > default: > return false;