* [PATCH 01/10] platform/x86/amd/hsmp: Create hsmp/ directory
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-06-27 5:39 ` [PATCH 02/10] platform/x86/amd/hsmp: Create wrapper function init_acpi() Suma Hegde
` (8 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
This is in preparation to splitting ACPI and platform device drivers.
Create and move hsmp specific code into its own directory,
no logical changes.
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 | 8 ++++++++
drivers/platform/x86/amd/{ => hsmp}/hsmp.c | 0
6 files changed, 28 insertions(+), 16 deletions(-)
create mode 100644 drivers/platform/x86/amd/hsmp/Kconfig
create mode 100644 drivers/platform/x86/amd/hsmp/Makefile
rename drivers/platform/x86/amd/{ => hsmp}/hsmp.c (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index d6c90161c7bf..a7d79d1f7ec1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1037,7 +1037,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..2c671cc17d63 100644
--- a/drivers/platform/x86/amd/Kconfig
+++ b/drivers/platform/x86/amd/Kconfig
@@ -3,22 +3,10 @@
# AMD x86 Platform Specific Drivers
#
+source "drivers/platform/x86/amd/hsmp/Kconfig"
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.
-
config AMD_WBRF
bool "AMD Wifi RF Band mitigations (WBRF)"
depends on ACPI
diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
index dcec0a46f8af..96ec24c8701b 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-y += 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..b55d4ed9bceb
--- /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 && 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.
diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
new file mode 100644
index 000000000000..fda64906a5e8
--- /dev/null
+++ b/drivers/platform/x86/amd/hsmp/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for drivers/platform/x86/amd/hsmp
+# AMD HSMP Driver
+#
+
+obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
+amd_hsmp-objs := hsmp.o
diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
similarity index 100%
rename from drivers/platform/x86/amd/hsmp.c
rename to drivers/platform/x86/amd/hsmp/hsmp.c
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 02/10] platform/x86/amd/hsmp: Create wrapper function init_acpi()
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
2024-06-27 5:39 ` [PATCH 01/10] platform/x86/amd/hsmp: Create hsmp/ directory Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-07-09 10:24 ` Ilpo Järvinen
2024-06-27 5:39 ` [PATCH 03/10] platform/x86/amd/hsmp: Move strcuture and macros to header file Suma Hegde
` (7 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
This is in preparation to splitting ACPI and platform device drivers.
Having init_acpi() helps in smooth code movement.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/hsmp.c | 91 ++++++++++++++++++----------
1 file changed, 59 insertions(+), 32 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 8fcf38eed7f0..10ab9b2437f1 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -778,6 +778,11 @@ static int init_platform_device(struct device *dev)
dev_err(dev, "Is HSMP disabled in BIOS ?\n");
return ret;
}
+ ret = hsmp_cache_proto_ver(i);
+ if (ret) {
+ dev_err(dev, "Failed to read HSMP protocol version\n");
+ return ret;
+ }
}
return 0;
@@ -789,10 +794,53 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
};
MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
+static bool check_acpi_support(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
+ return true;
+
+ return false;
+}
+
+static int init_acpi(struct device *dev)
+{
+ u16 sock_ind;
+ int ret;
+
+ ret = hsmp_get_uid(dev, &sock_ind);
+ if (ret)
+ return ret;
+ if (sock_ind >= plat_dev.num_sockets)
+ return -EINVAL;
+
+ ret = hsmp_parse_acpi_table(dev, sock_ind);
+ if (ret) {
+ dev_err(dev, "Failed to parse ACPI table\n");
+ return ret;
+ }
+
+ /* Test the hsmp interface */
+ ret = hsmp_test(sock_ind, 0xDEADBEEF);
+ if (ret) {
+ dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
+ boot_cpu_data.x86, boot_cpu_data.x86_model);
+ dev_err(dev, "Is HSMP disabled in BIOS ?\n");
+ return ret;
+ }
+
+ ret = hsmp_cache_proto_ver(sock_ind);
+ if (ret) {
+ dev_err(dev, "Failed to read HSMP protocol version\n");
+ return ret;
+ }
+
+ return ret;
+}
+
static int hsmp_pltdrv_probe(struct platform_device *pdev)
{
- struct acpi_device *adev;
- u16 sock_ind = 0;
int ret;
/*
@@ -809,46 +857,25 @@ 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);
- if (ret) {
- dev_err(&pdev->dev, "Failed to parse ACPI table\n");
- return ret;
- }
- /* Test the hsmp interface */
- ret = hsmp_test(sock_ind, 0xDEADBEEF);
+ if (check_acpi_support(&pdev->dev)) {
+ ret = init_acpi(&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 init HSMP mailbox\n");
return ret;
}
+ ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
} else {
ret = init_platform_device(&pdev->dev);
if (ret) {
dev_err(&pdev->dev, "Failed to init HSMP mailbox\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");
+ }
if (!plat_dev.is_probed) {
plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 02/10] platform/x86/amd/hsmp: Create wrapper function init_acpi()
2024-06-27 5:39 ` [PATCH 02/10] platform/x86/amd/hsmp: Create wrapper function init_acpi() Suma Hegde
@ 2024-07-09 10:24 ` Ilpo Järvinen
0 siblings, 0 replies; 22+ messages in thread
From: Ilpo Järvinen @ 2024-07-09 10:24 UTC (permalink / raw)
To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
[-- Attachment #1: Type: text/plain, Size: 4357 bytes --]
On Thu, 27 Jun 2024, Suma Hegde wrote:
> This is in preparation to splitting ACPI and platform device drivers.
> Having init_acpi() helps in smooth code movement.
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/hsmp.c | 91 ++++++++++++++++++----------
> 1 file changed, 59 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 8fcf38eed7f0..10ab9b2437f1 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -778,6 +778,11 @@ static int init_platform_device(struct device *dev)
> dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> return ret;
> }
> + ret = hsmp_cache_proto_ver(i);
> + if (ret) {
> + dev_err(dev, "Failed to read HSMP protocol version\n");
> + return ret;
> + }
> }
>
> return 0;
> @@ -789,10 +794,53 @@ static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
> };
> MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
>
> +static bool check_acpi_support(struct device *dev)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
> + return true;
> +
> + return false;
> +}
> +
> +static int init_acpi(struct device *dev)
> +{
> + u16 sock_ind;
> + int ret;
> +
> + ret = hsmp_get_uid(dev, &sock_ind);
> + if (ret)
> + return ret;
> + if (sock_ind >= plat_dev.num_sockets)
> + return -EINVAL;
> +
> + ret = hsmp_parse_acpi_table(dev, sock_ind);
> + if (ret) {
> + dev_err(dev, "Failed to parse ACPI table\n");
> + return ret;
> + }
> +
> + /* Test the hsmp interface */
> + ret = hsmp_test(sock_ind, 0xDEADBEEF);
> + if (ret) {
> + dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> + boot_cpu_data.x86, boot_cpu_data.x86_model);
> + dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> + return ret;
> + }
> +
> + ret = hsmp_cache_proto_ver(sock_ind);
> + if (ret) {
> + dev_err(dev, "Failed to read HSMP protocol version\n");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> static int hsmp_pltdrv_probe(struct platform_device *pdev)
> {
> - struct acpi_device *adev;
> - u16 sock_ind = 0;
> int ret;
>
> /*
> @@ -809,46 +857,25 @@ 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);
> - if (ret) {
> - dev_err(&pdev->dev, "Failed to parse ACPI table\n");
> - return ret;
> - }
> - /* Test the hsmp interface */
> - ret = hsmp_test(sock_ind, 0xDEADBEEF);
> + if (check_acpi_support(&pdev->dev)) {
> + ret = init_acpi(&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 init HSMP mailbox\n");
> return ret;
> }
> + ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> } else {
> ret = init_platform_device(&pdev->dev);
> if (ret) {
> dev_err(&pdev->dev, "Failed to init HSMP mailbox\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");
> + }
>
> if (!plat_dev.is_probed) {
> plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 03/10] platform/x86/amd/hsmp: Move strcuture and macros to header file
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
2024-06-27 5:39 ` [PATCH 01/10] platform/x86/amd/hsmp: Create hsmp/ directory Suma Hegde
2024-06-27 5:39 ` [PATCH 02/10] platform/x86/amd/hsmp: Create wrapper function init_acpi() Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-06-27 19:30 ` Mario Limonciello
2024-07-09 10:21 ` Ilpo Järvinen
2024-06-27 5:39 ` [PATCH 04/10] platform/x86/amd/hsmp: Move platform device specific code to plat.c Suma Hegde
` (6 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
This is in preparation to splitting ACPI and platform device drivers.
No logical change, move common structures and macros to hsmp.h
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/hsmp.c | 42 ++--------------------
drivers/platform/x86/amd/hsmp/hsmp.h | 54 ++++++++++++++++++++++++++++
2 files changed, 56 insertions(+), 40 deletions(-)
create mode 100644 drivers/platform/x86/amd/hsmp/hsmp.h
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 10ab9b2437f1..2c9ba51b9614 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -9,15 +9,14 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include "hsmp.h"
+
#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>
#define DRIVER_NAME "amd_hsmp"
@@ -51,48 +50,11 @@
#define HSMP_INDEX_REG 0xc4
#define HSMP_DATA_REG 0xc8
-#define HSMP_CDEV_NAME "hsmp_cdev"
-#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,
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
new file mode 100644
index 000000000000..cc11e9303a83
--- /dev/null
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * AMD HSMP Platform Driver
+ * Copyright (c) 2024, AMD.
+ * All Rights Reserved.
+ *
+ * Header file for HSMP driver
+ */
+
+#ifndef HSMP_H
+#define HSMP_H
+
+#include <linux/io.h>
+#include <linux/semaphore.h>
+#include <linux/miscdevice.h>
+
+#define HSMP_METRICS_TABLE_NAME "metrics_bin"
+
+#define HSMP_ATTR_GRP_NAME_SIZE 10
+
+#define MAX_AMD_SOCKETS 8
+
+#define HSMP_CDEV_NAME "hsmp_cdev"
+#define HSMP_DEVNODE_NAME "hsmp"
+
+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;
+};
+#endif /* HSMP_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 03/10] platform/x86/amd/hsmp: Move strcuture and macros to header file
2024-06-27 5:39 ` [PATCH 03/10] platform/x86/amd/hsmp: Move strcuture and macros to header file Suma Hegde
@ 2024-06-27 19:30 ` Mario Limonciello
2024-07-09 10:21 ` Ilpo Järvinen
1 sibling, 0 replies; 22+ messages in thread
From: Mario Limonciello @ 2024-06-27 19:30 UTC (permalink / raw)
To: Suma Hegde, platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Naveen Krishna Chatradhi
You have a typo in the title.
On 6/27/2024 00:39, Suma Hegde wrote:
> This is in preparation to splitting ACPI and platform device drivers.
> No logical change, move common structures and macros to hsmp.h
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/hsmp.c | 42 ++--------------------
> drivers/platform/x86/amd/hsmp/hsmp.h | 54 ++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+), 40 deletions(-)
> create mode 100644 drivers/platform/x86/amd/hsmp/hsmp.h
>
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 10ab9b2437f1..2c9ba51b9614 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -9,15 +9,14 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include "hsmp.h"
> +
> #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>
>
> #define DRIVER_NAME "amd_hsmp"
> @@ -51,48 +50,11 @@
> #define HSMP_INDEX_REG 0xc4
> #define HSMP_DATA_REG 0xc8
>
> -#define HSMP_CDEV_NAME "hsmp_cdev"
> -#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,
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> new file mode 100644
> index 000000000000..cc11e9303a83
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2024, AMD.
> + * All Rights Reserved.
> + *
> + * Header file for HSMP driver
> + */
> +
> +#ifndef HSMP_H
> +#define HSMP_H
> +
> +#include <linux/io.h>
> +#include <linux/semaphore.h>
> +#include <linux/miscdevice.h>
> +
> +#define HSMP_METRICS_TABLE_NAME "metrics_bin"
> +
> +#define HSMP_ATTR_GRP_NAME_SIZE 10
> +
> +#define MAX_AMD_SOCKETS 8
> +
> +#define HSMP_CDEV_NAME "hsmp_cdev"
> +#define HSMP_DEVNODE_NAME "hsmp"
> +
> +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;
> +};
> +#endif /* HSMP_H */
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 03/10] platform/x86/amd/hsmp: Move strcuture and macros to header file
2024-06-27 5:39 ` [PATCH 03/10] platform/x86/amd/hsmp: Move strcuture and macros to header file Suma Hegde
2024-06-27 19:30 ` Mario Limonciello
@ 2024-07-09 10:21 ` Ilpo Järvinen
1 sibling, 0 replies; 22+ messages in thread
From: Ilpo Järvinen @ 2024-07-09 10:21 UTC (permalink / raw)
To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
[-- Attachment #1: Type: text/plain, Size: 4132 bytes --]
On Thu, 27 Jun 2024, Suma Hegde wrote:
> This is in preparation to splitting ACPI and platform device drivers.
> No logical change, move common structures and macros to hsmp.h
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/hsmp.c | 42 ++--------------------
> drivers/platform/x86/amd/hsmp/hsmp.h | 54 ++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+), 40 deletions(-)
> create mode 100644 drivers/platform/x86/amd/hsmp/hsmp.h
>
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 10ab9b2437f1..2c9ba51b9614 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -9,15 +9,14 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include "hsmp.h"
> +
> #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>
>
> #define DRIVER_NAME "amd_hsmp"
> @@ -51,48 +50,11 @@
> #define HSMP_INDEX_REG 0xc4
> #define HSMP_DATA_REG 0xc8
>
> -#define HSMP_CDEV_NAME "hsmp_cdev"
> -#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,
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> new file mode 100644
> index 000000000000..cc11e9303a83
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2024, AMD.
> + * All Rights Reserved.
> + *
> + * Header file for HSMP driver
> + */
> +
> +#ifndef HSMP_H
> +#define HSMP_H
> +
> +#include <linux/io.h>
> +#include <linux/semaphore.h>
> +#include <linux/miscdevice.h>
Please put these into alphabetical order.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
> +#define HSMP_METRICS_TABLE_NAME "metrics_bin"
> +
> +#define HSMP_ATTR_GRP_NAME_SIZE 10
> +
> +#define MAX_AMD_SOCKETS 8
> +
> +#define HSMP_CDEV_NAME "hsmp_cdev"
> +#define HSMP_DEVNODE_NAME "hsmp"
> +
> +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;
> +};
> +#endif /* HSMP_H */
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 04/10] platform/x86/amd/hsmp: Move platform device specific code to plat.c
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
` (2 preceding siblings ...)
2024-06-27 5:39 ` [PATCH 03/10] platform/x86/amd/hsmp: Move strcuture and macros to header file Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-07-09 10:20 ` Ilpo Järvinen
2024-06-27 5:39 ` [PATCH 05/10] platform/x86/amd/hsmp: Move ACPI code to acpi.c Suma Hegde
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
Move platform device part to plat.c.
No functinality/logical changes.
Common code which can be used by ACPI and platform device
remains in hsmp.c.
ACPI code in hsmp.c will be moved to acpi.c in next patch.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/Makefile | 2 +-
drivers/platform/x86/amd/hsmp/hsmp.c | 347 +------------------------
drivers/platform/x86/amd/hsmp/hsmp.h | 17 ++
drivers/platform/x86/amd/hsmp/plat.c | 336 ++++++++++++++++++++++++
4 files changed, 367 insertions(+), 335 deletions(-)
create mode 100644 drivers/platform/x86/amd/hsmp/plat.c
diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
index fda64906a5e8..fb8ba04b2f0d 100644
--- a/drivers/platform/x86/amd/hsmp/Makefile
+++ b/drivers/platform/x86/amd/hsmp/Makefile
@@ -5,4 +5,4 @@
#
obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
-amd_hsmp-objs := hsmp.o
+amd_hsmp-objs := hsmp.o plat.o
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 2c9ba51b9614..cd4de107a470 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -12,17 +12,9 @@
#include "hsmp.h"
#include <asm/amd_hsmp.h>
-#include <asm/amd_nb.h>
#include <linux/delay.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/platform_device.h>
#include <linux/acpi.h>
-#define DRIVER_NAME "amd_hsmp"
-#define DRIVER_VERSION "2.2"
-#define ACPI_HSMP_DEVICE_HID "AMDI0097"
-
/* HSMP Status / Error codes */
#define HSMP_STATUS_NOT_READY 0x00
#define HSMP_STATUS_OK 0x01
@@ -36,45 +28,12 @@
#define HSMP_WR true
#define HSMP_RD false
-/*
- * 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
-
-#define HSMP_INDEX_REG 0xc4
-#define HSMP_DATA_REG 0xc8
-
/* These are the strings specified in ACPI table */
#define MSG_IDOFF_STR "MsgIdOffset"
#define MSG_ARGOFF_STR "MsgArgOffset"
#define MSG_RESPOFF_STR "MsgRspOffset"
-static struct hsmp_plat_device plat_dev;
-
-static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
- u32 *value, bool write)
-{
- int ret;
-
- if (!sock->root)
- return -ENODEV;
-
- ret = pci_write_config_dword(sock->root, HSMP_INDEX_REG,
- sock->mbinfo.base_addr + offset);
- if (ret)
- return ret;
-
- ret = (write ? pci_write_config_dword(sock->root, HSMP_DATA_REG, *value)
- : pci_read_config_dword(sock->root, HSMP_DATA_REG, value));
-
- return ret;
-}
+struct hsmp_plat_device plat_dev;
static void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
u32 *value, bool write)
@@ -253,7 +212,7 @@ int hsmp_send_message(struct hsmp_message *msg)
}
EXPORT_SYMBOL_GPL(hsmp_send_message);
-static int hsmp_test(u16 sock_ind, u32 value)
+int hsmp_test(u16 sock_ind, u32 value)
{
struct hsmp_message msg = { 0 };
int ret;
@@ -283,7 +242,7 @@ static int hsmp_test(u16 sock_ind, u32 value)
return ret;
}
-static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
{
int __user *arguser = (int __user *)arg;
struct hsmp_message msg = { 0 };
@@ -339,12 +298,6 @@ static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
return 0;
}
-static const struct file_operations hsmp_fops = {
- .owner = THIS_MODULE,
- .unlocked_ioctl = hsmp_ioctl,
- .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,
@@ -520,9 +473,9 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
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)
+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)
{
struct hsmp_socket *sock = bin_attr->private;
struct hsmp_message msg = { 0 };
@@ -581,8 +534,8 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
return 0;
}
-static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
- struct bin_attribute *battr, int id)
+umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+ struct bin_attribute *battr, int id)
{
if (plat_dev.proto_ver == HSMP_PROTO_VER6)
return battr->attr.mode;
@@ -611,8 +564,8 @@ static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock
/* One bin sysfs for metrics table */
#define NUM_HSMP_ATTRS 1
-static int hsmp_create_attr_list(struct attribute_group *attr_grp,
- struct device *dev, u16 sock_ind)
+int hsmp_create_attr_list(struct attribute_group *attr_grp,
+ struct device *dev, u16 sock_ind)
{
struct bin_attribute **hsmp_bin_attrs;
@@ -628,37 +581,7 @@ static int hsmp_create_attr_list(struct attribute_group *attr_grp,
return hsmp_init_metric_tbl_bin_attr(hsmp_bin_attrs, sock_ind);
}
-static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
-{
- const struct attribute_group **hsmp_attr_grps;
- struct attribute_group *attr_grp;
- u16 i;
-
- hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
- sizeof(*hsmp_attr_grps),
- GFP_KERNEL);
- if (!hsmp_attr_grps)
- return -ENOMEM;
-
- /* Create a sysfs directory for each socket */
- for (i = 0; i < plat_dev.num_sockets; i++) {
- attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
- GFP_KERNEL);
- if (!attr_grp)
- return -ENOMEM;
-
- snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
- attr_grp->name = plat_dev.sock[i].name;
- attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
- hsmp_attr_grps[i] = attr_grp;
-
- hsmp_create_attr_list(attr_grp, dev, i);
- }
-
- return device_add_groups(dev, hsmp_attr_grps);
-}
-
-static int hsmp_create_acpi_sysfs_if(struct device *dev)
+int hsmp_create_acpi_sysfs_if(struct device *dev)
{
struct attribute_group *attr_grp;
u16 sock_ind;
@@ -681,7 +604,7 @@ 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)
+int hsmp_cache_proto_ver(u16 sock_ind)
{
struct hsmp_message msg = { 0 };
int ret;
@@ -697,76 +620,7 @@ static int hsmp_cache_proto_ver(u16 sock_ind)
return ret;
}
-static inline bool is_f1a_m0h(void)
-{
- if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
- return true;
-
- return false;
-}
-
-static int init_platform_device(struct device *dev)
-{
- struct hsmp_socket *sock;
- int ret, i;
-
- for (i = 0; i < plat_dev.num_sockets; i++) {
- if (!node_to_amd_nb(i))
- return -ENODEV;
- sock = &plat_dev.sock[i];
- sock->root = node_to_amd_nb(i)->root;
- sock->sock_ind = i;
- sock->dev = dev;
- sock->mbinfo.base_addr = SMN_HSMP_BASE;
-
- /*
- * This is a transitional change from non-ACPI to ACPI, only
- * family 0x1A, model 0x00 platform is supported for both ACPI and non-ACPI.
- */
- 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 */
- ret = hsmp_test(i, 0xDEADBEEF);
- if (ret) {
- dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
- boot_cpu_data.x86, boot_cpu_data.x86_model);
- dev_err(dev, "Is HSMP disabled in BIOS ?\n");
- return ret;
- }
- ret = hsmp_cache_proto_ver(i);
- if (ret) {
- dev_err(dev, "Failed to read HSMP protocol version\n");
- return ret;
- }
- }
-
- return 0;
-}
-
-static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
- {ACPI_HSMP_DEVICE_HID, 0},
- {}
-};
-MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
-
-static bool check_acpi_support(struct device *dev)
-{
- struct acpi_device *adev = ACPI_COMPANION(dev);
-
- if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
- return true;
-
- return false;
-}
-
-static int init_acpi(struct device *dev)
+int init_acpi(struct device *dev)
{
u16 sock_ind;
int ret;
@@ -800,178 +654,3 @@ static int init_acpi(struct device *dev)
return ret;
}
-
-static int hsmp_pltdrv_probe(struct platform_device *pdev)
-{
- int ret;
-
- /*
- * On ACPI supported BIOS, there is an ACPI HSMP device added for
- * each socket, so the per socket probing, but the memory allocated for
- * sockets should be contiguous to access it as an array,
- * Hence allocate memory for all the sockets at once instead of allocating
- * on each probe.
- */
- if (!plat_dev.is_probed) {
- plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
- sizeof(*plat_dev.sock),
- GFP_KERNEL);
- if (!plat_dev.sock)
- return -ENOMEM;
- }
- if (check_acpi_support(&pdev->dev)) {
- ret = init_acpi(&pdev->dev);
- if (ret) {
- dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
- return ret;
- }
- ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
- if (ret)
- dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
- } else {
- ret = init_platform_device(&pdev->dev);
- if (ret) {
- dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
- return ret;
- }
- ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
- if (ret)
- dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
- }
-
- if (!plat_dev.is_probed) {
- plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
- plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
- plat_dev.hsmp_device.fops = &hsmp_fops;
- plat_dev.hsmp_device.parent = &pdev->dev;
- plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
- plat_dev.hsmp_device.mode = 0644;
-
- ret = misc_register(&plat_dev.hsmp_device);
- if (ret)
- return ret;
-
- plat_dev.is_probed = true;
- }
-
- return 0;
-
-}
-
-static void hsmp_pltdrv_remove(struct platform_device *pdev)
-{
- /*
- * We register only one misc_device even on multi socket system.
- * So, deregister should happen only once.
- */
- if (plat_dev.is_probed) {
- misc_deregister(&plat_dev.hsmp_device);
- plat_dev.is_probed = false;
- }
-}
-
-static struct platform_driver amd_hsmp_driver = {
- .probe = hsmp_pltdrv_probe,
- .remove_new = hsmp_pltdrv_remove,
- .driver = {
- .name = DRIVER_NAME,
- .acpi_match_table = amd_hsmp_acpi_ids,
- },
-};
-
-static struct platform_device *amd_hsmp_platdev;
-
-static int hsmp_plat_dev_register(void)
-{
- int ret;
-
- amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE);
- if (!amd_hsmp_platdev)
- return -ENOMEM;
-
- ret = platform_device_add(amd_hsmp_platdev);
- if (ret)
- platform_device_put(amd_hsmp_platdev);
-
- return ret;
-}
-
-/*
- * This check is only needed for backward compatibility of previous platforms.
- * All new platforms are expected to support ACPI based probing.
- */
-static bool legacy_hsmp_support(void)
-{
- if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
- return false;
-
- switch (boot_cpu_data.x86) {
- case 0x19:
- switch (boot_cpu_data.x86_model) {
- case 0x00 ... 0x1F:
- case 0x30 ... 0x3F:
- case 0x90 ... 0x9F:
- case 0xA0 ... 0xAF:
- return true;
- default:
- return false;
- }
- case 0x1A:
- switch (boot_cpu_data.x86_model) {
- case 0x00 ... 0x1F:
- return true;
- default:
- return false;
- }
- default:
- return false;
- }
-
- return false;
-}
-
-static int __init hsmp_plt_init(void)
-{
- int ret = -ENODEV;
-
- /*
- * amd_nb_num() returns number of SMN/DF interfaces present in the system
- * if we have N SMN/DF interfaces that ideally means N sockets
- */
- plat_dev.num_sockets = amd_nb_num();
- if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
- return ret;
-
- ret = platform_driver_register(&amd_hsmp_driver);
- if (ret)
- return ret;
-
- if (!plat_dev.is_acpi_device) {
- if (legacy_hsmp_support()) {
- /* Not ACPI device, but supports HSMP, register a plat_dev */
- ret = hsmp_plat_dev_register();
- } else {
- /* Not ACPI, Does not support HSMP */
- pr_info("HSMP is not supported on Family:%x model:%x\n",
- boot_cpu_data.x86, boot_cpu_data.x86_model);
- ret = -ENODEV;
- }
- if (ret)
- platform_driver_unregister(&amd_hsmp_driver);
- }
-
- return ret;
-}
-
-static void __exit hsmp_plt_exit(void)
-{
- platform_device_unregister(amd_hsmp_platdev);
- platform_driver_unregister(&amd_hsmp_driver);
-}
-
-device_initcall(hsmp_plt_init);
-module_exit(hsmp_plt_exit);
-
-MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
-MODULE_VERSION(DRIVER_VERSION);
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index cc11e9303a83..d65ff2acdf3d 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -51,4 +51,21 @@ struct hsmp_plat_device {
bool is_acpi_device;
bool is_probed;
};
+
+extern struct hsmp_plat_device plat_dev;
+
+int init_acpi(struct device *dev);
+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);
+int hsmp_create_acpi_sysfs_if(struct device *dev);
+int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write);
+int hsmp_cache_proto_ver(u16 sock_ind);
+long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
+umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+ struct bin_attribute *battr, int id);
+int hsmp_create_attr_list(struct attribute_group *attr_grp,
+ struct device *dev, u16 sock_ind);
+int hsmp_test(u16 sock_ind, u32 value);
#endif /* HSMP_H */
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
new file mode 100644
index 000000000000..0f181688c972
--- /dev/null
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD HSMP Platform Driver
+ * Copyright (c) 2024, AMD.
+ * All Rights Reserved.
+ *
+ * This file provides platform device implementations.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include "hsmp.h"
+
+#include <asm/amd_nb.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/acpi.h>
+
+#define DRIVER_NAME "amd_hsmp"
+#define DRIVER_VERSION "2.2"
+#define ACPI_HSMP_DEVICE_HID "AMDI0097"
+
+/*
+ * 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
+
+#define HSMP_INDEX_REG 0xc4
+#define HSMP_DATA_REG 0xc8
+
+int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write)
+{
+ int ret;
+
+ if (!sock->root)
+ return -ENODEV;
+
+ ret = pci_write_config_dword(sock->root, HSMP_INDEX_REG,
+ sock->mbinfo.base_addr + offset);
+ if (ret)
+ return ret;
+
+ ret = (write ? pci_write_config_dword(sock->root, HSMP_DATA_REG, *value)
+ : pci_read_config_dword(sock->root, HSMP_DATA_REG, value));
+
+ return ret;
+}
+
+static const struct file_operations hsmp_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = hsmp_ioctl,
+ .compat_ioctl = hsmp_ioctl,
+};
+
+static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
+{
+ const struct attribute_group **hsmp_attr_grps;
+ struct attribute_group *attr_grp;
+ u16 i;
+
+ hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
+ sizeof(*hsmp_attr_grps),
+ GFP_KERNEL);
+ if (!hsmp_attr_grps)
+ return -ENOMEM;
+
+ /* Create a sysfs directory for each socket */
+ for (i = 0; i < plat_dev.num_sockets; i++) {
+ attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
+ GFP_KERNEL);
+ if (!attr_grp)
+ return -ENOMEM;
+
+ snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
+ attr_grp->name = plat_dev.sock[i].name;
+ attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
+ hsmp_attr_grps[i] = attr_grp;
+
+ hsmp_create_attr_list(attr_grp, dev, i);
+ }
+
+ return device_add_groups(dev, hsmp_attr_grps);
+}
+
+static inline bool is_f1a_m0h(void)
+{
+ if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
+ return true;
+
+ return false;
+}
+
+static int init_platform_device(struct device *dev)
+{
+ struct hsmp_socket *sock;
+ int ret, i;
+
+ for (i = 0; i < plat_dev.num_sockets; i++) {
+ if (!node_to_amd_nb(i))
+ return -ENODEV;
+ sock = &plat_dev.sock[i];
+ sock->root = node_to_amd_nb(i)->root;
+ sock->sock_ind = i;
+ sock->dev = dev;
+ sock->mbinfo.base_addr = SMN_HSMP_BASE;
+
+ /*
+ * This is a transitional change from non-ACPI to ACPI, only
+ * family 0x1A, model 0x00 platform is supported for both ACPI and non-ACPI.
+ */
+ 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 */
+ ret = hsmp_test(i, 0xDEADBEEF);
+ if (ret) {
+ dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
+ boot_cpu_data.x86, boot_cpu_data.x86_model);
+ dev_err(dev, "Is HSMP disabled in BIOS ?\n");
+ return ret;
+ }
+
+ ret = hsmp_cache_proto_ver(i);
+ if (ret) {
+ dev_err(dev, "Failed to read HSMP protocol version\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
+ {ACPI_HSMP_DEVICE_HID, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
+
+static bool check_acpi_support(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
+ return true;
+
+ return false;
+}
+
+static int hsmp_pltdrv_probe(struct platform_device *pdev)
+{
+ int ret;
+
+ /*
+ * On ACPI supported BIOS, there is an ACPI HSMP device added for
+ * each socket, so the per socket probing, but the memory allocated for
+ * sockets should be contiguous to access it as an array,
+ * Hence allocate memory for all the sockets at once instead of allocating
+ * on each probe.
+ */
+ if (!plat_dev.is_probed) {
+ plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
+ sizeof(*plat_dev.sock),
+ GFP_KERNEL);
+ if (!plat_dev.sock)
+ return -ENOMEM;
+ }
+
+ if (check_acpi_support(&pdev->dev)) {
+ ret = init_acpi(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
+ return ret;
+ }
+ ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
+ } else {
+ ret = init_platform_device(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
+ return ret;
+ }
+ ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
+ }
+
+ if (!plat_dev.is_probed) {
+ plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
+ plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
+ plat_dev.hsmp_device.fops = &hsmp_fops;
+ plat_dev.hsmp_device.parent = &pdev->dev;
+ plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
+ plat_dev.hsmp_device.mode = 0644;
+
+ ret = misc_register(&plat_dev.hsmp_device);
+ if (ret)
+ return ret;
+
+ plat_dev.is_probed = true;
+ }
+
+ return 0;
+}
+
+static void hsmp_pltdrv_remove(struct platform_device *pdev)
+{
+ /*
+ * We register only one misc_device even on multi socket system.
+ * So, deregister should happen only once.
+ */
+ if (plat_dev.is_probed) {
+ misc_deregister(&plat_dev.hsmp_device);
+ plat_dev.is_probed = false;
+ }
+}
+
+static struct platform_driver amd_hsmp_driver = {
+ .probe = hsmp_pltdrv_probe,
+ .remove_new = hsmp_pltdrv_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .acpi_match_table = amd_hsmp_acpi_ids,
+ },
+};
+
+static struct platform_device *amd_hsmp_platdev;
+
+static int hsmp_plat_dev_register(void)
+{
+ int ret;
+
+ amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE);
+ if (!amd_hsmp_platdev)
+ return -ENOMEM;
+
+ ret = platform_device_add(amd_hsmp_platdev);
+ if (ret)
+ platform_device_put(amd_hsmp_platdev);
+
+ return ret;
+}
+
+/*
+ * This check is only needed for backward compatibility of previous platforms.
+ * All new platforms are expected to support ACPI based probing.
+ */
+static bool legacy_hsmp_support(void)
+{
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+ return false;
+
+ switch (boot_cpu_data.x86) {
+ case 0x19:
+ switch (boot_cpu_data.x86_model) {
+ case 0x00 ... 0x1F:
+ case 0x30 ... 0x3F:
+ case 0x90 ... 0x9F:
+ case 0xA0 ... 0xAF:
+ return true;
+ default:
+ return false;
+ }
+ case 0x1A:
+ switch (boot_cpu_data.x86_model) {
+ case 0x00 ... 0x1F:
+ return true;
+ default:
+ return false;
+ }
+ default:
+ return false;
+ }
+
+ return false;
+}
+
+static int __init hsmp_plt_init(void)
+{
+ int ret = -ENODEV;
+
+ /*
+ * amd_nb_num() returns number of SMN/DF interfaces present in the system
+ * if we have N SMN/DF interfaces that ideally means N sockets
+ */
+ plat_dev.num_sockets = amd_nb_num();
+ if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
+ return ret;
+
+ ret = platform_driver_register(&amd_hsmp_driver);
+ if (ret)
+ return ret;
+
+ if (!plat_dev.is_acpi_device) {
+ if (legacy_hsmp_support()) {
+ /* Not ACPI device, but supports HSMP, register a plat_dev */
+ ret = hsmp_plat_dev_register();
+ } else {
+ /* Not ACPI, Does not support HSMP */
+ pr_info("HSMP is not supported on Family:%x model:%x\n",
+ boot_cpu_data.x86, boot_cpu_data.x86_model);
+ ret = -ENODEV;
+ }
+ if (ret)
+ platform_driver_unregister(&amd_hsmp_driver);
+ }
+
+ return ret;
+}
+
+static void __exit hsmp_plt_exit(void)
+{
+ platform_device_unregister(amd_hsmp_platdev);
+ platform_driver_unregister(&amd_hsmp_driver);
+}
+
+device_initcall(hsmp_plt_init);
+module_exit(hsmp_plt_exit);
+
+MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 04/10] platform/x86/amd/hsmp: Move platform device specific code to plat.c
2024-06-27 5:39 ` [PATCH 04/10] platform/x86/amd/hsmp: Move platform device specific code to plat.c Suma Hegde
@ 2024-07-09 10:20 ` Ilpo Järvinen
0 siblings, 0 replies; 22+ messages in thread
From: Ilpo Järvinen @ 2024-07-09 10:20 UTC (permalink / raw)
To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
On Thu, 27 Jun 2024, Suma Hegde wrote:
> Move platform device part to plat.c.
>
> No functinality/logical changes.
> Common code which can be used by ACPI and platform device
> remains in hsmp.c.
> ACPI code in hsmp.c will be moved to acpi.c in next patch.
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/Makefile | 2 +-
> drivers/platform/x86/amd/hsmp/hsmp.c | 347 +------------------------
> drivers/platform/x86/amd/hsmp/hsmp.h | 17 ++
> drivers/platform/x86/amd/hsmp/plat.c | 336 ++++++++++++++++++++++++
> 4 files changed, 367 insertions(+), 335 deletions(-)
> create mode 100644 drivers/platform/x86/amd/hsmp/plat.c
>
> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
> index fda64906a5e8..fb8ba04b2f0d 100644
> --- a/drivers/platform/x86/amd/hsmp/Makefile
> +++ b/drivers/platform/x86/amd/hsmp/Makefile
> @@ -5,4 +5,4 @@
> #
>
> obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> -amd_hsmp-objs := hsmp.o
> +amd_hsmp-objs := hsmp.o plat.o
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 2c9ba51b9614..cd4de107a470 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -12,17 +12,9 @@
> #include "hsmp.h"
>
> #include <asm/amd_hsmp.h>
> -#include <asm/amd_nb.h>
> #include <linux/delay.h>
> -#include <linux/module.h>
> -#include <linux/pci.h>
> -#include <linux/platform_device.h>
> #include <linux/acpi.h>
>
> -#define DRIVER_NAME "amd_hsmp"
> -#define DRIVER_VERSION "2.2"
> -#define ACPI_HSMP_DEVICE_HID "AMDI0097"
> -
> /* HSMP Status / Error codes */
> #define HSMP_STATUS_NOT_READY 0x00
> #define HSMP_STATUS_OK 0x01
> @@ -36,45 +28,12 @@
> #define HSMP_WR true
> #define HSMP_RD false
>
> -/*
> - * 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
> -
> -#define HSMP_INDEX_REG 0xc4
> -#define HSMP_DATA_REG 0xc8
> -
> /* These are the strings specified in ACPI table */
> #define MSG_IDOFF_STR "MsgIdOffset"
> #define MSG_ARGOFF_STR "MsgArgOffset"
> #define MSG_RESPOFF_STR "MsgRspOffset"
>
> -static struct hsmp_plat_device plat_dev;
> -
> -static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
> - u32 *value, bool write)
> -{
> - int ret;
> -
> - if (!sock->root)
> - return -ENODEV;
> -
> - ret = pci_write_config_dword(sock->root, HSMP_INDEX_REG,
> - sock->mbinfo.base_addr + offset);
> - if (ret)
> - return ret;
> -
> - ret = (write ? pci_write_config_dword(sock->root, HSMP_DATA_REG, *value)
> - : pci_read_config_dword(sock->root, HSMP_DATA_REG, value));
> -
> - return ret;
> -}
> +struct hsmp_plat_device plat_dev;
>
> static void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> u32 *value, bool write)
> @@ -253,7 +212,7 @@ int hsmp_send_message(struct hsmp_message *msg)
> }
> EXPORT_SYMBOL_GPL(hsmp_send_message);
>
> -static int hsmp_test(u16 sock_ind, u32 value)
> +int hsmp_test(u16 sock_ind, u32 value)
> {
> struct hsmp_message msg = { 0 };
> int ret;
> @@ -283,7 +242,7 @@ static int hsmp_test(u16 sock_ind, u32 value)
> return ret;
> }
>
> -static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> {
> int __user *arguser = (int __user *)arg;
> struct hsmp_message msg = { 0 };
> @@ -339,12 +298,6 @@ static long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> return 0;
> }
>
> -static const struct file_operations hsmp_fops = {
> - .owner = THIS_MODULE,
> - .unlocked_ioctl = hsmp_ioctl,
> - .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,
> @@ -520,9 +473,9 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
> 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)
> +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)
> {
> struct hsmp_socket *sock = bin_attr->private;
> struct hsmp_message msg = { 0 };
> @@ -581,8 +534,8 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
> return 0;
> }
>
> -static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> - struct bin_attribute *battr, int id)
> +umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> + struct bin_attribute *battr, int id)
> {
> if (plat_dev.proto_ver == HSMP_PROTO_VER6)
> return battr->attr.mode;
> @@ -611,8 +564,8 @@ static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock
> /* One bin sysfs for metrics table */
> #define NUM_HSMP_ATTRS 1
>
> -static int hsmp_create_attr_list(struct attribute_group *attr_grp,
> - struct device *dev, u16 sock_ind)
> +int hsmp_create_attr_list(struct attribute_group *attr_grp,
> + struct device *dev, u16 sock_ind)
> {
> struct bin_attribute **hsmp_bin_attrs;
>
> @@ -628,37 +581,7 @@ static int hsmp_create_attr_list(struct attribute_group *attr_grp,
> return hsmp_init_metric_tbl_bin_attr(hsmp_bin_attrs, sock_ind);
> }
>
> -static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
> -{
> - const struct attribute_group **hsmp_attr_grps;
> - struct attribute_group *attr_grp;
> - u16 i;
> -
> - hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
> - sizeof(*hsmp_attr_grps),
> - GFP_KERNEL);
> - if (!hsmp_attr_grps)
> - return -ENOMEM;
> -
> - /* Create a sysfs directory for each socket */
> - for (i = 0; i < plat_dev.num_sockets; i++) {
> - attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
> - GFP_KERNEL);
> - if (!attr_grp)
> - return -ENOMEM;
> -
> - snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
> - attr_grp->name = plat_dev.sock[i].name;
> - attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
> - hsmp_attr_grps[i] = attr_grp;
> -
> - hsmp_create_attr_list(attr_grp, dev, i);
> - }
> -
> - return device_add_groups(dev, hsmp_attr_grps);
> -}
> -
> -static int hsmp_create_acpi_sysfs_if(struct device *dev)
> +int hsmp_create_acpi_sysfs_if(struct device *dev)
> {
> struct attribute_group *attr_grp;
> u16 sock_ind;
> @@ -681,7 +604,7 @@ 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)
> +int hsmp_cache_proto_ver(u16 sock_ind)
> {
> struct hsmp_message msg = { 0 };
> int ret;
> @@ -697,76 +620,7 @@ static int hsmp_cache_proto_ver(u16 sock_ind)
> return ret;
> }
>
> -static inline bool is_f1a_m0h(void)
> -{
> - if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
> - return true;
> -
> - return false;
> -}
> -
> -static int init_platform_device(struct device *dev)
> -{
> - struct hsmp_socket *sock;
> - int ret, i;
> -
> - for (i = 0; i < plat_dev.num_sockets; i++) {
> - if (!node_to_amd_nb(i))
> - return -ENODEV;
> - sock = &plat_dev.sock[i];
> - sock->root = node_to_amd_nb(i)->root;
> - sock->sock_ind = i;
> - sock->dev = dev;
> - sock->mbinfo.base_addr = SMN_HSMP_BASE;
> -
> - /*
> - * This is a transitional change from non-ACPI to ACPI, only
> - * family 0x1A, model 0x00 platform is supported for both ACPI and non-ACPI.
> - */
> - 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 */
> - ret = hsmp_test(i, 0xDEADBEEF);
> - if (ret) {
> - dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> - boot_cpu_data.x86, boot_cpu_data.x86_model);
> - dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> - return ret;
> - }
> - ret = hsmp_cache_proto_ver(i);
> - if (ret) {
> - dev_err(dev, "Failed to read HSMP protocol version\n");
> - return ret;
> - }
> - }
> -
> - return 0;
> -}
> -
> -static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
> - {ACPI_HSMP_DEVICE_HID, 0},
> - {}
> -};
> -MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
> -
> -static bool check_acpi_support(struct device *dev)
> -{
> - struct acpi_device *adev = ACPI_COMPANION(dev);
> -
> - if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
> - return true;
> -
> - return false;
> -}
> -
> -static int init_acpi(struct device *dev)
> +int init_acpi(struct device *dev)
> {
> u16 sock_ind;
> int ret;
> @@ -800,178 +654,3 @@ static int init_acpi(struct device *dev)
>
> return ret;
> }
> -
> -static int hsmp_pltdrv_probe(struct platform_device *pdev)
> -{
> - int ret;
> -
> - /*
> - * On ACPI supported BIOS, there is an ACPI HSMP device added for
> - * each socket, so the per socket probing, but the memory allocated for
> - * sockets should be contiguous to access it as an array,
> - * Hence allocate memory for all the sockets at once instead of allocating
> - * on each probe.
> - */
> - if (!plat_dev.is_probed) {
> - plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
> - sizeof(*plat_dev.sock),
> - GFP_KERNEL);
> - if (!plat_dev.sock)
> - return -ENOMEM;
> - }
> - if (check_acpi_support(&pdev->dev)) {
> - ret = init_acpi(&pdev->dev);
> - if (ret) {
> - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
> - return ret;
> - }
> - ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> - if (ret)
> - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> - } else {
> - ret = init_platform_device(&pdev->dev);
> - if (ret) {
> - dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
> - return ret;
> - }
> - ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
> - if (ret)
> - dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> - }
> -
> - if (!plat_dev.is_probed) {
> - plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
> - plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
> - plat_dev.hsmp_device.fops = &hsmp_fops;
> - plat_dev.hsmp_device.parent = &pdev->dev;
> - plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
> - plat_dev.hsmp_device.mode = 0644;
> -
> - ret = misc_register(&plat_dev.hsmp_device);
> - if (ret)
> - return ret;
> -
> - plat_dev.is_probed = true;
> - }
> -
> - return 0;
> -
> -}
> -
> -static void hsmp_pltdrv_remove(struct platform_device *pdev)
> -{
> - /*
> - * We register only one misc_device even on multi socket system.
> - * So, deregister should happen only once.
> - */
> - if (plat_dev.is_probed) {
> - misc_deregister(&plat_dev.hsmp_device);
> - plat_dev.is_probed = false;
> - }
> -}
> -
> -static struct platform_driver amd_hsmp_driver = {
> - .probe = hsmp_pltdrv_probe,
> - .remove_new = hsmp_pltdrv_remove,
> - .driver = {
> - .name = DRIVER_NAME,
> - .acpi_match_table = amd_hsmp_acpi_ids,
> - },
> -};
> -
> -static struct platform_device *amd_hsmp_platdev;
> -
> -static int hsmp_plat_dev_register(void)
> -{
> - int ret;
> -
> - amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE);
> - if (!amd_hsmp_platdev)
> - return -ENOMEM;
> -
> - ret = platform_device_add(amd_hsmp_platdev);
> - if (ret)
> - platform_device_put(amd_hsmp_platdev);
> -
> - return ret;
> -}
> -
> -/*
> - * This check is only needed for backward compatibility of previous platforms.
> - * All new platforms are expected to support ACPI based probing.
> - */
> -static bool legacy_hsmp_support(void)
> -{
> - if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> - return false;
> -
> - switch (boot_cpu_data.x86) {
> - case 0x19:
> - switch (boot_cpu_data.x86_model) {
> - case 0x00 ... 0x1F:
> - case 0x30 ... 0x3F:
> - case 0x90 ... 0x9F:
> - case 0xA0 ... 0xAF:
> - return true;
> - default:
> - return false;
> - }
> - case 0x1A:
> - switch (boot_cpu_data.x86_model) {
> - case 0x00 ... 0x1F:
> - return true;
> - default:
> - return false;
> - }
> - default:
> - return false;
> - }
> -
> - return false;
> -}
> -
> -static int __init hsmp_plt_init(void)
> -{
> - int ret = -ENODEV;
> -
> - /*
> - * amd_nb_num() returns number of SMN/DF interfaces present in the system
> - * if we have N SMN/DF interfaces that ideally means N sockets
> - */
> - plat_dev.num_sockets = amd_nb_num();
> - if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
> - return ret;
> -
> - ret = platform_driver_register(&amd_hsmp_driver);
> - if (ret)
> - return ret;
> -
> - if (!plat_dev.is_acpi_device) {
> - if (legacy_hsmp_support()) {
> - /* Not ACPI device, but supports HSMP, register a plat_dev */
> - ret = hsmp_plat_dev_register();
> - } else {
> - /* Not ACPI, Does not support HSMP */
> - pr_info("HSMP is not supported on Family:%x model:%x\n",
> - boot_cpu_data.x86, boot_cpu_data.x86_model);
> - ret = -ENODEV;
> - }
> - if (ret)
> - platform_driver_unregister(&amd_hsmp_driver);
> - }
> -
> - return ret;
> -}
> -
> -static void __exit hsmp_plt_exit(void)
> -{
> - platform_device_unregister(amd_hsmp_platdev);
> - platform_driver_unregister(&amd_hsmp_driver);
> -}
> -
> -device_initcall(hsmp_plt_init);
> -module_exit(hsmp_plt_exit);
> -
> -MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
> -MODULE_VERSION(DRIVER_VERSION);
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index cc11e9303a83..d65ff2acdf3d 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -51,4 +51,21 @@ struct hsmp_plat_device {
> bool is_acpi_device;
> bool is_probed;
> };
> +
> +extern struct hsmp_plat_device plat_dev;
This is awfully generic variable name to be exposed with extern.
> +int init_acpi(struct device *dev);
> +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);
> +int hsmp_create_acpi_sysfs_if(struct device *dev);
> +int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
> + u32 *value, bool write);
> +int hsmp_cache_proto_ver(u16 sock_ind);
> +long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
> +umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> + struct bin_attribute *battr, int id);
> +int hsmp_create_attr_list(struct attribute_group *attr_grp,
> + struct device *dev, u16 sock_ind);
> +int hsmp_test(u16 sock_ind, u32 value);
> #endif /* HSMP_H */
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> new file mode 100644
> index 000000000000..0f181688c972
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -0,0 +1,336 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2024, AMD.
> + * All Rights Reserved.
> + *
> + * This file provides platform device implementations.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include "hsmp.h"
> +
> +#include <asm/amd_nb.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/acpi.h>
These should be kept in alphabetical order. And add empty line between
asm/ and linux/ ones.
> +#define DRIVER_NAME "amd_hsmp"
> +#define DRIVER_VERSION "2.2"
> +#define ACPI_HSMP_DEVICE_HID "AMDI0097"
> +
> +/*
> + * 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
> +
> +#define HSMP_INDEX_REG 0xc4
> +#define HSMP_DATA_REG 0xc8
> +
> +int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
> + u32 *value, bool write)
> +{
> + int ret;
> +
> + if (!sock->root)
> + return -ENODEV;
> +
> + ret = pci_write_config_dword(sock->root, HSMP_INDEX_REG,
> + sock->mbinfo.base_addr + offset);
> + if (ret)
> + return ret;
> +
> + ret = (write ? pci_write_config_dword(sock->root, HSMP_DATA_REG, *value)
> + : pci_read_config_dword(sock->root, HSMP_DATA_REG, value));
> +
> + return ret;
> +}
> +
> +static const struct file_operations hsmp_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = hsmp_ioctl,
> + .compat_ioctl = hsmp_ioctl,
> +};
> +
> +static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
> +{
> + const struct attribute_group **hsmp_attr_grps;
> + struct attribute_group *attr_grp;
> + u16 i;
> +
> + hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
> + sizeof(*hsmp_attr_grps),
> + GFP_KERNEL);
#include for devm_kcalloc()
> + if (!hsmp_attr_grps)
> + return -ENOMEM;
> +
> + /* Create a sysfs directory for each socket */
> + for (i = 0; i < plat_dev.num_sockets; i++) {
> + attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
> + GFP_KERNEL);
> + if (!attr_grp)
> + return -ENOMEM;
> +
> + snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
> + attr_grp->name = plat_dev.sock[i].name;
> + attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
> + hsmp_attr_grps[i] = attr_grp;
> +
> + hsmp_create_attr_list(attr_grp, dev, i);
> + }
> +
> + return device_add_groups(dev, hsmp_attr_grps);
#include for device_add_groups()
--
i.
> +}
> +
> +static inline bool is_f1a_m0h(void)
> +{
> + if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
> + return true;
> +
> + return false;
> +}
> +
> +static int init_platform_device(struct device *dev)
> +{
> + struct hsmp_socket *sock;
> + int ret, i;
> +
> + for (i = 0; i < plat_dev.num_sockets; i++) {
> + if (!node_to_amd_nb(i))
> + return -ENODEV;
> + sock = &plat_dev.sock[i];
> + sock->root = node_to_amd_nb(i)->root;
> + sock->sock_ind = i;
> + sock->dev = dev;
> + sock->mbinfo.base_addr = SMN_HSMP_BASE;
> +
> + /*
> + * This is a transitional change from non-ACPI to ACPI, only
> + * family 0x1A, model 0x00 platform is supported for both ACPI and non-ACPI.
> + */
> + 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 */
> + ret = hsmp_test(i, 0xDEADBEEF);
> + if (ret) {
> + dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> + boot_cpu_data.x86, boot_cpu_data.x86_model);
> + dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> + return ret;
> + }
> +
> + ret = hsmp_cache_proto_ver(i);
> + if (ret) {
> + dev_err(dev, "Failed to read HSMP protocol version\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
> + {ACPI_HSMP_DEVICE_HID, 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
> +
> +static bool check_acpi_support(struct device *dev)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
> + return true;
> +
> + return false;
> +}
> +
> +static int hsmp_pltdrv_probe(struct platform_device *pdev)
> +{
> + int ret;
> +
> + /*
> + * On ACPI supported BIOS, there is an ACPI HSMP device added for
> + * each socket, so the per socket probing, but the memory allocated for
> + * sockets should be contiguous to access it as an array,
> + * Hence allocate memory for all the sockets at once instead of allocating
> + * on each probe.
> + */
> + if (!plat_dev.is_probed) {
> + plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
> + sizeof(*plat_dev.sock),
> + GFP_KERNEL);
> + if (!plat_dev.sock)
> + return -ENOMEM;
> + }
> +
> + if (check_acpi_support(&pdev->dev)) {
> + ret = init_acpi(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
> + return ret;
> + }
> + ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> + } else {
> + ret = init_platform_device(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
> + return ret;
> + }
> + ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> + }
> +
> + if (!plat_dev.is_probed) {
> + plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
> + plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
> + plat_dev.hsmp_device.fops = &hsmp_fops;
> + plat_dev.hsmp_device.parent = &pdev->dev;
> + plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
> + plat_dev.hsmp_device.mode = 0644;
> +
> + ret = misc_register(&plat_dev.hsmp_device);
> + if (ret)
> + return ret;
> +
> + plat_dev.is_probed = true;
> + }
> +
> + return 0;
> +}
> +
> +static void hsmp_pltdrv_remove(struct platform_device *pdev)
> +{
> + /*
> + * We register only one misc_device even on multi socket system.
> + * So, deregister should happen only once.
> + */
> + if (plat_dev.is_probed) {
> + misc_deregister(&plat_dev.hsmp_device);
> + plat_dev.is_probed = false;
> + }
> +}
> +
> +static struct platform_driver amd_hsmp_driver = {
> + .probe = hsmp_pltdrv_probe,
> + .remove_new = hsmp_pltdrv_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .acpi_match_table = amd_hsmp_acpi_ids,
> + },
> +};
> +
> +static struct platform_device *amd_hsmp_platdev;
> +
> +static int hsmp_plat_dev_register(void)
> +{
> + int ret;
> +
> + amd_hsmp_platdev = platform_device_alloc(DRIVER_NAME, PLATFORM_DEVID_NONE);
> + if (!amd_hsmp_platdev)
> + return -ENOMEM;
> +
> + ret = platform_device_add(amd_hsmp_platdev);
> + if (ret)
> + platform_device_put(amd_hsmp_platdev);
> +
> + return ret;
> +}
> +
> +/*
> + * This check is only needed for backward compatibility of previous platforms.
> + * All new platforms are expected to support ACPI based probing.
> + */
> +static bool legacy_hsmp_support(void)
> +{
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> + return false;
> +
> + switch (boot_cpu_data.x86) {
> + case 0x19:
> + switch (boot_cpu_data.x86_model) {
> + case 0x00 ... 0x1F:
> + case 0x30 ... 0x3F:
> + case 0x90 ... 0x9F:
> + case 0xA0 ... 0xAF:
> + return true;
> + default:
> + return false;
> + }
> + case 0x1A:
> + switch (boot_cpu_data.x86_model) {
> + case 0x00 ... 0x1F:
> + return true;
> + default:
> + return false;
> + }
> + default:
> + return false;
> + }
> +
> + return false;
> +}
> +
> +static int __init hsmp_plt_init(void)
> +{
> + int ret = -ENODEV;
> +
> + /*
> + * amd_nb_num() returns number of SMN/DF interfaces present in the system
> + * if we have N SMN/DF interfaces that ideally means N sockets
> + */
> + plat_dev.num_sockets = amd_nb_num();
> + if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
> + return ret;
> +
> + ret = platform_driver_register(&amd_hsmp_driver);
> + if (ret)
> + return ret;
> +
> + if (!plat_dev.is_acpi_device) {
> + if (legacy_hsmp_support()) {
> + /* Not ACPI device, but supports HSMP, register a plat_dev */
> + ret = hsmp_plat_dev_register();
> + } else {
> + /* Not ACPI, Does not support HSMP */
> + pr_info("HSMP is not supported on Family:%x model:%x\n",
> + boot_cpu_data.x86, boot_cpu_data.x86_model);
> + ret = -ENODEV;
> + }
> + if (ret)
> + platform_driver_unregister(&amd_hsmp_driver);
> + }
> +
> + return ret;
> +}
> +
> +static void __exit hsmp_plt_exit(void)
> +{
> + platform_device_unregister(amd_hsmp_platdev);
> + platform_driver_unregister(&amd_hsmp_driver);
> +}
> +
> +device_initcall(hsmp_plt_init);
> +module_exit(hsmp_plt_exit);
> +
> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 05/10] platform/x86/amd/hsmp: Move ACPI code to acpi.c
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
` (3 preceding siblings ...)
2024-06-27 5:39 ` [PATCH 04/10] platform/x86/amd/hsmp: Move platform device specific code to plat.c Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-07-09 10:09 ` Ilpo Järvinen
2024-06-27 5:39 ` [PATCH 06/10] platform/x86/amd/hsmp: Create mutually exclusive ACPI and plat drivers Suma Hegde
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
Move ACPI related code to acpi.c from hsmp.c.
We still have one driver, the driver probe will be split in the next patch.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/Makefile | 2 +-
drivers/platform/x86/amd/hsmp/acpi.c | 261 +++++++++++++++++++++++++
drivers/platform/x86/amd/hsmp/hsmp.c | 247 -----------------------
drivers/platform/x86/amd/hsmp/hsmp.h | 2 +
4 files changed, 264 insertions(+), 248 deletions(-)
create mode 100644 drivers/platform/x86/amd/hsmp/acpi.c
diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
index fb8ba04b2f0d..0cc92865c0a2 100644
--- a/drivers/platform/x86/amd/hsmp/Makefile
+++ b/drivers/platform/x86/amd/hsmp/Makefile
@@ -5,4 +5,4 @@
#
obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
-amd_hsmp-objs := hsmp.o plat.o
+amd_hsmp-objs := hsmp.o plat.o acpi.o
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
new file mode 100644
index 000000000000..90bfa1ddadbf
--- /dev/null
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD HSMP Platform Driver
+ * Copyright (c) 2024, AMD.
+ * All Rights Reserved.
+ *
+ * This file provides a device implementation for HSMP interface
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include "hsmp.h"
+
+#include <linux/acpi.h>
+
+/* These are the strings specified in ACPI table */
+#define MSG_IDOFF_STR "MsgIdOffset"
+#define MSG_ARGOFF_STR "MsgArgOffset"
+#define MSG_RESPOFF_STR "MsgRspOffset"
+
+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);
+}
+
+/* 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);
+}
+
+int hsmp_create_acpi_sysfs_if(struct device *dev)
+{
+ struct attribute_group *attr_grp;
+ u16 sock_ind;
+ int ret;
+
+ attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
+ 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_create_attr_list(attr_grp, dev, sock_ind);
+ if (ret)
+ return ret;
+
+ return devm_device_add_group(dev, attr_grp);
+}
+
+int init_acpi(struct device *dev)
+{
+ u16 sock_ind;
+ int ret;
+
+ ret = hsmp_get_uid(dev, &sock_ind);
+ if (ret)
+ return ret;
+ if (sock_ind >= plat_dev.num_sockets)
+ return -EINVAL;
+
+ ret = hsmp_parse_acpi_table(dev, sock_ind);
+ if (ret) {
+ dev_err(dev, "Failed to parse ACPI table\n");
+ return ret;
+ }
+
+ /* Test the hsmp interface */
+ ret = hsmp_test(sock_ind, 0xDEADBEEF);
+ if (ret) {
+ dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
+ boot_cpu_data.x86, boot_cpu_data.x86_model);
+ dev_err(dev, "Is HSMP disabled in BIOS ?\n");
+ return ret;
+ }
+
+ ret = hsmp_cache_proto_ver(sock_ind);
+ if (ret) {
+ dev_err(dev, "Failed to read HSMP protocol version\n");
+ return ret;
+ }
+
+ return ret;
+}
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index cd4de107a470..d3f0f08cebf7 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -28,22 +28,8 @@
#define HSMP_WR true
#define HSMP_RD false
-/* These are the strings specified in ACPI table */
-#define MSG_IDOFF_STR "MsgIdOffset"
-#define MSG_ARGOFF_STR "MsgArgOffset"
-#define MSG_RESPOFF_STR "MsgRspOffset"
-
struct hsmp_plat_device plat_dev;
-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)
{
@@ -298,181 +284,6 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
return 0;
}
-/* 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);
-}
-
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)
@@ -581,29 +392,6 @@ int hsmp_create_attr_list(struct attribute_group *attr_grp,
return hsmp_init_metric_tbl_bin_attr(hsmp_bin_attrs, sock_ind);
}
-int hsmp_create_acpi_sysfs_if(struct device *dev)
-{
- struct attribute_group *attr_grp;
- u16 sock_ind;
- int ret;
-
- attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
- 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_create_attr_list(attr_grp, dev, sock_ind);
- if (ret)
- return ret;
-
- return devm_device_add_group(dev, attr_grp);
-}
-
int hsmp_cache_proto_ver(u16 sock_ind)
{
struct hsmp_message msg = { 0 };
@@ -619,38 +407,3 @@ int hsmp_cache_proto_ver(u16 sock_ind)
return ret;
}
-
-int init_acpi(struct device *dev)
-{
- u16 sock_ind;
- int ret;
-
- ret = hsmp_get_uid(dev, &sock_ind);
- if (ret)
- return ret;
- if (sock_ind >= plat_dev.num_sockets)
- return -EINVAL;
-
- ret = hsmp_parse_acpi_table(dev, sock_ind);
- if (ret) {
- dev_err(dev, "Failed to parse ACPI table\n");
- return ret;
- }
-
- /* Test the hsmp interface */
- ret = hsmp_test(sock_ind, 0xDEADBEEF);
- if (ret) {
- dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
- boot_cpu_data.x86, boot_cpu_data.x86_model);
- dev_err(dev, "Is HSMP disabled in BIOS ?\n");
- return ret;
- }
-
- ret = hsmp_cache_proto_ver(sock_ind);
- if (ret) {
- dev_err(dev, "Failed to read HSMP protocol version\n");
- return ret;
- }
-
- return ret;
-}
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index d65ff2acdf3d..2baeef57ca54 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -68,4 +68,6 @@ umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
int hsmp_create_attr_list(struct attribute_group *attr_grp,
struct device *dev, u16 sock_ind);
int hsmp_test(u16 sock_ind, u32 value);
+void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write);
#endif /* HSMP_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 05/10] platform/x86/amd/hsmp: Move ACPI code to acpi.c
2024-06-27 5:39 ` [PATCH 05/10] platform/x86/amd/hsmp: Move ACPI code to acpi.c Suma Hegde
@ 2024-07-09 10:09 ` Ilpo Järvinen
2024-07-09 10:50 ` Suma Hegde
0 siblings, 1 reply; 22+ messages in thread
From: Ilpo Järvinen @ 2024-07-09 10:09 UTC (permalink / raw)
To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
On Thu, 27 Jun 2024, Suma Hegde wrote:
> Move ACPI related code to acpi.c from hsmp.c.
> We still have one driver, the driver probe will be split in the next patch.
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/Makefile | 2 +-
> drivers/platform/x86/amd/hsmp/acpi.c | 261 +++++++++++++++++++++++++
> drivers/platform/x86/amd/hsmp/hsmp.c | 247 -----------------------
> drivers/platform/x86/amd/hsmp/hsmp.h | 2 +
> 4 files changed, 264 insertions(+), 248 deletions(-)
> create mode 100644 drivers/platform/x86/amd/hsmp/acpi.c
>
> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
> index fb8ba04b2f0d..0cc92865c0a2 100644
> --- a/drivers/platform/x86/amd/hsmp/Makefile
> +++ b/drivers/platform/x86/amd/hsmp/Makefile
> @@ -5,4 +5,4 @@
> #
>
> obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> -amd_hsmp-objs := hsmp.o plat.o
> +amd_hsmp-objs := hsmp.o plat.o acpi.o
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> new file mode 100644
> index 000000000000..90bfa1ddadbf
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2024, AMD.
> + * All Rights Reserved.
> + *
> + * This file provides a device implementation for HSMP interface
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include "hsmp.h"
> +
> +#include <linux/acpi.h>
> +
> +/* These are the strings specified in ACPI table */
> +#define MSG_IDOFF_STR "MsgIdOffset"
> +#define MSG_ARGOFF_STR "MsgArgOffset"
> +#define MSG_RESPOFF_STR "MsgRspOffset"
> +
> +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);
> +}
> +
> +/* 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);
#include for GUID_INIT() and guid_equal().
> + 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);
#include for kstrtou16().
I think I mentioned already earlier that each file should include what
they use, but it seems that not even the original hsmp.c contained these
so they came from somewhere deeper in the include chain.
> +}
> +
> +static acpi_status hsmp_resource(struct acpi_resource *res, void *data)
> +{
> + struct hsmp_socket *sock = data;
> + struct resource r;
#include for struct resource
> +
> + 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);
#include for resource_size()
> + 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));
#include for dev_err()
> + return -ENODEV;
#include for -Exx codes.
> + }
> +
> + 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 */
uncached
> + 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);
> +}
> +
> +int hsmp_create_acpi_sysfs_if(struct device *dev)
> +{
> + struct attribute_group *attr_grp;
> + u16 sock_ind;
> + int ret;
> +
> + attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
#include for devm_kzalloc()
> + 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_create_attr_list(attr_grp, dev, sock_ind);
> + if (ret)
> + return ret;
> +
> + return devm_device_add_group(dev, attr_grp);
#include for devm_device_add_group()
--
i.
> +}
> +
> +int init_acpi(struct device *dev)
> +{
> + u16 sock_ind;
> + int ret;
> +
> + ret = hsmp_get_uid(dev, &sock_ind);
> + if (ret)
> + return ret;
> + if (sock_ind >= plat_dev.num_sockets)
> + return -EINVAL;
> +
> + ret = hsmp_parse_acpi_table(dev, sock_ind);
> + if (ret) {
> + dev_err(dev, "Failed to parse ACPI table\n");
> + return ret;
> + }
> +
> + /* Test the hsmp interface */
> + ret = hsmp_test(sock_ind, 0xDEADBEEF);
> + if (ret) {
> + dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> + boot_cpu_data.x86, boot_cpu_data.x86_model);
> + dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> + return ret;
> + }
> +
> + ret = hsmp_cache_proto_ver(sock_ind);
> + if (ret) {
> + dev_err(dev, "Failed to read HSMP protocol version\n");
> + return ret;
> + }
> +
> + return ret;
> +}
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index cd4de107a470..d3f0f08cebf7 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -28,22 +28,8 @@
> #define HSMP_WR true
> #define HSMP_RD false
>
> -/* These are the strings specified in ACPI table */
> -#define MSG_IDOFF_STR "MsgIdOffset"
> -#define MSG_ARGOFF_STR "MsgArgOffset"
> -#define MSG_RESPOFF_STR "MsgRspOffset"
> -
> struct hsmp_plat_device plat_dev;
>
> -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)
> {
> @@ -298,181 +284,6 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> return 0;
> }
>
> -/* 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);
> -}
> -
> 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)
> @@ -581,29 +392,6 @@ int hsmp_create_attr_list(struct attribute_group *attr_grp,
> return hsmp_init_metric_tbl_bin_attr(hsmp_bin_attrs, sock_ind);
> }
>
> -int hsmp_create_acpi_sysfs_if(struct device *dev)
> -{
> - struct attribute_group *attr_grp;
> - u16 sock_ind;
> - int ret;
> -
> - attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
> - 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_create_attr_list(attr_grp, dev, sock_ind);
> - if (ret)
> - return ret;
> -
> - return devm_device_add_group(dev, attr_grp);
> -}
> -
> int hsmp_cache_proto_ver(u16 sock_ind)
> {
> struct hsmp_message msg = { 0 };
> @@ -619,38 +407,3 @@ int hsmp_cache_proto_ver(u16 sock_ind)
>
> return ret;
> }
> -
> -int init_acpi(struct device *dev)
> -{
> - u16 sock_ind;
> - int ret;
> -
> - ret = hsmp_get_uid(dev, &sock_ind);
> - if (ret)
> - return ret;
> - if (sock_ind >= plat_dev.num_sockets)
> - return -EINVAL;
> -
> - ret = hsmp_parse_acpi_table(dev, sock_ind);
> - if (ret) {
> - dev_err(dev, "Failed to parse ACPI table\n");
> - return ret;
> - }
> -
> - /* Test the hsmp interface */
> - ret = hsmp_test(sock_ind, 0xDEADBEEF);
> - if (ret) {
> - dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> - boot_cpu_data.x86, boot_cpu_data.x86_model);
> - dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> - return ret;
> - }
> -
> - ret = hsmp_cache_proto_ver(sock_ind);
> - if (ret) {
> - dev_err(dev, "Failed to read HSMP protocol version\n");
> - return ret;
> - }
> -
> - return ret;
> -}
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index d65ff2acdf3d..2baeef57ca54 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -68,4 +68,6 @@ umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> int hsmp_create_attr_list(struct attribute_group *attr_grp,
> struct device *dev, u16 sock_ind);
> int hsmp_test(u16 sock_ind, u32 value);
> +void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> + u32 *value, bool write);
> #endif /* HSMP_H */
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 05/10] platform/x86/amd/hsmp: Move ACPI code to acpi.c
2024-07-09 10:09 ` Ilpo Järvinen
@ 2024-07-09 10:50 ` Suma Hegde
0 siblings, 0 replies; 22+ messages in thread
From: Suma Hegde @ 2024-07-09 10:50 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
Hi Ilpo,
On 7/9/2024 3:39 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Thu, 27 Jun 2024, Suma Hegde wrote:
>
>> Move ACPI related code to acpi.c from hsmp.c.
>> We still have one driver, the driver probe will be split in the next patch.
>>
>> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>> ---
>> drivers/platform/x86/amd/hsmp/Makefile | 2 +-
>> drivers/platform/x86/amd/hsmp/acpi.c | 261 +++++++++++++++++++++++++
>> drivers/platform/x86/amd/hsmp/hsmp.c | 247 -----------------------
>> drivers/platform/x86/amd/hsmp/hsmp.h | 2 +
>> 4 files changed, 264 insertions(+), 248 deletions(-)
>> create mode 100644 drivers/platform/x86/amd/hsmp/acpi.c
>>
>> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
>> index fb8ba04b2f0d..0cc92865c0a2 100644
>> --- a/drivers/platform/x86/amd/hsmp/Makefile
>> +++ b/drivers/platform/x86/amd/hsmp/Makefile
>> @@ -5,4 +5,4 @@
>> #
>>
>> obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
>> -amd_hsmp-objs := hsmp.o plat.o
>> +amd_hsmp-objs := hsmp.o plat.o acpi.o
>> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
>> new file mode 100644
>> index 000000000000..90bfa1ddadbf
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
>> @@ -0,0 +1,261 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * AMD HSMP Platform Driver
>> + * Copyright (c) 2024, AMD.
>> + * All Rights Reserved.
>> + *
>> + * This file provides a device implementation for HSMP interface
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include "hsmp.h"
>> +
>> +#include <linux/acpi.h>
>> +
>> +/* These are the strings specified in ACPI table */
>> +#define MSG_IDOFF_STR "MsgIdOffset"
>> +#define MSG_ARGOFF_STR "MsgArgOffset"
>> +#define MSG_RESPOFF_STR "MsgRspOffset"
>> +
>> +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);
>> +}
>> +
>> +/* 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);
> #include for GUID_INIT() and guid_equal().
Ok, will change it.
>> + 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);
> #include for kstrtou16().
>
> I think I mentioned already earlier that each file should include what
> they use, but it seems that not even the original hsmp.c contained these
> so they came from somewhere deeper in the include chain.
Sorry, probably i had not understood the comment clearly.
Will include all the header files explicitly in alphabetical order.
Also will address all the comments for this patch series and will send v2.
>> +}
>> +
>> +static acpi_status hsmp_resource(struct acpi_resource *res, void *data)
>> +{
>> + struct hsmp_socket *sock = data;
>> + struct resource r;
> #include for struct resource
>
>> +
>> + 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);
> #include for resource_size()
ok.
>> + 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));
> #include for dev_err()
ok.
>> + return -ENODEV;
> #include for -Exx codes.
ok
>> + }
>> +
>> + 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 */
> uncached
ok.
>> + 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);
>> +}
>> +
>> +int hsmp_create_acpi_sysfs_if(struct device *dev)
>> +{
>> + struct attribute_group *attr_grp;
>> + u16 sock_ind;
>> + int ret;
>> +
>> + attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
> #include for devm_kzalloc()
ok.
>> + 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_create_attr_list(attr_grp, dev, sock_ind);
>> + if (ret)
>> + return ret;
>> +
>> + return devm_device_add_group(dev, attr_grp);
> #include for devm_device_add_group()
ok.
>
> --
> i.
>
>> +}
>> +
>> +int init_acpi(struct device *dev)
>> +{
>> + u16 sock_ind;
>> + int ret;
>> +
>> + ret = hsmp_get_uid(dev, &sock_ind);
>> + if (ret)
>> + return ret;
>> + if (sock_ind >= plat_dev.num_sockets)
>> + return -EINVAL;
>> +
>> + ret = hsmp_parse_acpi_table(dev, sock_ind);
>> + if (ret) {
>> + dev_err(dev, "Failed to parse ACPI table\n");
>> + return ret;
>> + }
>> +
>> + /* Test the hsmp interface */
>> + ret = hsmp_test(sock_ind, 0xDEADBEEF);
>> + if (ret) {
>> + dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
>> + boot_cpu_data.x86, boot_cpu_data.x86_model);
>> + dev_err(dev, "Is HSMP disabled in BIOS ?\n");
>> + return ret;
>> + }
>> +
>> + ret = hsmp_cache_proto_ver(sock_ind);
>> + if (ret) {
>> + dev_err(dev, "Failed to read HSMP protocol version\n");
>> + return ret;
>> + }
>> +
>> + return ret;
>> +}
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
>> index cd4de107a470..d3f0f08cebf7 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
>> @@ -28,22 +28,8 @@
>> #define HSMP_WR true
>> #define HSMP_RD false
>>
>> -/* These are the strings specified in ACPI table */
>> -#define MSG_IDOFF_STR "MsgIdOffset"
>> -#define MSG_ARGOFF_STR "MsgArgOffset"
>> -#define MSG_RESPOFF_STR "MsgRspOffset"
>> -
>> struct hsmp_plat_device plat_dev;
>>
>> -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)
>> {
>> @@ -298,181 +284,6 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
>> return 0;
>> }
>>
>> -/* 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);
>> -}
>> -
>> 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)
>> @@ -581,29 +392,6 @@ int hsmp_create_attr_list(struct attribute_group *attr_grp,
>> return hsmp_init_metric_tbl_bin_attr(hsmp_bin_attrs, sock_ind);
>> }
>>
>> -int hsmp_create_acpi_sysfs_if(struct device *dev)
>> -{
>> - struct attribute_group *attr_grp;
>> - u16 sock_ind;
>> - int ret;
>> -
>> - attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
>> - 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_create_attr_list(attr_grp, dev, sock_ind);
>> - if (ret)
>> - return ret;
>> -
>> - return devm_device_add_group(dev, attr_grp);
>> -}
>> -
>> int hsmp_cache_proto_ver(u16 sock_ind)
>> {
>> struct hsmp_message msg = { 0 };
>> @@ -619,38 +407,3 @@ int hsmp_cache_proto_ver(u16 sock_ind)
>>
>> return ret;
>> }
>> -
>> -int init_acpi(struct device *dev)
>> -{
>> - u16 sock_ind;
>> - int ret;
>> -
>> - ret = hsmp_get_uid(dev, &sock_ind);
>> - if (ret)
>> - return ret;
>> - if (sock_ind >= plat_dev.num_sockets)
>> - return -EINVAL;
>> -
>> - ret = hsmp_parse_acpi_table(dev, sock_ind);
>> - if (ret) {
>> - dev_err(dev, "Failed to parse ACPI table\n");
>> - return ret;
>> - }
>> -
>> - /* Test the hsmp interface */
>> - ret = hsmp_test(sock_ind, 0xDEADBEEF);
>> - if (ret) {
>> - dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
>> - boot_cpu_data.x86, boot_cpu_data.x86_model);
>> - dev_err(dev, "Is HSMP disabled in BIOS ?\n");
>> - return ret;
>> - }
>> -
>> - ret = hsmp_cache_proto_ver(sock_ind);
>> - if (ret) {
>> - dev_err(dev, "Failed to read HSMP protocol version\n");
>> - return ret;
>> - }
>> -
>> - return ret;
>> -}
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
>> index d65ff2acdf3d..2baeef57ca54 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
>> @@ -68,4 +68,6 @@ umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>> int hsmp_create_attr_list(struct attribute_group *attr_grp,
>> struct device *dev, u16 sock_ind);
>> int hsmp_test(u16 sock_ind, u32 value);
>> +void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
>> + u32 *value, bool write);
>> #endif /* HSMP_H */
Thanks and Regards,
Suma
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 06/10] platform/x86/amd/hsmp: Create mutually exclusive ACPI and plat drivers
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
` (4 preceding siblings ...)
2024-06-27 5:39 ` [PATCH 05/10] platform/x86/amd/hsmp: Move ACPI code to acpi.c Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-07-08 10:15 ` Ilpo Järvinen
2024-06-27 5:39 ` [PATCH 07/10] platform/x86/amd/hsmp: Use name space while exporting module symbols Suma Hegde
` (3 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
Separate the probes for ACPI and platform device drivers.
Provide a Kconfig option to select either the
ACPI or the platform device based driver.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
arch/x86/include/asm/amd_hsmp.h | 2 +-
drivers/platform/x86/amd/hsmp/Kconfig | 25 ++++-
drivers/platform/x86/amd/hsmp/Makefile | 10 +-
drivers/platform/x86/amd/hsmp/acpi.c | 114 ++++++++++++++++++++++-
drivers/platform/x86/amd/hsmp/hsmp.c | 25 ++---
drivers/platform/x86/amd/hsmp/hsmp.h | 8 +-
drivers/platform/x86/amd/hsmp/plat.c | 122 +++++++------------------
7 files changed, 183 insertions(+), 123 deletions(-)
diff --git a/arch/x86/include/asm/amd_hsmp.h b/arch/x86/include/asm/amd_hsmp.h
index 03c2ce3edaf5..ada14e55f9f4 100644
--- a/arch/x86/include/asm/amd_hsmp.h
+++ b/arch/x86/include/asm/amd_hsmp.h
@@ -5,7 +5,7 @@
#include <uapi/asm/amd_hsmp.h>
-#if IS_ENABLED(CONFIG_AMD_HSMP)
+#if IS_ENABLED(CONFIG_AMD_HSMP) || IS_ENABLED(CONFIG_AMD_HSMP_ACPI)
int hsmp_send_message(struct hsmp_message *msg);
#else
static inline int hsmp_send_message(struct hsmp_message *msg)
diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
index b55d4ed9bceb..1cb10d2aac77 100644
--- a/drivers/platform/x86/amd/hsmp/Kconfig
+++ b/drivers/platform/x86/amd/hsmp/Kconfig
@@ -3,9 +3,30 @@
# AMD HSMP Driver
#
+menu "AMD Host System Management Port driver"
+ depends on AMD_NB
+
+config AMD_HSMP_ACPI
+ tristate "AMD HSMP ACPI driver"
+ depends on 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.
+
+ This driver supports ACPI based probing.
+
+ You may enable this, if your platform bios provides an ACPI object
+ as described in the documentation.
+
+ If you choose to compile this driver as a module the module will be
+ called amd_hsmp.
+
config AMD_HSMP
tristate "AMD HSMP Driver"
- depends on AMD_NB && X86_64 && ACPI
+ depends on !(AMD_HSMP_ACPI || AMD_HSMP_ACPI=m)
help
The driver provides a way for user space tools to monitor and manage
system management functionality on EPYC server CPUs from AMD.
@@ -15,3 +36,5 @@ config AMD_HSMP
If you choose to compile this driver as a module the module will be
called amd_hsmp.
+
+endmenu
diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
index 0cc92865c0a2..53ebc462b0f9 100644
--- a/drivers/platform/x86/amd/hsmp/Makefile
+++ b/drivers/platform/x86/amd/hsmp/Makefile
@@ -4,5 +4,11 @@
# AMD HSMP Driver
#
-obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
-amd_hsmp-objs := hsmp.o plat.o acpi.o
+ifneq ($(CONFIG_AMD_HSMP), )
+obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
+amd_hsmp-objs = hsmp.o plat.o
+endif
+ifneq ($(CONFIG_AMD_HSMP_ACPI), )
+obj-$(CONFIG_AMD_HSMP_ACPI) += amd_hsmp.o
+amd_hsmp-objs = hsmp.o acpi.o
+endif
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 90bfa1ddadbf..0307f4e7176d 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -12,21 +12,34 @@
#include "hsmp.h"
#include <linux/acpi.h>
+#include <asm/amd_nb.h>
+#include <linux/platform_device.h>
+
+#define DRIVER_NAME "amd_hsmp"
+#define DRIVER_VERSION "2.3"
+#define ACPI_HSMP_DEVICE_HID "AMDI0097"
/* These are the strings specified in ACPI table */
#define MSG_IDOFF_STR "MsgIdOffset"
#define MSG_ARGOFF_STR "MsgArgOffset"
#define MSG_RESPOFF_STR "MsgRspOffset"
-void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
- u32 *value, bool write)
+static int 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);
+ return 0;
}
+static const struct file_operations hsmp_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = hsmp_ioctl,
+ .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,
@@ -189,7 +202,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
sock->sock_ind = sock_ind;
sock->dev = dev;
- plat_dev.is_acpi_device = true;
+ sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr;
sema_init(&sock->hsmp_sem, 1);
@@ -202,7 +215,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
return hsmp_read_acpi_dsd(sock);
}
-int hsmp_create_acpi_sysfs_if(struct device *dev)
+static int hsmp_create_acpi_sysfs_if(struct device *dev)
{
struct attribute_group *attr_grp;
u16 sock_ind;
@@ -225,7 +238,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev)
return devm_device_add_group(dev, attr_grp);
}
-int init_acpi(struct device *dev)
+static int init_acpi(struct device *dev)
{
u16 sock_ind;
int ret;
@@ -259,3 +272,94 @@ int init_acpi(struct device *dev)
return ret;
}
+
+static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
+ {ACPI_HSMP_DEVICE_HID, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
+
+static bool check_acpi_support(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
+ return true;
+
+ return false;
+}
+
+static int hsmp_acpi_probe(struct platform_device *pdev)
+{
+ int ret;
+
+ if (!plat_dev.is_probed) {
+ plat_dev.num_sockets = amd_nb_num();
+ if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
+ return -ENODEV;
+
+ plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
+ sizeof(*plat_dev.sock),
+ GFP_KERNEL);
+ if (!plat_dev.sock)
+ return -ENOMEM;
+ }
+
+ if (!check_acpi_support(&pdev->dev)) {
+ dev_err(&pdev->dev, "Not ACPI device?\n");
+ return -ENODEV;
+ }
+
+ ret = init_acpi(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
+ return ret;
+ }
+
+ ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
+
+ if (!plat_dev.is_probed) {
+ plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
+ plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
+ plat_dev.hsmp_device.fops = &hsmp_fops;
+ plat_dev.hsmp_device.parent = &pdev->dev;
+ plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
+ plat_dev.hsmp_device.mode = 0644;
+
+ ret = misc_register(&plat_dev.hsmp_device);
+ if (ret)
+ return ret;
+ plat_dev.is_probed = true;
+ }
+
+ return 0;
+}
+
+static void hsmp_acpi_remove(struct platform_device *pdev)
+{
+ /*
+ * We register only one misc_device even on multi socket system.
+ * So, deregister should happen only once.
+ */
+ if (plat_dev.is_probed) {
+ misc_deregister(&plat_dev.hsmp_device);
+ plat_dev.is_probed = false;
+ }
+}
+
+static struct platform_driver amd_hsmp_driver = {
+ .probe = hsmp_acpi_probe,
+ .remove_new = hsmp_acpi_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .acpi_match_table = amd_hsmp_acpi_ids,
+ },
+};
+
+module_platform_driver(amd_hsmp_driver);
+
+MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index d3f0f08cebf7..8cad5e813947 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -30,17 +30,6 @@
struct hsmp_plat_device plat_dev;
-static int amd_hsmp_rdwr(struct hsmp_socket *sock, u32 offset,
- u32 *value, bool write)
-{
- if (plat_dev.is_acpi_device)
- amd_hsmp_acpi_rdwr(sock, offset, value, write);
- else
- return amd_hsmp_pci_rdwr(sock, offset, value, write);
-
- return 0;
-}
-
/*
* Send a message to the HSMP port via PCI-e config space registers
* or by writing to MMIO space.
@@ -63,7 +52,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
/* Clear the status register */
mbox_status = HSMP_STATUS_NOT_READY;
- ret = amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_WR);
+ ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_WR);
if (ret) {
pr_err("Error %d clearing mailbox status register\n", ret);
return ret;
@@ -72,8 +61,8 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
index = 0;
/* Write any message arguments */
while (index < msg->num_args) {
- ret = amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
- &msg->args[index], HSMP_WR);
+ ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
+ &msg->args[index], HSMP_WR);
if (ret) {
pr_err("Error %d writing message argument %d\n", ret, index);
return ret;
@@ -82,7 +71,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
}
/* Write the message ID which starts the operation */
- ret = amd_hsmp_rdwr(sock, mbinfo->msg_id_off, &msg->msg_id, HSMP_WR);
+ ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_id_off, &msg->msg_id, HSMP_WR);
if (ret) {
pr_err("Error %d writing message ID %u\n", ret, msg->msg_id);
return ret;
@@ -99,7 +88,7 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
timeout = jiffies + msecs_to_jiffies(HSMP_MSG_TIMEOUT);
while (time_before(jiffies, timeout)) {
- ret = amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_RD);
+ ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_resp_off, &mbox_status, HSMP_RD);
if (ret) {
pr_err("Error %d reading mailbox status\n", ret);
return ret;
@@ -134,8 +123,8 @@ static int __hsmp_send_message(struct hsmp_socket *sock, struct hsmp_message *ms
*/
index = 0;
while (index < msg->response_sz) {
- ret = amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
- &msg->args[index], HSMP_RD);
+ ret = sock->amd_hsmp_rdwr(sock, mbinfo->msg_arg_off + (index << 2),
+ &msg->args[index], HSMP_RD);
if (ret) {
pr_err("Error %d reading response %u for message ID:%u\n",
ret, index, msg->msg_id);
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index 2baeef57ca54..f876370ba65c 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -41,6 +41,7 @@ struct hsmp_socket {
struct pci_dev *root;
struct device *dev;
u16 sock_ind;
+ int (*amd_hsmp_rdwr)(struct hsmp_socket *sock, u32 off, u32 *val, bool rw);
};
struct hsmp_plat_device {
@@ -48,19 +49,14 @@ struct hsmp_plat_device {
struct hsmp_socket *sock;
u32 proto_ver;
u16 num_sockets;
- bool is_acpi_device;
bool is_probed;
};
extern struct hsmp_plat_device plat_dev;
-int init_acpi(struct device *dev);
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);
-int hsmp_create_acpi_sysfs_if(struct device *dev);
-int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
- u32 *value, bool write);
int hsmp_cache_proto_ver(u16 sock_ind);
long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
@@ -68,6 +64,4 @@ umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
int hsmp_create_attr_list(struct attribute_group *attr_grp,
struct device *dev, u16 sock_ind);
int hsmp_test(u16 sock_ind, u32 value);
-void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
- u32 *value, bool write);
#endif /* HSMP_H */
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index 0f181688c972..62423581d839 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -15,11 +15,9 @@
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
-#include <linux/acpi.h>
#define DRIVER_NAME "amd_hsmp"
-#define DRIVER_VERSION "2.2"
-#define ACPI_HSMP_DEVICE_HID "AMDI0097"
+#define DRIVER_VERSION "2.3"
/*
* To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox
@@ -35,8 +33,8 @@
#define HSMP_INDEX_REG 0xc4
#define HSMP_DATA_REG 0xc8
-int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
- u32 *value, bool write)
+static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write)
{
int ret;
@@ -111,6 +109,7 @@ static int init_platform_device(struct device *dev)
sock->sock_ind = i;
sock->dev = dev;
sock->mbinfo.base_addr = SMN_HSMP_BASE;
+ sock->amd_hsmp_rdwr = amd_hsmp_pci_rdwr;
/*
* This is a transitional change from non-ACPI to ACPI, only
@@ -144,89 +143,39 @@ static int init_platform_device(struct device *dev)
return 0;
}
-static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
- {ACPI_HSMP_DEVICE_HID, 0},
- {}
-};
-MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
-
-static bool check_acpi_support(struct device *dev)
-{
- struct acpi_device *adev = ACPI_COMPANION(dev);
-
- if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
- return true;
-
- return false;
-}
-
static int hsmp_pltdrv_probe(struct platform_device *pdev)
{
int ret;
- /*
- * On ACPI supported BIOS, there is an ACPI HSMP device added for
- * each socket, so the per socket probing, but the memory allocated for
- * sockets should be contiguous to access it as an array,
- * Hence allocate memory for all the sockets at once instead of allocating
- * on each probe.
- */
- if (!plat_dev.is_probed) {
- plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
- sizeof(*plat_dev.sock),
- GFP_KERNEL);
- if (!plat_dev.sock)
- return -ENOMEM;
- }
+ plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
+ sizeof(*plat_dev.sock),
+ GFP_KERNEL);
+ if (!plat_dev.sock)
+ return -ENOMEM;
- if (check_acpi_support(&pdev->dev)) {
- ret = init_acpi(&pdev->dev);
- if (ret) {
- dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
- return ret;
- }
- ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
- if (ret)
- dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
- } else {
- ret = init_platform_device(&pdev->dev);
- if (ret) {
- dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
- return ret;
- }
- ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
- if (ret)
- dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
+ ret = init_platform_device(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to init HSMP mailbox\n");
+ return ret;
}
- if (!plat_dev.is_probed) {
- plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
- plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
- plat_dev.hsmp_device.fops = &hsmp_fops;
- plat_dev.hsmp_device.parent = &pdev->dev;
- plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
- plat_dev.hsmp_device.mode = 0644;
-
- ret = misc_register(&plat_dev.hsmp_device);
- if (ret)
- return ret;
+ ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
- plat_dev.is_probed = true;
- }
+ plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
+ plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
+ plat_dev.hsmp_device.fops = &hsmp_fops;
+ plat_dev.hsmp_device.parent = &pdev->dev;
+ plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
+ plat_dev.hsmp_device.mode = 0644;
- return 0;
+ return misc_register(&plat_dev.hsmp_device);
}
static void hsmp_pltdrv_remove(struct platform_device *pdev)
{
- /*
- * We register only one misc_device even on multi socket system.
- * So, deregister should happen only once.
- */
- if (plat_dev.is_probed) {
- misc_deregister(&plat_dev.hsmp_device);
- plat_dev.is_probed = false;
- }
+ misc_deregister(&plat_dev.hsmp_device);
}
static struct platform_driver amd_hsmp_driver = {
@@ -234,7 +183,6 @@ static struct platform_driver amd_hsmp_driver = {
.remove_new = hsmp_pltdrv_remove,
.driver = {
.name = DRIVER_NAME,
- .acpi_match_table = amd_hsmp_acpi_ids,
},
};
@@ -293,6 +241,12 @@ static int __init hsmp_plt_init(void)
{
int ret = -ENODEV;
+ if (!legacy_hsmp_support()) {
+ pr_info("HSMP is not supported on Family:%x model:%x\n",
+ boot_cpu_data.x86, boot_cpu_data.x86_model);
+ return ret;
+ }
+
/*
* amd_nb_num() returns number of SMN/DF interfaces present in the system
* if we have N SMN/DF interfaces that ideally means N sockets
@@ -305,19 +259,9 @@ static int __init hsmp_plt_init(void)
if (ret)
return ret;
- if (!plat_dev.is_acpi_device) {
- if (legacy_hsmp_support()) {
- /* Not ACPI device, but supports HSMP, register a plat_dev */
- ret = hsmp_plat_dev_register();
- } else {
- /* Not ACPI, Does not support HSMP */
- pr_info("HSMP is not supported on Family:%x model:%x\n",
- boot_cpu_data.x86, boot_cpu_data.x86_model);
- ret = -ENODEV;
- }
- if (ret)
- platform_driver_unregister(&amd_hsmp_driver);
- }
+ ret = hsmp_plat_dev_register();
+ if (ret)
+ platform_driver_unregister(&amd_hsmp_driver);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 06/10] platform/x86/amd/hsmp: Create mutually exclusive ACPI and plat drivers
2024-06-27 5:39 ` [PATCH 06/10] platform/x86/amd/hsmp: Create mutually exclusive ACPI and plat drivers Suma Hegde
@ 2024-07-08 10:15 ` Ilpo Järvinen
2024-07-09 6:41 ` Suma Hegde
0 siblings, 1 reply; 22+ messages in thread
From: Ilpo Järvinen @ 2024-07-08 10:15 UTC (permalink / raw)
To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
On Thu, 27 Jun 2024, Suma Hegde wrote:
> Separate the probes for ACPI and platform device drivers.
> Provide a Kconfig option to select either the
> ACPI or the platform device based driver.
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> arch/x86/include/asm/amd_hsmp.h | 2 +-
> drivers/platform/x86/amd/hsmp/Kconfig | 25 ++++-
> drivers/platform/x86/amd/hsmp/Makefile | 10 +-
> drivers/platform/x86/amd/hsmp/acpi.c | 114 ++++++++++++++++++++++-
> drivers/platform/x86/amd/hsmp/hsmp.c | 25 ++---
> drivers/platform/x86/amd/hsmp/hsmp.h | 8 +-
> drivers/platform/x86/amd/hsmp/plat.c | 122 +++++++------------------
> 7 files changed, 183 insertions(+), 123 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd_hsmp.h b/arch/x86/include/asm/amd_hsmp.h
> index 03c2ce3edaf5..ada14e55f9f4 100644
> --- a/arch/x86/include/asm/amd_hsmp.h
> +++ b/arch/x86/include/asm/amd_hsmp.h
> @@ -5,7 +5,7 @@
>
> #include <uapi/asm/amd_hsmp.h>
>
> -#if IS_ENABLED(CONFIG_AMD_HSMP)
> +#if IS_ENABLED(CONFIG_AMD_HSMP) || IS_ENABLED(CONFIG_AMD_HSMP_ACPI)
> int hsmp_send_message(struct hsmp_message *msg);
> #else
> static inline int hsmp_send_message(struct hsmp_message *msg)
> diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
> index b55d4ed9bceb..1cb10d2aac77 100644
> --- a/drivers/platform/x86/amd/hsmp/Kconfig
> +++ b/drivers/platform/x86/amd/hsmp/Kconfig
> @@ -3,9 +3,30 @@
> # AMD HSMP Driver
> #
>
> +menu "AMD Host System Management Port driver"
> + depends on AMD_NB
> +
> +config AMD_HSMP_ACPI
> + tristate "AMD HSMP ACPI driver"
> + depends on 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.
> +
> + This driver supports ACPI based probing.
> +
> + You may enable this, if your platform bios provides an ACPI object
> + as described in the documentation.
> +
> + If you choose to compile this driver as a module the module will be
> + called amd_hsmp.
> +
> config AMD_HSMP
> tristate "AMD HSMP Driver"
> - depends on AMD_NB && X86_64 && ACPI
> + depends on !(AMD_HSMP_ACPI || AMD_HSMP_ACPI=m)
depends on AMD_HSMP_ACPI=n is simpler.
> help
> The driver provides a way for user space tools to monitor and manage
> system management functionality on EPYC server CPUs from AMD.
> @@ -15,3 +36,5 @@ config AMD_HSMP
>
> If you choose to compile this driver as a module the module will be
> called amd_hsmp.
> +
> +endmenu
> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
> index 0cc92865c0a2..53ebc462b0f9 100644
> --- a/drivers/platform/x86/amd/hsmp/Makefile
> +++ b/drivers/platform/x86/amd/hsmp/Makefile
> @@ -4,5 +4,11 @@
> # AMD HSMP Driver
> #
>
> -obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> -amd_hsmp-objs := hsmp.o plat.o acpi.o
> +ifneq ($(CONFIG_AMD_HSMP), )
> +obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> +amd_hsmp-objs = hsmp.o plat.o
> +endif
> +ifneq ($(CONFIG_AMD_HSMP_ACPI), )
> +obj-$(CONFIG_AMD_HSMP_ACPI) += amd_hsmp.o
> +amd_hsmp-objs = hsmp.o acpi.o
> +endif
You should add a third symbol for the core that is selected to make
the Makefile side less convoluted.
The build system might work with something as simple as this (I'm not sure
if ...-m postfix works for composite objects, ...-y is documented to
work):
obj-$(CONFIG_xx) += amd_hsmp.o
amd_hsmp-$(CONFIG_xx) = hsmp.o
amd_hsmp-$(CONFIG_AMD_HSMP) += plat.o
amd_hsmp-$(CONFIG_AMD_HSMP_ACPI) += acpi.o
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index 90bfa1ddadbf..0307f4e7176d 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -12,21 +12,34 @@
> #include "hsmp.h"
>
> #include <linux/acpi.h>
> +#include <asm/amd_nb.h>
> +#include <linux/platform_device.h>
> +
> +#define DRIVER_NAME "amd_hsmp"
> +#define DRIVER_VERSION "2.3"
> +#define ACPI_HSMP_DEVICE_HID "AMDI0097"
>
> /* These are the strings specified in ACPI table */
> #define MSG_IDOFF_STR "MsgIdOffset"
> #define MSG_ARGOFF_STR "MsgArgOffset"
> #define MSG_RESPOFF_STR "MsgRspOffset"
>
> -void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> - u32 *value, bool write)
> +static int 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);
> + return 0;
> }
>
> +static const struct file_operations hsmp_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = hsmp_ioctl,
> + .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,
> @@ -189,7 +202,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
>
> sock->sock_ind = sock_ind;
> sock->dev = dev;
> - plat_dev.is_acpi_device = true;
> + sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr;
>
> sema_init(&sock->hsmp_sem, 1);
>
> @@ -202,7 +215,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
> return hsmp_read_acpi_dsd(sock);
> }
>
> -int hsmp_create_acpi_sysfs_if(struct device *dev)
> +static int hsmp_create_acpi_sysfs_if(struct device *dev)
> {
> struct attribute_group *attr_grp;
> u16 sock_ind;
> @@ -225,7 +238,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev)
> return devm_device_add_group(dev, attr_grp);
> }
>
> -int init_acpi(struct device *dev)
> +static int init_acpi(struct device *dev)
> {
> u16 sock_ind;
> int ret;
> @@ -259,3 +272,94 @@ int init_acpi(struct device *dev)
>
> return ret;
> }
> +
> +static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
> + {ACPI_HSMP_DEVICE_HID, 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
> +
> +static bool check_acpi_support(struct device *dev)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
> + return true;
> +
> + return false;
> +}
> +
> +static int hsmp_acpi_probe(struct platform_device *pdev)
> +{
> + int ret;
> +
> + if (!plat_dev.is_probed) {
> + plat_dev.num_sockets = amd_nb_num();
> + if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
> + return -ENODEV;
> +
> + plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
> + sizeof(*plat_dev.sock),
> + GFP_KERNEL);
> + if (!plat_dev.sock)
> + return -ENOMEM;
> + }
> +
> + if (!check_acpi_support(&pdev->dev)) {
> + dev_err(&pdev->dev, "Not ACPI device?\n");
> + return -ENODEV;
> + }
> +
> + ret = init_acpi(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
> + return ret;
> + }
> +
> + ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> +
> + if (!plat_dev.is_probed) {
> + plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
> + plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
> + plat_dev.hsmp_device.fops = &hsmp_fops;
> + plat_dev.hsmp_device.parent = &pdev->dev;
> + plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
> + plat_dev.hsmp_device.mode = 0644;
> +
> + ret = misc_register(&plat_dev.hsmp_device);
> + if (ret)
> + return ret;
> + plat_dev.is_probed = true;
> + }
> +
> + return 0;
> +}
> +
> +static void hsmp_acpi_remove(struct platform_device *pdev)
> +{
> + /*
> + * We register only one misc_device even on multi socket system.
multi-socket
> + * So, deregister should happen only once.
> + */
> + if (plat_dev.is_probed) {
> + misc_deregister(&plat_dev.hsmp_device);
> + plat_dev.is_probed = false;
> + }
> +}
> +
> +static struct platform_driver amd_hsmp_driver = {
> + .probe = hsmp_acpi_probe,
> + .remove_new = hsmp_acpi_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .acpi_match_table = amd_hsmp_acpi_ids,
> + },
> +};
> +
> +module_platform_driver(amd_hsmp_driver);
> +
> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
--
i.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 06/10] platform/x86/amd/hsmp: Create mutually exclusive ACPI and plat drivers
2024-07-08 10:15 ` Ilpo Järvinen
@ 2024-07-09 6:41 ` Suma Hegde
0 siblings, 0 replies; 22+ messages in thread
From: Suma Hegde @ 2024-07-09 6:41 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
Hi Ilpo,
Thank you for your review.
On 7/8/2024 3:45 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Thu, 27 Jun 2024, Suma Hegde wrote:
>
>> Separate the probes for ACPI and platform device drivers.
>> Provide a Kconfig option to select either the
>> ACPI or the platform device based driver.
>>
>> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>> ---
>> arch/x86/include/asm/amd_hsmp.h | 2 +-
>> drivers/platform/x86/amd/hsmp/Kconfig | 25 ++++-
>> drivers/platform/x86/amd/hsmp/Makefile | 10 +-
>> drivers/platform/x86/amd/hsmp/acpi.c | 114 ++++++++++++++++++++++-
>> drivers/platform/x86/amd/hsmp/hsmp.c | 25 ++---
>> drivers/platform/x86/amd/hsmp/hsmp.h | 8 +-
>> drivers/platform/x86/amd/hsmp/plat.c | 122 +++++++------------------
>> 7 files changed, 183 insertions(+), 123 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/amd_hsmp.h b/arch/x86/include/asm/amd_hsmp.h
>> index 03c2ce3edaf5..ada14e55f9f4 100644
>> --- a/arch/x86/include/asm/amd_hsmp.h
>> +++ b/arch/x86/include/asm/amd_hsmp.h
>> @@ -5,7 +5,7 @@
>>
>> #include <uapi/asm/amd_hsmp.h>
>>
>> -#if IS_ENABLED(CONFIG_AMD_HSMP)
>> +#if IS_ENABLED(CONFIG_AMD_HSMP) || IS_ENABLED(CONFIG_AMD_HSMP_ACPI)
>> int hsmp_send_message(struct hsmp_message *msg);
>> #else
>> static inline int hsmp_send_message(struct hsmp_message *msg)
>> diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
>> index b55d4ed9bceb..1cb10d2aac77 100644
>> --- a/drivers/platform/x86/amd/hsmp/Kconfig
>> +++ b/drivers/platform/x86/amd/hsmp/Kconfig
>> @@ -3,9 +3,30 @@
>> # AMD HSMP Driver
>> #
>>
>> +menu "AMD Host System Management Port driver"
>> + depends on AMD_NB
>> +
>> +config AMD_HSMP_ACPI
>> + tristate "AMD HSMP ACPI driver"
>> + depends on 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.
>> +
>> + This driver supports ACPI based probing.
>> +
>> + You may enable this, if your platform bios provides an ACPI object
>> + as described in the documentation.
>> +
>> + If you choose to compile this driver as a module the module will be
>> + called amd_hsmp.
>> +
>> config AMD_HSMP
>> tristate "AMD HSMP Driver"
>> - depends on AMD_NB && X86_64 && ACPI
>> + depends on !(AMD_HSMP_ACPI || AMD_HSMP_ACPI=m)
> depends on AMD_HSMP_ACPI=n is simpler.
ok, will change it to AMD_HSMP_ACPI=n.
>
>> help
>> The driver provides a way for user space tools to monitor and manage
>> system management functionality on EPYC server CPUs from AMD.
>> @@ -15,3 +36,5 @@ config AMD_HSMP
>>
>> If you choose to compile this driver as a module the module will be
>> called amd_hsmp.
>> +
>> +endmenu
>> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
>> index 0cc92865c0a2..53ebc462b0f9 100644
>> --- a/drivers/platform/x86/amd/hsmp/Makefile
>> +++ b/drivers/platform/x86/amd/hsmp/Makefile
>> @@ -4,5 +4,11 @@
>> # AMD HSMP Driver
>> #
>>
>> -obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
>> -amd_hsmp-objs := hsmp.o plat.o acpi.o
>> +ifneq ($(CONFIG_AMD_HSMP), )
>> +obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
>> +amd_hsmp-objs = hsmp.o plat.o
>> +endif
>> +ifneq ($(CONFIG_AMD_HSMP_ACPI), )
>> +obj-$(CONFIG_AMD_HSMP_ACPI) += amd_hsmp.o
>> +amd_hsmp-objs = hsmp.o acpi.o
>> +endif
> You should add a third symbol for the core that is selected to make
> the Makefile side less convoluted.
>
> The build system might work with something as simple as this (I'm not sure
> if ...-m postfix works for composite objects, ...-y is documented to
> work):
>
> obj-$(CONFIG_xx) += amd_hsmp.o
> amd_hsmp-$(CONFIG_xx) = hsmp.o
> amd_hsmp-$(CONFIG_AMD_HSMP) += plat.o
> amd_hsmp-$(CONFIG_AMD_HSMP_ACPI) += acpi.o
I'm trying to keep the config symbols changes minimal. Existing users
need not change their build setup. New ACPI users can include the ACPI
one instead.
>> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
>> index 90bfa1ddadbf..0307f4e7176d 100644
>> --- a/drivers/platform/x86/amd/hsmp/acpi.c
>> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
>> @@ -12,21 +12,34 @@
>> #include "hsmp.h"
>>
>> #include <linux/acpi.h>
>> +#include <asm/amd_nb.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define DRIVER_NAME "amd_hsmp"
>> +#define DRIVER_VERSION "2.3"
>> +#define ACPI_HSMP_DEVICE_HID "AMDI0097"
>>
>> /* These are the strings specified in ACPI table */
>> #define MSG_IDOFF_STR "MsgIdOffset"
>> #define MSG_ARGOFF_STR "MsgArgOffset"
>> #define MSG_RESPOFF_STR "MsgRspOffset"
>>
>> -void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
>> - u32 *value, bool write)
>> +static int 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);
>> + return 0;
>> }
>>
>> +static const struct file_operations hsmp_fops = {
>> + .owner = THIS_MODULE,
>> + .unlocked_ioctl = hsmp_ioctl,
>> + .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,
>> @@ -189,7 +202,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
>>
>> sock->sock_ind = sock_ind;
>> sock->dev = dev;
>> - plat_dev.is_acpi_device = true;
>> + sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr;
>>
>> sema_init(&sock->hsmp_sem, 1);
>>
>> @@ -202,7 +215,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
>> return hsmp_read_acpi_dsd(sock);
>> }
>>
>> -int hsmp_create_acpi_sysfs_if(struct device *dev)
>> +static int hsmp_create_acpi_sysfs_if(struct device *dev)
>> {
>> struct attribute_group *attr_grp;
>> u16 sock_ind;
>> @@ -225,7 +238,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev)
>> return devm_device_add_group(dev, attr_grp);
>> }
>>
>> -int init_acpi(struct device *dev)
>> +static int init_acpi(struct device *dev)
>> {
>> u16 sock_ind;
>> int ret;
>> @@ -259,3 +272,94 @@ int init_acpi(struct device *dev)
>>
>> return ret;
>> }
>> +
>> +static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
>> + {ACPI_HSMP_DEVICE_HID, 0},
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
>> +
>> +static bool check_acpi_support(struct device *dev)
>> +{
>> + struct acpi_device *adev = ACPI_COMPANION(dev);
>> +
>> + if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static int hsmp_acpi_probe(struct platform_device *pdev)
>> +{
>> + int ret;
>> +
>> + if (!plat_dev.is_probed) {
>> + plat_dev.num_sockets = amd_nb_num();
>> + if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
>> + return -ENODEV;
>> +
>> + plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
>> + sizeof(*plat_dev.sock),
>> + GFP_KERNEL);
>> + if (!plat_dev.sock)
>> + return -ENOMEM;
>> + }
>> +
>> + if (!check_acpi_support(&pdev->dev)) {
>> + dev_err(&pdev->dev, "Not ACPI device?\n");
>> + return -ENODEV;
>> + }
>> +
>> + ret = init_acpi(&pdev->dev);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
>> + return ret;
>> + }
>> +
>> + ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
>> + if (ret)
>> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
>> +
>> + if (!plat_dev.is_probed) {
>> + plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
>> + plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
>> + plat_dev.hsmp_device.fops = &hsmp_fops;
>> + plat_dev.hsmp_device.parent = &pdev->dev;
>> + plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
>> + plat_dev.hsmp_device.mode = 0644;
>> +
>> + ret = misc_register(&plat_dev.hsmp_device);
>> + if (ret)
>> + return ret;
>> + plat_dev.is_probed = true;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void hsmp_acpi_remove(struct platform_device *pdev)
>> +{
>> + /*
>> + * We register only one misc_device even on multi socket system.
> multi-socket
will correct it.
Could you please also review the other patches in this patch series?
>
>> + * So, deregister should happen only once.
>> + */
>> + if (plat_dev.is_probed) {
>> + misc_deregister(&plat_dev.hsmp_device);
>> + plat_dev.is_probed = false;
>> + }
>> +}
>> +
>> +static struct platform_driver amd_hsmp_driver = {
>> + .probe = hsmp_acpi_probe,
>> + .remove_new = hsmp_acpi_remove,
>> + .driver = {
>> + .name = DRIVER_NAME,
>> + .acpi_match_table = amd_hsmp_acpi_ids,
>> + },
>> +};
>> +
>> +module_platform_driver(amd_hsmp_driver);
>> +
>> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL v2");
> --
> i.
Thanks and Regards,
Suma
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 07/10] platform/x86/amd/hsmp: Use name space while exporting module symbols
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
` (5 preceding siblings ...)
2024-06-27 5:39 ` [PATCH 06/10] platform/x86/amd/hsmp: Create mutually exclusive ACPI and plat drivers Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-07-08 10:23 ` Ilpo Järvinen
2024-06-27 5:39 ` [PATCH 08/10] platform/x86/amd/hsmp: Move read and is_visible to respective files Suma Hegde
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
hsmp_send_message() is exported with AMD_HSMP name space.
The other modules who would like to use this symbol,
need to import AMD_HSMP namespace using MODULE_IMPORT_NS()
to get away with warning.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/hsmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 8cad5e813947..4bf598021f4a 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -185,7 +185,7 @@ int hsmp_send_message(struct hsmp_message *msg)
return ret;
}
-EXPORT_SYMBOL_GPL(hsmp_send_message);
+EXPORT_SYMBOL_NS_GPL(hsmp_send_message, AMD_HSMP);
int hsmp_test(u16 sock_ind, u32 value)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 07/10] platform/x86/amd/hsmp: Use name space while exporting module symbols
2024-06-27 5:39 ` [PATCH 07/10] platform/x86/amd/hsmp: Use name space while exporting module symbols Suma Hegde
@ 2024-07-08 10:23 ` Ilpo Järvinen
0 siblings, 0 replies; 22+ messages in thread
From: Ilpo Järvinen @ 2024-07-08 10:23 UTC (permalink / raw)
To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi
On Thu, 27 Jun 2024, Suma Hegde wrote:
> hsmp_send_message() is exported with AMD_HSMP name space.
> The other modules who would like to use this symbol,
> need to import AMD_HSMP namespace using MODULE_IMPORT_NS()
> to get away with warning.
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/hsmp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 8cad5e813947..4bf598021f4a 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -185,7 +185,7 @@ int hsmp_send_message(struct hsmp_message *msg)
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(hsmp_send_message);
> +EXPORT_SYMBOL_NS_GPL(hsmp_send_message, AMD_HSMP);
>
> int hsmp_test(u16 sock_ind, u32 value)
> {
>
Hmm, there seem to be no in-kernel users of this as you didn't need to do
any user side import adaptation...
--
i.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 08/10] platform/x86/amd/hsmp: Move read and is_visible to respective files
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
` (6 preceding siblings ...)
2024-06-27 5:39 ` [PATCH 07/10] platform/x86/amd/hsmp: Use name space while exporting module symbols Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-06-27 19:48 ` Mario Limonciello
2024-06-27 5:39 ` [PATCH 09/10] platform/x86/amd/hsmp: Use dev_groups in the driver structure Suma Hegde
2024-06-27 5:39 ` [PATCH 10/10] platform/x86/amd/hsmp: Fix potential spectre issue Suma Hegde
9 siblings, 1 reply; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
The .read() and .is_visibile() needs to be handled differently in acpi and
platform drivers, due to the way the sysfs files are created.
This is in preparation to using .dev_groups instead of dynamic sysfs
creation. The sysfs at this point is not functional, it will be enabled in
the next patch.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 41 ++++++++++++++++++++
drivers/platform/x86/amd/hsmp/hsmp.c | 37 ------------------
drivers/platform/x86/amd/hsmp/plat.c | 57 ++++++++++++++++++++++++++++
3 files changed, 98 insertions(+), 37 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 0307f4e7176d..1ea17aa296c7 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -12,6 +12,7 @@
#include "hsmp.h"
#include <linux/acpi.h>
+#include <asm/amd_hsmp.h>
#include <asm/amd_nb.h>
#include <linux/platform_device.h>
@@ -206,6 +207,8 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
sema_init(&sock->hsmp_sem, 1);
+ dev_set_drvdata(dev, sock);
+
/* Read MP1 base address from CRS method */
ret = hsmp_read_acpi_crs(sock);
if (ret)
@@ -238,6 +241,44 @@ static int hsmp_create_acpi_sysfs_if(struct device *dev)
return devm_device_add_group(dev, attr_grp);
}
+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)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct hsmp_socket *sock = dev_get_drvdata(dev);
+ struct hsmp_message msg = { 0 };
+ int ret;
+
+ if (!sock)
+ return -EINVAL;
+
+ /* Do not support lseek(), reads entire metric table */
+ if (count < bin_attr->size) {
+ dev_err(sock->dev, "Wrong buffer size\n");
+ return -EINVAL;
+ }
+
+ 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, bin_attr->size);
+
+ return bin_attr->size;
+}
+
+umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+ struct bin_attribute *battr, int id)
+{
+ if (plat_dev.proto_ver == HSMP_PROTO_VER6)
+ return battr->attr.mode;
+ else
+ return 0;
+}
+
static int init_acpi(struct device *dev)
{
u16 sock_ind;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 4bf598021f4a..c199a0ff457d 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -273,34 +273,6 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
return 0;
}
-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)
-{
- struct hsmp_socket *sock = bin_attr->private;
- struct hsmp_message msg = { 0 };
- int ret;
-
- if (!sock)
- return -EINVAL;
-
- /* Do not support lseek(), reads entire metric table */
- if (count < bin_attr->size) {
- dev_err(sock->dev, "Wrong buffer size\n");
- return -EINVAL;
- }
-
- 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, bin_attr->size);
-
- return bin_attr->size;
-}
-
static int hsmp_get_tbl_dram_base(u16 sock_ind)
{
struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
@@ -334,15 +306,6 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
return 0;
}
-umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
- struct bin_attribute *battr, int id)
-{
- if (plat_dev.proto_ver == HSMP_PROTO_VER6)
- return battr->attr.mode;
- else
- return 0;
-}
-
static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock_ind)
{
struct bin_attribute *hattr = &plat_dev.sock[sock_ind].hsmp_attr;
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index 62423581d839..57aa64b18e0d 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -11,6 +11,7 @@
#include "hsmp.h"
+#include <asm/amd_hsmp.h>
#include <asm/amd_nb.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -88,6 +89,62 @@ static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
return device_add_groups(dev, hsmp_attr_grps);
}
+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)
+{
+ struct hsmp_message msg = { 0 };
+ struct hsmp_socket *sock;
+ u8 sock_ind;
+ int ret;
+
+ ret = kstrtou8(bin_attr->private, 10, &sock_ind);
+ if (ret)
+ return ret;
+
+ if (sock_ind >= plat_dev.num_sockets)
+ return -EINVAL;
+
+ sock = &plat_dev.sock[sock_ind];
+ if (!sock)
+ return -EINVAL;
+
+ /* Do not support lseek(), reads entire metric table */
+ if (count < bin_attr->size) {
+ dev_err(sock->dev, "Wrong buffer size\n");
+ return -EINVAL;
+ }
+
+ msg.msg_id = HSMP_GET_METRIC_TABLE;
+ msg.sock_ind = sock_ind;
+
+ ret = hsmp_send_message(&msg);
+ if (ret)
+ return ret;
+ memcpy_fromio(buf, sock->metric_tbl_addr, bin_attr->size);
+
+ return bin_attr->size;
+}
+
+umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+ struct bin_attribute *battr, int id)
+{
+ u8 sock_ind;
+ int ret;
+
+ ret = kstrtou8(battr->private, 10, &sock_ind);
+ if (ret)
+ return ret;
+
+ if (id == 0 && sock_ind >= plat_dev.num_sockets)
+ return SYSFS_GROUP_INVISIBLE;
+
+ if (plat_dev.proto_ver == HSMP_PROTO_VER6)
+ return battr->attr.mode;
+ else
+ return 0;
+}
+
static inline bool is_f1a_m0h(void)
{
if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 08/10] platform/x86/amd/hsmp: Move read and is_visible to respective files
2024-06-27 5:39 ` [PATCH 08/10] platform/x86/amd/hsmp: Move read and is_visible to respective files Suma Hegde
@ 2024-06-27 19:48 ` Mario Limonciello
2024-06-28 3:50 ` Suma Hegde
0 siblings, 1 reply; 22+ messages in thread
From: Mario Limonciello @ 2024-06-27 19:48 UTC (permalink / raw)
To: Suma Hegde, platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Naveen Krishna Chatradhi
On 6/27/2024 00:39, Suma Hegde wrote:
> The .read() and .is_visibile() needs to be handled differently in acpi and
is_visible()
> platform drivers, due to the way the sysfs files are created.
>
> This is in preparation to using .dev_groups instead of dynamic sysfs
> creation. The sysfs at this point is not functional, it will be enabled in
> the next patch.
>
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> drivers/platform/x86/amd/hsmp/acpi.c | 41 ++++++++++++++++++++
> drivers/platform/x86/amd/hsmp/hsmp.c | 37 ------------------
> drivers/platform/x86/amd/hsmp/plat.c | 57 ++++++++++++++++++++++++++++
> 3 files changed, 98 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index 0307f4e7176d..1ea17aa296c7 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -12,6 +12,7 @@
> #include "hsmp.h"
>
> #include <linux/acpi.h>
> +#include <asm/amd_hsmp.h>
> #include <asm/amd_nb.h>
> #include <linux/platform_device.h>
>
> @@ -206,6 +207,8 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
>
> sema_init(&sock->hsmp_sem, 1);
>
> + dev_set_drvdata(dev, sock);
> +
> /* Read MP1 base address from CRS method */
> ret = hsmp_read_acpi_crs(sock);
> if (ret)
> @@ -238,6 +241,44 @@ static int hsmp_create_acpi_sysfs_if(struct device *dev)
> return devm_device_add_group(dev, attr_grp);
> }
>
> +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)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct hsmp_socket *sock = dev_get_drvdata(dev);
> + struct hsmp_message msg = { 0 };
> + int ret;
> +
> + if (!sock)
> + return -EINVAL;
> +
> + /* Do not support lseek(), reads entire metric table */
> + if (count < bin_attr->size) {
> + dev_err(sock->dev, "Wrong buffer size\n");
> + return -EINVAL;
> + }
> +
> + 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, bin_attr->size);
> +
> + return bin_attr->size;
> +}
> +
> +umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> + struct bin_attribute *battr, int id)
> +{
> + if (plat_dev.proto_ver == HSMP_PROTO_VER6)
> + return battr->attr.mode;
> + else
Since your only path in the "if" returns this else is redundant.
> + return 0;
> +}
> +
> static int init_acpi(struct device *dev)
> {
> u16 sock_ind;
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 4bf598021f4a..c199a0ff457d 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -273,34 +273,6 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> return 0;
> }
>
> -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)
> -{
> - struct hsmp_socket *sock = bin_attr->private;
> - struct hsmp_message msg = { 0 };
> - int ret;
> -
> - if (!sock)
> - return -EINVAL;
> -
> - /* Do not support lseek(), reads entire metric table */
> - if (count < bin_attr->size) {
> - dev_err(sock->dev, "Wrong buffer size\n");
> - return -EINVAL;
> - }
> -
> - 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, bin_attr->size);
> -
> - return bin_attr->size;
> -}
> -
> static int hsmp_get_tbl_dram_base(u16 sock_ind)
> {
> struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
> @@ -334,15 +306,6 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
> return 0;
> }
>
> -umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> - struct bin_attribute *battr, int id)
> -{
> - if (plat_dev.proto_ver == HSMP_PROTO_VER6)
> - return battr->attr.mode;
> - else
> - return 0;
> -}
> -
> static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock_ind)
> {
> struct bin_attribute *hattr = &plat_dev.sock[sock_ind].hsmp_attr;
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index 62423581d839..57aa64b18e0d 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -11,6 +11,7 @@
>
> #include "hsmp.h"
>
> +#include <asm/amd_hsmp.h>
> #include <asm/amd_nb.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> @@ -88,6 +89,62 @@ static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
> return device_add_groups(dev, hsmp_attr_grps);
> }
>
> +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)
> +{
> + struct hsmp_message msg = { 0 };
> + struct hsmp_socket *sock;
> + u8 sock_ind;
> + int ret;
> +
> + ret = kstrtou8(bin_attr->private, 10, &sock_ind);
> + if (ret)
> + return ret;
> +
> + if (sock_ind >= plat_dev.num_sockets)
> + return -EINVAL;
> +
> + sock = &plat_dev.sock[sock_ind];
> + if (!sock)
> + return -EINVAL;
> +
> + /* Do not support lseek(), reads entire metric table */
> + if (count < bin_attr->size) {
> + dev_err(sock->dev, "Wrong buffer size\n");
> + return -EINVAL;
> + }
> +
> + msg.msg_id = HSMP_GET_METRIC_TABLE;
> + msg.sock_ind = sock_ind;
> +
> + ret = hsmp_send_message(&msg);
> + if (ret)
> + return ret;
> + memcpy_fromio(buf, sock->metric_tbl_addr, bin_attr->size);
> +
> + return bin_attr->size;
> +}
> +
> +umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> + struct bin_attribute *battr, int id)
> +{
> + u8 sock_ind;
> + int ret;
> +
> + ret = kstrtou8(battr->private, 10, &sock_ind);
> + if (ret)
> + return ret;
> +
> + if (id == 0 && sock_ind >= plat_dev.num_sockets)
> + return SYSFS_GROUP_INVISIBLE;
> +
> + if (plat_dev.proto_ver == HSMP_PROTO_VER6)
> + return battr->attr.mode;
> + else
> + return 0;
Since your only path in the "if" returns this else is redundant.
> +}
> +
> static inline bool is_f1a_m0h(void)
> {
> if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 08/10] platform/x86/amd/hsmp: Move read and is_visible to respective files
2024-06-27 19:48 ` Mario Limonciello
@ 2024-06-28 3:50 ` Suma Hegde
0 siblings, 0 replies; 22+ messages in thread
From: Suma Hegde @ 2024-06-28 3:50 UTC (permalink / raw)
To: Mario Limonciello, platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Naveen Krishna Chatradhi
Hi Mario,
Thank you for your review. I will address these review comments in v2.
On 6/28/2024 1:18 AM, Mario Limonciello wrote:
> On 6/27/2024 00:39, Suma Hegde wrote:
>> The .read() and .is_visibile() needs to be handled differently in
>> acpi and
>
> is_visible()
>
>> platform drivers, due to the way the sysfs files are created.
>>
>> This is in preparation to using .dev_groups instead of dynamic sysfs
>> creation. The sysfs at this point is not functional, it will be
>> enabled in
>> the next patch.
>>
>> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>> ---
>> drivers/platform/x86/amd/hsmp/acpi.c | 41 ++++++++++++++++++++
>> drivers/platform/x86/amd/hsmp/hsmp.c | 37 ------------------
>> drivers/platform/x86/amd/hsmp/plat.c | 57 ++++++++++++++++++++++++++++
>> 3 files changed, 98 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c
>> b/drivers/platform/x86/amd/hsmp/acpi.c
>> index 0307f4e7176d..1ea17aa296c7 100644
>> --- a/drivers/platform/x86/amd/hsmp/acpi.c
>> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
>> @@ -12,6 +12,7 @@
>> #include "hsmp.h"
>> #include <linux/acpi.h>
>> +#include <asm/amd_hsmp.h>
>> #include <asm/amd_nb.h>
>> #include <linux/platform_device.h>
>> @@ -206,6 +207,8 @@ static int hsmp_parse_acpi_table(struct device
>> *dev, u16 sock_ind)
>> sema_init(&sock->hsmp_sem, 1);
>> + dev_set_drvdata(dev, sock);
>> +
>> /* Read MP1 base address from CRS method */
>> ret = hsmp_read_acpi_crs(sock);
>> if (ret)
>> @@ -238,6 +241,44 @@ static int hsmp_create_acpi_sysfs_if(struct
>> device *dev)
>> return devm_device_add_group(dev, attr_grp);
>> }
>> +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)
>> +{
>> + struct device *dev = container_of(kobj, struct device, kobj);
>> + struct hsmp_socket *sock = dev_get_drvdata(dev);
>> + struct hsmp_message msg = { 0 };
>> + int ret;
>> +
>> + if (!sock)
>> + return -EINVAL;
>> +
>> + /* Do not support lseek(), reads entire metric table */
>> + if (count < bin_attr->size) {
>> + dev_err(sock->dev, "Wrong buffer size\n");
>> + return -EINVAL;
>> + }
>> +
>> + 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, bin_attr->size);
>> +
>> + return bin_attr->size;
>> +}
>> +
>> +umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>> + struct bin_attribute *battr, int id)
>> +{
>> + if (plat_dev.proto_ver == HSMP_PROTO_VER6)
>> + return battr->attr.mode;
>> + else
>
> Since your only path in the "if" returns this else is redundant.
>
>> + return 0;
>> +}
>> +
>> static int init_acpi(struct device *dev)
>> {
>> u16 sock_ind;
>> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c
>> b/drivers/platform/x86/amd/hsmp/hsmp.c
>> index 4bf598021f4a..c199a0ff457d 100644
>> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
>> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
>> @@ -273,34 +273,6 @@ long hsmp_ioctl(struct file *fp, unsigned int
>> cmd, unsigned long arg)
>> return 0;
>> }
>> -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)
>> -{
>> - struct hsmp_socket *sock = bin_attr->private;
>> - struct hsmp_message msg = { 0 };
>> - int ret;
>> -
>> - if (!sock)
>> - return -EINVAL;
>> -
>> - /* Do not support lseek(), reads entire metric table */
>> - if (count < bin_attr->size) {
>> - dev_err(sock->dev, "Wrong buffer size\n");
>> - return -EINVAL;
>> - }
>> -
>> - 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, bin_attr->size);
>> -
>> - return bin_attr->size;
>> -}
>> -
>> static int hsmp_get_tbl_dram_base(u16 sock_ind)
>> {
>> struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
>> @@ -334,15 +306,6 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
>> return 0;
>> }
>> -umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>> - struct bin_attribute *battr, int id)
>> -{
>> - if (plat_dev.proto_ver == HSMP_PROTO_VER6)
>> - return battr->attr.mode;
>> - else
>> - return 0;
>> -}
>> -
>> static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute
>> **hattrs, u16 sock_ind)
>> {
>> struct bin_attribute *hattr = &plat_dev.sock[sock_ind].hsmp_attr;
>> diff --git a/drivers/platform/x86/amd/hsmp/plat.c
>> b/drivers/platform/x86/amd/hsmp/plat.c
>> index 62423581d839..57aa64b18e0d 100644
>> --- a/drivers/platform/x86/amd/hsmp/plat.c
>> +++ b/drivers/platform/x86/amd/hsmp/plat.c
>> @@ -11,6 +11,7 @@
>> #include "hsmp.h"
>> +#include <asm/amd_hsmp.h>
>> #include <asm/amd_nb.h>
>> #include <linux/module.h>
>> #include <linux/pci.h>
>> @@ -88,6 +89,62 @@ static int hsmp_create_non_acpi_sysfs_if(struct
>> device *dev)
>> return device_add_groups(dev, hsmp_attr_grps);
>> }
>> +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)
>> +{
>> + struct hsmp_message msg = { 0 };
>> + struct hsmp_socket *sock;
>> + u8 sock_ind;
>> + int ret;
>> +
>> + ret = kstrtou8(bin_attr->private, 10, &sock_ind);
>> + if (ret)
>> + return ret;
>> +
>> + if (sock_ind >= plat_dev.num_sockets)
>> + return -EINVAL;
>> +
>> + sock = &plat_dev.sock[sock_ind];
>> + if (!sock)
>> + return -EINVAL;
>> +
>> + /* Do not support lseek(), reads entire metric table */
>> + if (count < bin_attr->size) {
>> + dev_err(sock->dev, "Wrong buffer size\n");
>> + return -EINVAL;
>> + }
>> +
>> + msg.msg_id = HSMP_GET_METRIC_TABLE;
>> + msg.sock_ind = sock_ind;
>> +
>> + ret = hsmp_send_message(&msg);
>> + if (ret)
>> + return ret;
>> + memcpy_fromio(buf, sock->metric_tbl_addr, bin_attr->size);
>> +
>> + return bin_attr->size;
>> +}
>> +
>> +umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>> + struct bin_attribute *battr, int id)
>> +{
>> + u8 sock_ind;
>> + int ret;
>> +
>> + ret = kstrtou8(battr->private, 10, &sock_ind);
>> + if (ret)
>> + return ret;
>> +
>> + if (id == 0 && sock_ind >= plat_dev.num_sockets)
>> + return SYSFS_GROUP_INVISIBLE;
>> +
>> + if (plat_dev.proto_ver == HSMP_PROTO_VER6)
>> + return battr->attr.mode;
>> + else
>> + return 0;
>
> Since your only path in the "if" returns this else is redundant.
>
>> +}
>> +
>> static inline bool is_f1a_m0h(void)
>> {
>> if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
Thanks and Regards,
Suma
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 09/10] platform/x86/amd/hsmp: Use dev_groups in the driver structure
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
` (7 preceding siblings ...)
2024-06-27 5:39 ` [PATCH 08/10] platform/x86/amd/hsmp: Move read and is_visible to respective files Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
2024-06-27 5:39 ` [PATCH 10/10] platform/x86/amd/hsmp: Fix potential spectre issue Suma Hegde
9 siblings, 0 replies; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
Move out of device_add_group() variants, instead assign static array of
attribute groups to .dev_groups in platform_driver structure.
Then use is_visible to enable only the necessary files on the platform.
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 65 ++++++++--------
drivers/platform/x86/amd/hsmp/hsmp.c | 40 +---------
drivers/platform/x86/amd/hsmp/hsmp.h | 8 +-
drivers/platform/x86/amd/hsmp/plat.c | 106 +++++++++++++++++----------
4 files changed, 102 insertions(+), 117 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 1ea17aa296c7..087cb360814a 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -218,32 +218,9 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
return hsmp_read_acpi_dsd(sock);
}
-static int hsmp_create_acpi_sysfs_if(struct device *dev)
-{
- struct attribute_group *attr_grp;
- u16 sock_ind;
- int ret;
-
- attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group), GFP_KERNEL);
- 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_create_attr_list(attr_grp, dev, sock_ind);
- if (ret)
- return ret;
-
- return devm_device_add_group(dev, attr_grp);
-}
-
-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)
+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)
{
struct device *dev = container_of(kobj, struct device, kobj);
struct hsmp_socket *sock = dev_get_drvdata(dev);
@@ -270,8 +247,8 @@ ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
return bin_attr->size;
}
-umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
- struct bin_attribute *battr, int id)
+static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+ struct bin_attribute *battr, int id)
{
if (plat_dev.proto_ver == HSMP_PROTO_VER6)
return battr->attr.mode;
@@ -311,9 +288,36 @@ static int init_acpi(struct device *dev)
return ret;
}
+ if (plat_dev.proto_ver == HSMP_PROTO_VER6) {
+ ret = hsmp_get_tbl_dram_base(sock_ind);
+ if (ret)
+ dev_err(dev, "Failed to init metric table\n");
+ }
+
return ret;
}
+static struct bin_attribute hsmp_metric_tbl_attr = {
+ .attr = { .name = HSMP_METRICS_TABLE_NAME, .mode = 0444},
+ .read = hsmp_metric_tbl_read,
+ .size = sizeof(struct hsmp_metric_table),
+};
+
+static struct bin_attribute *hsmp_attr_list[] = {
+ &hsmp_metric_tbl_attr,
+ NULL
+};
+
+static struct attribute_group hsmp_attr_grp = {
+ .bin_attrs = hsmp_attr_list,
+ .is_bin_visible = hsmp_is_sock_attr_visible,
+};
+
+static const struct attribute_group *hsmp_groups[] = {
+ &hsmp_attr_grp,
+ NULL
+};
+
static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
{ACPI_HSMP_DEVICE_HID, 0},
{}
@@ -357,10 +361,6 @@ static int hsmp_acpi_probe(struct platform_device *pdev)
return ret;
}
- ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
- if (ret)
- dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
-
if (!plat_dev.is_probed) {
plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
@@ -396,6 +396,7 @@ static struct platform_driver amd_hsmp_driver = {
.driver = {
.name = DRIVER_NAME,
.acpi_match_table = amd_hsmp_acpi_ids,
+ .dev_groups = hsmp_groups,
},
};
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index c199a0ff457d..5120d8ff707b 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -273,7 +273,7 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
return 0;
}
-static int hsmp_get_tbl_dram_base(u16 sock_ind)
+int hsmp_get_tbl_dram_base(u16 sock_ind)
{
struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
struct hsmp_message msg = { 0 };
@@ -306,44 +306,6 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
return 0;
}
-static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock_ind)
-{
- struct bin_attribute *hattr = &plat_dev.sock[sock_ind].hsmp_attr;
-
- sysfs_bin_attr_init(hattr);
- hattr->attr.name = HSMP_METRICS_TABLE_NAME;
- hattr->attr.mode = 0444;
- hattr->read = hsmp_metric_tbl_read;
- hattr->size = sizeof(struct hsmp_metric_table);
- hattr->private = &plat_dev.sock[sock_ind];
- hattrs[0] = hattr;
-
- if (plat_dev.proto_ver == HSMP_PROTO_VER6)
- return hsmp_get_tbl_dram_base(sock_ind);
- else
- return 0;
-}
-
-/* One bin sysfs for metrics table */
-#define NUM_HSMP_ATTRS 1
-
-int hsmp_create_attr_list(struct attribute_group *attr_grp,
- struct device *dev, u16 sock_ind)
-{
- struct bin_attribute **hsmp_bin_attrs;
-
- /* Null terminated list of attributes */
- hsmp_bin_attrs = devm_kcalloc(dev, NUM_HSMP_ATTRS + 1,
- sizeof(*hsmp_bin_attrs),
- GFP_KERNEL);
- if (!hsmp_bin_attrs)
- return -ENOMEM;
-
- attr_grp->bin_attrs = hsmp_bin_attrs;
-
- return hsmp_init_metric_tbl_bin_attr(hsmp_bin_attrs, sock_ind);
-}
-
int hsmp_cache_proto_ver(u16 sock_ind)
{
struct hsmp_message msg = { 0 };
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
index f876370ba65c..c2d451c7c9fd 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.h
+++ b/drivers/platform/x86/amd/hsmp/hsmp.h
@@ -54,14 +54,8 @@ struct hsmp_plat_device {
extern struct hsmp_plat_device plat_dev;
-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);
int hsmp_cache_proto_ver(u16 sock_ind);
long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg);
-umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
- struct bin_attribute *battr, int id);
-int hsmp_create_attr_list(struct attribute_group *attr_grp,
- struct device *dev, u16 sock_ind);
int hsmp_test(u16 sock_ind, u32 value);
+int hsmp_get_tbl_dram_base(u16 sock_ind);
#endif /* HSMP_H */
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index 57aa64b18e0d..da4ddb6e9819 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -59,39 +59,9 @@ static const struct file_operations hsmp_fops = {
.compat_ioctl = hsmp_ioctl,
};
-static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
-{
- const struct attribute_group **hsmp_attr_grps;
- struct attribute_group *attr_grp;
- u16 i;
-
- hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
- sizeof(*hsmp_attr_grps),
- GFP_KERNEL);
- if (!hsmp_attr_grps)
- return -ENOMEM;
-
- /* Create a sysfs directory for each socket */
- for (i = 0; i < plat_dev.num_sockets; i++) {
- attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
- GFP_KERNEL);
- if (!attr_grp)
- return -ENOMEM;
-
- snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
- attr_grp->name = plat_dev.sock[i].name;
- attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
- hsmp_attr_grps[i] = attr_grp;
-
- hsmp_create_attr_list(attr_grp, dev, i);
- }
-
- return device_add_groups(dev, hsmp_attr_grps);
-}
-
-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)
+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)
{
struct hsmp_message msg = { 0 };
struct hsmp_socket *sock;
@@ -126,8 +96,8 @@ ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
return bin_attr->size;
}
-umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
- struct bin_attribute *battr, int id)
+static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+ struct bin_attribute *battr, int id)
{
u8 sock_ind;
int ret;
@@ -145,6 +115,61 @@ umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
return 0;
}
+/*
+ * AMD supports maximum of 8 sockets in a system.
+ * Static array of 8 + 1(for NULL) elements is created below
+ * to create sysfs groups for sockets.
+ * is_bin_visible function is used to show / hide the necessary groups.
+ */
+#define HSMP_BIN_ATTR(index, _list) \
+static struct bin_attribute attr##index = { \
+ .attr = { .name = HSMP_METRICS_TABLE_NAME, .mode = 0444}, \
+ .private = #index, \
+ .read = hsmp_metric_tbl_read, \
+ .size = sizeof(struct hsmp_metric_table), \
+}; \
+static struct bin_attribute _list[] = { \
+ &attr##index, \
+ NULL \
+}
+
+HSMP_BIN_ATTR(0, *sock0_attr_list);
+HSMP_BIN_ATTR(1, *sock1_attr_list);
+HSMP_BIN_ATTR(2, *sock2_attr_list);
+HSMP_BIN_ATTR(3, *sock3_attr_list);
+HSMP_BIN_ATTR(4, *sock4_attr_list);
+HSMP_BIN_ATTR(5, *sock5_attr_list);
+HSMP_BIN_ATTR(6, *sock6_attr_list);
+HSMP_BIN_ATTR(7, *sock7_attr_list);
+
+#define HSMP_BIN_ATTR_GRP(index, _list, _name) \
+static struct attribute_group sock##index##_attr_grp = { \
+ .bin_attrs = _list, \
+ .is_bin_visible = hsmp_is_sock_attr_visible, \
+ .name = #_name, \
+}
+
+HSMP_BIN_ATTR_GRP(0, sock0_attr_list, socket0);
+HSMP_BIN_ATTR_GRP(1, sock1_attr_list, socket1);
+HSMP_BIN_ATTR_GRP(2, sock2_attr_list, socket2);
+HSMP_BIN_ATTR_GRP(3, sock3_attr_list, socket3);
+HSMP_BIN_ATTR_GRP(4, sock4_attr_list, socket4);
+HSMP_BIN_ATTR_GRP(5, sock5_attr_list, socket5);
+HSMP_BIN_ATTR_GRP(6, sock6_attr_list, socket6);
+HSMP_BIN_ATTR_GRP(7, sock7_attr_list, socket7);
+
+static const struct attribute_group *hsmp_groups[] = {
+ &sock0_attr_grp,
+ &sock1_attr_grp,
+ &sock2_attr_grp,
+ &sock3_attr_grp,
+ &sock4_attr_grp,
+ &sock5_attr_grp,
+ &sock6_attr_grp,
+ &sock7_attr_grp,
+ NULL
+};
+
static inline bool is_f1a_m0h(void)
{
if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
@@ -195,6 +220,12 @@ static int init_platform_device(struct device *dev)
dev_err(dev, "Failed to read HSMP protocol version\n");
return ret;
}
+
+ if (plat_dev.proto_ver == HSMP_PROTO_VER6) {
+ ret = hsmp_get_tbl_dram_base(i);
+ if (ret)
+ dev_err(dev, "Failed to init metric table\n");
+ }
}
return 0;
@@ -216,10 +247,6 @@ static int hsmp_pltdrv_probe(struct platform_device *pdev)
return ret;
}
- ret = hsmp_create_non_acpi_sysfs_if(&pdev->dev);
- if (ret)
- dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
-
plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
plat_dev.hsmp_device.fops = &hsmp_fops;
@@ -240,6 +267,7 @@ static struct platform_driver amd_hsmp_driver = {
.remove_new = hsmp_pltdrv_remove,
.driver = {
.name = DRIVER_NAME,
+ .dev_groups = hsmp_groups,
},
};
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 10/10] platform/x86/amd/hsmp: Fix potential spectre issue
2024-06-27 5:39 [PATCH 00/10] platform/x86/amd/hsmp: Split ACPI and plat device driver Suma Hegde
` (8 preceding siblings ...)
2024-06-27 5:39 ` [PATCH 09/10] platform/x86/amd/hsmp: Use dev_groups in the driver structure Suma Hegde
@ 2024-06-27 5:39 ` Suma Hegde
9 siblings, 0 replies; 22+ messages in thread
From: Suma Hegde @ 2024-06-27 5:39 UTC (permalink / raw)
To: platform-driver-x86
Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi
Fix below warning caused by smatch by using array_index_nospec()
to clamp the index within the range.
"warn: potential spectre issue 'plat_dev.sock' [r] (local cap)"
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
drivers/platform/x86/amd/hsmp/acpi.c | 3 +++
drivers/platform/x86/amd/hsmp/plat.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 087cb360814a..f6d990fe0cb1 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include <asm/amd_hsmp.h>
#include <asm/amd_nb.h>
+#include <linux/nospec.h>
#include <linux/platform_device.h>
#define DRIVER_NAME "amd_hsmp"
@@ -267,6 +268,8 @@ static int init_acpi(struct device *dev)
if (sock_ind >= plat_dev.num_sockets)
return -EINVAL;
+ sock_ind = array_index_nospec(sock_ind, plat_dev.num_sockets);
+
ret = hsmp_parse_acpi_table(dev, sock_ind);
if (ret) {
dev_err(dev, "Failed to parse ACPI table\n");
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index da4ddb6e9819..04d1da455257 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -15,6 +15,7 @@
#include <asm/amd_nb.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/nospec.h>
#include <linux/platform_device.h>
#define DRIVER_NAME "amd_hsmp"
@@ -75,6 +76,8 @@ static ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
if (sock_ind >= plat_dev.num_sockets)
return -EINVAL;
+ sock_ind = array_index_nospec(sock_ind, plat_dev.num_sockets);
+
sock = &plat_dev.sock[sock_ind];
if (!sock)
return -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread