* [PATCH v4 1/7] x86/Documentation: Describe the Intel Hardware Feedback Interface
2022-01-08 3:47 [PATCH v4 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
@ 2022-01-08 3:47 ` Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 2/7] x86/cpu: Add definitions for " Ricardo Neri
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Ricardo Neri @ 2022-01-08 3:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm
Cc: x86, linux-doc, Len Brown, Srinivas Pandruvada, Aubrey Li,
Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, linux-kernel, Ricardo Neri
Start a documentation file to describe the purpose and operation of Intel's
Hardware Feedback Interface. Describe how this interface is used in Linux
to relay performance and energy efficiency updates to userspace.
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Suggested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* None
Changes since v2:
* None
Changes since v1:
* Clarified that HFI capabilities are independent. (Daniel)
* Provided examples on changes reflected in the HFI table. (Daniel)
---
Documentation/x86/index.rst | 1 +
Documentation/x86/intel-hfi.rst | 72 +++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
create mode 100644 Documentation/x86/intel-hfi.rst
diff --git a/Documentation/x86/index.rst b/Documentation/x86/index.rst
index f498f1d36cd3..982c8af853b9 100644
--- a/Documentation/x86/index.rst
+++ b/Documentation/x86/index.rst
@@ -21,6 +21,7 @@ x86-specific Documentation
tlb
mtrr
pat
+ intel-hfi
intel-iommu
intel_txt
amd-memory-encryption
diff --git a/Documentation/x86/intel-hfi.rst b/Documentation/x86/intel-hfi.rst
new file mode 100644
index 000000000000..49dea58ea4fb
--- /dev/null
+++ b/Documentation/x86/intel-hfi.rst
@@ -0,0 +1,72 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================================================
+Hardware-Feedback Interface for scheduling on Intel Hardware
+============================================================
+
+Overview
+--------
+
+Intel has described the Hardware Feedback Interface (HFI) in the Intel 64 and
+IA-32 Architectures Software Developer's Manual (Intel SDM) Volume 3 Section
+14.6 [1]_.
+
+The HFI gives the operating system a performance and energy efficiency
+capability data for each CPU in the system. Linux can use the information from
+the HFI to influence task placement decisions.
+
+The 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]. Higher values
+indicate higher capability. Energy efficiency and performance are reported in
+separate capabilities. Even though on some systems these two metrics may be
+related, they are specified as independent capabilities in the Intel SDM.
+
+These capabilities may change at runtime as a result of changes in the
+operating conditions of the system or the action of external factors. The rate
+at which these capabilities are updated is specific to each processor model. On
+some models, capabilities are set at boot time and never change. On others,
+capabilities may change every tens of milliseconds. For instance, a remote
+mechanism may be used to lower Thermal Design Power. Such change can be
+reflected in the HFI. Likewise, if the system needs to be throttled due to
+excessive heat, the HFI may reflect reduced performance on specific CPUs.
+
+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 infrastructure to handle thermal event interrupts has two parts. In the
+Local Vector Table of a CPU's local APIC, there exists a register for the
+Thermal Monitor Register. This register controls how interrupts are delivered
+to a CPU when the thermal monitor generates and interrupt. Further details
+can be found in the Intel SDM Vol. 3 Section 10.5 [1]_.
+
+The thermal monitor may generate interrupts per CPU or per package. The HFI
+generates package-level interrupts. This monitor is configured and initialized
+via a set of machine-specific registers. Specifically, the HFI interrupt and
+status are controlled via designated bits in the IA32_PACKAGE_THERM_INTERRUPT
+and IA32_PACKAGE_THERM_STATUS registers, respectively. There exists one HFI
+table per package. Further details can be found in the Intel SDM Vol. 3
+Section 14.9 [1]_.
+
+The hardware issues an HFI interrupt after updating the HFI table and is ready
+for the operating system to consume it. CPUs receive such interrupt via the
+thermal entry in the Local APIC's Local Vector Table.
+
+When servicing such interrupt, the HFI driver parses the updated table and
+relays the update to userspace using the thermal notification framework. Given
+that there may be many HFI updates every second, the updates relayed to
+userspace are throttled at a rate of CONFIG_HZ jiffies.
+
+References
+----------
+
+.. [1] https://www.intel.com/sdm
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 2/7] x86/cpu: Add definitions for the Intel Hardware Feedback Interface
2022-01-08 3:47 [PATCH v4 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 1/7] x86/Documentation: Describe the Intel Hardware Feedback Interface Ricardo Neri
@ 2022-01-08 3:47 ` Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 3/7] thermal: intel: hfi: Minimally initialize the " Ricardo Neri
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Ricardo Neri @ 2022-01-08 3:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm
Cc: x86, linux-doc, Len Brown, Srinivas Pandruvada, Aubrey Li,
Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, linux-kernel, Ricardo Neri
Add the CPUID feature bit and the model-specific registers needed to
identify and configure the Intel Hardware Feedback Interface.
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Acked-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* None
Changes since v2:
* Replaced subject prefix to be "x86/cpu:". (Boris)
Changes since v1:
* Renamed X86_FEATURE_INTEL_HFI as X86_FEATURE_HFI. (Boris)
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/msr-index.h | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index d5b5f2ab87a0..1a31b3ef15f0 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -327,6 +327,7 @@
#define X86_FEATURE_HWP_ACT_WINDOW (14*32+ 9) /* HWP Activity Window */
#define X86_FEATURE_HWP_EPP (14*32+10) /* HWP Energy Perf. Preference */
#define X86_FEATURE_HWP_PKG_REQ (14*32+11) /* HWP Package Level Request */
+#define X86_FEATURE_HFI (14*32+19) /* Hardware Feedback Interface */
/* AMD SVM Feature Identification, CPUID level 0x8000000a (EDX), word 15 */
#define X86_FEATURE_NPT (15*32+ 0) /* Nested Page Table support */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 01e2650b9585..ad958a49b2bb 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -687,12 +687,14 @@
#define PACKAGE_THERM_STATUS_PROCHOT (1 << 0)
#define PACKAGE_THERM_STATUS_POWER_LIMIT (1 << 10)
+#define PACKAGE_THERM_STATUS_HFI_UPDATED (1 << 26)
#define MSR_IA32_PACKAGE_THERM_INTERRUPT 0x000001b2
#define PACKAGE_THERM_INT_HIGH_ENABLE (1 << 0)
#define PACKAGE_THERM_INT_LOW_ENABLE (1 << 1)
#define PACKAGE_THERM_INT_PLN_ENABLE (1 << 24)
+#define PACKAGE_THERM_INT_HFI_ENABLE (1 << 25)
/* Thermal Thresholds Support */
#define THERM_INT_THRESHOLD0_ENABLE (1 << 15)
@@ -941,4 +943,8 @@
#define MSR_VM_IGNNE 0xc0010115
#define MSR_VM_HSAVE_PA 0xc0010117
+/* Hardware Feedback Interface */
+#define MSR_IA32_HW_FEEDBACK_PTR 0x17d0
+#define MSR_IA32_HW_FEEDBACK_CONFIG 0x17d1
+
#endif /* _ASM_X86_MSR_INDEX_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 3/7] thermal: intel: hfi: Minimally initialize the Hardware Feedback Interface
2022-01-08 3:47 [PATCH v4 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 1/7] x86/Documentation: Describe the Intel Hardware Feedback Interface Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 2/7] x86/cpu: Add definitions for " Ricardo Neri
@ 2022-01-08 3:47 ` Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 4/7] thermal: intel: hfi: Handle CPU hotplug events Ricardo Neri
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Ricardo Neri @ 2022-01-08 3:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm
Cc: x86, linux-doc, Len Brown, Srinivas Pandruvada, Aubrey Li,
Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, linux-kernel, Ricardo Neri
The Intel Hardware Feedback Interface provides guidance to the operating
system about the performance and energy efficiency capabilities of each
CPU in the system. Capabilities are numbers between 0 and 255 where a
higher number represents a higher capability. For each CPU, energy
efficiency and performance are reported as separate capabilities.
Hardware computes these capabilities based on the operating conditions of
the system such as power and thermal limits. These capabilities are shared
with the operating system in a table resident in memory. Each package in
the system has its own HFI instance. Every logical CPU in the package is
represented in the table. More than one logical CPUs may be represented in
a single table entry. When the hardware updates the table, it generates a
package-level thermal interrupt.
The size and format of the HFI table depend on the supported features and
can only be determined at runtime. To minimally initialize the HFI, parse
its features and allocate one instance per package of a data structure with
the necessary parameters to read and navigate a local copy (i.e., owned by
the driver) of individual HFI tables.
A subsequent changeset will provide per-CPU initialization and interrupt
handling.
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Co-developed by: Aubrey Li <aubrey.li@linux.intel.com>
Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* None
Changes since v2:
* Rename the INTEL_HFI Kconfig option as INTEL_HFI_THERMAL to reflect
better the nature of the feature. (Rafael)
* Added missing #include files.
Changes since v1:
* Renamed X86_FEATURE_INTEL_HFI as X86_FEATURE_HFI. (Boris)
* Reworked parsing of HFI features using bitfields instead of bitmasks.
(PeterZ).
* Removed hfi_instance::parsed as hfi_parse_features() is called only
once via intel_hfi_init() via thermal_throttle_init_device().
(Rafael)
* Converted pr_err() to pr_debug(). (Srinivas, Rafael)
* Removed unnecessary dependency on CONFIG_SCHED_MC.
* Renamed hfi_instance::ts_counter as hfi_instance::timestamp.
* Renamed hfi_instance::table_base as hfi_instance::local_table and
relocated its definition to this patch.
* Wrapped hfi_instance::timestamp and hfi_instance:local_table in an
anonymous union, since both point at the same location.
---
drivers/thermal/intel/Kconfig | 12 ++
drivers/thermal/intel/Makefile | 1 +
drivers/thermal/intel/intel_hfi.c | 181 ++++++++++++++++++++++++++++
drivers/thermal/intel/intel_hfi.h | 11 ++
drivers/thermal/intel/therm_throt.c | 3 +
5 files changed, 208 insertions(+)
create mode 100644 drivers/thermal/intel/intel_hfi.c
create mode 100644 drivers/thermal/intel/intel_hfi.h
diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
index c83ea5d04a1d..e9d2925227d4 100644
--- a/drivers/thermal/intel/Kconfig
+++ b/drivers/thermal/intel/Kconfig
@@ -99,3 +99,15 @@ config INTEL_MENLOW
Intel Menlow platform.
If unsure, say N.
+
+config INTEL_HFI_THERMAL
+ bool "Intel Hardware Feedback Interface"
+ depends on CPU_SUP_INTEL
+ depends on X86_THERMAL_VECTOR
+ help
+ Select this option to enable the Hardware Feedback Interface. If
+ selected, hardware provides guidance to the operating system on
+ the performance and energy efficiency capabilities of each CPU.
+ These capabilities may change as a result of changes in the operating
+ conditions of the system such power and thermal limits. If selected,
+ the kernel relays updates in CPUs' capabilities to userspace.
diff --git a/drivers/thermal/intel/Makefile b/drivers/thermal/intel/Makefile
index 960b56268b4a..9a8d8054f316 100644
--- a/drivers/thermal/intel/Makefile
+++ b/drivers/thermal/intel/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_INTEL_PCH_THERMAL) += intel_pch_thermal.o
obj-$(CONFIG_INTEL_TCC_COOLING) += intel_tcc_cooling.o
obj-$(CONFIG_X86_THERMAL_VECTOR) += therm_throt.o
obj-$(CONFIG_INTEL_MENLOW) += intel_menlow.o
+obj-$(CONFIG_INTEL_HFI_THERMAL) += intel_hfi.o
diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
new file mode 100644
index 000000000000..52e16d2bc957
--- /dev/null
+++ b/drivers/thermal/intel/intel_hfi.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Hardware Feedback Interface Driver
+ *
+ * Copyright (c) 2021, Intel Corporation.
+ *
+ * Authors: Aubrey Li <aubrey.li@linux.intel.com>
+ * Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
+ *
+ *
+ * The Hardware Feedback Interface provides a performance and energy efficiency
+ * capability information for each CPU in the system. Depending on the processor
+ * model, hardware may periodically update these capabilities as a result of
+ * changes in the operating conditions (e.g., power limits or thermal
+ * constraints). On other processor models, there is a single HFI update
+ * at boot.
+ *
+ * This file provides functionality to process HFI updates and relay these
+ * updates to userspace.
+ */
+
+#define pr_fmt(fmt) "intel-hfi: " fmt
+
+#include <linux/bitops.h>
+#include <linux/cpufeature.h>
+#include <linux/math.h>
+#include <linux/printk.h>
+#include <linux/processor.h>
+#include <linux/slab.h>
+#include <linux/topology.h>
+
+#include "intel_hfi.h"
+
+/* CPUID detection and enumeration definitions for HFI */
+
+#define CPUID_HFI_LEAF 6
+
+union hfi_capabilities {
+ struct {
+ u8 performance:1;
+ u8 energy_efficiency:1;
+ u8 __reserved:6;
+ } split;
+ u8 bits;
+};
+
+union cpuid6_edx {
+ struct {
+ union hfi_capabilities capabilities;
+ u32 table_pages:4;
+ u32 __reserved:4;
+ s32 index:16;
+ } split;
+ u32 full;
+};
+
+/**
+ * struct hfi_cpu_data - HFI capabilities per CPU
+ * @perf_cap: Performance capability
+ * @ee_cap: Energy efficiency capability
+ *
+ * Capabilities of a logical processor in the HFI table. These capabilities are
+ * unitless.
+ */
+struct hfi_cpu_data {
+ u8 perf_cap;
+ u8 ee_cap;
+} __packed;
+
+/**
+ * struct hfi_hdr - Header of the HFI table
+ * @perf_updated: Hardware updated performance capabilities
+ * @ee_updated: Hardware updated energy efficiency capabilities
+ *
+ * Properties of the data in an HFI table.
+ */
+struct hfi_hdr {
+ u8 perf_updated;
+ u8 ee_updated;
+} __packed;
+
+/**
+ * struct hfi_instance - Representation of an HFI instance (i.e., a table)
+ * @local_table: Base of the local copy of the HFI table
+ * @timestamp: Timestamp of the last update of the local table.
+ * Located at the base of the local table.
+ * @hdr: Base address of the local table header
+ * @data: Base address of the local table data
+ *
+ * A set of parameters to parse and navigate a specific HFI table.
+ */
+struct hfi_instance {
+ union {
+ void *local_table;
+ u64 *timestamp;
+ };
+ void *hdr;
+ void *data;
+};
+
+/**
+ * struct hfi_features - Supported HFI features
+ * @nr_table_pages: Size of the HFI table in 4KB pages
+ * @cpu_stride: Stride size to locate capability data of a logical
+ * processor within the table (i.e., row stride)
+ * @hdr_size: Size of table header
+ *
+ * Parameters and supported features that are common to all HFI instances
+ */
+struct hfi_features {
+ unsigned int nr_table_pages;
+ unsigned int cpu_stride;
+ unsigned int hdr_size;
+};
+
+static int max_hfi_instances;
+static struct hfi_instance *hfi_instances;
+
+static struct hfi_features hfi_features;
+
+static __init int hfi_parse_features(void)
+{
+ unsigned int nr_capabilities;
+ union cpuid6_edx edx;
+
+ if (!boot_cpu_has(X86_FEATURE_HFI))
+ return -ENODEV;
+
+ /*
+ * If we are here we know that CPUID_HFI_LEAF exists. Parse the
+ * supported capabilities and the size of the HFI table.
+ */
+ edx.full = cpuid_edx(CPUID_HFI_LEAF);
+
+ if (!edx.split.capabilities.split.performance) {
+ pr_debug("Performance reporting not supported! Not using HFI\n");
+ return -ENODEV;
+ }
+
+ /*
+ * The number of supported capabilities determines the number of
+ * columns in the HFI table. Exclude the reserved bits.
+ */
+ edx.split.capabilities.split.__reserved = 0;
+ nr_capabilities = hweight8(edx.split.capabilities.bits);
+
+ /* The number of 4KB pages required by the table */
+ hfi_features.nr_table_pages = edx.split.table_pages + 1;
+
+ /*
+ * The header contains change indications for each supported feature.
+ * The size of the table header is rounded up to be a multiple of 8
+ * bytes.
+ */
+ hfi_features.hdr_size = DIV_ROUND_UP(nr_capabilities, 8) * 8;
+
+ /*
+ * Data of each logical processor is also rounded up to be a multiple
+ * of 8 bytes.
+ */
+ hfi_features.cpu_stride = DIV_ROUND_UP(nr_capabilities, 8) * 8;
+
+ return 0;
+}
+
+void __init intel_hfi_init(void)
+{
+ if (hfi_parse_features())
+ return;
+
+ /* There is one HFI instance per die/package. */
+ max_hfi_instances = topology_max_packages() *
+ topology_max_die_per_package();
+
+ /*
+ * This allocation may fail. CPU hotplug callbacks must check
+ * for a null pointer.
+ */
+ hfi_instances = kcalloc(max_hfi_instances, sizeof(*hfi_instances),
+ GFP_KERNEL);
+}
diff --git a/drivers/thermal/intel/intel_hfi.h b/drivers/thermal/intel/intel_hfi.h
new file mode 100644
index 000000000000..05f748b48a4e
--- /dev/null
+++ b/drivers/thermal/intel/intel_hfi.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _INTEL_HFI_H
+#define _INTEL_HFI_H
+
+#if defined(CONFIG_INTEL_HFI_THERMAL)
+void __init intel_hfi_init(void);
+#else
+static inline void intel_hfi_init(void) { }
+#endif /* CONFIG_INTEL_HFI_THERMAL */
+
+#endif /* _INTEL_HFI_H */
diff --git a/drivers/thermal/intel/therm_throt.c b/drivers/thermal/intel/therm_throt.c
index dab7e8fb1059..ac408714d52b 100644
--- a/drivers/thermal/intel/therm_throt.c
+++ b/drivers/thermal/intel/therm_throt.c
@@ -32,6 +32,7 @@
#include <asm/irq.h>
#include <asm/msr.h>
+#include "intel_hfi.h"
#include "thermal_interrupt.h"
/* How long to wait between reporting thermal events */
@@ -509,6 +510,8 @@ static __init int thermal_throttle_init_device(void)
if (!atomic_read(&therm_throt_en))
return 0;
+ intel_hfi_init();
+
ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/therm:online",
thermal_throttle_online,
thermal_throttle_offline);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 4/7] thermal: intel: hfi: Handle CPU hotplug events
2022-01-08 3:47 [PATCH v4 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
` (2 preceding siblings ...)
2022-01-08 3:47 ` [PATCH v4 3/7] thermal: intel: hfi: Minimally initialize the " Ricardo Neri
@ 2022-01-08 3:47 ` Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 5/7] thermal: intel: hfi: Enable notification interrupt Ricardo Neri
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Ricardo Neri @ 2022-01-08 3:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm
Cc: x86, linux-doc, Len Brown, Srinivas Pandruvada, Aubrey Li,
Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, linux-kernel, Ricardo Neri
All CPUs in a package are represented in an HFI table. There exists an
HFI table per package. Thus, CPUs in a package need to coordinate to
initialize and access the table. Do such coordination during CPU hotplug.
Use the first CPU to come online in a package to initialize the HFI table
and the data structure representing it. Other CPUs in the same package need
only to register or unregister themselves in that data structure.
The HFI depends on both the package-level thermal management and the local
APIC thermal local vector. Thus, ensure that both are properly configured
before calling intel_hfi_online(). The CPU hotplug callbacks of the thermal
throttle events code already meet these conditions. Enable the HFI from
thermal_throttle_online().
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* Corrected bug when initializing a CPU's HFI instance in
intel_hfi_online().
Changes since v2:
* Removed hfi_instance::initialized. Instead, use hfi_instance::
hdr to determine if the instance has been initialized. (Rafael)
* Renamed hfi_lock as hfi_instance_lock to reflect the fact that it is
used to protect accesses to HFI instances. (Rafael)
* Removed unnecessary locking when linking a CPU to an HFI instance
in intel_hfi_online(). (Rafael)
* Improved locking in error paths in intel_hfi_online(). (Rafael)
* Unconditionally link the hfi_instance of a package/die to CPUs in
the same package in intel_hfi_online(). Initialization is taken care
of separately.
* Removed a pointless check for NULL when taking a pointer of an element
of hfi_instances.
* Added missing #include files.
Changes since v1:
* Renamed X86_FEATURE_INTEL_HFI as X86_FEATURE_HFI. (Boris)
* Relocated definitions of MSR bits from intel_hfi.h to intel_hfi.c.
* Renamed HFI_PTR_VALID_BIT as HW_FEEDBACK_PTR_VALID_BIT for clarity.
* Reworked init_hfi_cpu_index() to take a pointer of type struct
hfi_cpu_info. This makes the function more concise. (Rafael)
* In intel_hfi_online(), check for null hfi_instances and improve checks
of the die_id. (Rafael)
* Use a local cpumask_var_t to keep track of the CPUs of each
hfi_instance. (Rafael)
* Removed hfi_instance::die_id. It is not used anywhere.
* Renamed hfi_instance::table_base as hfi_instance::local_table for
clarity.
---
drivers/thermal/intel/intel_hfi.c | 204 ++++++++++++++++++++++++++++
drivers/thermal/intel/intel_hfi.h | 4 +
drivers/thermal/intel/therm_throt.c | 8 ++
3 files changed, 216 insertions(+)
diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
index 52e16d2bc957..37f979a9e833 100644
--- a/drivers/thermal/intel/intel_hfi.c
+++ b/drivers/thermal/intel/intel_hfi.c
@@ -23,14 +23,24 @@
#include <linux/bitops.h>
#include <linux/cpufeature.h>
+#include <linux/cpumask.h>
+#include <linux/gfp.h>
+#include <linux/io.h>
#include <linux/math.h>
+#include <linux/mutex.h>
+#include <linux/percpu-defs.h>
#include <linux/printk.h>
#include <linux/processor.h>
#include <linux/slab.h>
#include <linux/topology.h>
+#include <asm/msr.h>
+
#include "intel_hfi.h"
+/* Hardware Feedback Interface MSR configuration bits */
+#define HW_FEEDBACK_PTR_VALID_BIT BIT(0)
+
/* CPUID detection and enumeration definitions for HFI */
#define CPUID_HFI_LEAF 6
@@ -86,6 +96,8 @@ struct hfi_hdr {
* Located at the base of the local table.
* @hdr: Base address of the local table header
* @data: Base address of the local table data
+ * @cpus: CPUs represented in this HFI table instance
+ * @hw_table: Pointer to the HFI table of this instance
*
* A set of parameters to parse and navigate a specific HFI table.
*/
@@ -96,6 +108,8 @@ struct hfi_instance {
};
void *hdr;
void *data;
+ cpumask_var_t cpus;
+ void *hw_table;
};
/**
@@ -113,10 +127,179 @@ struct hfi_features {
unsigned int hdr_size;
};
+/**
+ * struct hfi_cpu_info - Per-CPU attributes to consume HFI data
+ * @index: Row of this CPU in its HFI table
+ * @hfi_instance: Attributes of the HFI table to which this CPU belongs
+ *
+ * Parameters to link a logical processor to an HFI table and a row within it.
+ */
+struct hfi_cpu_info {
+ s16 index;
+ struct hfi_instance *hfi_instance;
+};
+
+static DEFINE_PER_CPU(struct hfi_cpu_info, hfi_cpu_info) = { .index = -1 };
+
static int max_hfi_instances;
static struct hfi_instance *hfi_instances;
static struct hfi_features hfi_features;
+static DEFINE_MUTEX(hfi_instance_lock);
+
+static void init_hfi_cpu_index(struct hfi_cpu_info *info)
+{
+ union cpuid6_edx edx;
+
+ /* Do not re-read @cpu's index if it has already been initialized. */
+ if (info->index > -1)
+ return;
+
+ edx.full = cpuid_edx(CPUID_HFI_LEAF);
+ info->index = edx.split.index;
+}
+
+/*
+ * The format of the HFI table depends on the number of capabilities that the
+ * hardware supports. Keep a data structure to navigate the table.
+ */
+static void init_hfi_instance(struct hfi_instance *hfi_instance)
+{
+ /* The HFI header is below the time-stamp. */
+ hfi_instance->hdr = hfi_instance->local_table +
+ sizeof(*hfi_instance->timestamp);
+
+ /* The HFI data starts below the header. */
+ hfi_instance->data = hfi_instance->hdr + hfi_features.hdr_size;
+}
+
+/**
+ * intel_hfi_online() - Enable HFI on @cpu
+ * @cpu: CPU in which the HFI will be enabled
+ *
+ * Enable the HFI to be used in @cpu. The HFI is enabled at the die/package
+ * level. The first CPU in the die/package to come online does the full HFI
+ * initialization. Subsequent CPUs will just link themselves to the HFI
+ * instance of their die/package.
+ */
+void intel_hfi_online(unsigned int cpu)
+{
+ struct hfi_cpu_info *info = &per_cpu(hfi_cpu_info, cpu);
+ u16 die_id = topology_logical_die_id(cpu);
+ struct hfi_instance *hfi_instance;
+ phys_addr_t hw_table_pa;
+ u64 msr_val;
+
+ if (!boot_cpu_has(X86_FEATURE_HFI))
+ return;
+
+ /* Not much to do if hfi_instances are missing. */
+ if (!hfi_instances)
+ return;
+
+ /*
+ * The HFI instance of this @cpu may exist already but they have not
+ * been linked to @cpu.
+ */
+ hfi_instance = info->hfi_instance;
+ if (!hfi_instance) {
+ if (die_id < 0 || die_id >= max_hfi_instances)
+ return;
+
+ /*
+ * Link @cpu to the HFI instance of its package/die. It does
+ * not matter whether the instance has been initialized.
+ */
+ hfi_instance = &hfi_instances[die_id];
+ info->hfi_instance = hfi_instance;
+ }
+
+ init_hfi_cpu_index(info);
+
+ /*
+ * Now check if the HFI instance of the package/die of @cpu has been
+ * initialized (by checking its header). In such case, all we have to
+ * do is to add @cpu to this instance's cpumask.
+ */
+ mutex_lock(&hfi_instance_lock);
+ if (hfi_instance->hdr) {
+ cpumask_set_cpu(cpu, hfi_instance->cpus);
+ goto unlock;
+ }
+
+ /*
+ * Hardware is programmed with the physical address of the first page
+ * frame of the table. Hence, the allocated memory must be page-aligned.
+ */
+ hfi_instance->hw_table = alloc_pages_exact(hfi_features.nr_table_pages,
+ GFP_KERNEL | __GFP_ZERO);
+ if (!hfi_instance->hw_table)
+ goto unlock;
+
+ hw_table_pa = virt_to_phys(hfi_instance->hw_table);
+
+ /*
+ * Allocate memory to keep a local copy of the table that
+ * hardware generates.
+ */
+ hfi_instance->local_table = kzalloc(hfi_features.nr_table_pages << PAGE_SHIFT,
+ GFP_KERNEL);
+ if (!hfi_instance->local_table)
+ goto free_hw_table;
+
+ /*
+ * Program the address of the feedback table of this die/package. On
+ * some processors, hardware remembers the old address of the HFI table
+ * even after having been reprogrammed and re-enabled. Thus, do not free
+ * the pages allocated for the table or reprogram the hardware with a
+ * new base address. Namely, program the hardware only once.
+ */
+ msr_val = hw_table_pa | HW_FEEDBACK_PTR_VALID_BIT;
+ wrmsrl(MSR_IA32_HW_FEEDBACK_PTR, msr_val);
+
+ init_hfi_instance(hfi_instance);
+
+ cpumask_set_cpu(cpu, hfi_instance->cpus);
+
+unlock:
+ mutex_unlock(&hfi_instance_lock);
+ return;
+
+free_hw_table:
+ free_pages_exact(hfi_instance->hw_table, hfi_features.nr_table_pages);
+ goto unlock;
+}
+
+/**
+ * intel_hfi_offline() - Disable HFI on @cpu
+ * @cpu: CPU in which the HFI will be disabled
+ *
+ * Remove @cpu from those covered by its HFI instance.
+ *
+ * On some processors, hardware remembers previous programming settings even
+ * after being reprogrammed. Thus, keep HFI enabled even if all CPUs in the
+ * die/package of @cpu are offline. See note in intel_hfi_online().
+ */
+void intel_hfi_offline(unsigned int cpu)
+{
+ struct hfi_cpu_info *info = &per_cpu(hfi_cpu_info, cpu);
+ struct hfi_instance *hfi_instance;
+
+ if (!boot_cpu_has(X86_FEATURE_HFI))
+ return;
+
+ hfi_instance = info->hfi_instance;
+ if (!hfi_instance)
+ return;
+
+ /* A non-NULL header means this HFI instance has been initialized. */
+ if (!hfi_instance->hdr)
+ return;
+
+ mutex_lock(&hfi_instance_lock);
+ cpumask_clear_cpu(cpu, hfi_instance->cpus);
+ mutex_unlock(&hfi_instance_lock);
+}
static __init int hfi_parse_features(void)
{
@@ -165,6 +348,9 @@ static __init int hfi_parse_features(void)
void __init intel_hfi_init(void)
{
+ struct hfi_instance *hfi_instance;
+ int i;
+
if (hfi_parse_features())
return;
@@ -178,4 +364,22 @@ void __init intel_hfi_init(void)
*/
hfi_instances = kcalloc(max_hfi_instances, sizeof(*hfi_instances),
GFP_KERNEL);
+ if (!hfi_instances)
+ return;
+
+ for (i = 0; i < max_hfi_instances; i++) {
+ hfi_instance = &hfi_instances[i];
+ if (!zalloc_cpumask_var(&hfi_instance->cpus, GFP_KERNEL))
+ goto err_nomem;
+ }
+
+ return;
+
+err_nomem:
+ for (i = 0; i < max_hfi_instances; i++) {
+ hfi_instance = &hfi_instances[i];
+ free_cpumask_var(hfi_instance->cpus);
+ }
+
+ kfree(hfi_instances);
}
diff --git a/drivers/thermal/intel/intel_hfi.h b/drivers/thermal/intel/intel_hfi.h
index 05f748b48a4e..56c6b2d75202 100644
--- a/drivers/thermal/intel/intel_hfi.h
+++ b/drivers/thermal/intel/intel_hfi.h
@@ -4,8 +4,12 @@
#if defined(CONFIG_INTEL_HFI_THERMAL)
void __init intel_hfi_init(void);
+void intel_hfi_online(unsigned int cpu);
+void intel_hfi_offline(unsigned int cpu);
#else
static inline void intel_hfi_init(void) { }
+static inline void intel_hfi_online(unsigned int cpu) { }
+static inline void intel_hfi_offline(unsigned int cpu) { }
#endif /* CONFIG_INTEL_HFI_THERMAL */
#endif /* _INTEL_HFI_H */
diff --git a/drivers/thermal/intel/therm_throt.c b/drivers/thermal/intel/therm_throt.c
index ac408714d52b..2a79598a7f7a 100644
--- a/drivers/thermal/intel/therm_throt.c
+++ b/drivers/thermal/intel/therm_throt.c
@@ -480,6 +480,12 @@ static int thermal_throttle_online(unsigned int cpu)
l = apic_read(APIC_LVTTHMR);
apic_write(APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
+ /*
+ * Enable the package-level HFI interrupt. By now the local APIC is
+ * ready to get thermal interrupts.
+ */
+ intel_hfi_online(cpu);
+
return thermal_throttle_add_dev(dev, cpu);
}
@@ -489,6 +495,8 @@ static int thermal_throttle_offline(unsigned int cpu)
struct device *dev = get_cpu_device(cpu);
u32 l;
+ intel_hfi_offline(cpu);
+
/* Mask the thermal vector before draining evtl. pending work */
l = apic_read(APIC_LVTTHMR);
apic_write(APIC_LVTTHMR, l | APIC_LVT_MASKED);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 5/7] thermal: intel: hfi: Enable notification interrupt
2022-01-08 3:47 [PATCH v4 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
` (3 preceding siblings ...)
2022-01-08 3:47 ` [PATCH v4 4/7] thermal: intel: hfi: Handle CPU hotplug events Ricardo Neri
@ 2022-01-08 3:47 ` Ricardo Neri
2022-01-12 19:47 ` Rafael J. Wysocki
2022-01-08 3:47 ` [PATCH v4 6/7] thermal: netlink: Add a new event to notify CPU capabilities change Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 7/7] thermal: intel: hfi: Notify user space for HFI events Ricardo Neri
6 siblings, 1 reply; 14+ messages in thread
From: Ricardo Neri @ 2022-01-08 3:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm
Cc: x86, linux-doc, Len Brown, Srinivas Pandruvada, Aubrey Li,
Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, linux-kernel, Ricardo Neri
When hardware wants to inform the operating system about updates in the HFI
table, it issues a package-level thermal event interrupt. For this,
hardware has new interrupt and status bits in the IA32_PACKAGE_THERM_
INTERRUPT and IA32_PACKAGE_THERM_STATUS registers. The existing thermal
throttle driver already handles thermal event interrupts: it initializes
the thermal vector of the local APIC as well as per-CPU and package-level
interrupt reporting. It also provides routines to service such interrupts.
Extend its functionality to also handle HFI interrupts.
The frequency of the thermal HFI interrupt is specific to each processor
model. On some processors, a single interrupt happens as soon as the HFI is
enabled and hardware will never update HFI capabilities afterwards. On
other processors, thermal and power constraints may cause thermal HFI
interrupts every tens of milliseconds.
To not overwhelm consumers of the HFI data, use delayed work to throttle
the rate at which HFI updates are processed.
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* None
Changes since v2:
* Added missing #include files.
Changes since v1:
* Renamed X86_FEATURE_INTEL_HFI as X86_FEATURE_HFI. (Boris)
* Repurposed hfi_instance::event_lock to handle HFI interrupt
events to avoid keeping CPUs spinning needlessly. Added a new
hfi_instance::table_lock to serialize access to an HFI table.
(PeterZ)
* Replaced raw_spin_[un]lock[restore|save]() with raw_spin_[un]lock().
intel_hfi_process_event() runs in interrupt context and hence there
is no need to save interrupt flags.
* Renamed HFI_CONFIG_ENABLE_BIT as HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT
for clarity.
* Reworked timestamp updates for readability. Removed redundant
hfi_instance::timestamp.
* Relaxed timestamp check to allow time wrap-around.
---
drivers/thermal/intel/intel_hfi.c | 103 ++++++++++++++++++++++++++++
drivers/thermal/intel/intel_hfi.h | 2 +
drivers/thermal/intel/therm_throt.c | 10 +++
3 files changed, 115 insertions(+)
diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
index 37f979a9e833..1a08c58f26f6 100644
--- a/drivers/thermal/intel/intel_hfi.c
+++ b/drivers/thermal/intel/intel_hfi.c
@@ -26,20 +26,28 @@
#include <linux/cpumask.h>
#include <linux/gfp.h>
#include <linux/io.h>
+#include <linux/kernel.h>
#include <linux/math.h>
#include <linux/mutex.h>
#include <linux/percpu-defs.h>
#include <linux/printk.h>
#include <linux/processor.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
#include <linux/topology.h>
+#include <linux/workqueue.h>
#include <asm/msr.h>
#include "intel_hfi.h"
+#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | \
+ BIT(9) | BIT(11) | BIT(26))
+
/* Hardware Feedback Interface MSR configuration bits */
#define HW_FEEDBACK_PTR_VALID_BIT BIT(0)
+#define HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT BIT(0)
/* CPUID detection and enumeration definitions for HFI */
@@ -98,6 +106,9 @@ struct hfi_hdr {
* @data: Base address of the local table data
* @cpus: CPUs represented in this HFI table instance
* @hw_table: Pointer to the HFI table of this instance
+ * @update_work: Delayed work to process HFI updates
+ * @table_lock: Lock to protect acceses to the table of this instance
+ * @event_lock: Lock to process HFI interrupts
*
* A set of parameters to parse and navigate a specific HFI table.
*/
@@ -110,6 +121,9 @@ struct hfi_instance {
void *data;
cpumask_var_t cpus;
void *hw_table;
+ struct delayed_work update_work;
+ raw_spinlock_t table_lock;
+ raw_spinlock_t event_lock;
};
/**
@@ -147,6 +161,83 @@ static struct hfi_instance *hfi_instances;
static struct hfi_features hfi_features;
static DEFINE_MUTEX(hfi_instance_lock);
+#define HFI_UPDATE_INTERVAL HZ
+
+static void hfi_update_work_fn(struct work_struct *work)
+{
+ struct hfi_instance *hfi_instance;
+
+ hfi_instance = container_of(to_delayed_work(work), struct hfi_instance,
+ update_work);
+ if (!hfi_instance)
+ return;
+
+ /* TODO: Consume update here. */
+}
+
+void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
+{
+ struct hfi_instance *hfi_instance;
+ int cpu = smp_processor_id();
+ struct hfi_cpu_info *info;
+ u64 new_timestamp;
+
+ if (!pkg_therm_status_msr_val)
+ return;
+
+ info = &per_cpu(hfi_cpu_info, cpu);
+ if (!info)
+ return;
+
+ /*
+ * It is possible that we get an HFI thermal interrupt on this CPU
+ * before its HFI instance is initialized. This is not a problem. The
+ * CPU that enabled the interrupt for this package will also get the
+ * interrupt and is fully initialized.
+ */
+ hfi_instance = info->hfi_instance;
+ if (!hfi_instance)
+ return;
+
+ /*
+ * On most systems, all CPUs in the package receive a package-level
+ * thermal interrupt when there is an HFI update. It is sufficient to
+ * let a single CPU to acknowledge the update and schedule work to
+ * process it. The remaining CPUs can resume their work.
+ */
+ if (!raw_spin_trylock(&hfi_instance->event_lock))
+ return;
+
+ /* Skip duplicated updates. */
+ new_timestamp = *(u64 *)hfi_instance->hw_table;
+ if (*hfi_instance->timestamp == new_timestamp) {
+ raw_spin_unlock(&hfi_instance->event_lock);
+ return;
+ }
+
+ raw_spin_lock(&hfi_instance->table_lock);
+
+ /*
+ * Copy the updated table into our local copy. This includes the new
+ * timestamp.
+ */
+ memcpy(hfi_instance->local_table, hfi_instance->hw_table,
+ hfi_features.nr_table_pages << PAGE_SHIFT);
+
+ raw_spin_unlock(&hfi_instance->table_lock);
+ raw_spin_unlock(&hfi_instance->event_lock);
+
+ /*
+ * Let hardware know that we are done reading the HFI table and it is
+ * free to update it again.
+ */
+ pkg_therm_status_msr_val &= THERM_STATUS_CLEAR_PKG_MASK &
+ ~PACKAGE_THERM_STATUS_HFI_UPDATED;
+ wrmsrl(MSR_IA32_PACKAGE_THERM_STATUS, pkg_therm_status_msr_val);
+
+ schedule_delayed_work(&hfi_instance->update_work, HFI_UPDATE_INTERVAL);
+}
+
static void init_hfi_cpu_index(struct hfi_cpu_info *info)
{
union cpuid6_edx edx;
@@ -259,8 +350,20 @@ void intel_hfi_online(unsigned int cpu)
init_hfi_instance(hfi_instance);
+ INIT_DELAYED_WORK(&hfi_instance->update_work, hfi_update_work_fn);
+ raw_spin_lock_init(&hfi_instance->table_lock);
+ raw_spin_lock_init(&hfi_instance->event_lock);
+
cpumask_set_cpu(cpu, hfi_instance->cpus);
+ /*
+ * Enable the hardware feedback interface and never disable it. See
+ * comment on programming the address of the table.
+ */
+ rdmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val);
+ msr_val |= HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT;
+ wrmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val);
+
unlock:
mutex_unlock(&hfi_instance_lock);
return;
diff --git a/drivers/thermal/intel/intel_hfi.h b/drivers/thermal/intel/intel_hfi.h
index 56c6b2d75202..325aa78b745c 100644
--- a/drivers/thermal/intel/intel_hfi.h
+++ b/drivers/thermal/intel/intel_hfi.h
@@ -6,10 +6,12 @@
void __init intel_hfi_init(void);
void intel_hfi_online(unsigned int cpu);
void intel_hfi_offline(unsigned int cpu);
+void intel_hfi_process_event(__u64 pkg_therm_status_msr_val);
#else
static inline void intel_hfi_init(void) { }
static inline void intel_hfi_online(unsigned int cpu) { }
static inline void intel_hfi_offline(unsigned int cpu) { }
+static inline void intel_hfi_process_event(__u64 pkg_therm_status_msr_val) { }
#endif /* CONFIG_INTEL_HFI_THERMAL */
#endif /* _INTEL_HFI_H */
diff --git a/drivers/thermal/intel/therm_throt.c b/drivers/thermal/intel/therm_throt.c
index 2a79598a7f7a..930e19eeeac6 100644
--- a/drivers/thermal/intel/therm_throt.c
+++ b/drivers/thermal/intel/therm_throt.c
@@ -619,6 +619,10 @@ void intel_thermal_interrupt(void)
PACKAGE_THERM_STATUS_POWER_LIMIT,
POWER_LIMIT_EVENT,
PACKAGE_LEVEL);
+
+ if (this_cpu_has(X86_FEATURE_HFI))
+ intel_hfi_process_event(msr_val &
+ PACKAGE_THERM_STATUS_HFI_UPDATED);
}
}
@@ -728,6 +732,12 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
wrmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT,
l | (PACKAGE_THERM_INT_LOW_ENABLE
| PACKAGE_THERM_INT_HIGH_ENABLE), h);
+
+ if (cpu_has(c, X86_FEATURE_HFI)) {
+ rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, l, h);
+ wrmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT,
+ l | PACKAGE_THERM_INT_HFI_ENABLE, h);
+ }
}
rdmsr(MSR_IA32_MISC_ENABLE, l, h);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 5/7] thermal: intel: hfi: Enable notification interrupt
2022-01-08 3:47 ` [PATCH v4 5/7] thermal: intel: hfi: Enable notification interrupt Ricardo Neri
@ 2022-01-12 19:47 ` Rafael J. Wysocki
2022-01-12 23:26 ` Srinivas Pandruvada
0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2022-01-12 19:47 UTC (permalink / raw)
To: Ricardo Neri
Cc: Rafael J. Wysocki, Daniel Lezcano, Linux PM,
the arch/x86 maintainers, open list:DOCUMENTATION, Len Brown,
Srinivas Pandruvada, Aubrey Li, Amit Kucheria, Andi Kleen,
Tim Chen, Lukasz Luba, Ravi V. Shankar, Ricardo Neri,
Linux Kernel Mailing List
On Sat, Jan 8, 2022 at 4:46 AM Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
>
> When hardware wants to inform the operating system about updates in the HFI
> table, it issues a package-level thermal event interrupt. For this,
> hardware has new interrupt and status bits in the IA32_PACKAGE_THERM_
> INTERRUPT and IA32_PACKAGE_THERM_STATUS registers. The existing thermal
> throttle driver already handles thermal event interrupts: it initializes
> the thermal vector of the local APIC as well as per-CPU and package-level
> interrupt reporting. It also provides routines to service such interrupts.
> Extend its functionality to also handle HFI interrupts.
>
> The frequency of the thermal HFI interrupt is specific to each processor
> model. On some processors, a single interrupt happens as soon as the HFI is
> enabled and hardware will never update HFI capabilities afterwards. On
> other processors, thermal and power constraints may cause thermal HFI
> interrupts every tens of milliseconds.
>
> To not overwhelm consumers of the HFI data, use delayed work to throttle
> the rate at which HFI updates are processed.
>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Aubrey Li <aubrey.li@linux.intel.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
> Reviewed-by: Len Brown <len.brown@intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v3:
> * None
>
> Changes since v2:
> * Added missing #include files.
>
> Changes since v1:
> * Renamed X86_FEATURE_INTEL_HFI as X86_FEATURE_HFI. (Boris)
> * Repurposed hfi_instance::event_lock to handle HFI interrupt
> events to avoid keeping CPUs spinning needlessly. Added a new
> hfi_instance::table_lock to serialize access to an HFI table.
> (PeterZ)
> * Replaced raw_spin_[un]lock[restore|save]() with raw_spin_[un]lock().
> intel_hfi_process_event() runs in interrupt context and hence there
> is no need to save interrupt flags.
> * Renamed HFI_CONFIG_ENABLE_BIT as HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT
> for clarity.
> * Reworked timestamp updates for readability. Removed redundant
> hfi_instance::timestamp.
> * Relaxed timestamp check to allow time wrap-around.
> ---
> drivers/thermal/intel/intel_hfi.c | 103 ++++++++++++++++++++++++++++
> drivers/thermal/intel/intel_hfi.h | 2 +
> drivers/thermal/intel/therm_throt.c | 10 +++
> 3 files changed, 115 insertions(+)
>
> diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
> index 37f979a9e833..1a08c58f26f6 100644
> --- a/drivers/thermal/intel/intel_hfi.c
> +++ b/drivers/thermal/intel/intel_hfi.c
> @@ -26,20 +26,28 @@
> #include <linux/cpumask.h>
> #include <linux/gfp.h>
> #include <linux/io.h>
> +#include <linux/kernel.h>
> #include <linux/math.h>
> #include <linux/mutex.h>
> #include <linux/percpu-defs.h>
> #include <linux/printk.h>
> #include <linux/processor.h>
> #include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
> #include <linux/topology.h>
> +#include <linux/workqueue.h>
>
> #include <asm/msr.h>
>
> #include "intel_hfi.h"
>
> +#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | \
> + BIT(9) | BIT(11) | BIT(26))
> +
> /* Hardware Feedback Interface MSR configuration bits */
> #define HW_FEEDBACK_PTR_VALID_BIT BIT(0)
> +#define HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT BIT(0)
>
> /* CPUID detection and enumeration definitions for HFI */
>
> @@ -98,6 +106,9 @@ struct hfi_hdr {
> * @data: Base address of the local table data
> * @cpus: CPUs represented in this HFI table instance
> * @hw_table: Pointer to the HFI table of this instance
> + * @update_work: Delayed work to process HFI updates
> + * @table_lock: Lock to protect acceses to the table of this instance
> + * @event_lock: Lock to process HFI interrupts
> *
> * A set of parameters to parse and navigate a specific HFI table.
> */
> @@ -110,6 +121,9 @@ struct hfi_instance {
> void *data;
> cpumask_var_t cpus;
> void *hw_table;
> + struct delayed_work update_work;
> + raw_spinlock_t table_lock;
> + raw_spinlock_t event_lock;
> };
>
> /**
> @@ -147,6 +161,83 @@ static struct hfi_instance *hfi_instances;
> static struct hfi_features hfi_features;
> static DEFINE_MUTEX(hfi_instance_lock);
>
> +#define HFI_UPDATE_INTERVAL HZ
> +
> +static void hfi_update_work_fn(struct work_struct *work)
> +{
> + struct hfi_instance *hfi_instance;
> +
> + hfi_instance = container_of(to_delayed_work(work), struct hfi_instance,
> + update_work);
> + if (!hfi_instance)
> + return;
> +
> + /* TODO: Consume update here. */
> +}
> +
> +void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
> +{
> + struct hfi_instance *hfi_instance;
> + int cpu = smp_processor_id();
> + struct hfi_cpu_info *info;
> + u64 new_timestamp;
> +
> + if (!pkg_therm_status_msr_val)
> + return;
> +
> + info = &per_cpu(hfi_cpu_info, cpu);
> + if (!info)
> + return;
> +
> + /*
> + * It is possible that we get an HFI thermal interrupt on this CPU
> + * before its HFI instance is initialized. This is not a problem. The
> + * CPU that enabled the interrupt for this package will also get the
> + * interrupt and is fully initialized.
> + */
> + hfi_instance = info->hfi_instance;
> + if (!hfi_instance)
> + return;
Generally, a CPU whose info has been initialized can be offline, so
this code may run on an offline CPU.
I'm not actually sure if this is a concern, but just mentioning it in
case it is.
> +
> + /*
> + * On most systems, all CPUs in the package receive a package-level
> + * thermal interrupt when there is an HFI update. It is sufficient to
> + * let a single CPU to acknowledge the update and schedule work to
> + * process it. The remaining CPUs can resume their work.
> + */
> + if (!raw_spin_trylock(&hfi_instance->event_lock))
> + return;
> +
> + /* Skip duplicated updates. */
> + new_timestamp = *(u64 *)hfi_instance->hw_table;
> + if (*hfi_instance->timestamp == new_timestamp) {
> + raw_spin_unlock(&hfi_instance->event_lock);
> + return;
> + }
> +
> + raw_spin_lock(&hfi_instance->table_lock);
> +
> + /*
> + * Copy the updated table into our local copy. This includes the new
> + * timestamp.
> + */
> + memcpy(hfi_instance->local_table, hfi_instance->hw_table,
> + hfi_features.nr_table_pages << PAGE_SHIFT);
> +
> + raw_spin_unlock(&hfi_instance->table_lock);
> + raw_spin_unlock(&hfi_instance->event_lock);
> +
> + /*
> + * Let hardware know that we are done reading the HFI table and it is
> + * free to update it again.
> + */
> + pkg_therm_status_msr_val &= THERM_STATUS_CLEAR_PKG_MASK &
> + ~PACKAGE_THERM_STATUS_HFI_UPDATED;
> + wrmsrl(MSR_IA32_PACKAGE_THERM_STATUS, pkg_therm_status_msr_val);
> +
> + schedule_delayed_work(&hfi_instance->update_work, HFI_UPDATE_INTERVAL);
AFAICS, if update_work has been scheduled already, but is not pending
yet, the delay will be set to the current time plus
HFI_UPDATE_INTERVAL, but shouldn't it actually run earlier in that
case?
Also it looks like the processing introduced in the next patch can
take quite a bit of time if there is a sufficiently large number of
CPUs in the package, so is it suitable for system_wq in all cases?
> +}
> +
> static void init_hfi_cpu_index(struct hfi_cpu_info *info)
> {
> union cpuid6_edx edx;
> @@ -259,8 +350,20 @@ void intel_hfi_online(unsigned int cpu)
>
> init_hfi_instance(hfi_instance);
>
> + INIT_DELAYED_WORK(&hfi_instance->update_work, hfi_update_work_fn);
> + raw_spin_lock_init(&hfi_instance->table_lock);
> + raw_spin_lock_init(&hfi_instance->event_lock);
> +
> cpumask_set_cpu(cpu, hfi_instance->cpus);
>
> + /*
> + * Enable the hardware feedback interface and never disable it. See
> + * comment on programming the address of the table.
> + */
> + rdmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val);
> + msr_val |= HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT;
> + wrmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val);
> +
> unlock:
> mutex_unlock(&hfi_instance_lock);
> return;
> diff --git a/drivers/thermal/intel/intel_hfi.h b/drivers/thermal/intel/intel_hfi.h
> index 56c6b2d75202..325aa78b745c 100644
> --- a/drivers/thermal/intel/intel_hfi.h
> +++ b/drivers/thermal/intel/intel_hfi.h
> @@ -6,10 +6,12 @@
> void __init intel_hfi_init(void);
> void intel_hfi_online(unsigned int cpu);
> void intel_hfi_offline(unsigned int cpu);
> +void intel_hfi_process_event(__u64 pkg_therm_status_msr_val);
> #else
> static inline void intel_hfi_init(void) { }
> static inline void intel_hfi_online(unsigned int cpu) { }
> static inline void intel_hfi_offline(unsigned int cpu) { }
> +static inline void intel_hfi_process_event(__u64 pkg_therm_status_msr_val) { }
> #endif /* CONFIG_INTEL_HFI_THERMAL */
>
> #endif /* _INTEL_HFI_H */
> diff --git a/drivers/thermal/intel/therm_throt.c b/drivers/thermal/intel/therm_throt.c
> index 2a79598a7f7a..930e19eeeac6 100644
> --- a/drivers/thermal/intel/therm_throt.c
> +++ b/drivers/thermal/intel/therm_throt.c
> @@ -619,6 +619,10 @@ void intel_thermal_interrupt(void)
> PACKAGE_THERM_STATUS_POWER_LIMIT,
> POWER_LIMIT_EVENT,
> PACKAGE_LEVEL);
> +
> + if (this_cpu_has(X86_FEATURE_HFI))
> + intel_hfi_process_event(msr_val &
> + PACKAGE_THERM_STATUS_HFI_UPDATED);
> }
> }
>
> @@ -728,6 +732,12 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> wrmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT,
> l | (PACKAGE_THERM_INT_LOW_ENABLE
> | PACKAGE_THERM_INT_HIGH_ENABLE), h);
> +
> + if (cpu_has(c, X86_FEATURE_HFI)) {
> + rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, l, h);
> + wrmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT,
> + l | PACKAGE_THERM_INT_HFI_ENABLE, h);
> + }
> }
>
> rdmsr(MSR_IA32_MISC_ENABLE, l, h);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 5/7] thermal: intel: hfi: Enable notification interrupt
2022-01-12 19:47 ` Rafael J. Wysocki
@ 2022-01-12 23:26 ` Srinivas Pandruvada
2022-01-13 13:48 ` Ricardo Neri
0 siblings, 1 reply; 14+ messages in thread
From: Srinivas Pandruvada @ 2022-01-12 23:26 UTC (permalink / raw)
To: Rafael J. Wysocki, Ricardo Neri
Cc: Rafael J. Wysocki, Daniel Lezcano, Linux PM,
the arch/x86 maintainers, open list:DOCUMENTATION, Len Brown,
Aubrey Li, Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, Linux Kernel Mailing List
On Wed, 2022-01-12 at 20:47 +0100, Rafael J. Wysocki wrote:
> On Sat, Jan 8, 2022 at 4:46 AM Ricardo Neri
> <ricardo.neri-calderon@linux.intel.com> wrote:
> > When hardware wants to inform the operating system about updates in
> > the HFI
> > table, it issues a package-level thermal event interrupt. For this,
> > hardware has new interrupt and status bits in the
> > IA32_PACKAGE_THERM_
> > INTERRUPT and IA32_PACKAGE_THERM_STATUS registers. The existing
> > thermal
> > throttle driver already handles thermal event interrupts: it
> > initializes
> > the thermal vector of the local APIC as well as per-CPU and
> > package-level
> > interrupt reporting. It also provides routines to service such
> > interrupts.
> > Extend its functionality to also handle HFI interrupts.
> >
> > The frequency of the thermal HFI interrupt is specific to each
> > processor
> > model. On some processors, a single interrupt happens as soon as
> > the HFI is
> > enabled and hardware will never update HFI capabilities afterwards.
> > On
> > other processors, thermal and power constraints may cause thermal
> > HFI
> > interrupts every tens of milliseconds.
> >
> > To not overwhelm consumers of the HFI data, use delayed work to
> > throttle
> > the rate at which HFI updates are processed.
> >
> >
[...]
> > +void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
> > +{
> > + struct hfi_instance *hfi_instance;
> > + int cpu = smp_processor_id();
> > + struct hfi_cpu_info *info;
> > + u64 new_timestamp;
> > +
> > + if (!pkg_therm_status_msr_val)
> > + return;
> > +
> > + info = &per_cpu(hfi_cpu_info, cpu);
> > + if (!info)
> > + return;
> > +
> > + /*
> > + * It is possible that we get an HFI thermal interrupt on
> > this CPU
> > + * before its HFI instance is initialized.
Although this code can handle this situation, you can avoid this.
You can call intel_hfi_online(cpu) before
"
l = apic_read(APIC_LVTTHMR);
apic_write(APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
"
in thermal_throttle_online()
In the same way call intel_hfi_offline(cpu)
after
/* Mask the thermal vector before draining evtl. pending work */
l = apic_read(APIC_LVTTHMR);
apic_write(APIC_LVTTHMR, l | APIC_LVT_MASKED);
in thermal_throttle_offline()
> > This is not a problem. The
> > + * CPU that enabled the interrupt for this package will
> > also get the
> > + * interrupt and is fully initialized.
> > + */
> > + hfi_instance = info->hfi_instance;
> > + if (!hfi_instance)
> > + return;
>
> Generally, a CPU whose info has been initialized can be offline, so
> this code may run on an offline CPU.
>
> I'm not actually sure if this is a concern, but just mentioning it in
> case it is.
>
It will not matter as the handler of the message should be handle case
as CPU can go offline later after the message even if the CPU was
offline.
But I think we can avoid this situation.
> > +
> > + /*
> > + * On most systems, all CPUs in the package receive a
> > package-level
> > + * thermal interrupt when there is an HFI update. It is
> > sufficient to
> > + * let a single CPU to acknowledge the update and schedule
> > work to
> > + * process it. The remaining CPUs can resume their work.
> > + */
> > + if (!raw_spin_trylock(&hfi_instance->event_lock))
> > + return;
> > +
> > + /* Skip duplicated updates. */
> > + new_timestamp = *(u64 *)hfi_instance->hw_table;
> > + if (*hfi_instance->timestamp == new_timestamp) {
> > + raw_spin_unlock(&hfi_instance->event_lock);
> > + return;
> > + }
> > +
> > + raw_spin_lock(&hfi_instance->table_lock);
> > +
> > + /*
> > + * Copy the updated table into our local copy. This
> > includes the new
> > + * timestamp.
> > + */
> > + memcpy(hfi_instance->local_table, hfi_instance->hw_table,
> > + hfi_features.nr_table_pages << PAGE_SHIFT);
> > +
> > + raw_spin_unlock(&hfi_instance->table_lock);
> > + raw_spin_unlock(&hfi_instance->event_lock);
> > +
> > + /*
> > + * Let hardware know that we are done reading the HFI table
> > and it is
> > + * free to update it again.
> > + */
> > + pkg_therm_status_msr_val &= THERM_STATUS_CLEAR_PKG_MASK &
> > + ~PACKAGE_THERM_STATUS_HFI_UPDAT
> > ED;
> > + wrmsrl(MSR_IA32_PACKAGE_THERM_STATUS,
> > pkg_therm_status_msr_val);
> > +
> > + schedule_delayed_work(&hfi_instance->update_work,
> > HFI_UPDATE_INTERVAL);
>
> AFAICS,
For my understanding:
> if update_work has been scheduled already,
queue_delayed_work_on is called
> but is not pending
> yet, the delay will be set to the current time plus
> HFI_UPDATE_INTERVAL, but shouldn't it actually run earlier in that
> case?
"
if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT,
work_data_bits(work))) {
__queue_delayed_work(cpu, wq, dwork, delay);
ret = true;
}
"
Pending bit will be set only one time, so delay will be from
the first call of queue_delayed_work_on() + HFI_UPDATE_INTERVAL.
So on subsequent calls of schedule_delayed_work() the delay is always
with reference to first call.
>
> Also it looks like the processing introduced in the next patch can
> take quite a bit of time if there is a sufficiently large number of
> CPUs in the package, so is it suitable for system_wq in all cases?
>
Good question. What is the threshold of not using system_wq?
With current set of max cpus/package, I did experiments with busy
systems with a test functions with several workqueues and check if they
drift in expiry. But I think we can move away from system_wq. Can this
be add on patch?
Thanks,
Srinivas
> > +}
> > +
> > static void init_hfi_cpu_index(struct hfi_cpu_info *info)
> > {
> > union cpuid6_edx edx;
> > @@ -259,8 +350,20 @@ void intel_hfi_online(unsigned int cpu)
> >
> > init_hfi_instance(hfi_instance);
> >
> > + INIT_DELAYED_WORK(&hfi_instance->update_work,
> > hfi_update_work_fn);
> > + raw_spin_lock_init(&hfi_instance->table_lock);
> > + raw_spin_lock_init(&hfi_instance->event_lock);
> > +
> > cpumask_set_cpu(cpu, hfi_instance->cpus);
> >
> > + /*
> > + * Enable the hardware feedback interface and never disable
> > it. See
> > + * comment on programming the address of the table.
> > + */
> > + rdmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val);
> > + msr_val |= HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT;
> > + wrmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val);
> > +
> > unlock:
> > mutex_unlock(&hfi_instance_lock);
> > return;
> > diff --git a/drivers/thermal/intel/intel_hfi.h
> > b/drivers/thermal/intel/intel_hfi.h
> > index 56c6b2d75202..325aa78b745c 100644
> > --- a/drivers/thermal/intel/intel_hfi.h
> > +++ b/drivers/thermal/intel/intel_hfi.h
> > @@ -6,10 +6,12 @@
> > void __init intel_hfi_init(void);
> > void intel_hfi_online(unsigned int cpu);
> > void intel_hfi_offline(unsigned int cpu);
> > +void intel_hfi_process_event(__u64 pkg_therm_status_msr_val);
> > #else
> > static inline void intel_hfi_init(void) { }
> > static inline void intel_hfi_online(unsigned int cpu) { }
> > static inline void intel_hfi_offline(unsigned int cpu) { }
> > +static inline void intel_hfi_process_event(__u64
> > pkg_therm_status_msr_val) { }
> > #endif /* CONFIG_INTEL_HFI_THERMAL */
> >
> > #endif /* _INTEL_HFI_H */
> > diff --git a/drivers/thermal/intel/therm_throt.c
> > b/drivers/thermal/intel/therm_throt.c
> > index 2a79598a7f7a..930e19eeeac6 100644
> > --- a/drivers/thermal/intel/therm_throt.c
> > +++ b/drivers/thermal/intel/therm_throt.c
> > @@ -619,6 +619,10 @@ void intel_thermal_interrupt(void)
> > PACKAGE_THERM_STATUS_POWER_
> > LIMIT,
> > POWER_LIMIT_EVENT,
> > PACKAGE_LEVEL);
> > +
> > + if (this_cpu_has(X86_FEATURE_HFI))
> > + intel_hfi_process_event(msr_val &
> > + PACKAGE_THERM_STATU
> > S_HFI_UPDATED);
> > }
> > }
> >
> > @@ -728,6 +732,12 @@ void intel_init_thermal(struct cpuinfo_x86 *c)
> > wrmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT,
> > l | (PACKAGE_THERM_INT_LOW_ENABLE
> > | PACKAGE_THERM_INT_HIGH_ENABLE),
> > h);
> > +
> > + if (cpu_has(c, X86_FEATURE_HFI)) {
> > + rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, l,
> > h);
> > + wrmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT,
> > + l | PACKAGE_THERM_INT_HFI_ENABLE, h);
> > + }
> > }
> >
> > rdmsr(MSR_IA32_MISC_ENABLE, l, h);
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 5/7] thermal: intel: hfi: Enable notification interrupt
2022-01-12 23:26 ` Srinivas Pandruvada
@ 2022-01-13 13:48 ` Ricardo Neri
0 siblings, 0 replies; 14+ messages in thread
From: Ricardo Neri @ 2022-01-13 13:48 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Daniel Lezcano, Linux PM,
the arch/x86 maintainers, open list:DOCUMENTATION, Len Brown,
Aubrey Li, Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, Linux Kernel Mailing List
On Wed, Jan 12, 2022 at 03:26:41PM -0800, Srinivas Pandruvada wrote:
> On Wed, 2022-01-12 at 20:47 +0100, Rafael J. Wysocki wrote:
> > On Sat, Jan 8, 2022 at 4:46 AM Ricardo Neri
> > <ricardo.neri-calderon@linux.intel.com> wrote:
> > > When hardware wants to inform the operating system about updates in
> > > the HFI
> > > table, it issues a package-level thermal event interrupt. For this,
> > > hardware has new interrupt and status bits in the
> > > IA32_PACKAGE_THERM_
> > > INTERRUPT and IA32_PACKAGE_THERM_STATUS registers. The existing
> > > thermal
> > > throttle driver already handles thermal event interrupts: it
> > > initializes
> > > the thermal vector of the local APIC as well as per-CPU and
> > > package-level
> > > interrupt reporting. It also provides routines to service such
> > > interrupts.
> > > Extend its functionality to also handle HFI interrupts.
> > >
> > > The frequency of the thermal HFI interrupt is specific to each
> > > processor
> > > model. On some processors, a single interrupt happens as soon as
> > > the HFI is
> > > enabled and hardware will never update HFI capabilities afterwards.
> > > On
> > > other processors, thermal and power constraints may cause thermal
> > > HFI
> > > interrupts every tens of milliseconds.
> > >
> > > To not overwhelm consumers of the HFI data, use delayed work to
> > > throttle
> > > the rate at which HFI updates are processed.
> > >
> > >
>
> [...]
>
> > > +void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
> > > +{
> > > + struct hfi_instance *hfi_instance;
> > > + int cpu = smp_processor_id();
> > > + struct hfi_cpu_info *info;
> > > + u64 new_timestamp;
> > > +
> > > + if (!pkg_therm_status_msr_val)
> > > + return;
> > > +
> > > + info = &per_cpu(hfi_cpu_info, cpu);
> > > + if (!info)
> > > + return;
> > > +
> > > + /*
> > > + * It is possible that we get an HFI thermal interrupt on
> > > this CPU
> > > + * before its HFI instance is initialized.
> Although this code can handle this situation, you can avoid this.
>
> You can call intel_hfi_online(cpu) before
> "
> l = apic_read(APIC_LVTTHMR);
> apic_write(APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
> "
Indeed this could work.
intel_hfi_online() also enables the HFI. It is possible to get an HFI
interrupt as soon as we enable it. I was concerned about getting the
interrupt with APIC_LVTTHMR masked and miss it. This should not be
a problem since we will get it as soon as we unmask it.
> in thermal_throttle_online()
>
> In the same way call intel_hfi_offline(cpu)
> after
>
> /* Mask the thermal vector before draining evtl. pending work */
> l = apic_read(APIC_LVTTHMR);
> apic_write(APIC_LVTTHMR, l | APIC_LVT_MASKED);
>
> in thermal_throttle_offline()
Sure, I can do this.
>
>
> > > This is not a problem. The
> > > + * CPU that enabled the interrupt for this package will
> > > also get the
> > > + * interrupt and is fully initialized.
> > > + */
> > > + hfi_instance = info->hfi_instance;
> > > + if (!hfi_instance)
> > > + return;
> >
> > Generally, a CPU whose info has been initialized can be offline, so
> > this code may run on an offline CPU.
> >
> > I'm not actually sure if this is a concern, but just mentioning it in
> > case it is.
> >
> It will not matter as the handler of the message should be handle case
> as CPU can go offline later after the message even if the CPU was
> offline.
> But I think we can avoid this situation.
>
> > > +
> > > + /*
> > > + * On most systems, all CPUs in the package receive a
> > > package-level
> > > + * thermal interrupt when there is an HFI update. It is
> > > sufficient to
> > > + * let a single CPU to acknowledge the update and schedule
> > > work to
> > > + * process it. The remaining CPUs can resume their work.
> > > + */
> > > + if (!raw_spin_trylock(&hfi_instance->event_lock))
> > > + return;
> > > +
> > > + /* Skip duplicated updates. */
> > > + new_timestamp = *(u64 *)hfi_instance->hw_table;
> > > + if (*hfi_instance->timestamp == new_timestamp) {
> > > + raw_spin_unlock(&hfi_instance->event_lock);
> > > + return;
> > > + }
> > > +
> > > + raw_spin_lock(&hfi_instance->table_lock);
> > > +
> > > + /*
> > > + * Copy the updated table into our local copy. This
> > > includes the new
> > > + * timestamp.
> > > + */
> > > + memcpy(hfi_instance->local_table, hfi_instance->hw_table,
> > > + hfi_features.nr_table_pages << PAGE_SHIFT);
> > > +
> > > + raw_spin_unlock(&hfi_instance->table_lock);
> > > + raw_spin_unlock(&hfi_instance->event_lock);
> > > +
> > > + /*
> > > + * Let hardware know that we are done reading the HFI table
> > > and it is
> > > + * free to update it again.
> > > + */
> > > + pkg_therm_status_msr_val &= THERM_STATUS_CLEAR_PKG_MASK &
> > > + ~PACKAGE_THERM_STATUS_HFI_UPDAT
> > > ED;
> > > + wrmsrl(MSR_IA32_PACKAGE_THERM_STATUS,
> > > pkg_therm_status_msr_val);
> > > +
> > > + schedule_delayed_work(&hfi_instance->update_work,
> > > HFI_UPDATE_INTERVAL);
> >
> > AFAICS,
> For my understanding:
>
> > if update_work has been scheduled already,
> queue_delayed_work_on is called
>
> > but is not pending
> > yet, the delay will be set to the current time plus
> > HFI_UPDATE_INTERVAL, but shouldn't it actually run earlier in that
> > case?
>
>
> "
> if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT,
> work_data_bits(work))) {
> __queue_delayed_work(cpu, wq, dwork, delay);
> ret = true;
> }
> "
> Pending bit will be set only one time, so delay will be from
> the first call of queue_delayed_work_on() + HFI_UPDATE_INTERVAL.
>
> So on subsequent calls of schedule_delayed_work() the delay is always
> with reference to first call.
>
> >
> > Also it looks like the processing introduced in the next patch can
> > take quite a bit of time if there is a sufficiently large number of
> > CPUs in the package, so is it suitable for system_wq in all cases?
> >
> Good question. What is the threshold of not using system_wq?
> With current set of max cpus/package, I did experiments with busy
> systems with a test functions with several workqueues and check if they
> drift in expiry. But I think we can move away from system_wq. Can this
> be add on patch?
Sure, I can do this.
Thanks and BR,
Ricardo
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 6/7] thermal: netlink: Add a new event to notify CPU capabilities change
2022-01-08 3:47 [PATCH v4 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
` (4 preceding siblings ...)
2022-01-08 3:47 ` [PATCH v4 5/7] thermal: intel: hfi: Enable notification interrupt Ricardo Neri
@ 2022-01-08 3:47 ` Ricardo Neri
2022-01-08 3:47 ` [PATCH v4 7/7] thermal: intel: hfi: Notify user space for HFI events Ricardo Neri
6 siblings, 0 replies; 14+ messages in thread
From: Ricardo Neri @ 2022-01-08 3:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm
Cc: x86, linux-doc, Len Brown, Srinivas Pandruvada, Aubrey Li,
Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, linux-kernel
From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Add a new netlink event to notify change in CPU capabilities in terms of
performance and efficiency.
Firmware may change CPU capabilities as a result of thermal events in the
system or to account for changes in the TDP (thermal design power) level.
This notification type will allow user space to avoid running workloads
on certain CPUs or proactively adjust power limits to avoid future events.
The netlink message consists of a nested attribute
(THERMAL_GENL_ATTR_CPU_CAPABILITY) with three attributes:
* THERMAL_GENL_ATTR_CPU_CAPABILITY_ID (type u32):
-- logical CPU number
* THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE (type u32):
-- Scaled performance from 0-1023
* THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY (type u32):
-- Scaled efficiency from 0-1023
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Lukasz Luba <lukasz.luba@arm.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
Changes since v3:
* None
Changes since v2:
* None
Changes since v1:
* Reworded commit message.
* Reworded the members of struct cpu_capacity for clarity. (Lukasz)
* Defined the CPU capability attributes to be scaled in the [0, 1023]
interval. (Lukasz)
---
drivers/thermal/thermal_netlink.c | 55 +++++++++++++++++++++++++++++++
drivers/thermal/thermal_netlink.h | 13 ++++++++
include/uapi/linux/thermal.h | 6 +++-
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c
index a16dd4d5d710..38e6c20f460c 100644
--- a/drivers/thermal/thermal_netlink.c
+++ b/drivers/thermal/thermal_netlink.c
@@ -43,6 +43,11 @@ static const struct nla_policy thermal_genl_policy[THERMAL_GENL_ATTR_MAX + 1] =
[THERMAL_GENL_ATTR_CDEV_MAX_STATE] = { .type = NLA_U32 },
[THERMAL_GENL_ATTR_CDEV_NAME] = { .type = NLA_STRING,
.len = THERMAL_NAME_LENGTH },
+ /* CPU capabilities */
+ [THERMAL_GENL_ATTR_CPU_CAPABILITY] = { .type = NLA_NESTED },
+ [THERMAL_GENL_ATTR_CPU_CAPABILITY_ID] = { .type = NLA_U32 },
+ [THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE] = { .type = NLA_U32 },
+ [THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY] = { .type = NLA_U32 },
};
struct param {
@@ -58,6 +63,8 @@ struct param {
int temp;
int cdev_state;
int cdev_max_state;
+ struct cpu_capability *cpu_capabilities;
+ int cpu_capabilities_count;
};
typedef int (*cb_t)(struct param *);
@@ -190,6 +197,45 @@ static int thermal_genl_event_gov_change(struct param *p)
return 0;
}
+static int thermal_genl_event_cpu_capability_change(struct param *p)
+{
+ struct cpu_capability *cpu_cap = p->cpu_capabilities;
+ struct sk_buff *msg = p->msg;
+ struct nlattr *start_cap;
+ int i, ret;
+
+ start_cap = nla_nest_start(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY);
+ if (!start_cap)
+ return -EMSGSIZE;
+
+ for (i = 0; i < p->cpu_capabilities_count; ++i) {
+ if (nla_put_u32(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY_ID,
+ cpu_cap->cpu)) {
+ ret = -EMSGSIZE;
+ goto out_cancel_nest;
+ }
+ if (nla_put_u32(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE,
+ cpu_cap->performance)) {
+ ret = -EMSGSIZE;
+ goto out_cancel_nest;
+ }
+ if (nla_put_u32(msg, THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY,
+ cpu_cap->efficiency)) {
+ ret = -EMSGSIZE;
+ goto out_cancel_nest;
+ }
+ ++cpu_cap;
+ }
+
+ nla_nest_end(msg, start_cap);
+
+ return 0;
+out_cancel_nest:
+ nla_nest_cancel(msg, start_cap);
+
+ return ret;
+}
+
int thermal_genl_event_tz_delete(struct param *p)
__attribute__((alias("thermal_genl_event_tz")));
@@ -219,6 +265,7 @@ static cb_t event_cb[] = {
[THERMAL_GENL_EVENT_CDEV_DELETE] = thermal_genl_event_cdev_delete,
[THERMAL_GENL_EVENT_CDEV_STATE_UPDATE] = thermal_genl_event_cdev_state_update,
[THERMAL_GENL_EVENT_TZ_GOV_CHANGE] = thermal_genl_event_gov_change,
+ [THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE] = thermal_genl_event_cpu_capability_change,
};
/*
@@ -356,6 +403,14 @@ int thermal_notify_tz_gov_change(int tz_id, const char *name)
return thermal_genl_send_event(THERMAL_GENL_EVENT_TZ_GOV_CHANGE, &p);
}
+int thermal_genl_cpu_capability_event(int count, struct cpu_capability *caps)
+{
+ struct param p = { .cpu_capabilities_count = count, .cpu_capabilities = caps };
+
+ return thermal_genl_send_event(THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE, &p);
+}
+EXPORT_SYMBOL_GPL(thermal_genl_cpu_capability_event);
+
/*************************** Command encoding ********************************/
static int __thermal_genl_cmd_tz_get_id(struct thermal_zone_device *tz,
diff --git a/drivers/thermal/thermal_netlink.h b/drivers/thermal/thermal_netlink.h
index e554f76291f4..44bc3dec5568 100644
--- a/drivers/thermal/thermal_netlink.h
+++ b/drivers/thermal/thermal_netlink.h
@@ -4,6 +4,12 @@
* Author: Daniel Lezcano <daniel.lezcano@linaro.org>
*/
+struct cpu_capability {
+ int cpu;
+ int performance;
+ int efficiency;
+};
+
/* Netlink notification function */
#ifdef CONFIG_THERMAL_NETLINK
int __init thermal_netlink_init(void);
@@ -23,6 +29,7 @@ int thermal_notify_cdev_add(int cdev_id, const char *name, int max_state);
int thermal_notify_cdev_delete(int cdev_id);
int thermal_notify_tz_gov_change(int tz_id, const char *name);
int thermal_genl_sampling_temp(int id, int temp);
+int thermal_genl_cpu_capability_event(int count, struct cpu_capability *caps);
#else
static inline int thermal_netlink_init(void)
{
@@ -101,4 +108,10 @@ static inline int thermal_genl_sampling_temp(int id, int temp)
{
return 0;
}
+
+static inline int thermal_genl_cpu_capability_event(int count, struct cpu_capability *caps)
+{
+ return 0;
+}
+
#endif /* CONFIG_THERMAL_NETLINK */
diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
index 9aa2fedfa309..fc78bf3aead7 100644
--- a/include/uapi/linux/thermal.h
+++ b/include/uapi/linux/thermal.h
@@ -44,7 +44,10 @@ enum thermal_genl_attr {
THERMAL_GENL_ATTR_CDEV_MAX_STATE,
THERMAL_GENL_ATTR_CDEV_NAME,
THERMAL_GENL_ATTR_GOV_NAME,
-
+ THERMAL_GENL_ATTR_CPU_CAPABILITY,
+ THERMAL_GENL_ATTR_CPU_CAPABILITY_ID,
+ THERMAL_GENL_ATTR_CPU_CAPABILITY_PERFORMANCE,
+ THERMAL_GENL_ATTR_CPU_CAPABILITY_EFFICIENCY,
__THERMAL_GENL_ATTR_MAX,
};
#define THERMAL_GENL_ATTR_MAX (__THERMAL_GENL_ATTR_MAX - 1)
@@ -71,6 +74,7 @@ enum thermal_genl_event {
THERMAL_GENL_EVENT_CDEV_DELETE, /* Cdev unbound */
THERMAL_GENL_EVENT_CDEV_STATE_UPDATE, /* Cdev state updated */
THERMAL_GENL_EVENT_TZ_GOV_CHANGE, /* Governor policy changed */
+ THERMAL_GENL_EVENT_CPU_CAPABILITY_CHANGE, /* CPU capability changed */
__THERMAL_GENL_EVENT_MAX,
};
#define THERMAL_GENL_EVENT_MAX (__THERMAL_GENL_EVENT_MAX - 1)
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 7/7] thermal: intel: hfi: Notify user space for HFI events
2022-01-08 3:47 [PATCH v4 0/7] Thermal: Introduce the Hardware Feedback Interface for thermal and performance management Ricardo Neri
` (5 preceding siblings ...)
2022-01-08 3:47 ` [PATCH v4 6/7] thermal: netlink: Add a new event to notify CPU capabilities change Ricardo Neri
@ 2022-01-08 3:47 ` Ricardo Neri
2022-01-12 19:53 ` Rafael J. Wysocki
6 siblings, 1 reply; 14+ messages in thread
From: Ricardo Neri @ 2022-01-08 3:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm
Cc: x86, linux-doc, Len Brown, Srinivas Pandruvada, Aubrey Li,
Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, linux-kernel
From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
When the hardware issues an HFI event, relay a notification to user space.
This allows user space to respond by reading performance and efficiency of
each CPU and take appropriate action.
For example, when performance and efficiency of a CPU is 0, user space can
either offline the CPU or inject idle. Also, if user space notices a
downward trend in performance, it may proactively adjust power limits to
avoid future situations in which performance drops to 0.
To avoid excessive notifications, the rate is limited by one HZ per event.
To limit the netlink message size, parameters for only 16 CPUs at max are
sent in one message. If there are more than 16 CPUs, issue as many messages
as needed to notify the status of all CPUs.
In the HFI specification, both performance and efficiency capabilities are
set in the [0, 255] range. The existing implementations of HFI hardware
do not scale the maximum values to 255. Since userspace cares about
capability values that are either 0 or show a downward/upward trend, this
fact does not matter much. Relative changes in capabilities are enough. To
comply with the thermal netlink ABI, scale both performance and efficiency
capabilities to the [0, 1023] interval.
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Aubrey Li <aubrey.li@linux.intel.com>
Cc: Lukasz Luba <lukasz.luba@arm.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
Changes since v3:
* None
Changes since v2:
* None
Changes since v1:
* Made get_one_hfi_cap() return void. Removed unnecessary checks.
(Rafael)
* Replaced raw_spin_[un]lock_irq[restore|save]() with raw_spin_
[un]lock_irq() in get_one_hfi_cap(). This function is only called from
a workqueue and there is no need to save and restore irq flags.
* Scaled performance and energy efficiency values to a [0, 1023] interval
when reporting values to user space via thermal netlink notifications.
(Lucasz).
* Reworded commit message to comment on the scaling of HFI capabilities
to comply with the proposed thermal netlink ABI.
---
drivers/thermal/intel/Kconfig | 1 +
drivers/thermal/intel/intel_hfi.c | 57 ++++++++++++++++++++++++++++++-
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
index e9d2925227d4..6cf3fe36a4ae 100644
--- a/drivers/thermal/intel/Kconfig
+++ b/drivers/thermal/intel/Kconfig
@@ -104,6 +104,7 @@ config INTEL_HFI_THERMAL
bool "Intel Hardware Feedback Interface"
depends on CPU_SUP_INTEL
depends on X86_THERMAL_VECTOR
+ select THERMAL_NETLINK
help
Select this option to enable the Hardware Feedback Interface. If
selected, hardware provides guidance to the operating system on
diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
index 1a08c58f26f6..9fd66f176948 100644
--- a/drivers/thermal/intel/intel_hfi.c
+++ b/drivers/thermal/intel/intel_hfi.c
@@ -40,6 +40,7 @@
#include <asm/msr.h>
+#include "../thermal_core.h"
#include "intel_hfi.h"
#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | \
@@ -162,6 +163,60 @@ static struct hfi_features hfi_features;
static DEFINE_MUTEX(hfi_instance_lock);
#define HFI_UPDATE_INTERVAL HZ
+#define HFI_MAX_THERM_NOTIFY_COUNT 16
+
+static void get_one_hfi_cap(struct hfi_instance *hfi_instance, s16 index,
+ struct hfi_cpu_data *hfi_caps)
+{
+ struct hfi_cpu_data *caps;
+
+ /* Find the capabilities of @cpu */
+ raw_spin_lock_irq(&hfi_instance->table_lock);
+ caps = hfi_instance->data + index * hfi_features.cpu_stride;
+ memcpy(hfi_caps, caps, sizeof(*hfi_caps));
+ raw_spin_unlock_irq(&hfi_instance->table_lock);
+}
+
+/*
+ * Call update_capabilities() when there are changes in the HFI table.
+ */
+static void update_capabilities(struct hfi_instance *hfi_instance)
+{
+ struct cpu_capability cpu_caps[HFI_MAX_THERM_NOTIFY_COUNT];
+ int i = 0, cpu;
+
+ for_each_cpu(cpu, hfi_instance->cpus) {
+ struct hfi_cpu_data caps;
+ s16 index;
+
+ /*
+ * We know index is valid because this CPU is present
+ * in this instance.
+ */
+ index = per_cpu(hfi_cpu_info, cpu).index;
+
+ get_one_hfi_cap(hfi_instance, index, &caps);
+
+ cpu_caps[i].cpu = cpu;
+
+ /*
+ * Scale performance and energy efficiency to
+ * the [0, 1023] interval that thermal netlink uses.
+ */
+ cpu_caps[i].performance = caps.perf_cap << 2;
+ cpu_caps[i].efficiency = caps.ee_cap << 2;
+ ++i;
+
+ if (i >= HFI_MAX_THERM_NOTIFY_COUNT) {
+ thermal_genl_cpu_capability_event(HFI_MAX_THERM_NOTIFY_COUNT,
+ cpu_caps);
+ i = 0;
+ }
+ }
+
+ if (i)
+ thermal_genl_cpu_capability_event(i, cpu_caps);
+}
static void hfi_update_work_fn(struct work_struct *work)
{
@@ -172,7 +227,7 @@ static void hfi_update_work_fn(struct work_struct *work)
if (!hfi_instance)
return;
- /* TODO: Consume update here. */
+ update_capabilities(hfi_instance);
}
void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 7/7] thermal: intel: hfi: Notify user space for HFI events
2022-01-08 3:47 ` [PATCH v4 7/7] thermal: intel: hfi: Notify user space for HFI events Ricardo Neri
@ 2022-01-12 19:53 ` Rafael J. Wysocki
2022-01-12 23:54 ` Srinivas Pandruvada
0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2022-01-12 19:53 UTC (permalink / raw)
To: Ricardo Neri
Cc: Rafael J. Wysocki, Daniel Lezcano, Linux PM,
the arch/x86 maintainers, open list:DOCUMENTATION, Len Brown,
Srinivas Pandruvada, Aubrey Li, Amit Kucheria, Andi Kleen,
Tim Chen, Lukasz Luba, Ravi V. Shankar, Ricardo Neri,
Linux Kernel Mailing List
On Sat, Jan 8, 2022 at 4:46 AM Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
>
> From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>
> When the hardware issues an HFI event, relay a notification to user space.
> This allows user space to respond by reading performance and efficiency of
> each CPU and take appropriate action.
>
> For example, when performance and efficiency of a CPU is 0, user space can
> either offline the CPU or inject idle. Also, if user space notices a
> downward trend in performance, it may proactively adjust power limits to
> avoid future situations in which performance drops to 0.
>
> To avoid excessive notifications, the rate is limited by one HZ per event.
> To limit the netlink message size, parameters for only 16 CPUs at max are
> sent in one message. If there are more than 16 CPUs, issue as many messages
> as needed to notify the status of all CPUs.
>
> In the HFI specification, both performance and efficiency capabilities are
> set in the [0, 255] range. The existing implementations of HFI hardware
> do not scale the maximum values to 255. Since userspace cares about
> capability values that are either 0 or show a downward/upward trend, this
> fact does not matter much. Relative changes in capabilities are enough. To
> comply with the thermal netlink ABI, scale both performance and efficiency
> capabilities to the [0, 1023] interval.
>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Aubrey Li <aubrey.li@linux.intel.com>
> Cc: Lukasz Luba <lukasz.luba@arm.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
> Reviewed-by: Len Brown <len.brown@intel.com>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> Changes since v3:
> * None
>
> Changes since v2:
> * None
>
> Changes since v1:
> * Made get_one_hfi_cap() return void. Removed unnecessary checks.
> (Rafael)
> * Replaced raw_spin_[un]lock_irq[restore|save]() with raw_spin_
> [un]lock_irq() in get_one_hfi_cap(). This function is only called from
> a workqueue and there is no need to save and restore irq flags.
> * Scaled performance and energy efficiency values to a [0, 1023] interval
> when reporting values to user space via thermal netlink notifications.
> (Lucasz).
> * Reworded commit message to comment on the scaling of HFI capabilities
> to comply with the proposed thermal netlink ABI.
> ---
> drivers/thermal/intel/Kconfig | 1 +
> drivers/thermal/intel/intel_hfi.c | 57 ++++++++++++++++++++++++++++++-
> 2 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/intel/Kconfig b/drivers/thermal/intel/Kconfig
> index e9d2925227d4..6cf3fe36a4ae 100644
> --- a/drivers/thermal/intel/Kconfig
> +++ b/drivers/thermal/intel/Kconfig
> @@ -104,6 +104,7 @@ config INTEL_HFI_THERMAL
> bool "Intel Hardware Feedback Interface"
> depends on CPU_SUP_INTEL
> depends on X86_THERMAL_VECTOR
> + select THERMAL_NETLINK
> help
> Select this option to enable the Hardware Feedback Interface. If
> selected, hardware provides guidance to the operating system on
> diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c
> index 1a08c58f26f6..9fd66f176948 100644
> --- a/drivers/thermal/intel/intel_hfi.c
> +++ b/drivers/thermal/intel/intel_hfi.c
> @@ -40,6 +40,7 @@
>
> #include <asm/msr.h>
>
> +#include "../thermal_core.h"
> #include "intel_hfi.h"
>
> #define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | \
> @@ -162,6 +163,60 @@ static struct hfi_features hfi_features;
> static DEFINE_MUTEX(hfi_instance_lock);
>
> #define HFI_UPDATE_INTERVAL HZ
> +#define HFI_MAX_THERM_NOTIFY_COUNT 16
> +
> +static void get_one_hfi_cap(struct hfi_instance *hfi_instance, s16 index,
> + struct hfi_cpu_data *hfi_caps)
> +{
> + struct hfi_cpu_data *caps;
> +
> + /* Find the capabilities of @cpu */
> + raw_spin_lock_irq(&hfi_instance->table_lock);
> + caps = hfi_instance->data + index * hfi_features.cpu_stride;
> + memcpy(hfi_caps, caps, sizeof(*hfi_caps));
> + raw_spin_unlock_irq(&hfi_instance->table_lock);
> +}
> +
> +/*
> + * Call update_capabilities() when there are changes in the HFI table.
> + */
> +static void update_capabilities(struct hfi_instance *hfi_instance)
> +{
> + struct cpu_capability cpu_caps[HFI_MAX_THERM_NOTIFY_COUNT];
> + int i = 0, cpu;
> +
Wouldn't it be better to hold hfi_instance_lock for the duration of this loop?
Surely, CPU offline or online during it can be confusing.
> + for_each_cpu(cpu, hfi_instance->cpus) {
> + struct hfi_cpu_data caps;
> + s16 index;
> +
> + /*
> + * We know index is valid because this CPU is present
> + * in this instance.
> + */
> + index = per_cpu(hfi_cpu_info, cpu).index;
> +
> + get_one_hfi_cap(hfi_instance, index, &caps);
> +
> + cpu_caps[i].cpu = cpu;
> +
> + /*
> + * Scale performance and energy efficiency to
> + * the [0, 1023] interval that thermal netlink uses.
> + */
> + cpu_caps[i].performance = caps.perf_cap << 2;
> + cpu_caps[i].efficiency = caps.ee_cap << 2;
> + ++i;
> +
> + if (i >= HFI_MAX_THERM_NOTIFY_COUNT) {
> + thermal_genl_cpu_capability_event(HFI_MAX_THERM_NOTIFY_COUNT,
> + cpu_caps);
> + i = 0;
> + }
> + }
> +
> + if (i)
> + thermal_genl_cpu_capability_event(i, cpu_caps);
> +}
>
> static void hfi_update_work_fn(struct work_struct *work)
> {
> @@ -172,7 +227,7 @@ static void hfi_update_work_fn(struct work_struct *work)
> if (!hfi_instance)
> return;
>
> - /* TODO: Consume update here. */
> + update_capabilities(hfi_instance);
> }
>
> void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 7/7] thermal: intel: hfi: Notify user space for HFI events
2022-01-12 19:53 ` Rafael J. Wysocki
@ 2022-01-12 23:54 ` Srinivas Pandruvada
2022-01-13 5:50 ` Srinivas Pandruvada
0 siblings, 1 reply; 14+ messages in thread
From: Srinivas Pandruvada @ 2022-01-12 23:54 UTC (permalink / raw)
To: Rafael J. Wysocki, Ricardo Neri
Cc: Rafael J. Wysocki, Daniel Lezcano, Linux PM,
the arch/x86 maintainers, open list:DOCUMENTATION, Len Brown,
Aubrey Li, Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, Linux Kernel Mailing List
On Wed, 2022-01-12 at 20:53 +0100, Rafael J. Wysocki wrote:
> On Sat, Jan 8, 2022 at 4:46 AM Ricardo Neri
> <ricardo.neri-calderon@linux.intel.com> wrote:
> > From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> >
> > When the hardware issues an HFI event, relay a notification to user
> > space.
> > This allows user space to respond by reading performance and
> > efficiency of
> > each CPU and take appropriate action.
> >
> > For example, when performance and efficiency of a CPU is 0, user
> > space can
> > either offline the CPU or inject idle. Also, if user space notices
> > a
> > downward trend in performance, it may proactively adjust power
> > limits to
> > avoid future situations in which performance drops to 0.
> >
> > To avoid excessive notifications, the rate is limited by one HZ per
> > event.
> > To limit the netlink message size, parameters for only 16 CPUs at
> > max are
> > sent in one message. If there are more than 16 CPUs, issue as many
> > messages
> > as needed to notify the status of all CPUs.
> >
> > In the HFI specification, both performance and efficiency
> > capabilities are
> > set in the [0, 255] range. The existing implementations of HFI
> > hardware
> > do not scale the maximum values to 255. Since userspace cares about
> > capability values that are either 0 or show a downward/upward
> > trend, this
> > fact does not matter much. Relative changes in capabilities are
> > enough. To
> > comply with the thermal netlink ABI, scale both performance and
> > efficiency
> > capabilities to the [0, 1023] interval.
> >
> > Cc: Andi Kleen <ak@linux.intel.com>
> > Cc: Aubrey Li <aubrey.li@linux.intel.com>
> > Cc: Lukasz Luba <lukasz.luba@arm.com>
> > Cc: Tim Chen <tim.c.chen@linux.intel.com>
> > Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
> > Reviewed-by: Len Brown <len.brown@intel.com>
> > Signed-off-by: Srinivas Pandruvada <
> > srinivas.pandruvada@linux.intel.com>
> > ---
> > Changes since v3:
> > * None
> >
> > Changes since v2:
> > * None
> >
> > Changes since v1:
> > * Made get_one_hfi_cap() return void. Removed unnecessary checks.
> > (Rafael)
> > * Replaced raw_spin_[un]lock_irq[restore|save]() with raw_spin_
> > [un]lock_irq() in get_one_hfi_cap(). This function is only
> > called from
> > a workqueue and there is no need to save and restore irq flags.
> > * Scaled performance and energy efficiency values to a [0, 1023]
> > interval
> > when reporting values to user space via thermal netlink
> > notifications.
> > (Lucasz).
> > * Reworded commit message to comment on the scaling of HFI
> > capabilities
> > to comply with the proposed thermal netlink ABI.
> > ---
> > drivers/thermal/intel/Kconfig | 1 +
> > drivers/thermal/intel/intel_hfi.c | 57
> > ++++++++++++++++++++++++++++++-
> > 2 files changed, 57 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/intel/Kconfig
> > b/drivers/thermal/intel/Kconfig
> > index e9d2925227d4..6cf3fe36a4ae 100644
> > --- a/drivers/thermal/intel/Kconfig
> > +++ b/drivers/thermal/intel/Kconfig
> > @@ -104,6 +104,7 @@ config INTEL_HFI_THERMAL
> > bool "Intel Hardware Feedback Interface"
> > depends on CPU_SUP_INTEL
> > depends on X86_THERMAL_VECTOR
> > + select THERMAL_NETLINK
> > help
> > Select this option to enable the Hardware Feedback
> > Interface. If
> > selected, hardware provides guidance to the operating
> > system on
> > diff --git a/drivers/thermal/intel/intel_hfi.c
> > b/drivers/thermal/intel/intel_hfi.c
> > index 1a08c58f26f6..9fd66f176948 100644
> > --- a/drivers/thermal/intel/intel_hfi.c
> > +++ b/drivers/thermal/intel/intel_hfi.c
> > @@ -40,6 +40,7 @@
> >
> > #include <asm/msr.h>
> >
> > +#include "../thermal_core.h"
> > #include "intel_hfi.h"
> >
> > #define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) |
> > BIT(7) | \
> > @@ -162,6 +163,60 @@ static struct hfi_features hfi_features;
> > static DEFINE_MUTEX(hfi_instance_lock);
> >
> > #define HFI_UPDATE_INTERVAL HZ
> > +#define HFI_MAX_THERM_NOTIFY_COUNT 16
> > +
> > +static void get_one_hfi_cap(struct hfi_instance *hfi_instance, s16
> > index,
> > + struct hfi_cpu_data *hfi_caps)
> > +{
> > + struct hfi_cpu_data *caps;
> > +
> > + /* Find the capabilities of @cpu */
> > + raw_spin_lock_irq(&hfi_instance->table_lock);
> > + caps = hfi_instance->data + index *
> > hfi_features.cpu_stride;
> > + memcpy(hfi_caps, caps, sizeof(*hfi_caps));
> > + raw_spin_unlock_irq(&hfi_instance->table_lock);
> > +}
> > +
> > +/*
> > + * Call update_capabilities() when there are changes in the HFI
> > table.
> > + */
> > +static void update_capabilities(struct hfi_instance *hfi_instance)
> > +{
> > + struct cpu_capability cpu_caps[HFI_MAX_THERM_NOTIFY_COUNT];
> > + int i = 0, cpu;
> > +
>
> Wouldn't it be better to hold hfi_instance_lock for the duration of
> this loop?
As you expressed concern with more CPUs per package in future + netlink
processing the interrupts will be disabled for longer time.
But this can be optimized to have
void get_one_hfi_cap(struct hfi_instance *hfi_instance, s16 index,
struct hfi_cpu_data *hfi_caps)
with something like
void get_hfi_caps(struct hfi_instance *hfi_instance, s16 *cpu_count,
struct hfi_cpu_data **hfi_caps)
and take one lock for all
HFI_MAX_THERM_NOTIFY_COUNT CPUs.
Then keep thermal_genl_cpu_capability_event outside.
This ends up in calling thermal_genl_send_event() which has a long call
chain to netlink_broadcast() to format and broadcast message.
Thanks,
Srinivas
>
> Surely, CPU offline or online during it can be confusing.
>
> > + for_each_cpu(cpu, hfi_instance->cpus) {
> > + struct hfi_cpu_data caps;
> > + s16 index;
> > +
> > + /*
> > + * We know index is valid because this CPU is
> > present
> > + * in this instance.
> > + */
> > + index = per_cpu(hfi_cpu_info, cpu).index;
> > +
> > + get_one_hfi_cap(hfi_instance, index, &caps);
> > +
> > + cpu_caps[i].cpu = cpu;
> > +
> > + /*
> > + * Scale performance and energy efficiency to
> > + * the [0, 1023] interval that thermal netlink
> > uses.
> > + */
> > + cpu_caps[i].performance = caps.perf_cap << 2;
> > + cpu_caps[i].efficiency = caps.ee_cap << 2;
> > + ++i;
> > +
> > + if (i >= HFI_MAX_THERM_NOTIFY_COUNT) {
> > + thermal_genl_cpu_capability_event(HFI_MAX_T
> > HERM_NOTIFY_COUNT,
> > + cpu_caps)
> > ;
> > + i = 0;
> > + }
> > + }
> > +
> > + if (i)
> > + thermal_genl_cpu_capability_event(i, cpu_caps);
> > +}
> >
> > static void hfi_update_work_fn(struct work_struct *work)
> > {
> > @@ -172,7 +227,7 @@ static void hfi_update_work_fn(struct
> > work_struct *work)
> > if (!hfi_instance)
> > return;
> >
> > - /* TODO: Consume update here. */
> > + update_capabilities(hfi_instance);
> > }
> >
> > void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 7/7] thermal: intel: hfi: Notify user space for HFI events
2022-01-12 23:54 ` Srinivas Pandruvada
@ 2022-01-13 5:50 ` Srinivas Pandruvada
0 siblings, 0 replies; 14+ messages in thread
From: Srinivas Pandruvada @ 2022-01-13 5:50 UTC (permalink / raw)
To: Rafael J. Wysocki, Ricardo Neri
Cc: Rafael J. Wysocki, Daniel Lezcano, Linux PM,
the arch/x86 maintainers, open list:DOCUMENTATION, Len Brown,
Aubrey Li, Amit Kucheria, Andi Kleen, Tim Chen, Lukasz Luba,
Ravi V. Shankar, Ricardo Neri, Linux Kernel Mailing List
On Wed, 2022-01-12 at 15:54 -0800, Srinivas Pandruvada wrote:
> On Wed, 2022-01-12 at 20:53 +0100, Rafael J. Wysocki wrote:
> > On Sat, Jan 8, 2022 at 4:46 AM Ricardo Neri
> > <ricardo.neri-calderon@linux.intel.com> wrote:
> > > From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > >
> > > When the hardware issues an HFI event, relay a notification to
> > > user
> > > space.
> > > This allows user space to respond by reading performance and
> > > efficiency of
> > > each CPU and take appropriate action.
> > >
> > > For example, when performance and efficiency of a CPU is 0, user
> > > space can
> > > either offline the CPU or inject idle. Also, if user space
> > > notices
> > > a
> > > downward trend in performance, it may proactively adjust power
> > > limits to
> > > avoid future situations in which performance drops to 0.
> > >
> > > To avoid excessive notifications, the rate is limited by one HZ
> > > per
> > > event.
> > > To limit the netlink message size, parameters for only 16 CPUs at
> > > max are
> > > sent in one message. If there are more than 16 CPUs, issue as
> > > many
> > > messages
> > > as needed to notify the status of all CPUs.
> > >
> > > In the HFI specification, both performance and efficiency
> > > capabilities are
> > > set in the [0, 255] range. The existing implementations of HFI
> > > hardware
> > > do not scale the maximum values to 255. Since userspace cares
> > > about
> > > capability values that are either 0 or show a downward/upward
> > > trend, this
> > > fact does not matter much. Relative changes in capabilities are
> > > enough. To
> > > comply with the thermal netlink ABI, scale both performance and
> > > efficiency
> > > capabilities to the [0, 1023] interval.
> > >
> > > Cc: Andi Kleen <ak@linux.intel.com>
> > > Cc: Aubrey Li <aubrey.li@linux.intel.com>
> > > Cc: Lukasz Luba <lukasz.luba@arm.com>
> > > Cc: Tim Chen <tim.c.chen@linux.intel.com>
> > > Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com>
> > > Reviewed-by: Len Brown <len.brown@intel.com>
> > > Signed-off-by: Srinivas Pandruvada <
> > > srinivas.pandruvada@linux.intel.com>
> > > ---
> > > Changes since v3:
> > > * None
> > >
> > > Changes since v2:
> > > * None
> > >
> > > Changes since v1:
> > > * Made get_one_hfi_cap() return void. Removed unnecessary
> > > checks.
> > > (Rafael)
> > > * Replaced raw_spin_[un]lock_irq[restore|save]() with raw_spin_
> > > [un]lock_irq() in get_one_hfi_cap(). This function is only
> > > called from
> > > a workqueue and there is no need to save and restore irq
> > > flags.
> > > * Scaled performance and energy efficiency values to a [0,
> > > 1023]
> > > interval
> > > when reporting values to user space via thermal netlink
> > > notifications.
> > > (Lucasz).
> > > * Reworded commit message to comment on the scaling of HFI
> > > capabilities
> > > to comply with the proposed thermal netlink ABI.
> > > ---
> > > drivers/thermal/intel/Kconfig | 1 +
> > > drivers/thermal/intel/intel_hfi.c | 57
> > > ++++++++++++++++++++++++++++++-
> > > 2 files changed, 57 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/thermal/intel/Kconfig
> > > b/drivers/thermal/intel/Kconfig
> > > index e9d2925227d4..6cf3fe36a4ae 100644
> > > --- a/drivers/thermal/intel/Kconfig
> > > +++ b/drivers/thermal/intel/Kconfig
> > > @@ -104,6 +104,7 @@ config INTEL_HFI_THERMAL
> > > bool "Intel Hardware Feedback Interface"
> > > depends on CPU_SUP_INTEL
> > > depends on X86_THERMAL_VECTOR
> > > + select THERMAL_NETLINK
> > > help
> > > Select this option to enable the Hardware Feedback
> > > Interface. If
> > > selected, hardware provides guidance to the operating
> > > system on
> > > diff --git a/drivers/thermal/intel/intel_hfi.c
> > > b/drivers/thermal/intel/intel_hfi.c
> > > index 1a08c58f26f6..9fd66f176948 100644
> > > --- a/drivers/thermal/intel/intel_hfi.c
> > > +++ b/drivers/thermal/intel/intel_hfi.c
> > > @@ -40,6 +40,7 @@
> > >
> > > #include <asm/msr.h>
> > >
> > > +#include "../thermal_core.h"
> > > #include "intel_hfi.h"
> > >
> > > #define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) |
> > > BIT(7) | \
> > > @@ -162,6 +163,60 @@ static struct hfi_features hfi_features;
> > > static DEFINE_MUTEX(hfi_instance_lock);
> > >
> > > #define HFI_UPDATE_INTERVAL HZ
> > > +#define HFI_MAX_THERM_NOTIFY_COUNT 16
> > > +
> > > +static void get_one_hfi_cap(struct hfi_instance *hfi_instance,
> > > s16
> > > index,
> > > + struct hfi_cpu_data *hfi_caps)
> > > +{
> > > + struct hfi_cpu_data *caps;
> > > +
> > > + /* Find the capabilities of @cpu */
> > > + raw_spin_lock_irq(&hfi_instance->table_lock);
> > > + caps = hfi_instance->data + index *
> > > hfi_features.cpu_stride;
> > > + memcpy(hfi_caps, caps, sizeof(*hfi_caps));
> > > + raw_spin_unlock_irq(&hfi_instance->table_lock);
> > > +}
> > > +
> > > +/*
> > > + * Call update_capabilities() when there are changes in the HFI
> > > table.
> > > + */
> > > +static void update_capabilities(struct hfi_instance
> > > *hfi_instance)
> > > +{
> > > + struct cpu_capability
> > > cpu_caps[HFI_MAX_THERM_NOTIFY_COUNT];
> > > + int i = 0, cpu;
> > > +
> >
> > Wouldn't it be better to hold hfi_instance_lock for the duration of
> > this loop?
diff --git a/drivers/thermal/intel/intel_hfi.c
b/drivers/thermal/intel/intel_hfi.c
index 77e54f2b2455..a386a3462738 100644
--- a/drivers/thermal/intel/intel_hfi.c
+++ b/drivers/thermal/intel/intel_hfi.c
@@ -392,45 +392,74 @@ static void get_one_hfi_cap(struct hfi_instance
*hfi_instance, s16 index,
raw_spin_unlock_irq(&hfi_instance->table_lock);
}
-/*
- * Call update_capabilities() when there are changes in the HFI table.
- */
-static void update_capabilities(struct hfi_instance *hfi_instance)
+static void get_hfi_caps(struct hfi_instance *hfi_instance, int
*count, struct cpu_capability **cpu_caps)
{
- struct cpu_capability cpu_caps[HFI_MAX_THERM_NOTIFY_COUNT];
- int i = 0, cpu;
+ struct cpu_capability *_cpu_caps;
+ int _count, cpu, i = 0;
+
+ *count = 0;
+
+ raw_spin_lock_irq(&hfi_instance->table_lock);
+ _count = cpumask_weight(hfi_instance->cpus);
+ if (!_count)
+ goto unlock;
+
+ _cpu_caps = kcalloc(_count, sizeof(*_cpu_caps), GFP_ATOMIC);
+ if (!_cpu_caps)
+ goto unlock;
for_each_cpu(cpu, hfi_instance->cpus) {
- struct hfi_cpu_data caps;
+ struct hfi_cpu_data *caps;
s16 index;
- /*
- * We know index is valid because this CPU is present
- * in this instance.
- */
index = per_cpu(hfi_cpu_info, cpu).index;
-
- get_one_hfi_cap(hfi_instance, index, &caps, 0);
-
- cpu_caps[i].cpu = cpu;
+ caps = hfi_instance->data + index *
hfi_features.cpu_stride;
+ _cpu_caps[i].cpu = cpu;
/*
* Scale performance and energy efficiency to
* the [0, 1023] interval that thermal netlink uses.
*/
- cpu_caps[i].performance = caps.perf_cap << 2;
- cpu_caps[i].efficiency = caps.ee_cap << 2;
+ _cpu_caps[i].performance = caps->perf_cap << 2;
+ _cpu_caps[i].efficiency = caps->ee_cap << 2;
+
++i;
+ if (i >= _count)
+ break;
+ }
+ *count = i;
+ *cpu_caps = _cpu_caps;
- if (i >= HFI_MAX_THERM_NOTIFY_COUNT) {
- thermal_genl_cpu_capability_event(HFI_MAX_THERM
_NOTIFY_COUNT,
- cpu_caps);
- i = 0;
- }
+unlock:
+ raw_spin_unlock_irq(&hfi_instance->table_lock);
+}
+
+/*
+ * Call update_capabilities() when there are changes in the HFI table.
+ */
+static void update_capabilities(struct hfi_instance *hfi_instance)
+{
+ struct cpu_capability *cpu_caps;
+ int i, j = 0, count;
+
+ get_hfi_caps(hfi_instance, &count, &cpu_caps);
+ if (!count)
+ return;
+
+ if (count < HFI_MAX_THERM_NOTIFY_COUNT)
+ goto last_cmd;
+
+ for (i = 0; i < count; i += HFI_MAX_THERM_NOTIFY_COUNT) {
+ thermal_genl_cpu_capability_event(HFI_MAX_THERM_NOTIFY_
COUNT, &cpu_caps[i]);
+ j = i;
}
- if (i)
- thermal_genl_cpu_capability_event(i, cpu_caps);
+ count = i - count;
+last_cmd:
+ if (count)
+ thermal_genl_cpu_capability_event(count, &cpu_caps[j]);
+
+ kfree(cpu_caps);
}
> As you expressed concern with more CPUs per package in future +
> netlink
> processing the interrupts will be disabled for longer time.
>
> But this can be optimized to have
> void get_one_hfi_cap(struct hfi_instance *hfi_instance, s16 index,
> struct hfi_cpu_data *hfi_caps)
> with something like
> void get_hfi_caps(struct hfi_instance *hfi_instance, s16 *cpu_count,
> struct hfi_cpu_data **hfi_caps)
something like this:
diff --git a/drivers/thermal/intel/intel_hfi.c
b/drivers/thermal/intel/intel_hfi.c
index 77e54f2b2455..a386a3462738 100644
--- a/drivers/thermal/intel/intel_hfi.c
+++ b/drivers/thermal/intel/intel_hfi.c
@@ -392,45 +392,74 @@ static void get_one_hfi_cap(struct hfi_instance
*hfi_instance, s16 index,
raw_spin_unlock_irq(&hfi_instance->table_lock);
}
-/*
- * Call update_capabilities() when there are changes in the HFI table.
- */
-static void update_capabilities(struct hfi_instance *hfi_instance)
+static void get_hfi_caps(struct hfi_instance *hfi_instance, int
*count, struct cpu_capability **cpu_caps)
{
- struct cpu_capability cpu_caps[HFI_MAX_THERM_NOTIFY_COUNT];
- int i = 0, cpu;
+ struct cpu_capability *_cpu_caps;
+ int _count, cpu, i = 0;
+
+ *count = 0;
+
+ raw_spin_lock_irq(&hfi_instance->table_lock);
+ _count = cpumask_weight(hfi_instance->cpus);
+ if (!_count)
+ goto unlock;
+
+ _cpu_caps = kcalloc(_count, sizeof(*_cpu_caps), GFP_ATOMIC);
+ if (!_cpu_caps)
+ goto unlock;
for_each_cpu(cpu, hfi_instance->cpus) {
- struct hfi_cpu_data caps;
+ struct hfi_cpu_data *caps;
s16 index;
- /*
- * We know index is valid because this CPU is present
- * in this instance.
- */
index = per_cpu(hfi_cpu_info, cpu).index;
-
- get_one_hfi_cap(hfi_instance, index, &caps, 0);
-
- cpu_caps[i].cpu = cpu;
+ caps = hfi_instance->data + index *
hfi_features.cpu_stride;
+ _cpu_caps[i].cpu = cpu;
/*
* Scale performance and energy efficiency to
* the [0, 1023] interval that thermal netlink uses.
*/
- cpu_caps[i].performance = caps.perf_cap << 2;
- cpu_caps[i].efficiency = caps.ee_cap << 2;
+ _cpu_caps[i].performance = caps->perf_cap << 2;
+ _cpu_caps[i].efficiency = caps->ee_cap << 2;
+
++i;
+ if (i >= _count)
+ break;
+ }
+ *count = i;
+ *cpu_caps = _cpu_caps;
- if (i >= HFI_MAX_THERM_NOTIFY_COUNT) {
- thermal_genl_cpu_capability_event(HFI_MAX_THERM
_NOTIFY_COUNT,
- cpu_caps);
- i = 0;
- }
+unlock:
+ raw_spin_unlock_irq(&hfi_instance->table_lock);
+}
+
+/*
+ * Call update_capabilities() when there are changes in the HFI table.
+ */
+static void update_capabilities(struct hfi_instance *hfi_instance)
+{
+ struct cpu_capability *cpu_caps;
+ int i, j = 0, count;
+
+ get_hfi_caps(hfi_instance, &count, &cpu_caps);
+ if (!count)
+ return;
+
+ if (count < HFI_MAX_THERM_NOTIFY_COUNT)
+ goto last_cmd;
+
+ for (i = 0; i < count; i += HFI_MAX_THERM_NOTIFY_COUNT) {
+ thermal_genl_cpu_capability_event(HFI_MAX_THERM_NOTIFY_
COUNT, &cpu_caps[i]);
+ j = i;
}
- if (i)
- thermal_genl_cpu_capability_event(i, cpu_caps);
+ count = i - count;
+last_cmd:
+ if (count)
+ thermal_genl_cpu_capability_event(count, &cpu_caps[j]);
+
+ kfree(cpu_caps);
}
> and take one lock for all
> HFI_MAX_THERM_NOTIFY_COUNT CPUs.
>
> Then keep thermal_genl_cpu_capability_event outside.
> This ends up in calling thermal_genl_send_event() which has a long
> call
> chain to netlink_broadcast() to format and broadcast message.
>
> Thanks,
> Srinivas
>
> > Surely, CPU offline or online during it can be confusing.
> >
> > > + for_each_cpu(cpu, hfi_instance->cpus) {
> > > + struct hfi_cpu_data caps;
> > > + s16 index;
> > > +
> > > + /*
> > > + * We know index is valid because this CPU is
> > > present
> > > + * in this instance.
> > > + */
> > > + index = per_cpu(hfi_cpu_info, cpu).index;
> > > +
> > > + get_one_hfi_cap(hfi_instance, index, &caps);
> > > +
> > > + cpu_caps[i].cpu = cpu;
> > > +
> > > + /*
> > > + * Scale performance and energy efficiency to
> > > + * the [0, 1023] interval that thermal netlink
> > > uses.
> > > + */
> > > + cpu_caps[i].performance = caps.perf_cap << 2;
> > > + cpu_caps[i].efficiency = caps.ee_cap << 2;
> > > + ++i;
> > > +
> > > + if (i >= HFI_MAX_THERM_NOTIFY_COUNT) {
> > > + thermal_genl_cpu_capability_event(HFI_MAX
> > > _T
> > > HERM_NOTIFY_COUNT,
> > > + cpu_cap
> > > s)
> > > ;
> > > + i = 0;
> > > + }
> > > + }
> > > +
> > > + if (i)
> > > + thermal_genl_cpu_capability_event(i, cpu_caps);
> > > +}
> > >
> > > static void hfi_update_work_fn(struct work_struct *work)
> > > {
> > > @@ -172,7 +227,7 @@ static void hfi_update_work_fn(struct
> > > work_struct *work)
> > > if (!hfi_instance)
> > > return;
> > >
> > > - /* TODO: Consume update here. */
> > > + update_capabilities(hfi_instance);
> > > }
> > >
> > > void intel_hfi_process_event(__u64 pkg_therm_status_msr_val)
> > > --
> > > 2.17.1
> > >
^ permalink raw reply related [flat|nested] 14+ messages in thread