From: Perry Yuan <Perry.Yuan@amd.com>
The AMD Heterogeneous core design and Hardware Feedback Interface (HFI)
provide behavioral classification and a dynamically updated ranking table
for the scheduler to use when choosing cores for tasks.
There are two CPU core types defined: `Classic Core` and `Dense Core`.
"Classic" cores are the standard performance cores, while "Dense" cores
are optimized for area and efficiency.
Heterogeneous compute refers to CPU implementations that are comprised
of more than one architectural class, each with two capabilities. This
means each CPU reports two separate capabilities: "perf" and "eff".
Each capability lists all core ranking numbers between 0 and 255, where
a higher number represents a higher capability.
Heterogeneous systems can also extend to more than two architectural
classes.
The purpose of the scheduling feedback mechanism is to provide information
to the operating system scheduler in real time, allowing the scheduler to
direct threads to the optimal core during task scheduling.
All core ranking data are provided by the BIOS via a shared memory ranking
table, which the driver reads and uses to update core capabilities to the
scheduler. When the hardware updates the table, it generates a platform
interrupt to notify the OS to read the new ranking table.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
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>
---
v2:
* Drop unnecessary select
* Make tristate instead of bool
* Drop error messages
* Drop unnecessary function declarations for init
* Fix cleanup for amd_hfi_exit()
* Drop unnecessary variables for upcoming features
---
drivers/platform/x86/amd/Kconfig | 1 +
drivers/platform/x86/amd/Makefile | 1 +
drivers/platform/x86/amd/hfi/Kconfig | 20 +++
drivers/platform/x86/amd/hfi/Makefile | 7 ++
drivers/platform/x86/amd/hfi/hfi.c | 169 ++++++++++++++++++++++++++
5 files changed, 198 insertions(+)
create mode 100644 drivers/platform/x86/amd/hfi/Kconfig
create mode 100644 drivers/platform/x86/amd/hfi/Makefile
create mode 100644 drivers/platform/x86/amd/hfi/hfi.c
diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
index f88682d36447..c3f69dbe3037 100644
--- a/drivers/platform/x86/amd/Kconfig
+++ b/drivers/platform/x86/amd/Kconfig
@@ -5,6 +5,7 @@
source "drivers/platform/x86/amd/pmf/Kconfig"
source "drivers/platform/x86/amd/pmc/Kconfig"
+source "drivers/platform/x86/amd/hfi/Kconfig"
config AMD_HSMP
tristate "AMD HSMP Driver"
diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
index dcec0a46f8af..2676fc81fee5 100644
--- a/drivers/platform/x86/amd/Makefile
+++ b/drivers/platform/x86/amd/Makefile
@@ -9,3 +9,4 @@ amd_hsmp-y := hsmp.o
obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
obj-$(CONFIG_AMD_PMF) += pmf/
obj-$(CONFIG_AMD_WBRF) += wbrf.o
+obj-$(CONFIG_AMD_HFI) += hfi/
diff --git a/drivers/platform/x86/amd/hfi/Kconfig b/drivers/platform/x86/amd/hfi/Kconfig
new file mode 100644
index 000000000000..08051cd4f74d
--- /dev/null
+++ b/drivers/platform/x86/amd/hfi/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# AMD Hardware Feedback Interface Driver
+#
+
+config AMD_HFI
+ bool "AMD Hetero Core Hardware Feedback Driver"
+ depends on ACPI
+ depends on CPU_SUP_AMD
+ help
+ Select this option to enable the AMD Heterogeneous Core Hardware Feedback Interface. If
+ selected, hardware provides runtime thread classification guidance to the operating system
+ on the performance and energy efficiency capabilities of each heterogeneous CPU core.
+ These capabilities may vary due to the inherent differences in the core types and can
+ also change as a result of variations in the operating conditions of the system such
+ as power and thermal limits. If selected, the kernel relays updates in heterogeneous
+ CPUs' capabilities to userspace, allowing for more optimal task scheduling and
+ resource allocation, leveraging the diverse set of cores available.
+
+
diff --git a/drivers/platform/x86/amd/hfi/Makefile b/drivers/platform/x86/amd/hfi/Makefile
new file mode 100644
index 000000000000..672c6ac106e9
--- /dev/null
+++ b/drivers/platform/x86/amd/hfi/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# AMD Hardware Feedback Interface Driver
+#
+
+obj-$(CONFIG_AMD_HFI) += amd_hfi.o
+amd_hfi-objs := hfi.o
diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
new file mode 100644
index 000000000000..da2e667107e8
--- /dev/null
+++ b/drivers/platform/x86/amd/hfi/hfi.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD Hardware Feedback Interface Driver
+ *
+ * Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
+ *
+ * Author: Perry Yuan <Perry.Yuan@amd.com>
+ *
+ */
+
+#define pr_fmt(fmt) "amd-hfi: " fmt
+
+#include <linux/acpi.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/gfp.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/smp.h>
+#include <linux/string.h>
+
+#define AMD_HFI_DRIVER "amd_hfi"
+#define AMD_HETERO_CPUID_27 0x80000027
+static struct platform_device *device;
+
+struct amd_hfi_data {
+ const char *name;
+ struct device *dev;
+ struct mutex lock;
+};
+
+struct amd_hfi_classes {
+ u32 perf;
+ u32 eff;
+} __packed;
+
+/**
+ * struct amd_hfi_cpuinfo - HFI workload class info per CPU
+ * @cpu: cpu index
+ * @cpus: mask of cpus associated with amd_hfi_cpuinfo
+ * @class_index: workload class ID index
+ * @nr_class: max number of workload class supported
+ * @amd_hfi_classes: current cpu workload class ranking data
+ *
+ * Parameters of a logical processor linked with hardware feedback class
+ */
+struct amd_hfi_cpuinfo {
+ int cpu;
+ cpumask_var_t cpus;
+ s16 class_index;
+ u8 nr_class;
+ struct amd_hfi_classes *amd_hfi_classes;
+};
+
+static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
+
+static int amd_hfi_alloc_class_data(struct platform_device *pdev)
+{
+ struct amd_hfi_cpuinfo *hfi_cpuinfo;
+ struct device *dev = &pdev->dev;
+ int idx;
+ int nr_class_id;
+
+ nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27);
+ if (nr_class_id < 0 || nr_class_id > 255) {
+ dev_warn(dev, "failed to get supported class number from CPUID %d\n",
+ AMD_HETERO_CPUID_27);
+ return -EINVAL;
+ }
+
+ for_each_possible_cpu(idx) {
+ hfi_cpuinfo = per_cpu_ptr(&amd_hfi_cpuinfo, idx);
+ hfi_cpuinfo->amd_hfi_classes = devm_kmalloc(dev, nr_class_id *
+ sizeof(struct amd_hfi_classes), GFP_KERNEL);
+ if (!hfi_cpuinfo->amd_hfi_classes)
+ return -ENOMEM;
+
+ hfi_cpuinfo->nr_class = nr_class_id;
+ }
+
+ return 0;
+}
+
+static void amd_hfi_remove(struct platform_device *pdev)
+{
+ struct amd_hfi_data *dev = platform_get_drvdata(pdev);
+
+ mutex_destroy(&dev->lock);
+}
+
+static const struct acpi_device_id amd_hfi_platform_match[] = {
+ { "AMDI0104", 0},
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, amd_hfi_platform_match);
+
+static int amd_hfi_probe(struct platform_device *pdev)
+{
+ struct amd_hfi_data *amd_hfi_data;
+ int ret;
+
+ if (!acpi_match_device(amd_hfi_platform_match, &pdev->dev))
+ return -ENODEV;
+
+ amd_hfi_data = devm_kzalloc(&pdev->dev, sizeof(*amd_hfi_data), GFP_KERNEL);
+ if (!amd_hfi_data)
+ return -ENOMEM;
+
+ amd_hfi_data->dev = &pdev->dev;
+
+ mutex_init(&amd_hfi_data->lock);
+ platform_set_drvdata(pdev, amd_hfi_data);
+
+ /* alloc data array for hardware feedback class data */
+ ret = amd_hfi_alloc_class_data(pdev);
+ if (ret)
+ goto out;
+
+out:
+ return ret;
+}
+
+static struct platform_driver amd_hfi_driver = {
+ .driver = {
+ .name = AMD_HFI_DRIVER,
+ .owner = THIS_MODULE,
+ .acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
+ },
+ .probe = amd_hfi_probe,
+ .remove_new = amd_hfi_remove,
+};
+
+static int __init amd_hfi_init(void)
+{
+ int ret;
+
+ if (acpi_disabled ||
+ !boot_cpu_has(X86_FEATURE_HETERO_CORE_TOPOLOGY) ||
+ !boot_cpu_has(X86_FEATURE_WORKLOAD_CLASS))
+ return -ENODEV;
+
+ device = platform_device_register_simple(AMD_HFI_DRIVER, -1, NULL, 0);
+ if (IS_ERR(device)) {
+ pr_err("unable to register hfi platform device\n");
+ return PTR_ERR(device);
+ }
+
+ ret = platform_driver_register(&amd_hfi_driver);
+ if (ret)
+ pr_err("Failed to register hfi driver\n");
+
+ return ret;
+}
+
+static __exit void amd_hfi_exit(void)
+{
+ platform_device_unregister(device);
+ platform_driver_unregister(&amd_hfi_driver);
+}
+module_init(amd_hfi_init);
+module_exit(amd_hfi_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("AMD Hardware Feedback Interface Driver");
--
2.43.0Hello,
On Thu, Oct 10, 2024 at 02:36:57PM -0500, Mario Limonciello wrote:
> +static struct platform_driver amd_hfi_driver = {
> + .driver = {
> + .name = AMD_HFI_DRIVER,
> + .owner = THIS_MODULE,
> + .acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
> + },
> + .probe = amd_hfi_probe,
> + .remove_new = amd_hfi_remove,
> +};
After commit 0edb555a65d1 ("platform: Make platform_driver::remove()
return void") .remove() is (again) the right callback to implement for
platform drivers. Please just drop "_new".
Best regards
Uwe
Hi,
On 16-Oct-24 11:36 AM, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Oct 10, 2024 at 02:36:57PM -0500, Mario Limonciello wrote:
>> +static struct platform_driver amd_hfi_driver = {
>> + .driver = {
>> + .name = AMD_HFI_DRIVER,
>> + .owner = THIS_MODULE,
>> + .acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
>> + },
>> + .probe = amd_hfi_probe,
>> + .remove_new = amd_hfi_remove,
>> +};
>
> After commit 0edb555a65d1 ("platform: Make platform_driver::remove()
> return void") .remove() is (again) the right callback to implement for
> platform drivers. Please just drop "_new".
Note there is a "[v3,05/14] platform/x86: hfi: Introduce AMD Hardware
Feedback Interface Driver" patch superseding this one now; and that one
has the same issue.
Regards,
Hans
On 10/16/2024 04:59, Hans de Goede wrote:
> Hi,
>
> On 16-Oct-24 11:36 AM, Uwe Kleine-König wrote:
>> Hello,
>>
>> On Thu, Oct 10, 2024 at 02:36:57PM -0500, Mario Limonciello wrote:
>>> +static struct platform_driver amd_hfi_driver = {
>>> + .driver = {
>>> + .name = AMD_HFI_DRIVER,
>>> + .owner = THIS_MODULE,
>>> + .acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
>>> + },
>>> + .probe = amd_hfi_probe,
>>> + .remove_new = amd_hfi_remove,
>>> +};
>>
>> After commit 0edb555a65d1 ("platform: Make platform_driver::remove()
>> return void") .remove() is (again) the right callback to implement for
>> platform drivers. Please just drop "_new".
>
> Note there is a "[v3,05/14] platform/x86: hfi: Introduce AMD Hardware
> Feedback Interface Driver" patch superseding this one now; and that one
> has the same issue.
>
> Regards,
>
> Hans
>
Thanks! I'll gather more review feedback this week and fix this in a v4
next week.
On Thu, Oct 10, 2024 at 02:36:57PM -0500, Mario Limonciello wrote: > From: Perry Yuan <Perry.Yuan@amd.com> > > The AMD Heterogeneous core design and Hardware Feedback Interface (HFI) > provide behavioral classification and a dynamically updated ranking table > for the scheduler to use when choosing cores for tasks. > > There are two CPU core types defined: `Classic Core` and `Dense Core`. > "Classic" cores are the standard performance cores, while "Dense" cores > are optimized for area and efficiency. > > Heterogeneous compute refers to CPU implementations that are comprised > of more than one architectural class, each with two capabilities. This > means each CPU reports two separate capabilities: "perf" and "eff". > > Each capability lists all core ranking numbers between 0 and 255, where > a higher number represents a higher capability. > > Heterogeneous systems can also extend to more than two architectural > classes. > > The purpose of the scheduling feedback mechanism is to provide information > to the operating system scheduler in real time, allowing the scheduler to > direct threads to the optimal core during task scheduling. > > All core ranking data are provided by the BIOS via a shared memory ranking > table, which the driver reads and uses to update core capabilities to the > scheduler. When the hardware updates the table, it generates a platform > interrupt to notify the OS to read the new ranking table. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 I tried to find the HFI details on the documents in this "bug" but I could not find them. What document in specific could I look at? Thanks and BR, Ricardo
On 10/14/2024 22:52, Ricardo Neri wrote: > On Thu, Oct 10, 2024 at 02:36:57PM -0500, Mario Limonciello wrote: >> From: Perry Yuan <Perry.Yuan@amd.com> >> >> The AMD Heterogeneous core design and Hardware Feedback Interface (HFI) >> provide behavioral classification and a dynamically updated ranking table >> for the scheduler to use when choosing cores for tasks. >> >> There are two CPU core types defined: `Classic Core` and `Dense Core`. >> "Classic" cores are the standard performance cores, while "Dense" cores >> are optimized for area and efficiency. >> >> Heterogeneous compute refers to CPU implementations that are comprised >> of more than one architectural class, each with two capabilities. This >> means each CPU reports two separate capabilities: "perf" and "eff". >> >> Each capability lists all core ranking numbers between 0 and 255, where >> a higher number represents a higher capability. >> >> Heterogeneous systems can also extend to more than two architectural >> classes. >> >> The purpose of the scheduling feedback mechanism is to provide information >> to the operating system scheduler in real time, allowing the scheduler to >> direct threads to the optimal core during task scheduling. >> >> All core ranking data are provided by the BIOS via a shared memory ranking >> table, which the driver reads and uses to update core capabilities to the >> scheduler. When the hardware updates the table, it generates a platform >> interrupt to notify the OS to read the new ranking table. >> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 > > I tried to find the HFI details on the documents in this "bug" but I could > not find them. What document in specific could I look at? > > Thanks and BR, > Ricardo Hi Ricardo, It is spread out across multiple places. This is part of the reason for patch 1 in the series outlines details of how it works. The reason for that "collect all" Bugzilla for documentation is because the URLs for AMD documentation have undergone changes in the past and it makes it difficult to put stable URLs in commit messages. So teams that want to reference documentation put it on a dump all bug for a stable URL to reference. On that link you will find the APM, which will have some documentation specifically for the CPUID leafs used for topology identification and clearing history. Read patch 1 and let me know if it covers what specifically you're looking for. If it's still missing some info let me know what you would like added. Also; I do want to note something; this is the first series to lay some foundation for static information and not everything in patch 1 is implemented in this first series. There will be further follow-ups later.
On Tue, Oct 15, 2024 at 01:09:42PM -0500, Mario Limonciello wrote: > > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 > > > > I tried to find the HFI details on the documents in this "bug" but I could > > not find them. What document in specific could I look at? > > > > Thanks and BR, > > Ricardo > > Hi Ricardo, > > It is spread out across multiple places. This is part of the reason for > patch 1 in the series outlines details of how it works. > > The reason for that "collect all" Bugzilla for documentation is because the > URLs for AMD documentation have undergone changes in the past and it makes > it difficult to put stable URLs in commit messages. So teams that want to > reference documentation put it on a dump all bug for a stable URL to > reference. > > On that link you will find the APM, which will have some documentation > specifically for the CPUID leafs used for topology identification and > clearing history. > > Read patch 1 and let me know if it covers what specifically you're looking > for. If it's still missing some info let me know what you would like added. Thank you for your reply! I read patch 1. I was wondering specifically about more details of the Class ID. I see that they have associated counters and desired scheduling behavior. I was also curious about the layout of the HFI table. I guess I can infer it from patches 5 and 6 but if there is a picture already, I wouldn't mind. ;)
On 10/17/2024 18:33, Ricardo Neri wrote: > On Tue, Oct 15, 2024 at 01:09:42PM -0500, Mario Limonciello wrote: >>> >>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 >>> >>> I tried to find the HFI details on the documents in this "bug" but I could >>> not find them. What document in specific could I look at? >>> >>> Thanks and BR, >>> Ricardo >> >> Hi Ricardo, >> >> It is spread out across multiple places. This is part of the reason for >> patch 1 in the series outlines details of how it works. >> >> The reason for that "collect all" Bugzilla for documentation is because the >> URLs for AMD documentation have undergone changes in the past and it makes >> it difficult to put stable URLs in commit messages. So teams that want to >> reference documentation put it on a dump all bug for a stable URL to >> reference. >> >> On that link you will find the APM, which will have some documentation >> specifically for the CPUID leafs used for topology identification and >> clearing history. >> >> Read patch 1 and let me know if it covers what specifically you're looking >> for. If it's still missing some info let me know what you would like added. > > Thank you for your reply! I read patch 1. I was wondering specifically about > more details of the Class ID. I see that they have associated counters and > desired scheduling behavior. Ah thanks! Obviously in this version there is no utilization of the classifications, so this was an oversight. This is something that we'll worry about after the baseline support is landed. I'll make sure the documentation is updated in the next revision to explain these. > > I was also curious about the layout of the HFI table. I guess I can infer it > from patches 5 and 6 but if there is a picture already, I wouldn't mind. ;) > There's no picture right now, but in v3 I added a patch at the end of the series to dump the table. I'll see what makes sense to add to documentation.
On Thu, 10 Oct 2024, Mario Limonciello wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> The AMD Heterogeneous core design and Hardware Feedback Interface (HFI)
> provide behavioral classification and a dynamically updated ranking table
> for the scheduler to use when choosing cores for tasks.
>
> There are two CPU core types defined: `Classic Core` and `Dense Core`.
> "Classic" cores are the standard performance cores, while "Dense" cores
> are optimized for area and efficiency.
>
> Heterogeneous compute refers to CPU implementations that are comprised
> of more than one architectural class, each with two capabilities. This
> means each CPU reports two separate capabilities: "perf" and "eff".
>
> Each capability lists all core ranking numbers between 0 and 255, where
> a higher number represents a higher capability.
>
> Heterogeneous systems can also extend to more than two architectural
> classes.
>
> The purpose of the scheduling feedback mechanism is to provide information
> to the operating system scheduler in real time, allowing the scheduler to
> direct threads to the optimal core during task scheduling.
>
> All core ranking data are provided by the BIOS via a shared memory ranking
> table, which the driver reads and uses to update core capabilities to the
> scheduler. When the hardware updates the table, it generates a platform
> interrupt to notify the OS to read the new ranking table.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> 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>
> ---
> v2:
> * Drop unnecessary select
> * Make tristate instead of bool
> * Drop error messages
> * Drop unnecessary function declarations for init
> * Fix cleanup for amd_hfi_exit()
> * Drop unnecessary variables for upcoming features
> ---
> drivers/platform/x86/amd/Kconfig | 1 +
> drivers/platform/x86/amd/Makefile | 1 +
> drivers/platform/x86/amd/hfi/Kconfig | 20 +++
> drivers/platform/x86/amd/hfi/Makefile | 7 ++
> drivers/platform/x86/amd/hfi/hfi.c | 169 ++++++++++++++++++++++++++
> 5 files changed, 198 insertions(+)
> create mode 100644 drivers/platform/x86/amd/hfi/Kconfig
> create mode 100644 drivers/platform/x86/amd/hfi/Makefile
> create mode 100644 drivers/platform/x86/amd/hfi/hfi.c
>
> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
> index f88682d36447..c3f69dbe3037 100644
> --- a/drivers/platform/x86/amd/Kconfig
> +++ b/drivers/platform/x86/amd/Kconfig
> @@ -5,6 +5,7 @@
>
> source "drivers/platform/x86/amd/pmf/Kconfig"
> source "drivers/platform/x86/amd/pmc/Kconfig"
> +source "drivers/platform/x86/amd/hfi/Kconfig"
>
> config AMD_HSMP
> tristate "AMD HSMP Driver"
> diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
> index dcec0a46f8af..2676fc81fee5 100644
> --- a/drivers/platform/x86/amd/Makefile
> +++ b/drivers/platform/x86/amd/Makefile
> @@ -9,3 +9,4 @@ amd_hsmp-y := hsmp.o
> obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> obj-$(CONFIG_AMD_PMF) += pmf/
> obj-$(CONFIG_AMD_WBRF) += wbrf.o
> +obj-$(CONFIG_AMD_HFI) += hfi/
> diff --git a/drivers/platform/x86/amd/hfi/Kconfig b/drivers/platform/x86/amd/hfi/Kconfig
> new file mode 100644
> index 000000000000..08051cd4f74d
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hfi/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# AMD Hardware Feedback Interface Driver
> +#
> +
> +config AMD_HFI
> + bool "AMD Hetero Core Hardware Feedback Driver"
> + depends on ACPI
> + depends on CPU_SUP_AMD
> + help
> + Select this option to enable the AMD Heterogeneous Core Hardware Feedback Interface. If
> + selected, hardware provides runtime thread classification guidance to the operating system
> + on the performance and energy efficiency capabilities of each heterogeneous CPU core.
> + These capabilities may vary due to the inherent differences in the core types and can
> + also change as a result of variations in the operating conditions of the system such
> + as power and thermal limits. If selected, the kernel relays updates in heterogeneous
> + CPUs' capabilities to userspace, allowing for more optimal task scheduling and
> + resource allocation, leveraging the diverse set of cores available.
> +
> +
> diff --git a/drivers/platform/x86/amd/hfi/Makefile b/drivers/platform/x86/amd/hfi/Makefile
> new file mode 100644
> index 000000000000..672c6ac106e9
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hfi/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# AMD Hardware Feedback Interface Driver
> +#
> +
> +obj-$(CONFIG_AMD_HFI) += amd_hfi.o
> +amd_hfi-objs := hfi.o
> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
> new file mode 100644
> index 000000000000..da2e667107e8
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hfi/hfi.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD Hardware Feedback Interface Driver
> + *
> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + * Author: Perry Yuan <Perry.Yuan@amd.com>
> + *
> + */
> +
> +#define pr_fmt(fmt) "amd-hfi: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/gfp.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/smp.h>
> +#include <linux/string.h>
One more,
string.h doesn't look like being used but I could have failed to notice
something.
--
i.
> +#define AMD_HFI_DRIVER "amd_hfi"
> +#define AMD_HETERO_CPUID_27 0x80000027
> +static struct platform_device *device;
> +
> +struct amd_hfi_data {
> + const char *name;
> + struct device *dev;
> + struct mutex lock;
> +};
> +
> +struct amd_hfi_classes {
> + u32 perf;
> + u32 eff;
> +} __packed;
> +
> +/**
> + * struct amd_hfi_cpuinfo - HFI workload class info per CPU
> + * @cpu: cpu index
> + * @cpus: mask of cpus associated with amd_hfi_cpuinfo
> + * @class_index: workload class ID index
> + * @nr_class: max number of workload class supported
> + * @amd_hfi_classes: current cpu workload class ranking data
> + *
> + * Parameters of a logical processor linked with hardware feedback class
> + */
> +struct amd_hfi_cpuinfo {
> + int cpu;
> + cpumask_var_t cpus;
> + s16 class_index;
> + u8 nr_class;
> + struct amd_hfi_classes *amd_hfi_classes;
> +};
> +
> +static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
> +
> +static int amd_hfi_alloc_class_data(struct platform_device *pdev)
> +{
> + struct amd_hfi_cpuinfo *hfi_cpuinfo;
> + struct device *dev = &pdev->dev;
> + int idx;
> + int nr_class_id;
> +
> + nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27);
> + if (nr_class_id < 0 || nr_class_id > 255) {
> + dev_warn(dev, "failed to get supported class number from CPUID %d\n",
> + AMD_HETERO_CPUID_27);
> + return -EINVAL;
> + }
> +
> + for_each_possible_cpu(idx) {
> + hfi_cpuinfo = per_cpu_ptr(&amd_hfi_cpuinfo, idx);
> + hfi_cpuinfo->amd_hfi_classes = devm_kmalloc(dev, nr_class_id *
> + sizeof(struct amd_hfi_classes), GFP_KERNEL);
> + if (!hfi_cpuinfo->amd_hfi_classes)
> + return -ENOMEM;
> +
> + hfi_cpuinfo->nr_class = nr_class_id;
> + }
> +
> + return 0;
> +}
> +
> +static void amd_hfi_remove(struct platform_device *pdev)
> +{
> + struct amd_hfi_data *dev = platform_get_drvdata(pdev);
> +
> + mutex_destroy(&dev->lock);
> +}
> +
> +static const struct acpi_device_id amd_hfi_platform_match[] = {
> + { "AMDI0104", 0},
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, amd_hfi_platform_match);
> +
> +static int amd_hfi_probe(struct platform_device *pdev)
> +{
> + struct amd_hfi_data *amd_hfi_data;
> + int ret;
> +
> + if (!acpi_match_device(amd_hfi_platform_match, &pdev->dev))
> + return -ENODEV;
> +
> + amd_hfi_data = devm_kzalloc(&pdev->dev, sizeof(*amd_hfi_data), GFP_KERNEL);
> + if (!amd_hfi_data)
> + return -ENOMEM;
> +
> + amd_hfi_data->dev = &pdev->dev;
> +
> + mutex_init(&amd_hfi_data->lock);
> + platform_set_drvdata(pdev, amd_hfi_data);
> +
> + /* alloc data array for hardware feedback class data */
> + ret = amd_hfi_alloc_class_data(pdev);
> + if (ret)
> + goto out;
> +
> +out:
> + return ret;
> +}
> +
> +static struct platform_driver amd_hfi_driver = {
> + .driver = {
> + .name = AMD_HFI_DRIVER,
> + .owner = THIS_MODULE,
> + .acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
> + },
> + .probe = amd_hfi_probe,
> + .remove_new = amd_hfi_remove,
> +};
> +
> +static int __init amd_hfi_init(void)
> +{
> + int ret;
> +
> + if (acpi_disabled ||
> + !boot_cpu_has(X86_FEATURE_HETERO_CORE_TOPOLOGY) ||
> + !boot_cpu_has(X86_FEATURE_WORKLOAD_CLASS))
> + return -ENODEV;
> +
> + device = platform_device_register_simple(AMD_HFI_DRIVER, -1, NULL, 0);
> + if (IS_ERR(device)) {
> + pr_err("unable to register hfi platform device\n");
> + return PTR_ERR(device);
> + }
> +
> + ret = platform_driver_register(&amd_hfi_driver);
> + if (ret)
> + pr_err("Failed to register hfi driver\n");
> +
> + return ret;
> +}
> +
> +static __exit void amd_hfi_exit(void)
> +{
> + platform_device_unregister(device);
> + platform_driver_unregister(&amd_hfi_driver);
> +}
> +module_init(amd_hfi_init);
> +module_exit(amd_hfi_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("AMD Hardware Feedback Interface Driver");
>
On Thu, 10 Oct 2024, Mario Limonciello wrote:
> From: Perry Yuan <Perry.Yuan@amd.com>
>
> The AMD Heterogeneous core design and Hardware Feedback Interface (HFI)
> provide behavioral classification and a dynamically updated ranking table
> for the scheduler to use when choosing cores for tasks.
>
> There are two CPU core types defined: `Classic Core` and `Dense Core`.
> "Classic" cores are the standard performance cores, while "Dense" cores
> are optimized for area and efficiency.
>
> Heterogeneous compute refers to CPU implementations that are comprised
> of more than one architectural class, each with two capabilities. This
> means each CPU reports two separate capabilities: "perf" and "eff".
>
> Each capability lists all core ranking numbers between 0 and 255, where
> a higher number represents a higher capability.
>
> Heterogeneous systems can also extend to more than two architectural
> classes.
>
> The purpose of the scheduling feedback mechanism is to provide information
> to the operating system scheduler in real time, allowing the scheduler to
> direct threads to the optimal core during task scheduling.
>
> All core ranking data are provided by the BIOS via a shared memory ranking
> table, which the driver reads and uses to update core capabilities to the
> scheduler. When the hardware updates the table, it generates a platform
> interrupt to notify the OS to read the new ranking table.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> 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>
> ---
> v2:
> * Drop unnecessary select
> * Make tristate instead of bool
> * Drop error messages
> * Drop unnecessary function declarations for init
> * Fix cleanup for amd_hfi_exit()
> * Drop unnecessary variables for upcoming features
> ---
> drivers/platform/x86/amd/Kconfig | 1 +
> drivers/platform/x86/amd/Makefile | 1 +
> drivers/platform/x86/amd/hfi/Kconfig | 20 +++
> drivers/platform/x86/amd/hfi/Makefile | 7 ++
> drivers/platform/x86/amd/hfi/hfi.c | 169 ++++++++++++++++++++++++++
> 5 files changed, 198 insertions(+)
> create mode 100644 drivers/platform/x86/amd/hfi/Kconfig
> create mode 100644 drivers/platform/x86/amd/hfi/Makefile
> create mode 100644 drivers/platform/x86/amd/hfi/hfi.c
>
> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
> index f88682d36447..c3f69dbe3037 100644
> --- a/drivers/platform/x86/amd/Kconfig
> +++ b/drivers/platform/x86/amd/Kconfig
> @@ -5,6 +5,7 @@
>
> source "drivers/platform/x86/amd/pmf/Kconfig"
> source "drivers/platform/x86/amd/pmc/Kconfig"
> +source "drivers/platform/x86/amd/hfi/Kconfig"
>
> config AMD_HSMP
> tristate "AMD HSMP Driver"
> diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
> index dcec0a46f8af..2676fc81fee5 100644
> --- a/drivers/platform/x86/amd/Makefile
> +++ b/drivers/platform/x86/amd/Makefile
> @@ -9,3 +9,4 @@ amd_hsmp-y := hsmp.o
> obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> obj-$(CONFIG_AMD_PMF) += pmf/
> obj-$(CONFIG_AMD_WBRF) += wbrf.o
> +obj-$(CONFIG_AMD_HFI) += hfi/
> diff --git a/drivers/platform/x86/amd/hfi/Kconfig b/drivers/platform/x86/amd/hfi/Kconfig
> new file mode 100644
> index 000000000000..08051cd4f74d
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hfi/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# AMD Hardware Feedback Interface Driver
> +#
> +
> +config AMD_HFI
> + bool "AMD Hetero Core Hardware Feedback Driver"
> + depends on ACPI
> + depends on CPU_SUP_AMD
> + help
> + Select this option to enable the AMD Heterogeneous Core Hardware Feedback Interface. If
> + selected, hardware provides runtime thread classification guidance to the operating system
> + on the performance and energy efficiency capabilities of each heterogeneous CPU core.
> + These capabilities may vary due to the inherent differences in the core types and can
> + also change as a result of variations in the operating conditions of the system such
> + as power and thermal limits. If selected, the kernel relays updates in heterogeneous
> + CPUs' capabilities to userspace, allowing for more optimal task scheduling and
> + resource allocation, leveraging the diverse set of cores available.
> +
> +
> diff --git a/drivers/platform/x86/amd/hfi/Makefile b/drivers/platform/x86/amd/hfi/Makefile
> new file mode 100644
> index 000000000000..672c6ac106e9
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hfi/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# AMD Hardware Feedback Interface Driver
> +#
> +
> +obj-$(CONFIG_AMD_HFI) += amd_hfi.o
> +amd_hfi-objs := hfi.o
> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
> new file mode 100644
> index 000000000000..da2e667107e8
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hfi/hfi.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD Hardware Feedback Interface Driver
> + *
> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + * Author: Perry Yuan <Perry.Yuan@amd.com>
> + *
Extra line.
> + */
> +
> +#define pr_fmt(fmt) "amd-hfi: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/gfp.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/smp.h>
> +#include <linux/string.h>
> +
> +#define AMD_HFI_DRIVER "amd_hfi"
> +#define AMD_HETERO_CPUID_27 0x80000027
> +static struct platform_device *device;
> +
> +struct amd_hfi_data {
> + const char *name;
> + struct device *dev;
> + struct mutex lock;
> +};
> +
> +struct amd_hfi_classes {
> + u32 perf;
> + u32 eff;
> +} __packed;
Unnecessary packed.
> +
> +/**
> + * struct amd_hfi_cpuinfo - HFI workload class info per CPU
> + * @cpu: cpu index
> + * @cpus: mask of cpus associated with amd_hfi_cpuinfo
> + * @class_index: workload class ID index
> + * @nr_class: max number of workload class supported
> + * @amd_hfi_classes: current cpu workload class ranking data
> + *
> + * Parameters of a logical processor linked with hardware feedback class
> + */
> +struct amd_hfi_cpuinfo {
> + int cpu;
> + cpumask_var_t cpus;
> + s16 class_index;
> + u8 nr_class;
> + struct amd_hfi_classes *amd_hfi_classes;
> +};
> +
> +static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
> +
> +static int amd_hfi_alloc_class_data(struct platform_device *pdev)
> +{
> + struct amd_hfi_cpuinfo *hfi_cpuinfo;
> + struct device *dev = &pdev->dev;
> + int idx;
> + int nr_class_id;
> +
> + nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27);
> + if (nr_class_id < 0 || nr_class_id > 255) {
> + dev_warn(dev, "failed to get supported class number from CPUID %d\n",
> + AMD_HETERO_CPUID_27);
> + return -EINVAL;
> + }
> +
> + for_each_possible_cpu(idx) {
> + hfi_cpuinfo = per_cpu_ptr(&amd_hfi_cpuinfo, idx);
> + hfi_cpuinfo->amd_hfi_classes = devm_kmalloc(dev, nr_class_id *
> + sizeof(struct amd_hfi_classes), GFP_KERNEL);
I recommend splitting this line differently.
Why it's not using kzalloc?
> + if (!hfi_cpuinfo->amd_hfi_classes)
> + return -ENOMEM;
> +
> + hfi_cpuinfo->nr_class = nr_class_id;
> + }
> +
> + return 0;
> +}
> +
> +static void amd_hfi_remove(struct platform_device *pdev)
> +{
> + struct amd_hfi_data *dev = platform_get_drvdata(pdev);
> +
> + mutex_destroy(&dev->lock);
> +}
> +
> +static const struct acpi_device_id amd_hfi_platform_match[] = {
> + { "AMDI0104", 0},
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, amd_hfi_platform_match);
> +
> +static int amd_hfi_probe(struct platform_device *pdev)
> +{
> + struct amd_hfi_data *amd_hfi_data;
> + int ret;
> +
> + if (!acpi_match_device(amd_hfi_platform_match, &pdev->dev))
> + return -ENODEV;
> +
> + amd_hfi_data = devm_kzalloc(&pdev->dev, sizeof(*amd_hfi_data), GFP_KERNEL);
> + if (!amd_hfi_data)
> + return -ENOMEM;
> +
> + amd_hfi_data->dev = &pdev->dev;
> +
> + mutex_init(&amd_hfi_data->lock);
No idea why the empty line is put between these initializations of the
data structure fields.
> + platform_set_drvdata(pdev, amd_hfi_data);
> +
> + /* alloc data array for hardware feedback class data */
Unnecessary comment? Isn't the function name enough to tell it?
> + ret = amd_hfi_alloc_class_data(pdev);
> + if (ret)
> + goto out;
> +
> +out:
I'm guessing this might be to reduce churn in some patch after this,
if not, this is enough:
return amd_hfi_alloc_class_data(pdev);
> + return ret;
> +}
> +
> +static struct platform_driver amd_hfi_driver = {
> + .driver = {
> + .name = AMD_HFI_DRIVER,
> + .owner = THIS_MODULE,
> + .acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
> + },
> + .probe = amd_hfi_probe,
> + .remove_new = amd_hfi_remove,
> +};
> +
> +static int __init amd_hfi_init(void)
> +{
> + int ret;
> +
> + if (acpi_disabled ||
> + !boot_cpu_has(X86_FEATURE_HETERO_CORE_TOPOLOGY) ||
> + !boot_cpu_has(X86_FEATURE_WORKLOAD_CLASS))
> + return -ENODEV;
> +
> + device = platform_device_register_simple(AMD_HFI_DRIVER, -1, NULL, 0);
> + if (IS_ERR(device)) {
> + pr_err("unable to register hfi platform device\n");
I assume HFI is the correct capitalization given I see some comments with
that. Please correct any user visible print outs to use the capitalized
form.
> + return PTR_ERR(device);
> + }
> +
> + ret = platform_driver_register(&amd_hfi_driver);
> + if (ret)
> + pr_err("Failed to register hfi driver\n");
> +
> + return ret;
> +}
> +
> +static __exit void amd_hfi_exit(void)
> +{
> + platform_device_unregister(device);
> + platform_driver_unregister(&amd_hfi_driver);
> +}
> +module_init(amd_hfi_init);
> +module_exit(amd_hfi_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("AMD Hardware Feedback Interface Driver");
>
--
i.
On 10/14/2024 04:10, Ilpo Järvinen wrote:
> On Thu, 10 Oct 2024, Mario Limonciello wrote:
>
>> From: Perry Yuan <Perry.Yuan@amd.com>
>>
>> The AMD Heterogeneous core design and Hardware Feedback Interface (HFI)
>> provide behavioral classification and a dynamically updated ranking table
>> for the scheduler to use when choosing cores for tasks.
>>
>> There are two CPU core types defined: `Classic Core` and `Dense Core`.
>> "Classic" cores are the standard performance cores, while "Dense" cores
>> are optimized for area and efficiency.
>>
>> Heterogeneous compute refers to CPU implementations that are comprised
>> of more than one architectural class, each with two capabilities. This
>> means each CPU reports two separate capabilities: "perf" and "eff".
>>
>> Each capability lists all core ranking numbers between 0 and 255, where
>> a higher number represents a higher capability.
>>
>> Heterogeneous systems can also extend to more than two architectural
>> classes.
>>
>> The purpose of the scheduling feedback mechanism is to provide information
>> to the operating system scheduler in real time, allowing the scheduler to
>> direct threads to the optimal core during task scheduling.
>>
>> All core ranking data are provided by the BIOS via a shared memory ranking
>> table, which the driver reads and uses to update core capabilities to the
>> scheduler. When the hardware updates the table, it generates a platform
>> interrupt to notify the OS to read the new ranking table.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
>> 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>
>> ---
>> v2:
>> * Drop unnecessary select
>> * Make tristate instead of bool
>> * Drop error messages
>> * Drop unnecessary function declarations for init
>> * Fix cleanup for amd_hfi_exit()
>> * Drop unnecessary variables for upcoming features
>> ---
>> drivers/platform/x86/amd/Kconfig | 1 +
>> drivers/platform/x86/amd/Makefile | 1 +
>> drivers/platform/x86/amd/hfi/Kconfig | 20 +++
>> drivers/platform/x86/amd/hfi/Makefile | 7 ++
>> drivers/platform/x86/amd/hfi/hfi.c | 169 ++++++++++++++++++++++++++
>> 5 files changed, 198 insertions(+)
>> create mode 100644 drivers/platform/x86/amd/hfi/Kconfig
>> create mode 100644 drivers/platform/x86/amd/hfi/Makefile
>> create mode 100644 drivers/platform/x86/amd/hfi/hfi.c
>>
>> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
>> index f88682d36447..c3f69dbe3037 100644
>> --- a/drivers/platform/x86/amd/Kconfig
>> +++ b/drivers/platform/x86/amd/Kconfig
>> @@ -5,6 +5,7 @@
>>
>> source "drivers/platform/x86/amd/pmf/Kconfig"
>> source "drivers/platform/x86/amd/pmc/Kconfig"
>> +source "drivers/platform/x86/amd/hfi/Kconfig"
>>
>> config AMD_HSMP
>> tristate "AMD HSMP Driver"
>> diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
>> index dcec0a46f8af..2676fc81fee5 100644
>> --- a/drivers/platform/x86/amd/Makefile
>> +++ b/drivers/platform/x86/amd/Makefile
>> @@ -9,3 +9,4 @@ amd_hsmp-y := hsmp.o
>> obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
>> obj-$(CONFIG_AMD_PMF) += pmf/
>> obj-$(CONFIG_AMD_WBRF) += wbrf.o
>> +obj-$(CONFIG_AMD_HFI) += hfi/
>> diff --git a/drivers/platform/x86/amd/hfi/Kconfig b/drivers/platform/x86/amd/hfi/Kconfig
>> new file mode 100644
>> index 000000000000..08051cd4f74d
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/hfi/Kconfig
>> @@ -0,0 +1,20 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# AMD Hardware Feedback Interface Driver
>> +#
>> +
>> +config AMD_HFI
>> + bool "AMD Hetero Core Hardware Feedback Driver"
>> + depends on ACPI
>> + depends on CPU_SUP_AMD
>> + help
>> + Select this option to enable the AMD Heterogeneous Core Hardware Feedback Interface. If
>> + selected, hardware provides runtime thread classification guidance to the operating system
>> + on the performance and energy efficiency capabilities of each heterogeneous CPU core.
>> + These capabilities may vary due to the inherent differences in the core types and can
>> + also change as a result of variations in the operating conditions of the system such
>> + as power and thermal limits. If selected, the kernel relays updates in heterogeneous
>> + CPUs' capabilities to userspace, allowing for more optimal task scheduling and
>> + resource allocation, leveraging the diverse set of cores available.
>> +
>> +
>> diff --git a/drivers/platform/x86/amd/hfi/Makefile b/drivers/platform/x86/amd/hfi/Makefile
>> new file mode 100644
>> index 000000000000..672c6ac106e9
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/hfi/Makefile
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# AMD Hardware Feedback Interface Driver
>> +#
>> +
>> +obj-$(CONFIG_AMD_HFI) += amd_hfi.o
>> +amd_hfi-objs := hfi.o
>> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
>> new file mode 100644
>> index 000000000000..da2e667107e8
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/hfi/hfi.c
>> @@ -0,0 +1,169 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * AMD Hardware Feedback Interface Driver
>> + *
>> + * Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
>> + *
>> + * Author: Perry Yuan <Perry.Yuan@amd.com>
>> + *
>
> Extra line.
>
>> + */
>> +
>> +#define pr_fmt(fmt) "amd-hfi: " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/gfp.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/printk.h>
>> +#include <linux/smp.h>
>> +#include <linux/string.h>
>> +
>> +#define AMD_HFI_DRIVER "amd_hfi"
>> +#define AMD_HETERO_CPUID_27 0x80000027
>> +static struct platform_device *device;
>> +
>> +struct amd_hfi_data {
>> + const char *name;
>> + struct device *dev;
>> + struct mutex lock;
>> +};
>> +
>> +struct amd_hfi_classes {
>> + u32 perf;
>> + u32 eff;
>> +} __packed;
>
> Unnecessary packed.
>
>> +
>> +/**
>> + * struct amd_hfi_cpuinfo - HFI workload class info per CPU
>> + * @cpu: cpu index
>> + * @cpus: mask of cpus associated with amd_hfi_cpuinfo
>> + * @class_index: workload class ID index
>> + * @nr_class: max number of workload class supported
>> + * @amd_hfi_classes: current cpu workload class ranking data
>> + *
>> + * Parameters of a logical processor linked with hardware feedback class
>> + */
>> +struct amd_hfi_cpuinfo {
>> + int cpu;
>> + cpumask_var_t cpus;
>> + s16 class_index;
>> + u8 nr_class;
>> + struct amd_hfi_classes *amd_hfi_classes;
>> +};
>> +
>> +static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
>> +
>> +static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>> +{
>> + struct amd_hfi_cpuinfo *hfi_cpuinfo;
>> + struct device *dev = &pdev->dev;
>> + int idx;
>> + int nr_class_id;
>> +
>> + nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27);
>> + if (nr_class_id < 0 || nr_class_id > 255) {
>> + dev_warn(dev, "failed to get supported class number from CPUID %d\n",
>> + AMD_HETERO_CPUID_27);
>> + return -EINVAL;
>> + }
>> +
>> + for_each_possible_cpu(idx) {
>> + hfi_cpuinfo = per_cpu_ptr(&amd_hfi_cpuinfo, idx);
>> + hfi_cpuinfo->amd_hfi_classes = devm_kmalloc(dev, nr_class_id *
>> + sizeof(struct amd_hfi_classes), GFP_KERNEL);
>
> I recommend splitting this line differently.
>
> Why it's not using kzalloc?
>
>> + if (!hfi_cpuinfo->amd_hfi_classes)
>> + return -ENOMEM;
>> +
>> + hfi_cpuinfo->nr_class = nr_class_id;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void amd_hfi_remove(struct platform_device *pdev)
>> +{
>> + struct amd_hfi_data *dev = platform_get_drvdata(pdev);
>> +
>> + mutex_destroy(&dev->lock);
>> +}
>> +
>> +static const struct acpi_device_id amd_hfi_platform_match[] = {
>> + { "AMDI0104", 0},
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, amd_hfi_platform_match);
>> +
>> +static int amd_hfi_probe(struct platform_device *pdev)
>> +{
>> + struct amd_hfi_data *amd_hfi_data;
>> + int ret;
>> +
>> + if (!acpi_match_device(amd_hfi_platform_match, &pdev->dev))
>> + return -ENODEV;
>> +
>> + amd_hfi_data = devm_kzalloc(&pdev->dev, sizeof(*amd_hfi_data), GFP_KERNEL);
>> + if (!amd_hfi_data)
>> + return -ENOMEM;
>> +
>> + amd_hfi_data->dev = &pdev->dev;
>> +
>> + mutex_init(&amd_hfi_data->lock);
>
> No idea why the empty line is put between these initializations of the
> data structure fields.
>
>> + platform_set_drvdata(pdev, amd_hfi_data);
>> +
>> + /* alloc data array for hardware feedback class data */
>
> Unnecessary comment? Isn't the function name enough to tell it?
>
>> + ret = amd_hfi_alloc_class_data(pdev);
>> + if (ret)
>> + goto out;
>> +
>> +out:
>
> I'm guessing this might be to reduce churn in some patch after this,
> if not, this is enough:
Thanks for all the feedback.
This was intended to avoid churn, but some of that change isn't part of
this series and will be a future follow up that can adjust at that time.
I'll adjust to churn for "this series alone".
>
> return amd_hfi_alloc_class_data(pdev);
>
>> + return ret;
>> +}
>> +
>> +static struct platform_driver amd_hfi_driver = {
>> + .driver = {
>> + .name = AMD_HFI_DRIVER,
>> + .owner = THIS_MODULE,
>> + .acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
>> + },
>> + .probe = amd_hfi_probe,
>> + .remove_new = amd_hfi_remove,
>> +};
>> +
>> +static int __init amd_hfi_init(void)
>> +{
>> + int ret;
>> +
>> + if (acpi_disabled ||
>> + !boot_cpu_has(X86_FEATURE_HETERO_CORE_TOPOLOGY) ||
>> + !boot_cpu_has(X86_FEATURE_WORKLOAD_CLASS))
>> + return -ENODEV;
>> +
>> + device = platform_device_register_simple(AMD_HFI_DRIVER, -1, NULL, 0);
>> + if (IS_ERR(device)) {
>> + pr_err("unable to register hfi platform device\n");
>
> I assume HFI is the correct capitalization given I see some comments with
> that. Please correct any user visible print outs to use the capitalized
> form.
>
>> + return PTR_ERR(device);
>> + }
>> +
>> + ret = platform_driver_register(&amd_hfi_driver);
>> + if (ret)
>> + pr_err("Failed to register hfi driver\n");
>> +
>> + return ret;
>> +}
>> +
>> +static __exit void amd_hfi_exit(void)
>> +{
>> + platform_device_unregister(device);
>> + platform_driver_unregister(&amd_hfi_driver);
>> +}
>> +module_init(amd_hfi_init);
>> +module_exit(amd_hfi_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("AMD Hardware Feedback Interface Driver");
>>
>
© 2016 - 2024 Red Hat, Inc.