On Mon, Nov 29, 2021 at 07:55:18PM -0800, Srinivas Pandruvada wrote:

On Mon, 2021-11-29 at 19:20 -0800, Ricardo Neri wrote:

quoted

On Wed, Nov 24, 2021 at 03:09:20PM +0100, Rafael J. Wysocki wrote:

quoted

On Sat, Nov 6, 2021 at 2:34 AM Ricardo Neri
[off-list ref] wrote:

quoted

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 individual HFI
tables.

A subsequent changeset will provide per-CPU initialization and
interrupt
handling.

[...]

quoted

quoted

quoted

+       /*
+        * If we are here we know that CPUID_HFI_LEAF exists.
Parse the
+        * supported capabilities and the size of the HFI table.
+        */
+       reg = cpuid_edx(CPUID_HFI_LEAF);
+
+       hfi_features.capabilities = reg & HFI_CAPABILITIES_MASK;
+       if (!(hfi_features.capabilities &
HFI_CAPABILITIES_PERFORMANCE)) {
+               pr_err("Performance reporting not supported! Not
using HFI\n");

This doesn't need to be pr_err().

Should it be a pr_warn() or perhaps pr_info()?

May be even pr_debug as we can always enable dynamic debug, where we
need to debug.

Fair enough. That sounds good. I will make the change.

Thanks and BR,
Ricardo