[PATCH v2 00/13] Add support for AMD hardware feedback interface

[PATCH v2 00/13] Add support for AMD hardware feedback interface

[Mario Limonciello]

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.

Threads are classified during runtime into enumerated classes.
Currently, the driver supports 3 classes (0 through 2). These classes
represent thread performance/power characteristics that may benefit from
special scheduling behaviors. The real-time thread classification is
consumed by the operating system and is used to inform the scheduler of
where the thread should be placed for optimal performance or energy efficiency.

The thread classification helps to select CPU from a ranking table that describes
an efficiency and performance ranking for each classification from two dimensions.

The ranking data provided by the ranking table are numbers ranging from 0 to 255,
where a higher performance value indicates higher performance capability and a higher
efficiency value indicates greater efficiency. All the CPU cores are ranked into
different class IDs. Within each class ranking, the cores may have different ranking
values. Therefore, picking from each classification ID will later allow the scheduler
to select the best core while threads are classified into the specified workload class.

This series was originally submitted by Perry Yuan [1] but he is now doing a different
role and he asked me to take over.

Link: https://lore.kernel.org/all/cover.1724748733.git.perry.yuan@amd.com/

On applicable hardware this series has between a 2% and 5% improvement across various
benchmarks.

There is however a cost associated with clearing history on the process context switch.
On average it increases the delay by 119ns, and also has a wider range in delays
(the standard deviation is 25% greater).

Mario Limonciello (3):
  MAINTAINERS: Add maintainer entry for AMD Hardware Feedback Driver
  cpufreq/amd-pstate: Disable preferred cores on designs with workload
    classification
  platform/x86/amd: hfi: Set ITMT priority from ranking data

Perry Yuan (10):
  Documentation: x86: Add AMD Hardware Feedback Interface documentation
  x86/cpufeatures: add X86_FEATURE_WORKLOAD_CLASS feature bit
  x86/msr-index: define AMD heterogeneous CPU related MSR
  platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver
  platform/x86: hfi: parse CPU core ranking data from shared memory
  platform/x86: hfi: init per-cpu scores for each class
  platform/x86: hfi: add online and offline callback support
  platform/x86: hfi: add power management callback
  x86/cpu: Enable SD_ASYM_PACKING for DIE Domain on AMD Processors
  x86/process: Clear hardware feedback history for AMD processors

 Documentation/arch/x86/amd-hfi.rst    | 116 ++++++
 Documentation/arch/x86/index.rst      |   1 +
 MAINTAINERS                           |   9 +
 arch/x86/include/asm/cpufeatures.h    |   1 +
 arch/x86/include/asm/hreset.h         |   6 +
 arch/x86/include/asm/msr-index.h      |   5 +
 arch/x86/kernel/cpu/common.c          |  15 +
 arch/x86/kernel/cpu/scattered.c       |   1 +
 arch/x86/kernel/process_32.c          |   3 +
 arch/x86/kernel/process_64.c          |   3 +
 arch/x86/kernel/smpboot.c             |   5 +-
 drivers/cpufreq/amd-pstate.c          |   4 +
 drivers/platform/x86/amd/Kconfig      |   1 +
 drivers/platform/x86/amd/Makefile     |   1 +
 drivers/platform/x86/amd/hfi/Kconfig  |  21 +
 drivers/platform/x86/amd/hfi/Makefile |   7 +
 drivers/platform/x86/amd/hfi/hfi.c    | 541 ++++++++++++++++++++++++++
 17 files changed, 738 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/arch/x86/amd-hfi.rst
 create mode 100644 arch/x86/include/asm/hreset.h
 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

-- 
2.43.0

[PATCH v2 01/13] Documentation: x86: Add AMD Hardware Feedback Interface documentation

[Mario Limonciello]

From: Perry Yuan <Perry.Yuan@amd.com>

Introduce a new documentation file, `amd_hfi.rst`, which delves into the
implementation details of the AMD Hardware Feedback Interface and its
associated driver, `amd_hfi`. This documentation describes how the
driver provides hint to the OS scheduling which depends on the capability
of core performance and efficiency ranking data.

This documentation describes
* The design of the driver
* How the driver provides hints to the OS scheduling
* How the driver interfaces with the kernel for efficiency ranking data.

Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 Documentation/arch/x86/amd-hfi.rst | 116 +++++++++++++++++++++++++++++
 Documentation/arch/x86/index.rst   |   1 +
 2 files changed, 117 insertions(+)
 create mode 100644 Documentation/arch/x86/amd-hfi.rst

diff --git a/Documentation/arch/x86/amd-hfi.rst b/Documentation/arch/x86/amd-hfi.rst
new file mode 100644
index 000000000000..351641ce2821
--- /dev/null
+++ b/Documentation/arch/x86/amd-hfi.rst
@@ -0,0 +1,116 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======================================================================
+Hardware Feedback Interface For Hetero Core Scheduling On AMD Platform
+======================================================================
+
+:Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
+
+:Author: Perry Yuan <perry.yuan@amd.com>
+
+Overview
+--------
+
+AMD Heterogeneous Core implementations are comprised of more than one
+architectural class and CPUs are comprised of cores of various efficiency
+and power capabilities. Power management strategies must be designed to accommodate
+the complexities introduced by incorporating different core types.
+Heterogeneous systems can also extend to more than two architectural classes as well.
+The purpose of the scheduling feedback mechanism is to provide information to
+the operating system scheduler in real time such that the scheduler can direct
+threads to the optimal core.
+
+``Classic cores`` are generally more performant and ``Dense cores`` are generally more
+efficient.
+The goal of AMD's heterogeneous architecture is to attain power benefit by sending
+background thread to the dense cores while sending high priority threads to the classic
+cores. From a performance perspective, sending background threads to dense cores can free
+up power headroom and allow the classic cores to optimally service demanding threads.
+Furthermore, the area optimized nature of the dense cores allows for an increasing
+number of physical cores. This improved core density will have positive multithreaded
+performance impact.
+
+AMD Heterogeneous Core Driver
+-----------------------------
+
+The ``amd_hfi`` driver delivers the operating system a performance and energy efficiency
+capability data for each CPU in the system. The scheduler can use the ranking data
+from the HFI driver to make task placement decisions.
+
+Thread Classification and Ranking Table Interaction
+----------------------------------------------------
+
+The thread classification is used to select into a ranking table that describes
+an efficiency and performance ranking for each classification.
+
+Threads are classified during runtime into enumerated classes. The classes represent
+thread performance/power characteristics that may benefit from special scheduling behaviors.
+The below table depicts an example of thread classification and a preference where a given thread
+should be scheduled based on its thread class. The real time thread classification is consumed
+by the operating system and is used to inform the scheduler of where the thread should be placed.
+
+Thread Classification Example Table
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
++----------+----------------+-------------------------------+---------------------+---------+
+| class ID | Classification | Preferred scheduling behavior | Preemption priority | Counter |
++----------+----------------+-------------------------------+---------------------+---------+
+| 0        | Default        | Performant                    | Highest             |         |
++----------+----------------+-------------------------------+---------------------+---------+
+| 1        | Non-scalable   | Efficient                     | Lowest              | PMCx1A1 |
++----------+----------------+-------------------------------+---------------------+---------+
+| 2        | I/O bound      | Efficient                     | Lowest              | PMCx044 |
++----------+----------------+-------------------------------+---------------------+---------+
+
+
+AMD Hardware Feedback Interface
+--------------------------------
+
+The Hardware Feedback Interface provides to the operating system information
+about the performance and energy efficiency of each CPU in the system. Each
+capability is given as a unit-less quantity in the range [0-255]. A higher
+performance value indicates higher performance capability, and a higher
+efficiency value indicates more efficiency. Energy efficiency and performance
+are reported in separate capabilities in the shared memory based ranking table.
+
+These capabilities may change at runtime as a result of changes in the
+operating conditions of the system or the action of external factors.
+Power Management FW is responsible for detecting events that would require
+a reordering of the performance and efficiency ranking. Table updates would
+happen relatively infrequently and occur on the time scale of seconds or more.
+
+The mechanism used to trigger a table update like below events:
+    * Thermal Stress Events
+    * Silent Compute
+    * Extreme Low Battery Scenarios
+
+The kernel or a userspace policy daemon can use these capabilities to modify
+task placement decisions. For instance, if either the performance or energy
+capabilities of a given logical processor becomes zero, it is an indication that
+the hardware recommends to the operating system to not schedule any tasks on
+that processor for performance or energy efficiency reasons, respectively.
+
+Implementation details for Linux
+--------------------------------
+
+The implementation of threads scheduling consists of the following steps:
+
+1. A thread is spawned and scheduled to the ideal core using the default
+   heterogeneous scheduling policy.
+2. The processor profiles thread execution and assigns an enumerated classification ID.
+   This classification is communicated to the OS via logical processor scope MSR.
+3. During the thread context switch out the operating system consumes the workload(WL)
+   classification which resides in a logical processor scope MSR.
+4. The OS triggers the hardware to clear its history by writing to an MSR,
+   after consuming the WL classification and before switching in the new thread.
+5. If due to the classification, ranking table, and processor availability,
+   the thread is not on its ideal processor, the OS will then consider scheduling
+   the thread on its ideal processor (if available).
+
+Ranking Table update
+---------------------------
+The power management firmware issues an platform interrupt after updating the ranking
+table and is ready for the operating system to consume it. CPUs receive such interrupt
+and read new ranking table from shared memory which PCCT table has provided, then
+``amd_hfi`` driver parse the new table to provide new consume data for scheduling decisions.
+
+
diff --git a/Documentation/arch/x86/index.rst b/Documentation/arch/x86/index.rst
index 8ac64d7de4dc..7f47229f3104 100644
--- a/Documentation/arch/x86/index.rst
+++ b/Documentation/arch/x86/index.rst
@@ -43,3 +43,4 @@ x86-specific Documentation
    features
    elf_auxvec
    xstate
+   amd_hfi
-- 
2.43.0

Re: [PATCH v2 01/13] Documentation: x86: Add AMD Hardware Feedback Interface documentation

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 5792 bytes --]

On Thu, Oct 10, 2024 at 02:36:53PM -0500, Mario Limonciello wrote:
> +======================================================================
> +Hardware Feedback Interface For Hetero Core Scheduling On AMD Platform
> +======================================================================
> +
> +:Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
> +
> +:Author: Perry Yuan <perry.yuan@amd.com>

Don't forget to correct the copyright reST field:

diff --git a/Documentation/arch/x86/amd-hfi.rst b/Documentation/arch/x86/amd-hfi.rst
index 5ada5c5b79f4b5..82811be984799d 100644
--- a/Documentation/arch/x86/amd-hfi.rst
+++ b/Documentation/arch/x86/amd-hfi.rst
@@ -4,7 +4,7 @@
 Hardware Feedback Interface For Hetero Core Scheduling On AMD Platform
 ======================================================================
 
