Re: [PATCH 2/3] platform/x86/amd/hsmp: Split the ACPI and non-ACPI code

[Mario Limonciello <mario.limonciello@amd.com>]

On 4/23/2024 04:14, Suma Hegde wrote:
> HSMP driver is supported on ACPI as well as non-ACPI based platforms.
> Making it dependent on ACPI in Kconfig, causes it to be compiled only on
> systems which have ACPI enabled.
> Remove this dependency by seggregating acpi specific code into
segregating
> hsmp-acpi.c file.
> This ensures that HSMP can be compiled with or without
> ACPI enabled.
> 
> Also create hsmp/ directory with Kconfig and Makefile
> to keep hsmp related files.
> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
>   MAINTAINERS                                |   2 +-
>   drivers/platform/x86/amd/Kconfig           |  14 +-
>   drivers/platform/x86/amd/Makefile          |   3 +-
>   drivers/platform/x86/amd/hsmp/Kconfig      |  17 ++
>   drivers/platform/x86/amd/hsmp/Makefile     |   9 +
>   drivers/platform/x86/amd/hsmp/hsmp-acpi.c  | 219 ++++++++++++++
>   drivers/platform/x86/amd/{ => hsmp}/hsmp.c | 324 +++------------------
>   drivers/platform/x86/amd/hsmp/hsmp.h       |  82 ++++++
>   8 files changed, 377 insertions(+), 293 deletions(-)
>   create mode 100644 drivers/platform/x86/amd/hsmp/Kconfig
>   create mode 100644 drivers/platform/x86/amd/hsmp/Makefile
>   create mode 100644 drivers/platform/x86/amd/hsmp/hsmp-acpi.c
>   rename drivers/platform/x86/amd/{ => hsmp}/hsmp.c (72%)
>   create mode 100644 drivers/platform/x86/amd/hsmp/hsmp.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ebf03f5f0619..bdc390ddcc4b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -999,7 +999,7 @@ S:	Maintained
>   F:	Documentation/arch/x86/amd_hsmp.rst
>   F:	arch/x86/include/asm/amd_hsmp.h
>   F:	arch/x86/include/uapi/asm/amd_hsmp.h
> -F:	drivers/platform/x86/amd/hsmp.c
> +F:	drivers/platform/x86/amd/hsmp/
>   
>   AMD IOMMU (AMD-VI)
>   M:	Joerg Roedel <joro@8bytes.org>
> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
> index f88682d36447..c2f16ccebec4 100644
> --- a/drivers/platform/x86/amd/Kconfig
> +++ b/drivers/platform/x86/amd/Kconfig
> @@ -5,19 +5,7 @@
>   
>   source "drivers/platform/x86/amd/pmf/Kconfig"
>   source "drivers/platform/x86/amd/pmc/Kconfig"
> -
> -config AMD_HSMP
> -	tristate "AMD HSMP Driver"
> -	depends on AMD_NB && X86_64 && ACPI
> -	help
> -	  The driver provides a way for user space tools to monitor and manage
> -	  system management functionality on EPYC server CPUs from AMD.
> -
> -	  Host System Management Port (HSMP) interface is a mailbox interface
> -	  between the x86 core and the System Management Unit (SMU) firmware.
> -
> -	  If you choose to compile this driver as a module the module will be
> -	  called amd_hsmp.
> +source "drivers/platform/x86/amd/hsmp/Kconfig"
>   
>   config AMD_WBRF
>   	bool "AMD Wifi RF Band mitigations (WBRF)"
> diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
> index dcec0a46f8af..f0b2fe81c685 100644
> --- a/drivers/platform/x86/amd/Makefile
> +++ b/drivers/platform/x86/amd/Makefile
> @@ -5,7 +5,6 @@
>   #
>   
>   obj-$(CONFIG_AMD_PMC)		+= pmc/
> -amd_hsmp-y			:= hsmp.o
> -obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
> +obj-$(CONFIG_AMD_HSMP)		+= hsmp/
>   obj-$(CONFIG_AMD_PMF)		+= pmf/
>   obj-$(CONFIG_AMD_WBRF)		+= wbrf.o
> diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
> new file mode 100644
> index 000000000000..26096f901c22
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# AMD HSMP driver
> +#
> +
> +config AMD_HSMP
> +	tristate "AMD HSMP Driver"
> +	depends on AMD_NB && X86_64
> +	help
> +	  The driver provides a way for user space tools to monitor and manage
> +	  system management functionality on EPYC server CPUs from AMD.
> +
> +	  Host System Management Port (HSMP) interface is a mailbox interface
> +	  between the x86 core and the System Management Unit (SMU) firmware.
> +
> +	  If you choose to compile this driver as a module the module will be
> +	  called amd_hsmp.
> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
> new file mode 100644
> index 000000000000..0e5e4cc68145
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/Makefile
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for linux/drivers/platform/x86/amd/hsmp
> +# AMD Host System Management Port
> +#
> +
> +amd_hsmp-y			:= hsmp.o
> +amd_hsmp-$(CONFIG_ACPI)		+= hsmp-acpi.o
> +obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp-acpi.c b/drivers/platform/x86/amd/hsmp/hsmp-acpi.c
> new file mode 100644
> index 000000000000..90aa1bdbf7f1
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/hsmp-acpi.c
> @@ -0,0 +1,219 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2024, AMD.
> + * All Rights Reserved.
> + *
> + * This file provides ACPI parsing implementation for HSMP interface
> + */
> +
> +#include "hsmp.h"
> +
> +/* These are the strings specified in ACPI table */
> +#define MSG_IDOFF_STR           "MsgIdOffset"
> +#define MSG_ARGOFF_STR          "MsgArgOffset"
> +#define MSG_RESPOFF_STR         "MsgRspOffset"
> +
> +/* This is the UUID used for HSMP */
> +static const guid_t acpi_hsmp_uuid = GUID_INIT(0xb74d619d, 0x5707, 0x48bd,
> +						0xa6, 0x9f, 0x4e, 0xa2,
> +						0x87, 0x1f, 0xc2, 0xf6);
> +
> +static inline bool is_acpi_hsmp_uuid(union acpi_object *obj)
> +{
> +	if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length == UUID_SIZE)
> +		return guid_equal((guid_t *)obj->buffer.pointer, &acpi_hsmp_uuid);
> +
> +	return false;
> +}
> +
> +static acpi_status hsmp_resource(struct acpi_resource *res, void *data)
> +{
> +	struct hsmp_socket *sock = data;
> +	struct resource r;
> +
> +	switch (res->type) {
> +	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> +		if (!acpi_dev_resource_memory(res, &r))
> +			return AE_ERROR;
> +		if (!r.start || r.end < r.start || !(r.flags & IORESOURCE_MEM_WRITEABLE))
> +			return AE_ERROR;
> +		sock->mbinfo.base_addr = r.start;
> +		sock->mbinfo.size = resource_size(&r);
> +		break;
> +	case ACPI_RESOURCE_TYPE_END_TAG:
> +		break;
> +	default:
> +		return AE_ERROR;
> +	}
> +
> +	return AE_OK;
> +}
> +
> +static int hsmp_read_acpi_crs(struct hsmp_socket *sock)
> +{
> +	acpi_status status;
> +
> +	status = acpi_walk_resources(ACPI_HANDLE(sock->dev), METHOD_NAME__CRS,
> +				     hsmp_resource, sock);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(sock->dev, "Failed to look up MP1 base address from CRS method, err: %s\n",
> +			acpi_format_exception(status));
> +		return -EINVAL;
> +	}
> +	if (!sock->mbinfo.base_addr || !sock->mbinfo.size)
> +		return -EINVAL;
> +
> +	/* The mapped region should be un cached */
> +	sock->virt_base_addr = devm_ioremap_uc(sock->dev, sock->mbinfo.base_addr,
> +					       sock->mbinfo.size);
> +	if (!sock->virt_base_addr) {
> +		dev_err(sock->dev, "Failed to ioremap MP1 base address\n");
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hsmp_read_acpi_dsd(struct hsmp_socket *sock)
> +{
> +	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *guid, *mailbox_package;
> +	union acpi_object *dsd;
> +	acpi_status status;
> +	int ret = 0;
> +	int j;
> +
> +	status = acpi_evaluate_object_typed(ACPI_HANDLE(sock->dev), "_DSD", NULL,
> +					    &buf, ACPI_TYPE_PACKAGE);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(sock->dev, "Failed to read mailbox reg offsets from DSD table, err: %s\n",
> +			acpi_format_exception(status));
> +		return -ENODEV;
> +	}
> +
> +	dsd = buf.pointer;
> +
> +	/* HSMP _DSD property should contain 2 objects.
> +	 * 1. guid which is an acpi object of type ACPI_TYPE_BUFFER
> +	 * 2. mailbox which is an acpi object of type ACPI_TYPE_PACKAGE
> +	 *    This mailbox object contains 3 more acpi objects of type
> +	 *    ACPI_TYPE_PACKAGE for holding msgid, msgresp, msgarg offsets
> +	 *    these packages inturn contain 2 acpi objects of type
> +	 *    ACPI_TYPE_STRING and ACPI_TYPE_INTEGER
> +	 */
> +	if (!dsd || dsd->type != ACPI_TYPE_PACKAGE || dsd->package.count != 2) {
> +		ret = -EINVAL;
> +		goto free_buf;
> +	}
> +
> +	guid = &dsd->package.elements[0];
> +	mailbox_package = &dsd->package.elements[1];
> +	if (!is_acpi_hsmp_uuid(guid) || mailbox_package->type != ACPI_TYPE_PACKAGE) {
> +		dev_err(sock->dev, "Invalid hsmp _DSD table data\n");
> +		ret = -EINVAL;
> +		goto free_buf;
> +	}
> +
> +	for (j = 0; j < mailbox_package->package.count; j++) {
> +		union acpi_object *msgobj, *msgstr, *msgint;
> +
> +		msgobj	= &mailbox_package->package.elements[j];
> +		msgstr	= &msgobj->package.elements[0];
> +		msgint	= &msgobj->package.elements[1];
> +
> +		/* package should have 1 string and 1 integer object */
> +		if (msgobj->type != ACPI_TYPE_PACKAGE ||
> +		    msgstr->type != ACPI_TYPE_STRING ||
> +		    msgint->type != ACPI_TYPE_INTEGER) {
> +			ret = -EINVAL;
> +			goto free_buf;
> +		}
> +
> +		if (!strncmp(msgstr->string.pointer, MSG_IDOFF_STR,
> +			     msgstr->string.length)) {
> +			sock->mbinfo.msg_id_off = msgint->integer.value;
> +		} else if (!strncmp(msgstr->string.pointer, MSG_RESPOFF_STR,
> +				    msgstr->string.length)) {
> +			sock->mbinfo.msg_resp_off =  msgint->integer.value;
> +		} else if (!strncmp(msgstr->string.pointer, MSG_ARGOFF_STR,
> +				    msgstr->string.length)) {
> +			sock->mbinfo.msg_arg_off = msgint->integer.value;
> +		} else {
> +			ret = -ENOENT;
> +			goto free_buf;
> +		}
> +	}
> +
> +	if (!sock->mbinfo.msg_id_off || !sock->mbinfo.msg_resp_off ||
> +	    !sock->mbinfo.msg_arg_off)
> +		ret = -EINVAL;
> +
> +free_buf:
> +	ACPI_FREE(buf.pointer);
> +	return ret;
> +}
> +
> +int init_acpi(struct hsmp_plat_device *plat_dev, struct device *dev)
> +{
> +	struct hsmp_socket *sock;
> +	u16 sock_ind;
> +	int ret;
> +
> +	plat_dev->is_acpi_device = true;
> +
> +	ret = hsmp_get_uid(dev, &sock_ind);
> +	if (ret)
> +		return ret;
> +	if (sock_ind >= plat_dev->num_sockets)
> +		return -EINVAL;
> +
> +	sock = &plat_dev->sock[sock_ind];
> +
> +	sock->sock_ind		= sock_ind;
> +	sock->dev		= dev;
> +
> +	sema_init(&sock->hsmp_sem, 1);
> +
> +	/* Read MP1 base address from CRS method */
> +	ret = hsmp_read_acpi_crs(sock);
> +	if (ret)
> +		return ret;
> +
> +	/* Read mailbox offsets from DSD table */
> +	return hsmp_read_acpi_dsd(sock);
> +}
> +
> +int check_acpi_support(struct device *dev, const struct acpi_device_id *amd_hsmp_acpi_ids)
> +{
> +	struct acpi_device *adev;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
> +		return 0;
> +
> +	return -ENODEV;
> +}
> +
> +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.
> +	 */
> +	uid = acpi_device_uid(ACPI_COMPANION(dev));
> +
> +	return kstrtou16(uid + 2, 10, sock_ind);
> +}
> +
> +void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> +			u32 *value, bool write)
> +{
> +	if (write)
> +		iowrite32(*value, sock->virt_base_addr + offset);
> +	else
> +		*value = ioread32(sock->virt_base_addr + offset);
> +}
> diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> similarity index 72%
> rename from drivers/platform/x86/amd/hsmp.c
> rename to drivers/platform/x86/amd/hsmp/hsmp.c
> index 807a5066dacc..1df2b34ddbbd 100644
> --- a/drivers/platform/x86/amd/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -10,18 +10,10 @@
>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>   
>   #include <asm/amd_hsmp.h>
> -#include <asm/amd_nb.h>
> -#include <linux/delay.h>
> -#include <linux/io.h>
> -#include <linux/miscdevice.h>
> -#include <linux/module.h>
> -#include <linux/pci.h>
> -#include <linux/platform_device.h>
> -#include <linux/semaphore.h>
> -#include <linux/acpi.h>
> +#include "hsmp.h"
>   
>   #define DRIVER_NAME		"amd_hsmp"
> -#define DRIVER_VERSION		"2.2"
> +#define DRIVER_VERSION		"2.3"
>   #define ACPI_HSMP_DEVICE_HID	"AMDI0097"
>   
>   /* HSMP Status / Error codes */
> @@ -55,44 +47,8 @@
>   #define HSMP_DEVNODE_NAME	"hsmp"
>   #define HSMP_METRICS_TABLE_NAME	"metrics_bin"
>   
> -#define HSMP_ATTR_GRP_NAME_SIZE	10
> -
> -/* These are the strings specified in ACPI table */
> -#define MSG_IDOFF_STR		"MsgIdOffset"
> -#define MSG_ARGOFF_STR		"MsgArgOffset"
> -#define MSG_RESPOFF_STR		"MsgRspOffset"
> -
>   #define MAX_AMD_SOCKETS 8
>   
> -struct hsmp_mbaddr_info {
> -	u32 base_addr;
> -	u32 msg_id_off;
> -	u32 msg_resp_off;
> -	u32 msg_arg_off;
> -	u32 size;
> -};
> -
> -struct hsmp_socket {
> -	struct bin_attribute hsmp_attr;
> -	struct hsmp_mbaddr_info mbinfo;
> -	void __iomem *metric_tbl_addr;
> -	void __iomem *virt_base_addr;
> -	struct semaphore hsmp_sem;
> -	char name[HSMP_ATTR_GRP_NAME_SIZE];
> -	struct pci_dev *root;
> -	struct device *dev;
> -	u16 sock_ind;
> -};
> -
> -struct hsmp_plat_device {
> -	struct miscdevice hsmp_device;
> -	struct hsmp_socket *sock;
> -	u32 proto_ver;
> -	u16 num_sockets;
> -	bool is_acpi_device;
> -	bool is_probed;
> -};
> -
>   static struct hsmp_plat_device plat_dev;
>   
>   static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
> @@ -114,15 +70,6 @@ static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
>   	return ret;
>   }
>   
> -static void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> -			       u32 *value, bool write)
> -{
> -	if (write)
> -		iowrite32(*value, sock->virt_base_addr + offset);
> -	else
> -		*value = ioread32(sock->virt_base_addr + offset);
> -}
> -
>   static int amd_hsmp_rdwr(struct hsmp_socket *sock, u32 offset,
>   			 u32 *value, bool write)
>   {
> @@ -321,6 +268,22 @@ static int hsmp_test(u16 sock_ind, u32 value)
>   	return ret;
>   }
>   
> +static int hsmp_cache_proto_ver(u16 sock_ind)
> +{
> +	struct hsmp_message msg = { 0 };
> +	int ret;
> +
> +	msg.msg_id	= HSMP_GET_PROTO_VER;
> +	msg.sock_ind	= sock_ind;
> +	msg.response_sz = hsmp_msg_desc_table[HSMP_GET_PROTO_VER].response_sz;
> +
> +	ret = hsmp_send_message(&msg);
> +	if (!ret)
> +		plat_dev.proto_ver = msg.args[0];
> +
> +	return ret;
> +}
> +
>   static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>   {
>   	int __user *arguser = (int  __user *)arg;
> @@ -383,181 +346,6 @@ static const struct file_operations hsmp_fops = {
>   	.compat_ioctl	= hsmp_ioctl,
>   };
>   
> -/* This is the UUID used for HSMP */
> -static const guid_t acpi_hsmp_uuid = GUID_INIT(0xb74d619d, 0x5707, 0x48bd,
> -						0xa6, 0x9f, 0x4e, 0xa2,
> -						0x87, 0x1f, 0xc2, 0xf6);
> -
> -static inline bool is_acpi_hsmp_uuid(union acpi_object *obj)
> -{
> -	if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length == UUID_SIZE)
> -		return guid_equal((guid_t *)obj->buffer.pointer, &acpi_hsmp_uuid);
> -
> -	return false;
> -}
> -
> -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.
> -	 */
> -	uid = acpi_device_uid(ACPI_COMPANION(dev));
> -
> -	return kstrtou16(uid + 2, 10, sock_ind);
> -}
> -
> -static acpi_status hsmp_resource(struct acpi_resource *res, void *data)
> -{
> -	struct hsmp_socket *sock = data;
> -	struct resource r;
> -
> -	switch (res->type) {
> -	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> -		if (!acpi_dev_resource_memory(res, &r))
> -			return AE_ERROR;
> -		if (!r.start || r.end < r.start || !(r.flags & IORESOURCE_MEM_WRITEABLE))
> -			return AE_ERROR;
> -		sock->mbinfo.base_addr = r.start;
> -		sock->mbinfo.size = resource_size(&r);
> -		break;
> -	case ACPI_RESOURCE_TYPE_END_TAG:
> -		break;
> -	default:
> -		return AE_ERROR;
> -	}
> -
> -	return AE_OK;
> -}
> -
> -static int hsmp_read_acpi_dsd(struct hsmp_socket *sock)
> -{
> -	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> -	union acpi_object *guid, *mailbox_package;
> -	union acpi_object *dsd;
> -	acpi_status status;
> -	int ret = 0;
> -	int j;
> -
> -	status = acpi_evaluate_object_typed(ACPI_HANDLE(sock->dev), "_DSD", NULL,
> -					    &buf, ACPI_TYPE_PACKAGE);
> -	if (ACPI_FAILURE(status)) {
> -		dev_err(sock->dev, "Failed to read mailbox reg offsets from DSD table, err: %s\n",
> -			acpi_format_exception(status));
> -		return -ENODEV;
> -	}
> -
> -	dsd = buf.pointer;
> -
> -	/* HSMP _DSD property should contain 2 objects.
> -	 * 1. guid which is an acpi object of type ACPI_TYPE_BUFFER
> -	 * 2. mailbox which is an acpi object of type ACPI_TYPE_PACKAGE
> -	 *    This mailbox object contains 3 more acpi objects of type
> -	 *    ACPI_TYPE_PACKAGE for holding msgid, msgresp, msgarg offsets
> -	 *    these packages inturn contain 2 acpi objects of type
> -	 *    ACPI_TYPE_STRING and ACPI_TYPE_INTEGER
> -	 */
> -	if (!dsd || dsd->type != ACPI_TYPE_PACKAGE || dsd->package.count != 2) {
> -		ret = -EINVAL;
> -		goto free_buf;
> -	}
> -
> -	guid = &dsd->package.elements[0];
> -	mailbox_package = &dsd->package.elements[1];
> -	if (!is_acpi_hsmp_uuid(guid) || mailbox_package->type != ACPI_TYPE_PACKAGE) {
> -		dev_err(sock->dev, "Invalid hsmp _DSD table data\n");
> -		ret = -EINVAL;
> -		goto free_buf;
> -	}
> -
> -	for (j = 0; j < mailbox_package->package.count; j++) {
> -		union acpi_object *msgobj, *msgstr, *msgint;
> -
> -		msgobj	= &mailbox_package->package.elements[j];
> -		msgstr	= &msgobj->package.elements[0];
> -		msgint	= &msgobj->package.elements[1];
> -
> -		/* package should have 1 string and 1 integer object */
> -		if (msgobj->type != ACPI_TYPE_PACKAGE ||
> -		    msgstr->type != ACPI_TYPE_STRING ||
> -		    msgint->type != ACPI_TYPE_INTEGER) {
> -			ret = -EINVAL;
> -			goto free_buf;
> -		}
> -
> -		if (!strncmp(msgstr->string.pointer, MSG_IDOFF_STR,
> -			     msgstr->string.length)) {
> -			sock->mbinfo.msg_id_off = msgint->integer.value;
> -		} else if (!strncmp(msgstr->string.pointer, MSG_RESPOFF_STR,
> -				    msgstr->string.length)) {
> -			sock->mbinfo.msg_resp_off =  msgint->integer.value;
> -		} else if (!strncmp(msgstr->string.pointer, MSG_ARGOFF_STR,
> -				    msgstr->string.length)) {
> -			sock->mbinfo.msg_arg_off = msgint->integer.value;
> -		} else {
> -			ret = -ENOENT;
> -			goto free_buf;
> -		}
> -	}
> -
> -	if (!sock->mbinfo.msg_id_off || !sock->mbinfo.msg_resp_off ||
> -	    !sock->mbinfo.msg_arg_off)
> -		ret = -EINVAL;
> -
> -free_buf:
> -	ACPI_FREE(buf.pointer);
> -	return ret;
> -}
> -
> -static int hsmp_read_acpi_crs(struct hsmp_socket *sock)
> -{
> -	acpi_status status;
> -
> -	status = acpi_walk_resources(ACPI_HANDLE(sock->dev), METHOD_NAME__CRS,
> -				     hsmp_resource, sock);
> -	if (ACPI_FAILURE(status)) {
> -		dev_err(sock->dev, "Failed to look up MP1 base address from CRS method, err: %s\n",
> -			acpi_format_exception(status));
> -		return -EINVAL;
> -	}
> -	if (!sock->mbinfo.base_addr || !sock->mbinfo.size)
> -		return -EINVAL;
> -
> -	/* The mapped region should be un cached */
> -	sock->virt_base_addr = devm_ioremap_uc(sock->dev, sock->mbinfo.base_addr,
> -					       sock->mbinfo.size);
> -	if (!sock->virt_base_addr) {
> -		dev_err(sock->dev, "Failed to ioremap MP1 base address\n");
> -		return -ENOMEM;
> -	}
> -
> -	return 0;
> -}
> -
> -/* Parse the ACPI table to read the data */
> -static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
> -{
> -	struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
> -	int ret;
> -
> -	sock->sock_ind		= sock_ind;
> -	sock->dev		= dev;
> -	plat_dev.is_acpi_device	= true;
> -
> -	sema_init(&sock->hsmp_sem, 1);
> -
> -	/* Read MP1 base address from CRS method */
> -	ret = hsmp_read_acpi_crs(sock);
> -	if (ret)
> -		return ret;
> -
> -	/* Read mailbox offsets from DSD table */
> -	return hsmp_read_acpi_dsd(sock);
> -}
> -
>   static ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
>   				    struct bin_attribute *bin_attr, char *buf,
>   				    loff_t off, size_t count)
> @@ -670,8 +458,15 @@ static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
>   {
>   	const struct attribute_group **hsmp_attr_grps;
>   	struct attribute_group *attr_grp;
> +	int ret;
>   	u16 i;
>   
> +	ret = hsmp_cache_proto_ver(0);
> +	if (ret) {
> +		dev_err(dev, "Failed to read HSMP protocol version\n");
> +		return ret;
> +	}
> +
>   	hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
>   				      sizeof(*hsmp_attr_grps),
>   				      GFP_KERNEL);
> @@ -706,12 +501,18 @@ static int hsmp_create_acpi_sysfs_if(struct device *dev)
>   	if (!attr_grp)
>   		return -ENOMEM;
>   
> -	attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
> -
>   	ret = hsmp_get_uid(dev, &sock_ind);
>   	if (ret)
>   		return ret;
>   
> +	ret = hsmp_cache_proto_ver(sock_ind);
> +	if (ret) {
> +		dev_err(dev, "Failed to read HSMP protocol version\n");
> +		return ret;
> +	}
> +
> +	attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
> +
>   	ret = hsmp_create_attr_list(attr_grp, dev, sock_ind);
>   	if (ret)
>   		return ret;
> @@ -719,22 +520,6 @@ static int hsmp_create_acpi_sysfs_if(struct device *dev)
>   	return devm_device_add_group(dev, attr_grp);
>   }
>   
> -static int hsmp_cache_proto_ver(u16 sock_ind)
> -{
> -	struct hsmp_message msg = { 0 };
> -	int ret;
> -
> -	msg.msg_id	= HSMP_GET_PROTO_VER;
> -	msg.sock_ind	= sock_ind;
> -	msg.response_sz = hsmp_msg_desc_table[HSMP_GET_PROTO_VER].response_sz;
> -
> -	ret = hsmp_send_message(&msg);
> -	if (!ret)
> -		plat_dev.proto_ver = msg.args[0];
> -
> -	return ret;
> -}
> -
>   static inline bool is_f1a_m0h(void)
>   {
>   	if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
> @@ -791,8 +576,6 @@ MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
>   
>   static int hsmp_pltdrv_probe(struct platform_device *pdev)
>   {
> -	struct acpi_device *adev;
> -	u16 sock_ind = 0;
>   	int ret;
>   
>   	/*
> @@ -809,46 +592,33 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
>   		if (!plat_dev.sock)
>   			return -ENOMEM;
>   	}
> -	adev = ACPI_COMPANION(&pdev->dev);
> -	if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids)) {
> -		ret = hsmp_get_uid(&pdev->dev, &sock_ind);
> -		if (ret)
> -			return ret;
> -		if (sock_ind >= plat_dev.num_sockets)
> -			return -EINVAL;
> -		ret = hsmp_parse_acpi_table(&pdev->dev, sock_ind);
> +
> +	ret = check_acpi_support(&pdev->dev, amd_hsmp_acpi_ids);
> +	if (!ret) {
> +		ret = init_acpi(&plat_dev, &pdev->dev);
>   		if (ret) {
> -			dev_err(&pdev->dev, "Failed to parse ACPI table\n");
> +			dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
>   			return ret;
>   		}
> -		/* Test the hsmp interface */
> -		ret = hsmp_test(sock_ind, 0xDEADBEEF);
> +
> +		ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
>   		if (ret) {
> -			dev_err(&pdev->dev, "HSMP test message failed on Fam:%x model:%x\n",
> -				boot_cpu_data.x86, boot_cpu_data.x86_model);
> -			dev_err(&pdev->dev, "Is HSMP disabled in BIOS ?\n");
> +			dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
>   			return ret;
>   		}
>   	} else {
>   		ret = init_platform_device(&pdev->dev);
>   		if (ret) {
> -			dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
> +			dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
>   			return ret;
>   		}
> -	}
> -
> -	ret = hsmp_cache_proto_ver(sock_ind);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Failed to read HSMP protocol version\n");
> -		return ret;
> -	}
>   
> -	if (plat_dev.is_acpi_device)
> -		ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> -	else
>   		ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
> -	if (ret)
> -		dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> +			return ret;
> +		}
> +	}
>   
>   	if (!plat_dev.is_probed) {
>   		plat_dev.hsmp_device.name	= HSMP_CDEV_NAME;
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> new file mode 100644
> index 000000000000..23629ccb1fb0
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2024, AMD.
> + * All Rights Reserved.
> + *
> + * This file provides a header implementation for HSMP interface
> + */
> +
> +#ifndef _HSMP_H
> +#define _HSMP_H
> +
> +#include <asm/amd_nb.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/semaphore.h>
> +#include <linux/acpi.h>
> +#include <linux/miscdevice.h>
> +
> +#define HSMP_ATTR_GRP_NAME_SIZE 10
> +
> +struct hsmp_mbaddr_info {
> +	u32 base_addr;
> +	u32 msg_id_off;
> +	u32 msg_resp_off;
> +	u32 msg_arg_off;
> +	u32 size;
> +};
> +
> +struct hsmp_socket {
> +	struct bin_attribute hsmp_attr;
> +	struct hsmp_mbaddr_info mbinfo;
> +	void __iomem *metric_tbl_addr;
> +	void __iomem *virt_base_addr;
> +	struct semaphore hsmp_sem;
> +	char name[HSMP_ATTR_GRP_NAME_SIZE];
> +	struct pci_dev *root;
> +	struct device *dev;
> +	u16 sock_ind;
> +};
> +
> +struct hsmp_plat_device {
> +	struct miscdevice hsmp_device;
> +	struct hsmp_socket *sock;
> +	u32 proto_ver;
> +	u16 num_sockets;
> +	bool is_acpi_device;
> +	bool is_probed;
> +};
> +
> +#ifdef CONFIG_ACPI
> +int hsmp_get_uid(struct device *dev, u16 *sock_ind);
> +void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> +			u32 *value, bool write);
> +int check_acpi_support(struct device *dev, const struct acpi_device_id *amd_hsmp_acpi_ids);
> +int init_acpi(struct hsmp_plat_device *plat_dev, struct device *dev);
> +#else
> +static inline void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> +				      u32 *value, bool write)
> +{
> +}
> +
> +static inline int hsmp_get_uid(struct device *dev, u16 *sock_ind)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int check_acpi_support(struct device *dev,
> +				     const struct acpi_device_id *amd_hsmp_acpi_ids)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline int init_acpi(struct hsmp_plat_device *plat_dev, struct device *dev)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +#endif