* Re: [PATCH v5 1/2] platform/x86/amd: amd_3d_vcache: Add AMD 3D V-Cache optimizer driver
2024-11-11 10:46 ` [PATCH v5 1/2] platform/x86/amd: amd_3d_vcache: Add " Basavaraj Natikar
@ 2024-11-11 19:03 ` Armin Wolf
2024-11-12 10:00 ` Ilpo Järvinen
1 sibling, 0 replies; 5+ messages in thread
From: Armin Wolf @ 2024-11-11 19:03 UTC (permalink / raw)
To: Basavaraj Natikar, hdegoede, ilpo.jarvinen, platform-driver-x86
Cc: perry.yuan, mario.limonciello, Shyam-sundar.S-k
Am 11.11.24 um 11: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@amd.com>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
> MAINTAINERS | 7 ++
> drivers/platform/x86/amd/Kconfig | 12 ++
> drivers/platform/x86/amd/Makefile | 2 +
> drivers/platform/x86/amd/x3d_vcache.c | 173 ++++++++++++++++++++++++++
> 4 files changed, 194 insertions(+)
> create mode 100644 drivers/platform/x86/amd/x3d_vcache.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 21fdaa19229a..429ac6496f42 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@amd.com>
> +R: Mario Limonciello <mario.limonciello@amd.com>
> +L: platform-driver-x86@vger.kernel.org
> +S: Supported
> +F: drivers/platform/x86/amd/x3d_vcache.c
> +
> AMD ADDRESS TRANSLATION LIBRARY (ATL)
> M: Yazen Ghannam <Yazen.Ghannam@amd.com>
> L: linux-edac@vger.kernel.org
> 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..a1a147277814
> --- /dev/null
> +++ b/drivers/platform/x86/amd/x3d_vcache.c
> @@ -0,0 +1,173 @@
> +// 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@amd.com>
> + * Perry Yuan <perry.yuan@amd.com>
> + * Mario Limonciello <mario.limonciello@amd.com>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/array_size.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/sysfs.h>
> +#include <linux/uuid.h>
> +
> +static char *x3d_mode = "frequency";
> +module_param(x3d_mode, charp, 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_get_mode(struct amd_x3d_dev *data)
> +{
> + guard(mutex)(&data->lock);
> +
> + return data->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)
> + 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);
> + int mode = amd_x3d_get_mode(data);
> +
> + return sysfs_emit(buf, "%s\n", amd_x3d_mode_strings[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 int amd_x3d_resume_handler(struct device *dev)
> +{
> + struct amd_x3d_dev *data = dev_get_drvdata(dev);
> + int ret = amd_x3d_get_mode(data);
> +
> + return amd_x3d_mode_switch(data, ret);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(amd_x3d_pm, NULL, amd_x3d_resume_handler);
> +
> +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;
> +
> + if (!acpi_check_dsm(handle, &x3d_guid, DSM_REVISION_ID, BIT(DSM_SET_X3D_MODE)))
> + 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);
> +
> + 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);
Hi,
it could be that i overlooked this in my previous review, but devm_mutex_init() can fail, so
you need to do some error checking here.
Otherwise the driver looks good to me :).
Thanks,
Armin Wolf
> +
> + return amd_x3d_mode_switch(data, ret);
> +}
> +
> +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,
> + .pm = pm_sleep_ptr(&amd_x3d_pm),
> + },
> + .probe = amd_x3d_probe,
> +};
> +module_platform_driver(amd_3d_vcache_driver);
> +
> +MODULE_DESCRIPTION("AMD 3D V-Cache Performance Optimizer Driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v5 1/2] platform/x86/amd: amd_3d_vcache: Add AMD 3D V-Cache optimizer driver
2024-11-11 10:46 ` [PATCH v5 1/2] platform/x86/amd: amd_3d_vcache: Add " Basavaraj Natikar
2024-11-11 19:03 ` Armin Wolf
@ 2024-11-12 10:00 ` Ilpo Järvinen
1 sibling, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2024-11-12 10:00 UTC (permalink / raw)
To: Basavaraj Natikar
Cc: Hans de Goede, platform-driver-x86, perry.yuan, mario.limonciello,
Shyam-sundar.S-k, W_Armin
On Mon, 11 Nov 2024, Basavaraj Natikar wrote:
> 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@amd.com>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
> MAINTAINERS | 7 ++
> drivers/platform/x86/amd/Kconfig | 12 ++
> drivers/platform/x86/amd/Makefile | 2 +
> drivers/platform/x86/amd/x3d_vcache.c | 173 ++++++++++++++++++++++++++
> 4 files changed, 194 insertions(+)
> create mode 100644 drivers/platform/x86/amd/x3d_vcache.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 21fdaa19229a..429ac6496f42 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@amd.com>
> +R: Mario Limonciello <mario.limonciello@amd.com>
> +L: platform-driver-x86@vger.kernel.org
> +S: Supported
> +F: drivers/platform/x86/amd/x3d_vcache.c
> +
> AMD ADDRESS TRANSLATION LIBRARY (ATL)
> M: Yazen Ghannam <Yazen.Ghannam@amd.com>
> L: linux-edac@vger.kernel.org
> 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
Hi,
This doesn't apply cleanly to pxd86/for-next branch. Please
rebase.
--
i.
> 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..a1a147277814
> --- /dev/null
> +++ b/drivers/platform/x86/amd/x3d_vcache.c
> @@ -0,0 +1,173 @@
> +// 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@amd.com>
> + * Perry Yuan <perry.yuan@amd.com>
> + * Mario Limonciello <mario.limonciello@amd.com>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/array_size.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/sysfs.h>
> +#include <linux/uuid.h>
> +
> +static char *x3d_mode = "frequency";
> +module_param(x3d_mode, charp, 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_get_mode(struct amd_x3d_dev *data)
> +{
> + guard(mutex)(&data->lock);
> +
> + return data->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)
> + 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);
> + int mode = amd_x3d_get_mode(data);
> +
> + return sysfs_emit(buf, "%s\n", amd_x3d_mode_strings[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 int amd_x3d_resume_handler(struct device *dev)
> +{
> + struct amd_x3d_dev *data = dev_get_drvdata(dev);
> + int ret = amd_x3d_get_mode(data);
> +
> + return amd_x3d_mode_switch(data, ret);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(amd_x3d_pm, NULL, amd_x3d_resume_handler);
> +
> +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;
> +
> + if (!acpi_check_dsm(handle, &x3d_guid, DSM_REVISION_ID, BIT(DSM_SET_X3D_MODE)))
> + 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);
> +
> + 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);
> +
> + return amd_x3d_mode_switch(data, ret);
> +}
> +
> +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,
> + .pm = pm_sleep_ptr(&amd_x3d_pm),
> + },
> + .probe = amd_x3d_probe,
> +};
> +module_platform_driver(amd_3d_vcache_driver);
> +
> +MODULE_DESCRIPTION("AMD 3D V-Cache Performance Optimizer Driver");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply [flat|nested] 5+ messages in thread