-:Copyright (C) 2024 Advanced Micro Devices, Inc. All Rights Reserved.
+:Copyright: 2024 Advanced Micro Devices, Inc. All Rights Reserved.
 
 :Author: Perry Yuan <perry.yuan@amd.com>
 

> +
> +Overview
> +--------
> +
> +AMD Heterogeneous Core implementations are comprised of more than one
> +architectural class and CPUs are comprised of cores of various efficiency
> +and power capabilities. Power management strategies must be designed to accommodate
> +the complexities introduced by incorporating different core types.
> +Heterogeneous systems can also extend to more than two architectural classes as well.
> +The purpose of the scheduling feedback mechanism is to provide information to
> +the operating system scheduler in real time such that the scheduler can direct
> +threads to the optimal core.
> +
> +``Classic cores`` are generally more performant and ``Dense cores`` are generally more
> +efficient.
> +The goal of AMD's heterogeneous architecture is to attain power benefit by sending
> +background thread to the dense cores while sending high priority threads to the classic
> +cores. From a performance perspective, sending background threads to dense cores can free
> +up power headroom and allow the classic cores to optimally service demanding threads.
> +Furthermore, the area optimized nature of the dense cores allows for an increasing
> +number of physical cores. This improved core density will have positive multithreaded
> +performance impact.
> +
> <snipped>...
> +
> +The mechanism used to trigger a table update like below events:
> +    * Thermal Stress Events
> +    * Silent Compute
> +    * Extreme Low Battery Scenarios

What about below wording?

---- >8 ----
diff --git a/Documentation/arch/x86/amd-hfi.rst b/Documentation/arch/x86/amd-hfi.rst
index 351641ce28213c..5ada5c5b79f4b5 100644
--- a/Documentation/arch/x86/amd-hfi.rst
+++ b/Documentation/arch/x86/amd-hfi.rst
@@ -12,16 +12,15 @@ Overview
 --------
 
 AMD Heterogeneous Core implementations are comprised of more than one
-architectural class and CPUs are comprised of cores of various efficiency
-and power capabilities. Power management strategies must be designed to accommodate
-the complexities introduced by incorporating different core types.
-Heterogeneous systems can also extend to more than two architectural classes as well.
-The purpose of the scheduling feedback mechanism is to provide information to
-the operating system scheduler in real time such that the scheduler can direct
-threads to the optimal core.
+architectural class and CPUs are comprised of cores of various efficiency and
+power capabilities: performance-oriented *classic cores* and power-efficient
+*dense cores*. As such, power management strategies must be designed to
+accommodate the complexities introduced by incorporating different core types.
+Heterogeneous systems can also extend to more than two architectural classes as
+well. The purpose of the scheduling feedback mechanism is to provide
+information to the operating system scheduler in real time such that the
+scheduler can direct threads to the optimal core.
 
-``Classic cores`` are generally more performant and ``Dense cores`` are generally more
-efficient.
 The goal of AMD's heterogeneous architecture is to attain power benefit by sending
 background thread to the dense cores while sending high priority threads to the classic
 cores. From a performance perspective, sending background threads to dense cores can free
@@ -78,7 +77,8 @@ Power Management FW is responsible for detecting events that would require
 a reordering of the performance and efficiency ranking. Table updates would
 happen relatively infrequently and occur on the time scale of seconds or more.
 
-The mechanism used to trigger a table update like below events:
+The following events trigger a table update:
+
     * Thermal Stress Events
     * Silent Compute
     * Extreme Low Battery Scenarios

> diff --git a/Documentation/arch/x86/index.rst b/Documentation/arch/x86/index.rst
> index 8ac64d7de4dc..7f47229f3104 100644
> --- a/Documentation/arch/x86/index.rst
> +++ b/Documentation/arch/x86/index.rst
> @@ -43,3 +43,4 @@ x86-specific Documentation
>     features
>     elf_auxvec
>     xstate
> +   amd_hfi

Sphinx reports mismatched toctree entry name:

Documentation/arch/x86/index.rst:7: WARNING: toctree contains reference to nonexisting document 'arch/x86/amd_hfi'

I have to fix it up:

---- >8 ----
diff --git a/Documentation/arch/x86/index.rst b/Documentation/arch/x86/index.rst
index 7f47229f3104e1..56f2923f52597c 100644
--- a/Documentation/arch/x86/index.rst
+++ b/Documentation/arch/x86/index.rst
@@ -43,4 +43,4 @@ x86-specific Documentation
    features
    elf_auxvec
    xstate
-   amd_hfi
+   amd-hfi

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 02/13] MAINTAINERS: Add maintainer entry for AMD Hardware Feedback Driver

[Mario Limonciello]

Introduce the `amd_hfi` driver into the MAINTAINERS file.
The driver will support AMD Heterogeneous Core design which provides
hardware feedback to the OS scheduler.

Moving forward, Mario will be responsible for the maintenance
and Perry will assist on review of patches related to this driver.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2:
 * Make Perry Reviewer instead of Maintainer, he asked me to take
   over this series.
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a097afd76ded..90094e064d94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1083,6 +1083,15 @@ F:	arch/x86/include/asm/amd_hsmp.h
 F:	arch/x86/include/uapi/asm/amd_hsmp.h
 F:	drivers/platform/x86/amd/hsmp.c
 
+AMD HETERO CORE HARDWARE FEEDBACK DRIVER
+M:	Mario Limonciello <mario.limonciello@amd.com>
+R:	Perry Yuan <perry.yuan@amd.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Supported
+B:	https://gitlab.freedesktop.org/drm/amd/-/issues
+F:	Documentation/arch/x86/amd-hfi.rst
+F:	drivers/platform/x86/amd/hfi/
+
 AMD IOMMU (AMD-VI)
 M:	Joerg Roedel <joro@8bytes.org>
 R:	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
-- 
2.43.0

[PATCH v2 03/13] x86/cpufeatures: add X86_FEATURE_WORKLOAD_CLASS feature bit

[Mario Limonciello]

From: Perry Yuan <perry.yuan@amd.com>

Add new feature bit that indicates support for workload based
heuristic feedback to OS for scheduling decisions.
When the bit set, threads are classified during runtime into
enumerated classes. The classes represent thread performance/power
characteristics that may benefit from special scheduling behaviors.

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/scattered.c    | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index cea1ed82aeb4..3e8e67b8ec7a 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -474,6 +474,7 @@
 #define X86_FEATURE_CLEAR_BHB_LOOP_ON_VMEXIT (21*32+ 4) /* Clear branch history at vmexit using SW loop */
 #define X86_FEATURE_FAST_CPPC		(21*32 + 5) /* AMD Fast CPPC */
 #define X86_FEATURE_HETERO_CORE_TOPOLOGY	(21*32 + 6) /* Heterogeneous Core Topology */
+#define X86_FEATURE_WORKLOAD_CLASS	(21*32 + 7) /* Workload Classification */
 
 /*
  * BUG word(s)
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 3bba55323163..b6547bab9e23 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -49,6 +49,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
 	{ X86_FEATURE_SMBA,		CPUID_EBX,  2, 0x80000020, 0 },
 	{ X86_FEATURE_BMEC,		CPUID_EBX,  3, 0x80000020, 0 },
+	{ X86_FEATURE_WORKLOAD_CLASS,   CPUID_EAX,  22, 0x80000021, 0 },
 	{ X86_FEATURE_PERFMON_V2,	CPUID_EAX,  0, 0x80000022, 0 },
 	{ X86_FEATURE_AMD_LBR_V2,	CPUID_EAX,  1, 0x80000022, 0 },
 	{ X86_FEATURE_AMD_LBR_PMC_FREEZE,	CPUID_EAX,  2, 0x80000022, 0 },
-- 
2.43.0

[PATCH v2 04/13] x86/msr-index: define AMD heterogeneous CPU related MSR

[Mario Limonciello]

From: Perry Yuan <perry.yuan@amd.com>

Introduces new MSR registers for AMD hardware feedback support.
These registers enable the system to provide workload classification
and configuration capabilities.

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/x86/include/asm/msr-index.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 3ae84c3b8e6d..0cd5ffe50f4a 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -712,6 +712,11 @@
 #define MSR_AMD64_PERF_CNTR_GLOBAL_CTL		0xc0000301
 #define MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR	0xc0000302
 
+/* AMD Hardware Feedback Support MSRs */
+#define AMD_WORKLOAD_CLASS_CONFIG      0xc0000500
+#define AMD_WORKLOAD_CLASS_ID          0xc0000501
+#define AMD_WORKLOAD_HRST              0xc0000502
+
 /* AMD Last Branch Record MSRs */
 #define MSR_AMD64_LBR_SELECT			0xc000010e
 
-- 
2.43.0

[PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Mario Limonciello]

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.0

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Ilpo Järvinen]

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.

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Mario Limonciello]

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");
>>
> 

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Ilpo Järvinen]

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");
> 

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Ricardo Neri]

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

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Mario Limonciello]

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.

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Ricardo Neri]

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. ;)

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Mario Limonciello]

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.

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

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".

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Hans de Goede]

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

Re: [PATCH v2 05/13] platform/x86: hfi: Introduce AMD Hardware Feedback Interface Driver

[Mario Limonciello]

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.

[PATCH v2 06/13] platform/x86: hfi: parse CPU core ranking data from shared memory

[Mario Limonciello]

From: Perry Yuan <Perry.Yuan@amd.com>

When `amd_hfi` driver is loaded, it will use PCCT subspace type 4 table
to retrieve the shared memory address which contains the CPU core ranking
table. This table includes a header that specifies the number of ranking
data entries to be parsed and rank each CPU core with the Performance and
Energy Efficiency capability as implemented by the CPU power management
firmware.

Once the table has been parsed, each CPU is assigned a ranking score
within its class. Subsequently, when the scheduler selects cores, it
chooses from the ranking list based on the assigned scores in each class,
thereby ensuring the optimal selection of CPU cores according to their
predefined classifications and priorities.

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:
 * Rework amd_hfi_fill_metatadata to directly use structure instead of
   pointer math.
---
 drivers/platform/x86/amd/hfi/hfi.c | 215 ++++++++++++++++++++++++++++-
 1 file changed, 212 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
index da2e667107e8..10651399cf75 100644
--- a/drivers/platform/x86/amd/hfi/hfi.c
+++ b/drivers/platform/x86/amd/hfi/hfi.c
@@ -18,22 +18,78 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mailbox_client.h>
 #include <linux/mutex.h>
+#include <linux/percpu-defs.h>
 #include <linux/platform_device.h>
 #include <linux/printk.h>
 #include <linux/smp.h>
 #include <linux/string.h>
+#include <linux/topology.h>
+#include <linux/workqueue.h>
+
+#include <asm/cpu_device_id.h>
+
+#include <acpi/pcc.h>
+#include <acpi/cppc_acpi.h>
 
 #define AMD_HFI_DRIVER		"amd_hfi"
+#define AMD_HFI_MAILBOX_COUNT	1
+#define AMD_HETERO_RANKING_TABLE_VER	2
+
 #define AMD_HETERO_CPUID_27	0x80000027
