Re: [PATCH 07/11] platform/x86/amd/hsmp: Add per-platform bitmap to validate HSMP messages

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 8 Oct 2026, Suma Hegde wrote:

> From: Suma Hegde <Suma.Hegde@amd.com>
> 
> The HSMP driver previously allowed sending any message defined in
> amd_hsmp.h, including ones not supported on the running platform.
> 
> Add read and write bitmaps per socket to store which HSMP messages
> are enabled on the platform. Add validation in validate_message()
> to reject unsupported messages before they reach the SMU.
> 
> Key changes:
> - Introduce enabled_read_msg_ids and enabled_write_msg_ids bitmaps in
>   struct hsmp_socket.
> - Add is_read_msg_enabled() and is_write_msg_enabled() helpers in
>   hsmp.h.
> - Use is_get_msg() to distinguish read and write HSMP messages.
> - Mark HSMP_TEST, HSMP_GET_PROTO_VER, and HSMP_GET_SMU_VER as always
>   enabled during bitmap initialization, as these cannot be masked by
>   BIOS.
> - Skip bitmap init, populate, and validation on client platforms; the
>   client message set has no per-platform bitmaps yet.
> 
> Co-developed-by: Dhruv Gupta <dhruv.gupta@amd.com>
> Signed-off-by: Dhruv Gupta <dhruv.gupta@amd.com>
> Co-developed-by: Muthusamy Ramalingam <Muthusamy.Ramalingam@amd.com>
> Signed-off-by: Muthusamy Ramalingam <Muthusamy.Ramalingam@amd.com>
> Signed-off-by: Suma Hegde <Suma.Hegde@amd.com>
> ---
>  drivers/platform/x86/amd/hsmp/acpi.c | 13 ++++
>  drivers/platform/x86/amd/hsmp/hsmp.c | 94 ++++++++++++++++++++++++++++
>  drivers/platform/x86/amd/hsmp/hsmp.h | 27 ++++++++
>  drivers/platform/x86/amd/hsmp/plat.c | 13 ++++
>  4 files changed, 147 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index b6f54e8b91663..0d3b324fe2244 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -566,6 +566,11 @@ static int init_acpi(struct device *dev)
>  
>  	sock = &hsmp_pdev->sock[sock_ind];
>  
> +	if (!is_client_platform()) {
> +		init_read_enabled_msg_bitmap(sock);
> +		init_write_enabled_msg_bitmap(sock);
> +	}
> +
>  	/* Test the hsmp interface */
>  	ret = hsmp_test(sock_ind, 0xDEADBEEF);
>  	if (ret) {
> @@ -581,6 +586,14 @@ static int init_acpi(struct device *dev)
>  		return ret;
>  	}
>  
> +	if (!is_client_platform()) {
> +		ret = hsmp_populate_msg_bitmaps(sock_ind);
> +		if (ret) {
> +			dev_err(dev, "Failed to get enabled HSMP messages on this platform\n");
> +			return ret;
> +		}
> +	}
> +
>  	if ((is_client_platform() &&
>  	     sock->proto_ver >= RYZEN_MASTER_PROTO_VER1) ||
>  	    (!is_client_platform() &&
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 8dc1fc75bd9ba..89528632c9532 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -11,6 +11,7 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/array_size.h>
> +#include <linux/bitmap.h>
>  #include <linux/build_bug.h>
>  #include <linux/cleanup.h>
>  #include <linux/compiler.h>
> @@ -285,6 +286,8 @@ static int validate_message(struct hsmp_message *msg)
>  {
>  	const struct hsmp_plat_desc *plat_desc = hsmp_desc();
>  	const struct hsmp_msg_desc *desc;
> +	struct hsmp_socket *sock;
> +	unsigned int sock_ind;
>  
>  	if (!plat_desc->msg_desc) {
>  		/*
> @@ -294,6 +297,9 @@ static int validate_message(struct hsmp_message *msg)
>  		 * supports in each direction and leave msg_id correctness,
>  		 * such as an unsupported or out-of-range value, to the SMU
>  		 * status code __hsmp_send_message() already translates.
> +		 *
> +		 * Client platforms also have no per-platform enabled-message
> +		 * bitmaps yet, so skip that check here.

I'm unsure how to read this. This validation seems to be on driver side so 
why cannot it be validated for client?

>  		 */
>  		if (msg->num_args > HSMP_CLIENT_MAX_MSG_LEN ||
>  		    msg->response_sz > HSMP_CLIENT_MAX_MSG_LEN)
> @@ -307,6 +313,17 @@ static int validate_message(struct hsmp_message *msg)
>  	if (!desc)
>  		return -ENOMSG;
>  
> +	sock_ind = array_index_nospec(msg->sock_ind, hsmp_pdev.num_sockets);
> +	sock = &hsmp_pdev.sock[sock_ind];
> +
> +	if (is_get_msg(desc, msg)) {
> +		if (!is_read_msg_enabled(sock, msg->msg_id))
> +			return -ENOMSG;
> +	} else {
> +		if (!is_write_msg_enabled(sock, msg->msg_id))
> +			return -ENOMSG;
> +	}
> +
>  	/*
>  	 * num_args passed by user should match the num_args specified in
>  	 * message description table.
> @@ -822,6 +839,83 @@ int hsmp_cache_proto_ver(u16 sock_ind)
>  }
>  EXPORT_SYMBOL_NS_GPL(hsmp_cache_proto_ver, "AMD_HSMP");
>  
> +/*
> + * Predefine read and write enabled message bitmaps for use on platforms where
> + * HSMP_GET_ENABLED_HSMP_CMDS (37h) is not available.
> + */
> +static const u32 read_bitmap_f19h_m00h_0fh[] = {0x001b8cde, 0x00000000, 0x00000000};

These are just a combination of BIT(msg_id), right? If yes, these should 
be constructed from the named defines by OR'ing them together instead of 
using those obscure literals.

> +static const u32 write_bitmap_f19h_m00h_0fh[] = {0x00047320, 0x00000000, 0x00000000};
> +
> +static const u32 read_bitmap_zen4_variants[] = {0x7fdb8cde, 0x00000000, 0x00000000};
> +static const u32 write_bitmap_zen4_variants[] = {0x80047320, 0x00000007, 0x00000000};
> +
> +static const u32 read_bitmap_f19h_m80h_9fh[] = {0x7e0b8cde, 0x00000038, 0x00000000};
> +static const u32 write_bitmap_f19h_m80h_9fh[] = {0x00040320, 0x00000000, 0x00000000};
> +
> +static const u32 read_bitmap_f1ah_m00h_1fh[] = {0x7fdb8cde, 0x00070182, 0x00000000};
> +static const u32 write_bitmap_f1ah_m00h_1fh[] = {0x80047320, 0x000001c6, 0x00000000};

What's the general plan with these? Do we have to keep adding to them when 
more platforms appear?

> +/*
> + * Family 0x1A model 0x50-0x5f, family 0x1A model 0xC0-0xCF,
> + * family 0x1A model 0x90-0x9B and family 0x1A model 0xA0-0xAB
> + * belong to 6th Gen AMD EPYC Server family.
> + */
> +static const u32 read_bitmap_zen6_variants[] = {0x7fdbbcde, 0x05ff07fe, 0x00000000};
> +static const u32 write_bitmap_zen6_variants[] = {0x00047320, 0x070007c6, 0x00000000};
> +
> +#define set_read_enabled_msg_bitmap(sock, src)	\
> +	bitmap_from_arr32((sock)->enabled_read_msg_ids, (src), HSMP_MAX_MSGS)
> +#define set_write_enabled_msg_bitmap(sock, src)	\
> +	bitmap_from_arr32((sock)->enabled_write_msg_ids, (src), HSMP_MAX_MSGS)
> +
> +int hsmp_populate_msg_bitmaps(u16 sock_ind)
> +{
> +	struct hsmp_socket *sock = &hsmp_pdev.sock[sock_ind];
> +
> +	switch (boot_cpu_data.x86) {
> +	case 0x19:
> +		switch (boot_cpu_data.x86_model) {
> +		case 0x00 ... 0x0F:
> +			set_read_enabled_msg_bitmap(sock, read_bitmap_f19h_m00h_0fh);
> +			set_write_enabled_msg_bitmap(sock, write_bitmap_f19h_m00h_0fh);
> +			return 0;
> +		case 0x10 ... 0x1F:
> +		case 0xA0 ... 0xAF:
> +			set_read_enabled_msg_bitmap(sock, read_bitmap_zen4_variants);
> +			set_write_enabled_msg_bitmap(sock, write_bitmap_zen4_variants);
> +			return 0;
> +		case 0x80 ... 0x9F:
> +			set_read_enabled_msg_bitmap(sock, read_bitmap_f19h_m80h_9fh);
> +			set_write_enabled_msg_bitmap(sock, write_bitmap_f19h_m80h_9fh);
> +			return 0;
> +		}
> +		break;
> +	case 0x1A:
> +		switch (boot_cpu_data.x86_model) {
> +		case 0x00 ... 0x1F:
> +			set_read_enabled_msg_bitmap(sock, read_bitmap_f1ah_m00h_1fh);
> +			set_write_enabled_msg_bitmap(sock, write_bitmap_f1ah_m00h_1fh);
> +			return 0;
> +		case 0x50 ... 0x5F:
> +		case 0x90 ... 0x9B:
> +		case 0xA0 ... 0xAB:
> +		case 0xC0 ... 0xCF:
> +			set_read_enabled_msg_bitmap(sock, read_bitmap_zen6_variants);
> +			set_write_enabled_msg_bitmap(sock, write_bitmap_zen6_variants);
> +			/*
> +			 * PC6 control (msg 0x29) is not supported from
> +			 * protocol 0x70002 onwards on these models.
> +			 */
> +			if (sock->proto_ver >= 0x70002) {
> +				clear_bit(HSMP_PC6_ENABLE, sock->enabled_read_msg_ids);
> +				clear_bit(HSMP_PC6_ENABLE, sock->enabled_write_msg_ids);
> +			}
> +			return 0;
> +		}
> +	}
> +	return -ENODEV;
> +}
> +EXPORT_SYMBOL_NS_GPL(hsmp_populate_msg_bitmaps, "AMD_HSMP");
> +
>  static const struct file_operations hsmp_fops = {
>  	.owner		= THIS_MODULE,
>  	.unlocked_ioctl	= hsmp_ioctl,
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index 6c850c91a51b1..e67bbdd2b9bee 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -13,6 +13,8 @@
>  #include <asm/amd/hsmp.h>
>  
>  #include <linux/acpi.h>
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
>  #include <linux/compiler_types.h>
>  #include <linux/device.h>
>  #include <linux/hwmon.h>
> @@ -26,6 +28,8 @@
>  #include <linux/sysfs.h>
>  #include <linux/types.h>
>  
> +#define HSMP_MAX_MSGS			96
> +
>  #define HSMP_METRICS_TABLE_NAME	"metrics_bin"
>  
>  #define HSMP_ATTR_GRP_NAME_SIZE	10
> @@ -77,6 +81,8 @@ struct hsmp_socket {
>  	struct device *dev;
>  	u16 sock_ind;
>  	u32 proto_ver;
> +	DECLARE_BITMAP(enabled_read_msg_ids, HSMP_MAX_MSGS);
> +	DECLARE_BITMAP(enabled_write_msg_ids, HSMP_MAX_MSGS);
>  	int (*amd_hsmp_rdwr)(struct hsmp_socket *sock, u32 off, u32 *val, bool rw);
>  };
>  
> @@ -86,7 +92,28 @@ struct hsmp_plat_device {
>  	u16 num_sockets;
>  };
>  
> +/* HSMP_TEST, HSMP_GET_PROTO_VER, HSMP_GET_SMU_VER are always enabled in
> + * all the platforms and can't be masked by BIOS. Set those bits in the
> + * bitmap during bitmap initialization.
> + */
> +#define init_read_enabled_msg_bitmap(sock)					\
> +	do {									\
> +		bitmap_zero((sock)->enabled_read_msg_ids, HSMP_MAX_MSGS);	\
> +		bitmap_set((sock)->enabled_read_msg_ids, HSMP_TEST, 1);		\
> +		bitmap_set((sock)->enabled_read_msg_ids, HSMP_GET_PROTO_VER, 1);\
> +		bitmap_set((sock)->enabled_read_msg_ids, HSMP_GET_SMU_VER, 1);	\
> +	} while (0)
> +
> +#define init_write_enabled_msg_bitmap(sock)	\
> +	bitmap_zero((sock)->enabled_write_msg_ids, HSMP_MAX_MSGS)
> +
> +#define is_read_msg_enabled(sock, msg_id)	\
> +	test_bit((msg_id), (sock)->enabled_read_msg_ids)
> +#define is_write_msg_enabled(sock, msg_id)	\
> +	test_bit((msg_id), (sock)->enabled_write_msg_ids)
> +
>  int hsmp_cache_proto_ver(u16 sock_ind);
> +int hsmp_populate_msg_bitmaps(u16 sock_ind);
>  int hsmp_test(u16 sock_ind, u32 value);
>  long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
>  void hsmp_misc_deregister(void);
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index 40c98d9c66d4f..310a776125961 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -178,6 +178,11 @@ static int init_platform_device(struct device *dev)
>  		sock->mbinfo.msg_arg_off	= SMN_HSMP_MSG_DATA;
>  		sema_init(&sock->hsmp_sem, 1);
>  
> +		if (!is_client_platform()) {
> +			init_read_enabled_msg_bitmap(sock);
> +			init_write_enabled_msg_bitmap(sock);
> +		}
> +
>  		/* Test the hsmp interface on each socket */
>  		ret = hsmp_test(i, 0xDEADBEEF);
>  		if (ret) {
> @@ -193,6 +198,14 @@ static int init_platform_device(struct device *dev)
>  			return ret;
>  		}
>  
> +		if (!is_client_platform()) {
> +			ret = hsmp_populate_msg_bitmaps(i);
> +			if (ret) {
> +				dev_err(dev, "Failed to get enabled HSMP messages on this platform\n");
> +				return ret;
> +			}
> +		}
> +
>  		if (sock->proto_ver == HSMP_PROTO_VER6) {
>  			ret = hsmp_get_tbl_dram_base(i);
>  			if (ret)
> 

-- 
 i.