Am 21.10.24 um 15:46 schrieb Basavaraj Natikar:
AMD X3D processors, also known as AMD 3D V-Cache, feature dual Core
Complex Dies (CCDs) and enlarged L3 cache, enabling dynamic mode
switching between Frequency and Cache modes. To optimize performance,
implement the AMD 3D V-Cache Optimizer, which allows selecting either:
Frequency mode: cores within the faster CCD are prioritized before
those in the slower CCD.
Cache mode: cores within the larger L3 CCD are prioritized before
those in the smaller L3 CCD.
Co-developed-by: Perry Yuan <perry.yuan@xxxxxxx>
Signed-off-by: Perry Yuan <perry.yuan@xxxxxxx>
Co-developed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
---
MAINTAINERS | 7 ++
drivers/platform/x86/amd/Kconfig | 12 ++
drivers/platform/x86/amd/Makefile | 2 +
drivers/platform/x86/amd/x3d_vcache.c | 160 ++++++++++++++++++++++++++
4 files changed, 181 insertions(+)
create mode 100644 drivers/platform/x86/amd/x3d_vcache.c
diff --git a/MAINTAINERS b/MAINTAINERS
index e9659a5a7fb3..11b829956499 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -965,6 +965,13 @@ Q: https://patchwork.kernel.org/project/linux-rdma/list/
F: drivers/infiniband/hw/efa/
F: include/uapi/rdma/efa-abi.h
+AMD 3D V-CACHE PERFORMANCE OPTIMIZER DRIVER
+M: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
+R: Mario Limonciello <mario.limonciello@xxxxxxx>
+L: platform-driver-x86@xxxxxxxxxxxxxxx
+S: Supported
+F: drivers/platform/x86/amd/x3d_vcache.c
+
AMD ADDRESS TRANSLATION LIBRARY (ATL)
M: Yazen Ghannam <Yazen.Ghannam@xxxxxxx>
L: linux-edac@xxxxxxxxxxxxxxx
diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
index f88682d36447..d73f691020d0 100644
--- a/drivers/platform/x86/amd/Kconfig
+++ b/drivers/platform/x86/amd/Kconfig
@@ -6,6 +6,18 @@
source "drivers/platform/x86/amd/pmf/Kconfig"
source "drivers/platform/x86/amd/pmc/Kconfig"
+config AMD_3D_VCACHE
+ tristate "AMD 3D V-Cache Performance Optimizer Driver"
+ depends on X86_64 && ACPI
+ help
+ The driver provides a sysfs interface, enabling the setting of a bias
+ that alters CPU core reordering. This bias prefers cores with higher
+ frequencies or larger L3 caches on processors supporting AMD 3D V-Cache
+ technology.
+
+ If you choose to compile this driver as a module the module will be
+ called amd_3d_vcache.
+
config AMD_HSMP
tristate "AMD HSMP Driver"
depends on AMD_NB && X86_64 && ACPI
diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
index dcec0a46f8af..16e4cce02242 100644
--- a/drivers/platform/x86/amd/Makefile
+++ b/drivers/platform/x86/amd/Makefile
@@ -4,6 +4,8 @@
# AMD x86 Platform-Specific Drivers
#
+obj-$(CONFIG_AMD_3D_VCACHE) += amd_3d_vcache.o
+amd_3d_vcache-objs := x3d_vcache.o
obj-$(CONFIG_AMD_PMC) += pmc/
amd_hsmp-y := hsmp.o
obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
diff --git a/drivers/platform/x86/amd/x3d_vcache.c b/drivers/platform/x86/amd/x3d_vcache.c
new file mode 100644
index 000000000000..ec861e7b5e1e
--- /dev/null
+++ b/drivers/platform/x86/amd/x3d_vcache.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD 3D V-Cache Performance Optimizer Driver
+ *
+ * Copyright (c) 2024, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
+ * Perry Yuan <perry.yuan@xxxxxxx>
+ * Mario Limonciello <mario.limonciello@xxxxxxx>
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+
+static char *x3d_mode = "frequency";
+module_param(x3d_mode, charp, 0444);
Hi,
does userspace really need to know the value of this modparam? If no, then please
change the visibility to 0.
+MODULE_PARM_DESC(x3d_mode, "Initial 3D-VCache mode; 'frequency' (default) or 'cache'");
+
+#define DSM_REVISION_ID 0
+#define DSM_SET_X3D_MODE 1
+
+static guid_t x3d_guid = GUID_INIT(0xdff8e55f, 0xbcfd, 0x46fb, 0xba, 0x0a,
+ 0xef, 0xd0, 0x45, 0x0f, 0x34, 0xee);
+
+enum amd_x3d_mode_type {
+ MODE_INDEX_FREQ,
+ MODE_INDEX_CACHE,
+};
+
+static const char * const amd_x3d_mode_strings[] = {
+ [MODE_INDEX_FREQ] = "frequency",
+ [MODE_INDEX_CACHE] = "cache",
+};
+
+struct amd_x3d_dev {
+ struct device *dev;
+ acpi_handle ahandle;
+ /* To protect x3d mode setting */
+ struct mutex lock;
+ enum amd_x3d_mode_type curr_mode;
+};
+
+static int amd_x3d_mode_switch(struct amd_x3d_dev *data, int new_state)
+{
+ union acpi_object *out, argv;
+
+ guard(mutex)(&data->lock);
+ argv.type = ACPI_TYPE_INTEGER;
+ argv.integer.value = new_state;
+
+ out = acpi_evaluate_dsm(data->ahandle, &x3d_guid, DSM_REVISION_ID, DSM_SET_X3D_MODE,
+ &argv);
+ if (!out) {
+ dev_err(data->dev, "failed to evaluate _DSM\n");
+ return -EINVAL;
+ }
+
+ data->curr_mode = new_state;
+
+ kfree(out);
+
+ return 0;
+}
+
+static ssize_t amd_x3d_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct amd_x3d_dev *data = dev_get_drvdata(dev);
+ int ret;
+
+ ret = sysfs_match_string(amd_x3d_mode_strings, buf);
+ if (ret < 0) {
+ dev_err(dev, "no matching mode to set %s\n", buf);
Maybe just return ret here without printing anything. Otherwise userspace could spam
the kernel log by continuously writing invalid strings.
+ return ret;
+ }
+
+ ret = amd_x3d_mode_switch(data, ret);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+static ssize_t amd_x3d_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct amd_x3d_dev *data = dev_get_drvdata(dev);
+
+ if (!(data->curr_mode == MODE_INDEX_CACHE || data->curr_mode == MODE_INDEX_FREQ))
+ return -EINVAL;
Can this error condition even be triggered?
+
+ return sysfs_emit(buf, "%s\n", amd_x3d_mode_strings[data->curr_mode]);
Please use data->lock here to protect accesses to data->curr_mode.
+}
+static DEVICE_ATTR_RW(amd_x3d_mode);
+
+static struct attribute *amd_x3d_attrs[] = {
+ &dev_attr_amd_x3d_mode.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(amd_x3d);
+
+static const struct acpi_device_id amd_x3d_acpi_ids[] = {
+ {"AMDI0101"},
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, amd_x3d_acpi_ids);
+
+static int amd_x3d_probe(struct platform_device *pdev)
+{
+ struct amd_x3d_dev *data;
+ acpi_handle handle;
+ int ret;
+
+ handle = ACPI_HANDLE(&pdev->dev);
+ if (!handle)
+ return -ENODEV;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->dev = &pdev->dev;
+ data->ahandle = handle;
+ platform_set_drvdata(pdev, data);
+
+ if (!acpi_check_dsm(handle, &x3d_guid, DSM_REVISION_ID, BIT(DSM_SET_X3D_MODE)))
+ return dev_err_probe(&pdev->dev, -ENODEV, "not supported on this platform\n");
Please do this check before allocating anything, because the driver should bail out as soon as possible
when encountering unsupported hardware.
Also maybe being silent and just returning -ENODEV is better as this error message serves no purpose.
+
+ ret = match_string(amd_x3d_mode_strings, ARRAY_SIZE(amd_x3d_mode_strings), x3d_mode);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, -EINVAL, "invalid mode %s\n", x3d_mode);
+
+ devm_mutex_init(data->dev, &data->lock);
+
+ ret = amd_x3d_mode_switch(data, ret);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret, "mode switch failed\n");
amd_x3d_mode_switch() already prints an error when failing, so please just return the result directly:
return amd_x3d_mode_switch(data, ret);
Other than that the driver look good.
Thanks,
Armin Wolf
+
+ return 0;
+}
+
+static struct platform_driver amd_3d_vcache_driver = {
+ .driver = {
+ .name = "amd_x3d_vcache",
+ .dev_groups = amd_x3d_groups,
+ .acpi_match_table = amd_x3d_acpi_ids,
+ },
+ .probe = amd_x3d_probe,
+};
+module_platform_driver(amd_3d_vcache_driver);
+
+MODULE_DESCRIPTION("AMD 3D V-Cache Performance Optimizer Driver");
+MODULE_LICENSE("GPL");