+
 static struct platform_device *device;
 
+/**
+ * struct amd_shmem_info - Shared memory table for AMD HFI
+ *
+ * @signature:	The PCC signature. The signature of a subspace is computed by
+ *		a bitwise of the value 0x50434300 with the subspace ID.
+ * @flags:	Notify on completion
+ * @length:	Length of payload being transmitted including command field
+ * @command:	Command being sent over the subspace
+ * @version_number:		Version number of the table
+ * @n_logical_processors:	Number of logical processors
+ * @n_capabilities:		Number of ranking dimensions (performance, efficiency, etc)
+ * @table_update_context:	Command being sent over the subspace
+ * @n_bitmaps:			Number of 32-bit bitmaps to enumerate all the APIC IDs
+ *				This is based on the maximum APIC ID enumerated in the system
+ * @reserved:			24 bit spare
+ * @table_data:			Bit Map(s) of enabled logical processors
+ *				Followed by the ranking data for each logical processor
+ */
+struct amd_shmem_info {
+	struct acpi_pcct_ext_pcc_shared_memory header;
+	u32	version_number		:8,
+		n_logical_processors	:8,
+		n_capabilities		:8,
+		table_update_context	:8;
+	u32	n_bitmaps		:8,
+		reserved		:24;
+	u32	table_data[];
+} __packed;
+
 struct amd_hfi_data {
 	const char	*name;
 	struct device	*dev;
 	struct mutex	lock;
+
+	/* PCCT table related*/
+	struct pcc_mbox_chan	*pcc_chan;
+	void __iomem		*pcc_comm_addr;
+	struct acpi_subtable_header	*pcct_entry;
+	struct amd_shmem_info	*shmem;
 };
 
+/**
+ * struct amd_hfi_classes - HFI class capabilities per CPU
+ * @perf:	Performance capability
+ * @eff:	Power efficiency capability
+ *
+ * Capabilities of a logical processor in the ranking table. These capabilities
+ * are unitless and specific to each HFI class.
+ */
 struct amd_hfi_classes {
 	u32	perf;
 	u32	eff;
@@ -42,23 +98,105 @@ struct amd_hfi_classes {
 /**
  * struct amd_hfi_cpuinfo - HFI workload class info per CPU
  * @cpu:		cpu index
+ * @apic_id:		apic id of the current cpu
  * @cpus:		mask of cpus associated with amd_hfi_cpuinfo
  * @class_index:	workload class ID index
  * @nr_class:		max number of workload class supported
+ * @ipcc_scores:	ipcc scores for each class
  * @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;
+	u32		apic_id;
 	cpumask_var_t	cpus;
 	s16		class_index;
 	u8		nr_class;
+	int		*ipcc_scores;
 	struct amd_hfi_classes	*amd_hfi_classes;
 };
 
 static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
 
+static int find_cpu_index_by_apicid(unsigned int target_apicid)
+{
+	int cpu_index;
+
+	for_each_possible_cpu(cpu_index) {
+		struct cpuinfo_x86 *info = &cpu_data(cpu_index);
+
+		if (info->topo.apicid == target_apicid) {
+			pr_debug("match APIC id %d for CPU index: %d",
+				 info->topo.apicid, cpu_index);
+			return cpu_index;
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int amd_hfi_fill_metadata(struct amd_hfi_data *amd_hfi_data)
+{
+	struct acpi_pcct_ext_pcc_slave *pcct_ext =
+		(struct acpi_pcct_ext_pcc_slave *)amd_hfi_data->pcct_entry;
+	void __iomem *pcc_comm_addr;
+
+	pcc_comm_addr = acpi_os_ioremap(amd_hfi_data->pcc_chan->shmem_base_addr,
+					amd_hfi_data->pcc_chan->shmem_size);
+	if (!pcc_comm_addr) {
+		pr_err("failed to ioremap PCC common region mem\n");
+		return -ENOMEM;
+	}
+
+	memcpy_fromio(amd_hfi_data->shmem, pcc_comm_addr, pcct_ext->length);
+	iounmap(pcc_comm_addr);
+
+	if (amd_hfi_data->shmem->header.signature != PCC_SIGNATURE) {
+		pr_err("Invalid signature in shared memory\n");
+		return -EINVAL;
+	}
+	if (amd_hfi_data->shmem->version_number != AMD_HETERO_RANKING_TABLE_VER) {
+		pr_err("Invalid veresion %d\n", amd_hfi_data->shmem->version_number);
+		return -EINVAL;
+	}
+
+	for (u32 i = 0; i < amd_hfi_data->shmem->n_bitmaps; i++) {
+		u32 bitmap = amd_hfi_data->shmem->table_data[i];
+
+		for (u32 j = 0; j < BITS_PER_TYPE(u32); j++) {
+			struct amd_hfi_cpuinfo *info;
+			int apic_id = i * BITS_PER_TYPE(u32) + j;
+			int cpu_index;
+
+			if (!(bitmap & BIT(j)))
+				continue;
+
+			cpu_index = find_cpu_index_by_apicid(apic_id);
+			if (cpu_index < 0) {
+				pr_warn("APIC ID %d not found\n", apic_id);
+				continue;
+			}
+
+			info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index);
+			info->apic_id = apic_id;
+
+			/* Fill the ranking data for each logical processor */
+			info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index);
+			for (int k = 0; k < info->nr_class; k++) {
+				u32 *table = amd_hfi_data->shmem->table_data +
+					     amd_hfi_data->shmem->n_bitmaps +
+					     i * info->nr_class;
+
+				info->amd_hfi_classes[k].eff = table[apic_id + 2 * k];
+				info->amd_hfi_classes[k].perf = table[apic_id + 2 * k + 1];
+			}
+		}
+	}
+
+	return 0;
+}
+
 static int amd_hfi_alloc_class_data(struct platform_device *pdev)
 {
 	struct amd_hfi_cpuinfo *hfi_cpuinfo;
@@ -68,8 +206,7 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev)
 
 	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);
+		dev_warn(dev, "failed to get number of supported classes\n");
 		return -EINVAL;
 	}
 
@@ -79,7 +216,10 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev)
 				sizeof(struct amd_hfi_classes), GFP_KERNEL);
 		if (!hfi_cpuinfo->amd_hfi_classes)
 			return -ENOMEM;
-
+		hfi_cpuinfo->ipcc_scores = devm_kcalloc(dev, nr_class_id,
+							sizeof(int), GFP_KERNEL);
+		if (!hfi_cpuinfo->ipcc_scores)
+			return -ENOMEM;
 		hfi_cpuinfo->nr_class = nr_class_id;
 	}
 
@@ -93,6 +233,70 @@ static void amd_hfi_remove(struct platform_device *pdev)
 	mutex_destroy(&dev->lock);
 }
 
+static int amd_hfi_metadata_parser(struct platform_device *pdev,
+				   struct amd_hfi_data *amd_hfi_data)
+{
+	struct acpi_pcct_ext_pcc_slave *pcct_ext;
+	struct acpi_subtable_header *pcct_entry;
+	struct mbox_chan *pcc_mbox_channels;
+	struct acpi_table_header *pcct_tbl;
+	struct pcc_mbox_chan *pcc_chan;
+	acpi_status status;
+	int ret;
+
+	pcc_mbox_channels = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT,
+					 sizeof(*pcc_mbox_channels), GFP_KERNEL);
+	if (!pcc_mbox_channels) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	pcc_chan = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT,
+				sizeof(*pcc_chan), GFP_KERNEL);
+	if (!pcc_chan) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	status = acpi_get_table(ACPI_SIG_PCCT, 0, &pcct_tbl);
+	if (ACPI_FAILURE(status) || !pcct_tbl) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	/* get pointer to the first PCC subspace entry */
+	pcct_entry = (struct acpi_subtable_header *) (
+			(unsigned long)pcct_tbl + sizeof(struct acpi_table_pcct));
+
+	pcc_chan->mchan = &pcc_mbox_channels[0];
+
+	amd_hfi_data->pcc_chan = pcc_chan;
+	amd_hfi_data->pcct_entry = pcct_entry;
+	pcct_ext = (struct acpi_pcct_ext_pcc_slave *)pcct_entry;
+
+	if (pcct_ext->length <= 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	amd_hfi_data->shmem = devm_kmalloc(amd_hfi_data->dev, pcct_ext->length, GFP_KERNEL);
+	if (!amd_hfi_data->shmem) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	pcc_chan->shmem_base_addr = pcct_ext->base_address;
+	pcc_chan->shmem_size = pcct_ext->length;
+
+	/* parse the shared memory info from the pcct table */
+	ret = amd_hfi_fill_metadata(amd_hfi_data);
+
+	acpi_put_table(pcct_tbl);
+
+out:
+	return ret;
+}
+
 static const struct acpi_device_id amd_hfi_platform_match[] = {
 	{ "AMDI0104", 0},
 	{ }
@@ -121,6 +325,11 @@ static int amd_hfi_probe(struct platform_device *pdev)
 	if (ret)
 		goto out;
 
+	/* parse PCCT table */
+	ret = amd_hfi_metadata_parser(pdev, amd_hfi_data);
+	if (ret)
+		goto out;
+
 out:
 	return ret;
 }
-- 
2.43.0

Re: [PATCH v2 06/13] platform/x86: hfi: parse CPU core ranking data from shared memory

[Ilpo Järvinen]

On Thu, 10 Oct 2024, Mario Limonciello wrote:

> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> When `amd_hfi` driver is loaded, it will use PCCT subspace type 4 table
> to retrieve the shared memory address which contains the CPU core ranking
> table. This table includes a header that specifies the number of ranking
> data entries to be parsed and rank each CPU core with the Performance and
> Energy Efficiency capability as implemented by the CPU power management
> firmware.
> 
> Once the table has been parsed, each CPU is assigned a ranking score
> within its class. Subsequently, when the scheduler selects cores, it
> chooses from the ranking list based on the assigned scores in each class,
> thereby ensuring the optimal selection of CPU cores according to their
> predefined classifications and priorities.
> 
> 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:
>  * Rework amd_hfi_fill_metatadata to directly use structure instead of
>    pointer math.
> ---
>  drivers/platform/x86/amd/hfi/hfi.c | 215 ++++++++++++++++++++++++++++-
>  1 file changed, 212 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
> index da2e667107e8..10651399cf75 100644
> --- a/drivers/platform/x86/amd/hfi/hfi.c
> +++ b/drivers/platform/x86/amd/hfi/hfi.c
> @@ -18,22 +18,78 @@
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/mailbox_client.h>
>  #include <linux/mutex.h>
> +#include <linux/percpu-defs.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>  #include <linux/smp.h>
>  #include <linux/string.h>
> +#include <linux/topology.h>
> +#include <linux/workqueue.h>
> +
> +#include <asm/cpu_device_id.h>
> +
> +#include <acpi/pcc.h>
> +#include <acpi/cppc_acpi.h>
>  
>  #define AMD_HFI_DRIVER		"amd_hfi"
> +#define AMD_HFI_MAILBOX_COUNT	1
> +#define AMD_HETERO_RANKING_TABLE_VER	2
> +
>  #define AMD_HETERO_CPUID_27	0x80000027
> +
>  static struct platform_device *device;
>  
> +/**
> + * struct amd_shmem_info - Shared memory table for AMD HFI
> + *
> + * @signature:	The PCC signature. The signature of a subspace is computed by
> + *		a bitwise of the value 0x50434300 with the subspace ID.
> + * @flags:	Notify on completion
> + * @length:	Length of payload being transmitted including command field
> + * @command:	Command being sent over the subspace
> + * @version_number:		Version number of the table
> + * @n_logical_processors:	Number of logical processors
> + * @n_capabilities:		Number of ranking dimensions (performance, efficiency, etc)
> + * @table_update_context:	Command being sent over the subspace
> + * @n_bitmaps:			Number of 32-bit bitmaps to enumerate all the APIC IDs
> + *				This is based on the maximum APIC ID enumerated in the system
> + * @reserved:			24 bit spare
> + * @table_data:			Bit Map(s) of enabled logical processors
> + *				Followed by the ranking data for each logical processor
> + */
> +struct amd_shmem_info {
> +	struct acpi_pcct_ext_pcc_shared_memory header;
> +	u32	version_number		:8,
> +		n_logical_processors	:8,
> +		n_capabilities		:8,
> +		table_update_context	:8;
> +	u32	n_bitmaps		:8,
> +		reserved		:24;
> +	u32	table_data[];
> +} __packed;
> +
>  struct amd_hfi_data {
>  	const char	*name;
>  	struct device	*dev;
>  	struct mutex	lock;
> +
> +	/* PCCT table related*/
> +	struct pcc_mbox_chan	*pcc_chan;
> +	void __iomem		*pcc_comm_addr;
> +	struct acpi_subtable_header	*pcct_entry;
> +	struct amd_shmem_info	*shmem;
>  };
>  
> +/**
> + * struct amd_hfi_classes - HFI class capabilities per CPU
> + * @perf:	Performance capability
> + * @eff:	Power efficiency capability
> + *
> + * Capabilities of a logical processor in the ranking table. These capabilities
> + * are unitless and specific to each HFI class.
> + */
>  struct amd_hfi_classes {
>  	u32	perf;
>  	u32	eff;
> @@ -42,23 +98,105 @@ struct amd_hfi_classes {
>  /**
>   * struct amd_hfi_cpuinfo - HFI workload class info per CPU
>   * @cpu:		cpu index
> + * @apic_id:		apic id of the current cpu
>   * @cpus:		mask of cpus associated with amd_hfi_cpuinfo
>   * @class_index:	workload class ID index
>   * @nr_class:		max number of workload class supported
> + * @ipcc_scores:	ipcc scores for each class
>   * @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;
> +	u32		apic_id;
>  	cpumask_var_t	cpus;
>  	s16		class_index;
>  	u8		nr_class;
> +	int		*ipcc_scores;
>  	struct amd_hfi_classes	*amd_hfi_classes;
>  };
>  
>  static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
>  
> +static int find_cpu_index_by_apicid(unsigned int target_apicid)
> +{
> +	int cpu_index;
> +
> +	for_each_possible_cpu(cpu_index) {
> +		struct cpuinfo_x86 *info = &cpu_data(cpu_index);
> +
> +		if (info->topo.apicid == target_apicid) {
> +			pr_debug("match APIC id %d for CPU index: %d",

Missing \n

> +				 info->topo.apicid, cpu_index);
> +			return cpu_index;
> +		}
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +static int amd_hfi_fill_metadata(struct amd_hfi_data *amd_hfi_data)
> +{
> +	struct acpi_pcct_ext_pcc_slave *pcct_ext =
> +		(struct acpi_pcct_ext_pcc_slave *)amd_hfi_data->pcct_entry;
> +	void __iomem *pcc_comm_addr;
> +
> +	pcc_comm_addr = acpi_os_ioremap(amd_hfi_data->pcc_chan->shmem_base_addr,
> +					amd_hfi_data->pcc_chan->shmem_size);
> +	if (!pcc_comm_addr) {
> +		pr_err("failed to ioremap PCC common region mem\n");
> +		return -ENOMEM;
> +	}
> +
> +	memcpy_fromio(amd_hfi_data->shmem, pcc_comm_addr, pcct_ext->length);
> +	iounmap(pcc_comm_addr);
> +
> +	if (amd_hfi_data->shmem->header.signature != PCC_SIGNATURE) {
> +		pr_err("Invalid signature in shared memory\n");
> +		return -EINVAL;
> +	}
> +	if (amd_hfi_data->shmem->version_number != AMD_HETERO_RANKING_TABLE_VER) {
> +		pr_err("Invalid veresion %d\n", amd_hfi_data->shmem->version_number);

version

> +		return -EINVAL;
> +	}
> +
> +	for (u32 i = 0; i < amd_hfi_data->shmem->n_bitmaps; i++) {
> +		u32 bitmap = amd_hfi_data->shmem->table_data[i];
> +
> +		for (u32 j = 0; j < BITS_PER_TYPE(u32); j++) {

Are these u32 really the types you want to use for the loop vars, why?

> +			struct amd_hfi_cpuinfo *info;
> +			int apic_id = i * BITS_PER_TYPE(u32) + j;
> +			int cpu_index;
> +
> +			if (!(bitmap & BIT(j)))
> +				continue;
> +
> +			cpu_index = find_cpu_index_by_apicid(apic_id);
> +			if (cpu_index < 0) {
> +				pr_warn("APIC ID %d not found\n", apic_id);
> +				continue;
> +			}
> +
> +			info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index);
> +			info->apic_id = apic_id;
> +
> +			/* Fill the ranking data for each logical processor */
> +			info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index);
> +			for (int k = 0; k < info->nr_class; k++) {

unsigned int

> +				u32 *table = amd_hfi_data->shmem->table_data +
> +					     amd_hfi_data->shmem->n_bitmaps +
> +					     i * info->nr_class;
> +
> +				info->amd_hfi_classes[k].eff = table[apic_id + 2 * k];
> +				info->amd_hfi_classes[k].perf = table[apic_id + 2 * k + 1];
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>  {
>  	struct amd_hfi_cpuinfo *hfi_cpuinfo;
> @@ -68,8 +206,7 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>  
>  	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);
> +		dev_warn(dev, "failed to get number of supported classes\n");

This message was added in the previous patch and now immediately changed.

>  		return -EINVAL;
>  	}
>  
> @@ -79,7 +216,10 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>  				sizeof(struct amd_hfi_classes), GFP_KERNEL);
>  		if (!hfi_cpuinfo->amd_hfi_classes)
>  			return -ENOMEM;
> -
> +		hfi_cpuinfo->ipcc_scores = devm_kcalloc(dev, nr_class_id,
> +							sizeof(int), GFP_KERNEL);
> +		if (!hfi_cpuinfo->ipcc_scores)
> +			return -ENOMEM;
>  		hfi_cpuinfo->nr_class = nr_class_id;
>  	}
>  
> @@ -93,6 +233,70 @@ static void amd_hfi_remove(struct platform_device *pdev)
>  	mutex_destroy(&dev->lock);
>  }
>  
> +static int amd_hfi_metadata_parser(struct platform_device *pdev,
> +				   struct amd_hfi_data *amd_hfi_data)
> +{
> +	struct acpi_pcct_ext_pcc_slave *pcct_ext;
> +	struct acpi_subtable_header *pcct_entry;
> +	struct mbox_chan *pcc_mbox_channels;
> +	struct acpi_table_header *pcct_tbl;
> +	struct pcc_mbox_chan *pcc_chan;
> +	acpi_status status;
> +	int ret;
> +
> +	pcc_mbox_channels = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT,
> +					 sizeof(*pcc_mbox_channels), GFP_KERNEL);
> +	if (!pcc_mbox_channels) {
> +		ret = -ENOMEM;
> +		goto out;

Please return directly if there is nothing to rollback.

> +	}
> +
> +	pcc_chan = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT,
> +				sizeof(*pcc_chan), GFP_KERNEL);
> +	if (!pcc_chan) {
> +		ret = -ENOMEM;
> +		goto out;

Ditto.

> +	}
> +
> +	status = acpi_get_table(ACPI_SIG_PCCT, 0, &pcct_tbl);
> +	if (ACPI_FAILURE(status) || !pcct_tbl) {
> +		ret = -ENODEV;
> +		goto out;

Ditto.

> +	}
> +
> +	/* get pointer to the first PCC subspace entry */
> +	pcct_entry = (struct acpi_subtable_header *) (
> +			(unsigned long)pcct_tbl + sizeof(struct acpi_table_pcct));
> +
> +	pcc_chan->mchan = &pcc_mbox_channels[0];
> +
> +	amd_hfi_data->pcc_chan = pcc_chan;
> +	amd_hfi_data->pcct_entry = pcct_entry;
> +	pcct_ext = (struct acpi_pcct_ext_pcc_slave *)pcct_entry;
> +
> +	if (pcct_ext->length <= 0) {
> +		ret = -EINVAL;
> +		goto out;

Ditto.

> +	}
> +
> +	amd_hfi_data->shmem = devm_kmalloc(amd_hfi_data->dev, pcct_ext->length, GFP_KERNEL);

Why kmalloc ?

> +	if (!amd_hfi_data->shmem) {
> +		ret = -ENOMEM;
> +		goto out;

Return directly.

> +	}
> +
> +	pcc_chan->shmem_base_addr = pcct_ext->base_address;
> +	pcc_chan->shmem_size = pcct_ext->length;
> +
> +	/* parse the shared memory info from the pcct table */
> +	ret = amd_hfi_fill_metadata(amd_hfi_data);
> +
> +	acpi_put_table(pcct_tbl);
> +
> +out:
> +	return ret;
> +}
> +
>  static const struct acpi_device_id amd_hfi_platform_match[] = {
>  	{ "AMDI0104", 0},
>  	{ }
> @@ -121,6 +325,11 @@ static int amd_hfi_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto out;

This should do return ret; directly, not jump to out label which does 
nothing but return.

>  
> +	/* parse PCCT table */
> +	ret = amd_hfi_metadata_parser(pdev, amd_hfi_data);
> +	if (ret)
> +		goto out;
> +
>  out:
>  	return ret;

Might again be there for churn avoidance, otherwise, please consider:

	return amd_hfi_metadata_parser(pdev, amd_hfi_data);

That goto out should again just return ret directly.

>  }
> 

-- 
 i.

Re: [PATCH v2 06/13] platform/x86: hfi: parse CPU core ranking data from shared memory

[Mario Limonciello]

On 10/14/2024 05:14, Ilpo Järvinen wrote:
> On Thu, 10 Oct 2024, Mario Limonciello wrote:
> 
>> From: Perry Yuan <Perry.Yuan@amd.com>
>>
>> When `amd_hfi` driver is loaded, it will use PCCT subspace type 4 table
>> to retrieve the shared memory address which contains the CPU core ranking
>> table. This table includes a header that specifies the number of ranking
>> data entries to be parsed and rank each CPU core with the Performance and
>> Energy Efficiency capability as implemented by the CPU power management
>> firmware.
>>
>> Once the table has been parsed, each CPU is assigned a ranking score
>> within its class. Subsequently, when the scheduler selects cores, it
>> chooses from the ranking list based on the assigned scores in each class,
>> thereby ensuring the optimal selection of CPU cores according to their
>> predefined classifications and priorities.
>>
>> 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:
>>   * Rework amd_hfi_fill_metatadata to directly use structure instead of
>>     pointer math.
>> ---
>>   drivers/platform/x86/amd/hfi/hfi.c | 215 ++++++++++++++++++++++++++++-
>>   1 file changed, 212 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
>> index da2e667107e8..10651399cf75 100644
>> --- a/drivers/platform/x86/amd/hfi/hfi.c
>> +++ b/drivers/platform/x86/amd/hfi/hfi.c
>> @@ -18,22 +18,78 @@
>>   #include <linux/io.h>
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>> +#include <linux/mailbox_client.h>
>>   #include <linux/mutex.h>
>> +#include <linux/percpu-defs.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/printk.h>
>>   #include <linux/smp.h>
>>   #include <linux/string.h>
>> +#include <linux/topology.h>
>> +#include <linux/workqueue.h>
>> +
>> +#include <asm/cpu_device_id.h>
>> +
>> +#include <acpi/pcc.h>
>> +#include <acpi/cppc_acpi.h>
>>   
>>   #define AMD_HFI_DRIVER		"amd_hfi"
>> +#define AMD_HFI_MAILBOX_COUNT	1
>> +#define AMD_HETERO_RANKING_TABLE_VER	2
>> +
>>   #define AMD_HETERO_CPUID_27	0x80000027
>> +
>>   static struct platform_device *device;
>>   
>> +/**
>> + * struct amd_shmem_info - Shared memory table for AMD HFI
>> + *
>> + * @signature:	The PCC signature. The signature of a subspace is computed by
>> + *		a bitwise of the value 0x50434300 with the subspace ID.
>> + * @flags:	Notify on completion
>> + * @length:	Length of payload being transmitted including command field
>> + * @command:	Command being sent over the subspace
>> + * @version_number:		Version number of the table
>> + * @n_logical_processors:	Number of logical processors
>> + * @n_capabilities:		Number of ranking dimensions (performance, efficiency, etc)
>> + * @table_update_context:	Command being sent over the subspace
>> + * @n_bitmaps:			Number of 32-bit bitmaps to enumerate all the APIC IDs
>> + *				This is based on the maximum APIC ID enumerated in the system
>> + * @reserved:			24 bit spare
>> + * @table_data:			Bit Map(s) of enabled logical processors
>> + *				Followed by the ranking data for each logical processor
>> + */
>> +struct amd_shmem_info {
>> +	struct acpi_pcct_ext_pcc_shared_memory header;
>> +	u32	version_number		:8,
>> +		n_logical_processors	:8,
>> +		n_capabilities		:8,
>> +		table_update_context	:8;
>> +	u32	n_bitmaps		:8,
>> +		reserved		:24;
>> +	u32	table_data[];
>> +} __packed;
>> +
>>   struct amd_hfi_data {
>>   	const char	*name;
>>   	struct device	*dev;
>>   	struct mutex	lock;
>> +
>> +	/* PCCT table related*/
>> +	struct pcc_mbox_chan	*pcc_chan;
>> +	void __iomem		*pcc_comm_addr;
>> +	struct acpi_subtable_header	*pcct_entry;
>> +	struct amd_shmem_info	*shmem;
>>   };
>>   
>> +/**
>> + * struct amd_hfi_classes - HFI class capabilities per CPU
>> + * @perf:	Performance capability
>> + * @eff:	Power efficiency capability
>> + *
>> + * Capabilities of a logical processor in the ranking table. These capabilities
>> + * are unitless and specific to each HFI class.
>> + */
>>   struct amd_hfi_classes {
>>   	u32	perf;
>>   	u32	eff;
>> @@ -42,23 +98,105 @@ struct amd_hfi_classes {
>>   /**
>>    * struct amd_hfi_cpuinfo - HFI workload class info per CPU
>>    * @cpu:		cpu index
>> + * @apic_id:		apic id of the current cpu
>>    * @cpus:		mask of cpus associated with amd_hfi_cpuinfo
>>    * @class_index:	workload class ID index
>>    * @nr_class:		max number of workload class supported
>> + * @ipcc_scores:	ipcc scores for each class
>>    * @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;
>> +	u32		apic_id;
>>   	cpumask_var_t	cpus;
>>   	s16		class_index;
>>   	u8		nr_class;
>> +	int		*ipcc_scores;
>>   	struct amd_hfi_classes	*amd_hfi_classes;
>>   };
>>   
>>   static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
>>   
>> +static int find_cpu_index_by_apicid(unsigned int target_apicid)
>> +{
>> +	int cpu_index;
>> +
>> +	for_each_possible_cpu(cpu_index) {
>> +		struct cpuinfo_x86 *info = &cpu_data(cpu_index);
>> +
>> +		if (info->topo.apicid == target_apicid) {
>> +			pr_debug("match APIC id %d for CPU index: %d",
> 
> Missing \n
Ack
> 
>> +				 info->topo.apicid, cpu_index);
>> +			return cpu_index;
>> +		}
>> +	}
>> +
>> +	return -ENODEV;
>> +}
>> +
>> +static int amd_hfi_fill_metadata(struct amd_hfi_data *amd_hfi_data)
>> +{
>> +	struct acpi_pcct_ext_pcc_slave *pcct_ext =
>> +		(struct acpi_pcct_ext_pcc_slave *)amd_hfi_data->pcct_entry;
>> +	void __iomem *pcc_comm_addr;
>> +
>> +	pcc_comm_addr = acpi_os_ioremap(amd_hfi_data->pcc_chan->shmem_base_addr,
>> +					amd_hfi_data->pcc_chan->shmem_size);
>> +	if (!pcc_comm_addr) {
>> +		pr_err("failed to ioremap PCC common region mem\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	memcpy_fromio(amd_hfi_data->shmem, pcc_comm_addr, pcct_ext->length);
>> +	iounmap(pcc_comm_addr);
>> +
>> +	if (amd_hfi_data->shmem->header.signature != PCC_SIGNATURE) {
>> +		pr_err("Invalid signature in shared memory\n");
>> +		return -EINVAL;
>> +	}
>> +	if (amd_hfi_data->shmem->version_number != AMD_HETERO_RANKING_TABLE_VER) {
>> +		pr_err("Invalid veresion %d\n", amd_hfi_data->shmem->version_number);
> 
> version
Ack
> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	for (u32 i = 0; i < amd_hfi_data->shmem->n_bitmaps; i++) {
>> +		u32 bitmap = amd_hfi_data->shmem->table_data[i];
>> +
>> +		for (u32 j = 0; j < BITS_PER_TYPE(u32); j++) {
> 
> Are these u32 really the types you want to use for the loop vars, why?

I was going off the type of amd_hfi_data->shmem->n_bitmaps which is u32.
In practice I think an unsigned int should be fine though too.

> 
>> +			struct amd_hfi_cpuinfo *info;
>> +			int apic_id = i * BITS_PER_TYPE(u32) + j;
>> +			int cpu_index;
>> +
>> +			if (!(bitmap & BIT(j)))
>> +				continue;
>> +
>> +			cpu_index = find_cpu_index_by_apicid(apic_id);
>> +			if (cpu_index < 0) {
>> +				pr_warn("APIC ID %d not found\n", apic_id);
>> +				continue;
>> +			}
>> +
>> +			info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index);
>> +			info->apic_id = apic_id;
>> +
>> +			/* Fill the ranking data for each logical processor */
>> +			info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index);
>> +			for (int k = 0; k < info->nr_class; k++) {
> 
> unsigned int
> 
>> +				u32 *table = amd_hfi_data->shmem->table_data +
>> +					     amd_hfi_data->shmem->n_bitmaps +
>> +					     i * info->nr_class;
>> +
>> +				info->amd_hfi_classes[k].eff = table[apic_id + 2 * k];
>> +				info->amd_hfi_classes[k].perf = table[apic_id + 2 * k + 1];
>> +			}
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>>   {
>>   	struct amd_hfi_cpuinfo *hfi_cpuinfo;
>> @@ -68,8 +206,7 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>>   
>>   	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);
>> +		dev_warn(dev, "failed to get number of supported classes\n");
> 
> This message was added in the previous patch and now immediately changed.

Will drop this change.

> 
>>   		return -EINVAL;
>>   	}
>>   
>> @@ -79,7 +216,10 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev)
>>   				sizeof(struct amd_hfi_classes), GFP_KERNEL);
>>   		if (!hfi_cpuinfo->amd_hfi_classes)
>>   			return -ENOMEM;
>> -
>> +		hfi_cpuinfo->ipcc_scores = devm_kcalloc(dev, nr_class_id,
>> +							sizeof(int), GFP_KERNEL);
>> +		if (!hfi_cpuinfo->ipcc_scores)
>> +			return -ENOMEM;
>>   		hfi_cpuinfo->nr_class = nr_class_id;
>>   	}
>>   
>> @@ -93,6 +233,70 @@ static void amd_hfi_remove(struct platform_device *pdev)
>>   	mutex_destroy(&dev->lock);
>>   }
>>   
>> +static int amd_hfi_metadata_parser(struct platform_device *pdev,
>> +				   struct amd_hfi_data *amd_hfi_data)
>> +{
>> +	struct acpi_pcct_ext_pcc_slave *pcct_ext;
>> +	struct acpi_subtable_header *pcct_entry;
>> +	struct mbox_chan *pcc_mbox_channels;
>> +	struct acpi_table_header *pcct_tbl;
>> +	struct pcc_mbox_chan *pcc_chan;
>> +	acpi_status status;
>> +	int ret;
>> +
>> +	pcc_mbox_channels = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT,
>> +					 sizeof(*pcc_mbox_channels), GFP_KERNEL);
>> +	if (!pcc_mbox_channels) {
>> +		ret = -ENOMEM;
>> +		goto out;
> 
> Please return directly if there is nothing to rollback.
Ack
> 
>> +	}
>> +
>> +	pcc_chan = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT,
>> +				sizeof(*pcc_chan), GFP_KERNEL);
>> +	if (!pcc_chan) {
>> +		ret = -ENOMEM;
>> +		goto out;
> 
> Ditto.
> 
>> +	}
>> +
>> +	status = acpi_get_table(ACPI_SIG_PCCT, 0, &pcct_tbl);
>> +	if (ACPI_FAILURE(status) || !pcct_tbl) {
>> +		ret = -ENODEV;
>> +		goto out;
> 
> Ditto.
> 
>> +	}
>> +
>> +	/* get pointer to the first PCC subspace entry */
>> +	pcct_entry = (struct acpi_subtable_header *) (
>> +			(unsigned long)pcct_tbl + sizeof(struct acpi_table_pcct));
>> +
>> +	pcc_chan->mchan = &pcc_mbox_channels[0];
>> +
>> +	amd_hfi_data->pcc_chan = pcc_chan;
>> +	amd_hfi_data->pcct_entry = pcct_entry;
>> +	pcct_ext = (struct acpi_pcct_ext_pcc_slave *)pcct_entry;
>> +
>> +	if (pcct_ext->length <= 0) {
>> +		ret = -EINVAL;
>> +		goto out;
> 
> Ditto.
> 
>> +	}
>> +
>> +	amd_hfi_data->shmem = devm_kmalloc(amd_hfi_data->dev, pcct_ext->length, GFP_KERNEL);
> 
> Why kmalloc ?
> 
>> +	if (!amd_hfi_data->shmem) {
>> +		ret = -ENOMEM;
>> +		goto out;
> 
> Return directly.
> 
>> +	}
>> +
>> +	pcc_chan->shmem_base_addr = pcct_ext->base_address;
>> +	pcc_chan->shmem_size = pcct_ext->length;
>> +
>> +	/* parse the shared memory info from the pcct table */
>> +	ret = amd_hfi_fill_metadata(amd_hfi_data);
>> +
>> +	acpi_put_table(pcct_tbl);
>> +
>> +out:
>> +	return ret;
>> +}
>> +
>>   static const struct acpi_device_id amd_hfi_platform_match[] = {
>>   	{ "AMDI0104", 0},
>>   	{ }
>> @@ -121,6 +325,11 @@ static int amd_hfi_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		goto out;
> 
> This should do return ret; directly, not jump to out label which does
> nothing but return.
> 
>>   
>> +	/* parse PCCT table */
>> +	ret = amd_hfi_metadata_parser(pdev, amd_hfi_data);
>> +	if (ret)
>> +		goto out;
>> +
>>   out:
>>   	return ret;
> 
> Might again be there for churn avoidance, otherwise, please consider:
> 
> 	return amd_hfi_metadata_parser(pdev, amd_hfi_data);
> 
> That goto out should again just return ret directly.
> 
>>   }
>>
> 

[PATCH v2 07/13] platform/x86: hfi: init per-cpu scores for each class

[Mario Limonciello]

From: Perry Yuan <Perry.Yuan@amd.com>

Initialize per cpu score `amd_hfi_ipcc_scores` which store energy score
and performance score data for each class.

`Classic core` and `Dense core` are ranked according to those values as
energy efficiency capability or performance capability.
OS scheduler will pick cores from the ranking list on each class ID for
the thread which provide the class id got from hardware feedback
interface.

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:
 * Rework
---
 drivers/platform/x86/amd/hfi/hfi.c | 32 ++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
index 10651399cf75..c0065ba0ed18 100644
--- a/drivers/platform/x86/amd/hfi/hfi.c
+++ b/drivers/platform/x86/amd/hfi/hfi.c
@@ -119,6 +119,8 @@ struct amd_hfi_cpuinfo {
 
 static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
 
+static DEFINE_MUTEX(hfi_cpuinfo_lock);
+
 static int find_cpu_index_by_apicid(unsigned int target_apicid)
 {
 	int cpu_index;
@@ -233,6 +235,31 @@ static void amd_hfi_remove(struct platform_device *pdev)
 	mutex_destroy(&dev->lock);
 }
 
+static int amd_set_hfi_ipcc_score(struct amd_hfi_cpuinfo *hfi_cpuinfo, int cpu)
+{
+	for (int i = 0; i < hfi_cpuinfo->nr_class; i++)
+		WRITE_ONCE(hfi_cpuinfo->ipcc_scores[i],
+			   hfi_cpuinfo->amd_hfi_classes[i].perf);
+
+	return 0;
+}
+
+static int update_hfi_ipcc_scores(struct amd_hfi_data *amd_hfi_data)
+{
+	int cpu;
+	int ret;
+
+	for_each_online_cpu(cpu) {
+		struct amd_hfi_cpuinfo *hfi_cpuinfo = per_cpu_ptr(&amd_hfi_cpuinfo, cpu);
+
+		ret = amd_set_hfi_ipcc_score(hfi_cpuinfo, cpu);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int amd_hfi_metadata_parser(struct platform_device *pdev,
 				   struct amd_hfi_data *amd_hfi_data)
 {
@@ -330,6 +357,11 @@ static int amd_hfi_probe(struct platform_device *pdev)
 	if (ret)
 		goto out;
 
+	/* set up IPCC scores */
+	ret = update_hfi_ipcc_scores(amd_hfi_data);
+	if (ret)
+		goto out;
+
 out:
 	return ret;
 }
-- 
2.43.0

Re: [PATCH v2 07/13] platform/x86: hfi: init per-cpu scores for each class

[Ilpo Järvinen]

On Thu, 10 Oct 2024, Mario Limonciello wrote:

> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> Initialize per cpu score `amd_hfi_ipcc_scores` which store energy score
> and performance score data for each class.
> 
> `Classic core` and `Dense core` are ranked according to those values as
> energy efficiency capability or performance capability.
> OS scheduler will pick cores from the ranking list on each class ID for
> the thread which provide the class id got from hardware feedback
> interface.
> 
> 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:
>  * Rework
> ---
>  drivers/platform/x86/amd/hfi/hfi.c | 32 ++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
> index 10651399cf75..c0065ba0ed18 100644
> --- a/drivers/platform/x86/amd/hfi/hfi.c
> +++ b/drivers/platform/x86/amd/hfi/hfi.c
> @@ -119,6 +119,8 @@ struct amd_hfi_cpuinfo {
>  
>  static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1};
>  
> +static DEFINE_MUTEX(hfi_cpuinfo_lock);
> +
>  static int find_cpu_index_by_apicid(unsigned int target_apicid)
>  {
>  	int cpu_index;
> @@ -233,6 +235,31 @@ static void amd_hfi_remove(struct platform_device *pdev)
>  	mutex_destroy(&dev->lock);
>  }
>  
> +static int amd_set_hfi_ipcc_score(struct amd_hfi_cpuinfo *hfi_cpuinfo, int cpu)
> +{
> +	for (int i = 0; i < hfi_cpuinfo->nr_class; i++)
> +		WRITE_ONCE(hfi_cpuinfo->ipcc_scores[i],
> +			   hfi_cpuinfo->amd_hfi_classes[i].perf);
> +
> +	return 0;
> +}
> +
> +static int update_hfi_ipcc_scores(struct amd_hfi_data *amd_hfi_data)
> +{
> +	int cpu;
> +	int ret;
> +
> +	for_each_online_cpu(cpu) {
> +		struct amd_hfi_cpuinfo *hfi_cpuinfo = per_cpu_ptr(&amd_hfi_cpuinfo, cpu);
> +
> +		ret = amd_set_hfi_ipcc_score(hfi_cpuinfo, cpu);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int amd_hfi_metadata_parser(struct platform_device *pdev,
>  				   struct amd_hfi_data *amd_hfi_data)
>  {
> @@ -330,6 +357,11 @@ static int amd_hfi_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto out;
>  
> +	/* set up IPCC scores */
> +	ret = update_hfi_ipcc_scores(amd_hfi_data);
> +	if (ret)
> +		goto out;

return ret;

or

directly:

return update_hfi_ipcc_scores(amd_hfi_data);

> +
>  out:
>  	return ret;
>  }
> 

-- 
 i.

[PATCH v2 08/13] platform/x86: hfi: add online and offline callback support

[Mario Limonciello]

From: Perry Yuan <Perry.Yuan@amd.com>

There are some firmware parameters that need to be configured
when a CPU core is brought online or offline.

when CPU is online, it will initialize the workload classification
parameters to CPU firmware which will trigger the workload class ID
updating function.

Once the CPU is going to offline, it will need to disable the workload
classification function and clear the history.

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:
 * Rebase
---
 drivers/platform/x86/amd/hfi/hfi.c | 90 +++++++++++++++++++++++++++++-
 1 file changed, 89 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
index c0065ba0ed18..c969ee7ea5ee 100644
--- a/drivers/platform/x86/amd/hfi/hfi.c
+++ b/drivers/platform/x86/amd/hfi/hfi.c
@@ -244,6 +244,89 @@ static int amd_set_hfi_ipcc_score(struct amd_hfi_cpuinfo *hfi_cpuinfo, int cpu)
 	return 0;
 }
 
+static int amd_hfi_set_state(unsigned int cpu, bool state)
+{
+	int ret;
+
+	ret = wrmsrl_on_cpu(cpu, AMD_WORKLOAD_CLASS_CONFIG, state);
+	if (ret)
+		return ret;
+
+	return wrmsrl_on_cpu(cpu, AMD_WORKLOAD_HRST, 0x1);
+}
+
+/**
+ * amd_hfi_online() - Enable workload classification on @cpu
+ * @cpu: CPU in which the workload classification will be enabled
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int amd_hfi_online(unsigned int cpu)
+{
+	struct amd_hfi_cpuinfo *hfi_info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu);
+	struct amd_hfi_classes *hfi_classes;
+	int ret;
+
+	if (WARN_ON_ONCE(!hfi_info))
+		return -EINVAL;
+
+	if (!zalloc_cpumask_var(&hfi_info->cpus, GFP_KERNEL))
+		return -ENOMEM;
+
+	mutex_lock(&hfi_cpuinfo_lock);
+	cpumask_set_cpu(cpu, hfi_info->cpus);
+
+	/*
+	 * Check if @cpu as an associated, initialized and ranking data must be filled
+	 */
+	hfi_classes = hfi_info->amd_hfi_classes;
+	if (!hfi_classes)
+		goto unlock;
+
+	/* Enable the workload classification interface */
+	ret = amd_hfi_set_state(cpu, true);
+	if (ret)
+		pr_err("wct enable failed for cpu %d\n", cpu);
+
+	mutex_unlock(&hfi_cpuinfo_lock);
+	return 0;
+
+unlock:
+	free_cpumask_var(hfi_info->cpus);
+	mutex_unlock(&hfi_cpuinfo_lock);
+	return ret;
+}
+
+/**
+ * amd_hfi_offline() - Disable workload classification on @cpu
+ * @cpu: CPU in which the workload classification will be disabled
+ *
+ * Remove @cpu from those covered by its HFI instance.
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int amd_hfi_offline(unsigned int cpu)
+{
+	struct amd_hfi_cpuinfo *hfi_info = &per_cpu(amd_hfi_cpuinfo, cpu);
+	int ret;
+
+	if (WARN_ON_ONCE(!hfi_info))
+		return -EINVAL;
+
+	mutex_lock(&hfi_cpuinfo_lock);
+
+	/* Disable the workload classification interface */
+	ret = amd_hfi_set_state(cpu, false);
+	if (ret)
+		pr_err("wct disable failed for cpu %d\n", cpu);
+
+	mutex_unlock(&hfi_cpuinfo_lock);
+
+	free_cpumask_var(hfi_info->cpus);
+
+	return 0;
+}
+
 static int update_hfi_ipcc_scores(struct amd_hfi_data *amd_hfi_data)
 {
 	int cpu;
@@ -362,8 +445,13 @@ static int amd_hfi_probe(struct platform_device *pdev)
 	if (ret)
 		goto out;
 
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/amd_hfi:online",
+				amd_hfi_online, amd_hfi_offline);
+	if (ret < 0)
+		goto out;
+
 out:
-	return ret;
+	return ret < 0 ? ret : 0;
 }
 
 static struct platform_driver amd_hfi_driver = {
-- 
2.43.0

Re: [PATCH v2 08/13] platform/x86: hfi: add online and offline callback support

[Ilpo Järvinen]

On Thu, 10 Oct 2024, Mario Limonciello wrote:

> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> There are some firmware parameters that need to be configured
> when a CPU core is brought online or offline.
> 
> when CPU is online, it will initialize the workload classification
> parameters to CPU firmware which will trigger the workload class ID
> updating function.
> 
> Once the CPU is going to offline, it will need to disable the workload
> classification function and clear the history.
> 
> 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:
>  * Rebase
> ---
>  drivers/platform/x86/amd/hfi/hfi.c | 90 +++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
> index c0065ba0ed18..c969ee7ea5ee 100644
> --- a/drivers/platform/x86/amd/hfi/hfi.c
> +++ b/drivers/platform/x86/amd/hfi/hfi.c
> @@ -244,6 +244,89 @@ static int amd_set_hfi_ipcc_score(struct amd_hfi_cpuinfo *hfi_cpuinfo, int cpu)
>  	return 0;
>  }
>  
> +static int amd_hfi_set_state(unsigned int cpu, bool state)
> +{
> +	int ret;
> +
> +	ret = wrmsrl_on_cpu(cpu, AMD_WORKLOAD_CLASS_CONFIG, state);
> +	if (ret)
> +		return ret;
> +
> +	return wrmsrl_on_cpu(cpu, AMD_WORKLOAD_HRST, 0x1);
> +}
> +
> +/**
> + * amd_hfi_online() - Enable workload classification on @cpu
> + * @cpu: CPU in which the workload classification will be enabled
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int amd_hfi_online(unsigned int cpu)
> +{
> +	struct amd_hfi_cpuinfo *hfi_info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu);
> +	struct amd_hfi_classes *hfi_classes;
> +	int ret;
> +
> +	if (WARN_ON_ONCE(!hfi_info))
> +		return -EINVAL;
> +
> +	if (!zalloc_cpumask_var(&hfi_info->cpus, GFP_KERNEL))
> +		return -ENOMEM;
> +
> +	mutex_lock(&hfi_cpuinfo_lock);

Use guard()

> +	cpumask_set_cpu(cpu, hfi_info->cpus);
> +
> +	/*
> +	 * Check if @cpu as an associated, initialized and ranking data must be filled
> +	 */
> +	hfi_classes = hfi_info->amd_hfi_classes;
> +	if (!hfi_classes)
> +		goto unlock;
> +
> +	/* Enable the workload classification interface */
> +	ret = amd_hfi_set_state(cpu, true);
> +	if (ret)
> +		pr_err("wct enable failed for cpu %d\n", cpu);

CPU

Should wct too be capitalized?

Is it okay to return 0 when this error occurs?

> +
> +	mutex_unlock(&hfi_cpuinfo_lock);
> +	return 0;
> +
> +unlock:
> +	free_cpumask_var(hfi_info->cpus);
> +	mutex_unlock(&hfi_cpuinfo_lock);
> +	return ret;
> +}
> +
> +/**
> + * amd_hfi_offline() - Disable workload classification on @cpu
> + * @cpu: CPU in which the workload classification will be disabled
> + *
> + * Remove @cpu from those covered by its HFI instance.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int amd_hfi_offline(unsigned int cpu)
> +{
> +	struct amd_hfi_cpuinfo *hfi_info = &per_cpu(amd_hfi_cpuinfo, cpu);
> +	int ret;
> +
> +	if (WARN_ON_ONCE(!hfi_info))
> +		return -EINVAL;
> +
> +	mutex_lock(&hfi_cpuinfo_lock);

guard or scoped_guard.

> +
> +	/* Disable the workload classification interface */
> +	ret = amd_hfi_set_state(cpu, false);
> +	if (ret)
> +		pr_err("wct disable failed for cpu %d\n", cpu);
> +
> +	mutex_unlock(&hfi_cpuinfo_lock);
> +
> +	free_cpumask_var(hfi_info->cpus);
> +
> +	return 0;
> +}
> +
>  static int update_hfi_ipcc_scores(struct amd_hfi_data *amd_hfi_data)
>  {
>  	int cpu;
> @@ -362,8 +445,13 @@ static int amd_hfi_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto out;
>  
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/amd_hfi:online",
> +				amd_hfi_online, amd_hfi_offline);
> +	if (ret < 0)
> +		goto out;

return ret;

> +
>  out:
> -	return ret;
> +	return ret < 0 ? ret : 0;
>  }
>  
>  static struct platform_driver amd_hfi_driver = {
> 

-- 
 i.

[PATCH v2 09/13] platform/x86: hfi: add power management callback

[Mario Limonciello]

From: Perry Yuan <Perry.Yuan@amd.com>

Introduces power management callbacks for the `amd_hfi` driver.
Specifically, the `suspend` and `resume` callbacks have been added
to handle the necessary operations during system low power states
and wake-up.

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:
 * Whitespace changes
 * Use on online CPUs not present ones
---
 drivers/platform/x86/amd/hfi/hfi.c | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
index c969ee7ea5ee..0263993b0a94 100644
--- a/drivers/platform/x86/amd/hfi/hfi.c
+++ b/drivers/platform/x86/amd/hfi/hfi.c
@@ -407,6 +407,38 @@ static int amd_hfi_metadata_parser(struct platform_device *pdev,
 	return ret;
 }
 
+static int amd_hfi_pm_resume(struct device *dev)
+{
+	int ret, cpu;
+
+	for_each_present_cpu(cpu) {
+		ret = amd_hfi_set_state(cpu, true);
+		if (ret < 0) {
+			dev_err(dev, "failed to enable workload class config: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int amd_hfi_pm_suspend(struct device *dev)
+{
+	int ret, cpu;
+
+	for_each_online_cpu(cpu) {
+		ret = amd_hfi_set_state(cpu, false);
+		if (ret < 0) {
+			dev_err(dev, "failed to disable workload class config: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(amd_hfi_pm_ops, amd_hfi_pm_suspend, amd_hfi_pm_resume);
+
 static const struct acpi_device_id amd_hfi_platform_match[] = {
 	{ "AMDI0104", 0},
 	{ }
@@ -458,6 +490,7 @@ static struct platform_driver amd_hfi_driver = {
 	.driver = {
 		.name = AMD_HFI_DRIVER,
 		.owner = THIS_MODULE,
+		.pm	= &amd_hfi_pm_ops,
 		.acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
 	},
 	.probe = amd_hfi_probe,
-- 
2.43.0

Re: [PATCH v2 09/13] platform/x86: hfi: add power management callback

[Ilpo Järvinen]

On Thu, 10 Oct 2024, Mario Limonciello wrote:

> From: Perry Yuan <Perry.Yuan@amd.com>
> 
> Introduces power management callbacks for the `amd_hfi` driver.
> Specifically, the `suspend` and `resume` callbacks have been added
> to handle the necessary operations during system low power states
> and wake-up.
> 
> 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:
>  * Whitespace changes
>  * Use on online CPUs not present ones
> ---
>  drivers/platform/x86/amd/hfi/hfi.c | 33 ++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
> index c969ee7ea5ee..0263993b0a94 100644
> --- a/drivers/platform/x86/amd/hfi/hfi.c
> +++ b/drivers/platform/x86/amd/hfi/hfi.c
> @@ -407,6 +407,38 @@ static int amd_hfi_metadata_parser(struct platform_device *pdev,
>  	return ret;
>  }
>  
> +static int amd_hfi_pm_resume(struct device *dev)
> +{
> +	int ret, cpu;
> +
> +	for_each_present_cpu(cpu) {
> +		ret = amd_hfi_set_state(cpu, true);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to enable workload class config: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int amd_hfi_pm_suspend(struct device *dev)
> +{
> +	int ret, cpu;
> +
> +	for_each_online_cpu(cpu) {
> +		ret = amd_hfi_set_state(cpu, false);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to disable workload class config: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(amd_hfi_pm_ops, amd_hfi_pm_suspend, amd_hfi_pm_resume);
> +
>  static const struct acpi_device_id amd_hfi_platform_match[] = {
>  	{ "AMDI0104", 0},
>  	{ }
> @@ -458,6 +490,7 @@ static struct platform_driver amd_hfi_driver = {
>  	.driver = {
>  		.name = AMD_HFI_DRIVER,
>  		.owner = THIS_MODULE,
> +		.pm	= &amd_hfi_pm_ops,

This is inconsistent.

>  		.acpi_match_table = ACPI_PTR(amd_hfi_platform_match),
>  	},
>  	.probe = amd_hfi_probe,
> 

-- 
 i.

[PATCH v2 10/13] x86/cpu: Enable SD_ASYM_PACKING for DIE Domain on AMD Processors

[Mario Limonciello]

From: Perry Yuan <perry.yuan@amd.com>

Enable the SD_ASYM_PACKING domain flag for the DIE domain on AMD
heterogeneous processors.
This flag is beneficial for processors with one or more CCDs and
relies on x86_sched_itmt_flags().

Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2:
 * Whitespace
---
 arch/x86/kernel/smpboot.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 766f092dab80..c01bb75d0375 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -497,8 +497,9 @@ static int x86_cluster_flags(void)
 
 static int x86_die_flags(void)
 {
-	if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU))
-	       return x86_sched_itmt_flags();
+	if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU) ||
+	    cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY))
+		return x86_sched_itmt_flags();
 
 	return 0;
 }
-- 
2.43.0

[PATCH v2 11/13] x86/process: Clear hardware feedback history for AMD processors

[Mario Limonciello]

From: Perry Yuan <perry.yuan@amd.com>

Incorporate a mechanism within the context switching code to reset
the hardware history for AMD processors. Specifically, when a task
is switched in, the class ID was read and reset the hardware workload
classification history of CPU firmware and then it start to trigger
workload classification for the next running thread.

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:
 * Fix function name
 * Use static keys
---
 arch/x86/include/asm/hreset.h |  6 ++++++
 arch/x86/kernel/cpu/common.c  | 15 +++++++++++++++
 arch/x86/kernel/process_32.c  |  3 +++
 arch/x86/kernel/process_64.c  |  3 +++
 4 files changed, 27 insertions(+)
 create mode 100644 arch/x86/include/asm/hreset.h

diff --git a/arch/x86/include/asm/hreset.h b/arch/x86/include/asm/hreset.h
new file mode 100644
index 000000000000..ae1f72602bbd
--- /dev/null
+++ b/arch/x86/include/asm/hreset.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_HRESET_H
+
+void reset_hardware_history_hetero(void);
+
+#endif /* _ASM_X86_HRESET_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 07a34d723505..887821e97e54 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -57,6 +57,7 @@
 #include <asm/mce.h>
 #include <asm/msr.h>
 #include <asm/cacheinfo.h>
+#include <asm/hreset.h>
 #include <asm/memtype.h>
 #include <asm/microcode.h>
 #include <asm/intel-family.h>
@@ -403,6 +404,7 @@ static const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP | X86_C
 					     X86_CR4_FSGSBASE | X86_CR4_CET | X86_CR4_FRED;
 static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning);
 static unsigned long cr4_pinned_bits __ro_after_init;
+static DEFINE_STATIC_KEY_FALSE_RO(hardware_history_features);
 
 void native_write_cr0(unsigned long val)
 {
@@ -481,6 +483,12 @@ void cr4_init(void)
 	this_cpu_write(cpu_tlbstate.cr4, cr4);
 }
 
+static void __init setup_hreset(struct cpuinfo_x86 *c)
+{
+	if (cpu_feature_enabled(X86_FEATURE_WORKLOAD_CLASS))
+		static_key_enable_cpuslocked(&hardware_history_features.key);
+}
+
 /*
  * Once CPU feature detection is finished (and boot params have been
  * parsed), record any of the sensitive CR bits that are set, and
@@ -1844,6 +1852,7 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	setup_smep(c);
 	setup_smap(c);
 	setup_umip(c);
+	setup_hreset(c);
 
 	/* Enable FSGSBASE instructions if available. */
 	if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
@@ -2410,3 +2419,9 @@ void __init arch_cpu_finalize_init(void)
 	 */
 	mem_encrypt_init();
 }
+
+__always_inline void reset_hardware_history_hetero(void)
+{
+	if (static_branch_unlikely(&hardware_history_features))
+		wrmsrl(AMD_WORKLOAD_HRST, 0x1);
+}
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 0917c7f25720..6a3a1339f7a7 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -52,6 +52,7 @@
 #include <asm/switch_to.h>
 #include <asm/vm86.h>
 #include <asm/resctrl.h>
+#include <asm/hreset.h>
 #include <asm/proto.h>
 
 #include "process.h"
@@ -213,6 +214,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	/* Load the Intel cache allocation PQR MSR. */
 	resctrl_sched_in(next_p);
 
+	reset_hardware_history_hetero();
+
 	return prev_p;
 }
 
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 226472332a70..ea7f765c6262 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -54,6 +54,7 @@
 #include <asm/xen/hypervisor.h>
 #include <asm/vdso.h>
 #include <asm/resctrl.h>
+#include <asm/hreset.h>
 #include <asm/unistd.h>
 #include <asm/fsgsbase.h>
 #include <asm/fred.h>
@@ -709,6 +710,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 	/* Load the Intel cache allocation PQR MSR. */
 	resctrl_sched_in(next_p);
 
+	reset_hardware_history_hetero();
+
 	return prev_p;
 }
 
-- 
2.43.0

[PATCH v2 12/13] cpufreq/amd-pstate: Disable preferred cores on designs with workload classification

[Mario Limonciello]

On designs that have workload classification, it's preferred that
the amd-hfi driver is used to provide hints to the scheduler of
which cores to use instead of the amd-pstate driver.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 30415c30d8b4..0751579ef06c 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -780,6 +780,10 @@ static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata)
 	/* user disabled or not detected */
 	if (!amd_pstate_prefcore)
 		return;
+	/* should use amd-hfi instead */
+	if (boot_cpu_has(X86_FEATURE_WORKLOAD_CLASS) &&
+	    IS_ENABLED(CONFIG_AMD_HFI))
+		return;
 
 	cpudata->hw_prefcore = true;
 
-- 
2.43.0

[PATCH v2 13/13] platform/x86/amd: hfi: Set ITMT priority from ranking data

[Mario Limonciello]

The static ranking data that is read at module load should be used
to set up the priorities for the cores relative to the performance
values.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/hfi/Kconfig | 1 +
 drivers/platform/x86/amd/hfi/hfi.c   | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/platform/x86/amd/hfi/Kconfig b/drivers/platform/x86/amd/hfi/Kconfig
index 08051cd4f74d..4dfa7641b35b 100644
--- a/drivers/platform/x86/amd/hfi/Kconfig
+++ b/drivers/platform/x86/amd/hfi/Kconfig
@@ -7,6 +7,7 @@ config AMD_HFI
 	bool "AMD Hetero Core Hardware Feedback Driver"
 	depends on ACPI
 	depends on CPU_SUP_AMD
+	depends on SCHED_MC_PRIO
 	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
diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c
index 0263993b0a94..f212820a0e4a 100644
--- a/drivers/platform/x86/amd/hfi/hfi.c
+++ b/drivers/platform/x86/amd/hfi/hfi.c
@@ -121,6 +121,12 @@ static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index =
 
 static DEFINE_MUTEX(hfi_cpuinfo_lock);
 
+static void amd_hfi_sched_itmt_work(struct work_struct *work)
+{
+	sched_set_itmt_support();
+}
+static DECLARE_WORK(sched_amd_hfi_itmt_work, amd_hfi_sched_itmt_work);
+
 static int find_cpu_index_by_apicid(unsigned int target_apicid)
 {
 	int cpu_index;
@@ -241,6 +247,8 @@ static int amd_set_hfi_ipcc_score(struct amd_hfi_cpuinfo *hfi_cpuinfo, int cpu)
 		WRITE_ONCE(hfi_cpuinfo->ipcc_scores[i],
 			   hfi_cpuinfo->amd_hfi_classes[i].perf);
 
+	sched_set_itmt_core_prio(hfi_cpuinfo->ipcc_scores[0], cpu);
+
 	return 0;
 }
 
@@ -482,6 +490,7 @@ static int amd_hfi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto out;
 
+	schedule_work(&sched_amd_hfi_itmt_work);
 out:
 	return ret < 0 ? ret : 0;
 }
-- 
2.43.0

Re: [PATCH v2 00/13] Add support for AMD hardware feedback interface

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]

On Thu, Oct 10, 2024 at 02:36:52PM -0500, Mario Limonciello wrote:
> 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.
> 
> Threads are classified during runtime into enumerated classes.
> Currently, the driver supports 3 classes (0 through 2). These classes
> represent thread performance/power characteristics that may benefit from
> special scheduling behaviors. The real-time thread classification is
> consumed by the operating system and is used to inform the scheduler of
> where the thread should be placed for optimal performance or energy efficiency.
> 
> The thread classification helps to select CPU from a ranking table that describes
> an efficiency and performance ranking for each classification from two dimensions.
> 
> The ranking data provided by the ranking table are numbers ranging from 0 to 255,
> where a higher performance value indicates higher performance capability and a higher
> efficiency value indicates greater efficiency. All the CPU cores are ranked into
> different class IDs. Within each class ranking, the cores may have different ranking
> values. Therefore, picking from each classification ID will later allow the scheduler
> to select the best core while threads are classified into the specified workload class.
> 
> This series was originally submitted by Perry Yuan [1] but he is now doing a different
> role and he asked me to take over.

Sorry but can you specify the base commit/tree of this series? I can't apply
it on top of current platform-drivers-x86.git, though.

Confused...

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 00/13] Add support for AMD hardware feedback interface

[Mario Limonciello]

On 10/10/2024 19:54, Bagas Sanjaya wrote:
> On Thu, Oct 10, 2024 at 02:36:52PM -0500, Mario Limonciello wrote:
>> 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.
>>
>> Threads are classified during runtime into enumerated classes.
>> Currently, the driver supports 3 classes (0 through 2). These classes
>> represent thread performance/power characteristics that may benefit from
>> special scheduling behaviors. The real-time thread classification is
>> consumed by the operating system and is used to inform the scheduler of
>> where the thread should be placed for optimal performance or energy efficiency.
>>
>> The thread classification helps to select CPU from a ranking table that describes
>> an efficiency and performance ranking for each classification from two dimensions.
>>
>> The ranking data provided by the ranking table are numbers ranging from 0 to 255,
>> where a higher performance value indicates higher performance capability and a higher
>> efficiency value indicates greater efficiency. All the CPU cores are ranked into
>> different class IDs. Within each class ranking, the cores may have different ranking
>> values. Therefore, picking from each classification ID will later allow the scheduler
>> to select the best core while threads are classified into the specified workload class.
>>
>> This series was originally submitted by Perry Yuan [1] but he is now doing a different
>> role and he asked me to take over.
> 
> Sorry but can you specify the base commit/tree of this series? I can't apply
> it on top of current platform-drivers-x86.git, though.
> 
> Confused...
> 

Sorry about that.  As it crosses a few subsystems it will require some 
coordination to properly land once it has review comments.

It's (currently) based off:
https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git/log/?h=linux-next

commit 57533bc760ae ("cpufreq/amd-pstate: Fix non kerneldoc comment")

Re: [PATCH v2 00/13] Add support for AMD hardware feedback interface

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 2403 bytes --]

On Thu, Oct 10, 2024 at 08:29:47PM -0500, Mario Limonciello wrote:
> On 10/10/2024 19:54, Bagas Sanjaya wrote:
> > On Thu, Oct 10, 2024 at 02:36:52PM -0500, Mario Limonciello wrote:
> > > 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.
> > > 
> > > Threads are classified during runtime into enumerated classes.
> > > Currently, the driver supports 3 classes (0 through 2). These classes
> > > represent thread performance/power characteristics that may benefit from
> > > special scheduling behaviors. The real-time thread classification is
> > > consumed by the operating system and is used to inform the scheduler of
> > > where the thread should be placed for optimal performance or energy efficiency.
> > > 
> > > The thread classification helps to select CPU from a ranking table that describes
> > > an efficiency and performance ranking for each classification from two dimensions.
> > > 
> > > The ranking data provided by the ranking table are numbers ranging from 0 to 255,
> > > where a higher performance value indicates higher performance capability and a higher
> > > efficiency value indicates greater efficiency. All the CPU cores are ranked into
> > > different class IDs. Within each class ranking, the cores may have different ranking
> > > values. Therefore, picking from each classification ID will later allow the scheduler
> > > to select the best core while threads are classified into the specified workload class.
> > > 
> > > This series was originally submitted by Perry Yuan [1] but he is now doing a different
> > > role and he asked me to take over.
> > 
> > Sorry but can you specify the base commit/tree of this series? I can't apply
> > it on top of current platform-drivers-x86.git, though.
> > 
> > Confused...
> > 
> 
> Sorry about that.  As it crosses a few subsystems it will require some
> coordination to properly land once it has review comments.
> 
> It's (currently) based off:
> https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git/log/?h=linux-next
> 
> commit 57533bc760ae ("cpufreq/amd-pstate: Fix non kerneldoc comment")

Series successfully applied for review, thanks!

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]