LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] cpufreq: intel_pstate: Force intel_pstate to load when HWP disabled in firmware
@ 2021-05-13  7:59 Giovanni Gherdovich
  2021-05-13  9:24 ` Srinivas Pandruvada
  0 siblings, 1 reply; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-13  7:59 UTC (permalink / raw)
  To: Rafael J . Wysocki, Viresh Kumar
  Cc: Srinivas Pandruvada, Len Brown, linux-pm, linux-kernel,
	Giovanni Gherdovich

On CPUs succeeding SKX, eg. ICELAKE_X, intel_pstate doesn't load unless
CPUID advertises support for the HWP feature. Some OEMs, however, may offer
users the possibility to disable HWP from the BIOS config utility by
altering the output of CPUID.

Add the command line option "intel_pstate=hwp_broken_firmware" so that
intel_pstate still loads in that case, providing OS-driven frequency
scaling.

Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
---
 Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
 Documentation/admin-guide/pm/intel_pstate.rst   | 7 +++++++
 drivers/cpufreq/intel_pstate.c                  | 7 ++++++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index cb89dbdedc46..278ec0718dc9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1951,6 +1951,13 @@
 			per_cpu_perf_limits
 			  Allow per-logical-CPU P-State performance control limits using
 			  cpufreq sysfs interface
+			hwp_broken_firmware
+			  Register intel_pstate as the scaling driver despite the
+			  hardware-managed P-states (HWP) feature being disabled in
+			  firmware. On CPU models succeeding SKX, intel_pstate expects
+			  HWP to be supported. Some OEMs may use firmware that hides the
+			  feature from the OS. With this option intel_pstate will
+			  load regardless.
 
 	intremap=	[X86-64, Intel-IOMMU]
 			on	enable Interrupt Remapping (default)
diff --git a/Documentation/admin-guide/pm/intel_pstate.rst b/Documentation/admin-guide/pm/intel_pstate.rst
index df29b4f1f219..1e6f139d5b05 100644
--- a/Documentation/admin-guide/pm/intel_pstate.rst
+++ b/Documentation/admin-guide/pm/intel_pstate.rst
@@ -689,6 +689,13 @@ of them have to be prepended with the ``intel_pstate=`` prefix.
 	Use per-logical-CPU P-State limits (see `Coordination of P-state
 	Limits`_ for details).
 
+``hwp_broken_firmware``
+	Register ``intel_pstate`` as the scaling driver despite the
+	hardware-managed P-states (HWP) feature being disabled in firmware.
+
+	On CPU models succeeding SKX, ``intel_pstate`` expects HWP to be
+	supported. Some OEMs may use firmware that hides the feature from the
+	OS. With this option ``intel_pstate`` will load regardless.
 
 Diagnostics and Tuning
 ======================
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index f0401064d7aa..8635251f86f2 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2856,6 +2856,7 @@ static int intel_pstate_update_status(const char *buf, size_t size)
 static int no_load __initdata;
 static int no_hwp __initdata;
 static int hwp_only __initdata;
+static int hwp_broken_firmware __initdata;
 static unsigned int force_load __initdata;
 
 static int __init intel_pstate_msrs_not_valid(void)
@@ -3066,7 +3067,7 @@ static int __init intel_pstate_init(void)
 		}
 	} else {
 		id = x86_match_cpu(intel_pstate_cpu_ids);
-		if (!id) {
+		if (!id && !hwp_broken_firmware) {
 			pr_info("CPU model not supported\n");
 			return -ENODEV;
 		}
@@ -3149,6 +3150,10 @@ static int __init intel_pstate_setup(char *str)
 		force_load = 1;
 	if (!strcmp(str, "hwp_only"))
 		hwp_only = 1;
+	if (!strcmp(str, "hwp_broken_firmware")) {
+		pr_info("HWP disabled by firmware\n");
+		hwp_broken_firmware = 1;
+	}
 	if (!strcmp(str, "per_cpu_perf_limits"))
 		per_cpu_limits = true;
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] cpufreq: intel_pstate: Force intel_pstate to load when HWP disabled in firmware
  2021-05-13  7:59 [PATCH] cpufreq: intel_pstate: Force intel_pstate to load when HWP disabled in firmware Giovanni Gherdovich
@ 2021-05-13  9:24 ` Srinivas Pandruvada
  2021-05-13 10:10   ` Giovanni Gherdovich
  0 siblings, 1 reply; 23+ messages in thread
From: Srinivas Pandruvada @ 2021-05-13  9:24 UTC (permalink / raw)
  To: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar
  Cc: Len Brown, linux-pm, linux-kernel

On Thu, 2021-05-13 at 09:59 +0200, Giovanni Gherdovich wrote:
> On CPUs succeeding SKX, eg. ICELAKE_X, intel_pstate doesn't load
> unless
> CPUID advertises support for the HWP feature. Some OEMs, however, may
> offer
> users the possibility to disable HWP from the BIOS config utility by
> altering the output of CPUID.
Is someone providing a utility? What is the case for broken HWP?

It is possible that some user don't want to use HWP, because there
workloads works better without HWP. But that doesn't mean HWP is
broken.

Thanks,
Srinivas

> 
> Add the command line option "intel_pstate=hwp_broken_firmware" so
> that
> intel_pstate still loads in that case, providing OS-driven frequency
> scaling.
> 
> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
>  Documentation/admin-guide/pm/intel_pstate.rst   | 7 +++++++
>  drivers/cpufreq/intel_pstate.c                  | 7 ++++++-
>  3 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt
> b/Documentation/admin-guide/kernel-parameters.txt
> index cb89dbdedc46..278ec0718dc9 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1951,6 +1951,13 @@
>                         per_cpu_perf_limits
>                           Allow per-logical-CPU P-State performance
> control limits using
>                           cpufreq sysfs interface
> +                       hwp_broken_firmware
> +                         Register intel_pstate as the scaling driver
> despite the
> +                         hardware-managed P-states (HWP) feature
> being disabled in
> +                         firmware. On CPU models succeeding SKX,
> intel_pstate expects
> +                         HWP to be supported. Some OEMs may use
> firmware that hides the
> +                         feature from the OS. With this option
> intel_pstate will
> +                         load regardless.
>  
>         intremap=       [X86-64, Intel-IOMMU]
>                         on      enable Interrupt Remapping (default)
> diff --git a/Documentation/admin-guide/pm/intel_pstate.rst
> b/Documentation/admin-guide/pm/intel_pstate.rst
> index df29b4f1f219..1e6f139d5b05 100644
> --- a/Documentation/admin-guide/pm/intel_pstate.rst
> +++ b/Documentation/admin-guide/pm/intel_pstate.rst
> @@ -689,6 +689,13 @@ of them have to be prepended with the
> ``intel_pstate=`` prefix.
>         Use per-logical-CPU P-State limits (see `Coordination of P-
> state
>         Limits`_ for details).
>  
> +``hwp_broken_firmware``
> +       Register ``intel_pstate`` as the scaling driver despite the
> +       hardware-managed P-states (HWP) feature being disabled in
> firmware.
> +
> +       On CPU models succeeding SKX, ``intel_pstate`` expects HWP to
> be
> +       supported. Some OEMs may use firmware that hides the feature
> from the
> +       OS. With this option ``intel_pstate`` will load regardless.
>  
>  Diagnostics and Tuning
>  ======================
> diff --git a/drivers/cpufreq/intel_pstate.c
> b/drivers/cpufreq/intel_pstate.c
> index f0401064d7aa..8635251f86f2 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2856,6 +2856,7 @@ static int intel_pstate_update_status(const
> char *buf, size_t size)
>  static int no_load __initdata;
>  static int no_hwp __initdata;
>  static int hwp_only __initdata;
> +static int hwp_broken_firmware __initdata;
>  static unsigned int force_load __initdata;
>  
>  static int __init intel_pstate_msrs_not_valid(void)
> @@ -3066,7 +3067,7 @@ static int __init intel_pstate_init(void)
>                 }
>         } else {
>                 id = x86_match_cpu(intel_pstate_cpu_ids);
> -               if (!id) {
> +               if (!id && !hwp_broken_firmware) {
>                         pr_info("CPU model not supported\n");
>                         return -ENODEV;
>                 }
> @@ -3149,6 +3150,10 @@ static int __init intel_pstate_setup(char
> *str)
>                 force_load = 1;
>         if (!strcmp(str, "hwp_only"))
>                 hwp_only = 1;
> +       if (!strcmp(str, "hwp_broken_firmware")) {
> +               pr_info("HWP disabled by firmware\n");
> +               hwp_broken_firmware = 1;
> +       }
>         if (!strcmp(str, "per_cpu_perf_limits"))
>                 per_cpu_limits = true;
>  



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] cpufreq: intel_pstate: Force intel_pstate to load when HWP disabled in firmware
  2021-05-13  9:24 ` Srinivas Pandruvada
@ 2021-05-13 10:10   ` Giovanni Gherdovich
  2021-05-13 11:03     ` Srinivas Pandruvada
  0 siblings, 1 reply; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-13 10:10 UTC (permalink / raw)
  To: Srinivas Pandruvada, Rafael J . Wysocki, Viresh Kumar
  Cc: Len Brown, linux-pm, linux-kernel

On Thu, 2021-05-13 at 02:24 -0700, Srinivas Pandruvada wrote:
> On Thu, 2021-05-13 at 09:59 +0200, Giovanni Gherdovich wrote:
> > On CPUs succeeding SKX, eg. ICELAKE_X, intel_pstate doesn't load
> > unless
> > CPUID advertises support for the HWP feature. Some OEMs, however, may
> > offer
> > users the possibility to disable HWP from the BIOS config utility by
> > altering the output of CPUID.
>
> Is someone providing a utility? What is the case for broken HWP?

Yes, I know of at least one server manufacturer that ships a BIOS config
utility where the user can disable HWP.

On such server machine, which has an ICELAKE_X CPU, if the user unchecks HWP
via BIOS then intel_pstate will refuse to load saying:

    intel_pstate: CPU model not supported

because ICELAKE_X is not in the list intel_pstate_cpu_ids (defined in
intel_pstate.c) of CPUs that intel_pstate supports when HWP is absent from
CPUID; that list ends at SKYLAKE_X.

An alternative approach to register intel_pstate in the case I'm describing
would be to add ICELAKE_X (and every CPU model after that, forever?) to the
list intel_pstate_cpu_ids.

> It is possible that some user don't want to use HWP, because there
> workloads works better without HWP. But that doesn't mean HWP is
> broken.

That's true, a user may legitimate want to disable HWP, and we have the
intel_pstate=no_hwp option for that. But for that option to work CPUID must
still show that the CPU is HWP-capable; when disablement happens in BIOS, it's
not the case.

The wording "hwp_broken_firmware" deliberately has a negative connotation (the
intended meaning is: "firmware is broken, regarding HWP"), carrying the
not-so-subtle message "OEM folks, please don't do this". My understanding is
that the preferred way to disable HWP is with intel_pstate=no_hwp, the
firmware should stay out of it.

I hope this clarifies the problem (there is an ICELAKE_X somewhere out there
that can't load intel_pstate, which is not nice) and the intention
(discouraging disablement of HWP via firmware).


Giovanni


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] cpufreq: intel_pstate: Force intel_pstate to load when HWP disabled in firmware
  2021-05-13 10:10   ` Giovanni Gherdovich
@ 2021-05-13 11:03     ` Srinivas Pandruvada
  2021-05-13 12:10       ` Giovanni Gherdovich
  2021-05-13 13:20       ` [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode Giovanni Gherdovich
  0 siblings, 2 replies; 23+ messages in thread
From: Srinivas Pandruvada @ 2021-05-13 11:03 UTC (permalink / raw)
  To: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar
  Cc: Len Brown, linux-pm, linux-kernel

On Thu, 2021-05-13 at 12:10 +0200, Giovanni Gherdovich wrote:
> On Thu, 2021-05-13 at 02:24 -0700, Srinivas Pandruvada wrote:
> > On Thu, 2021-05-13 at 09:59 +0200, Giovanni Gherdovich wrote:
> > > On CPUs succeeding SKX, eg. ICELAKE_X, intel_pstate doesn't load
> > > unless
> > > CPUID advertises support for the HWP feature. Some OEMs, however,
> > > may
> > > offer
> > > users the possibility to disable HWP from the BIOS config utility
> > > by
> > > altering the output of CPUID.
> > 
> > Is someone providing a utility? What is the case for broken HWP?
> 
> Yes, I know of at least one server manufacturer that ships a BIOS
> config
> utility where the user can disable HWP.
> 
> On such server machine, which has an ICELAKE_X CPU, if the user
> unchecks HWP
> via BIOS then intel_pstate will refuse to load saying:
> 
>     intel_pstate: CPU model not supported
> 
> because ICELAKE_X is not in the list intel_pstate_cpu_ids (defined in
> intel_pstate.c) of CPUs that intel_pstate supports when HWP is absent
> from
> CPUID; that list ends at SKYLAKE_X.
> 
> An alternative approach to register intel_pstate in the case I'm
> describing
> would be to add ICELAKE_X (and every CPU model after that, forever?)
> to the
> list intel_pstate_cpu_ids.
This is not nice, but unlike client server CPUs don't get released
often. There is couple of years in between.

> 
> > It is possible that some user don't want to use HWP, because there
> > workloads works better without HWP. But that doesn't mean HWP is
> > broken.
> 
> That's true, a user may legitimate want to disable HWP, and we have
> the
> intel_pstate=no_hwp option for that. But for that option to work
> CPUID must
> still show that the CPU is HWP-capable; when disablement happens in
> BIOS, it's
> not the case.
Correct.

> 
> The wording "hwp_broken_firmware" deliberately has a negative
> connotation (the
> intended meaning is: "firmware is broken, regarding HWP"), carrying
> the
> not-so-subtle message "OEM folks, please don't do this". My
> understanding is
> that the preferred way to disable HWP is with intel_pstate=no_hwp,
> the
> firmware should stay out of it.
For me "broken" means that Intel has some bug, which is not the case,
even if the intention is to carry message to OEM.

no_hwp is for disabling HWP even if the HWP is supported.

The problem is that if we override the supported CPU list using some
kernel command line, some users may crash the system running on some
old hardware where some of the MSRs we rely are not present. We don't
read MSR in failsafe mode, so they will fault. We are checking some
MSRs but not all. Also what will be default struct pstate_funcs *)id-
>driver_data if the cpu model doesn't match.

I think better to add CPU model instead. We did that for SKX on user
requests.

Thanks,
Srinivas

> 
> I hope this clarifies the problem (there is an ICELAKE_X somewhere
> out there
> that can't load intel_pstate, which is not nice) and the intention
> (discouraging disablement of HWP via firmware).
> 
> 
> Giovanni
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] cpufreq: intel_pstate: Force intel_pstate to load when HWP disabled in firmware
  2021-05-13 11:03     ` Srinivas Pandruvada
@ 2021-05-13 12:10       ` Giovanni Gherdovich
  2021-05-13 13:20       ` [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode Giovanni Gherdovich
  1 sibling, 0 replies; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-13 12:10 UTC (permalink / raw)
  To: Srinivas Pandruvada, Rafael J . Wysocki, Viresh Kumar
  Cc: Len Brown, linux-pm, linux-kernel

On Thu, 2021-05-13 at 04:03 -0700, Srinivas Pandruvada wrote:
> On Thu, 2021-05-13 at 12:10 +0200, Giovanni Gherdovich wrote:
> > [...]
> > An alternative approach to register intel_pstate in the case I'm
> > describing
> > would be to add ICELAKE_X (and every CPU model after that, forever?)
> > to the
> > list intel_pstate_cpu_ids.
>
> This is not nice, but unlike client server CPUs don't get released
> often. There is couple of years in between.

True.

> > [...]
> > The wording "hwp_broken_firmware" deliberately has a negative
> > connotation (the
> > intended meaning is: "firmware is broken, regarding HWP"), carrying
> > the
> > not-so-subtle message "OEM folks, please don't do this". My
> > understanding is
> > that the preferred way to disable HWP is with intel_pstate=no_hwp,
> > the
> > firmware should stay out of it.
>
> For me "broken" means that Intel has some bug, which is not the case,
> even if the intention is to carry message to OEM.
> 
> no_hwp is for disabling HWP even if the HWP is supported.
> 
> The problem is that if we override the supported CPU list using some
> kernel command line, some users may crash the system running on some
> old hardware where some of the MSRs we rely are not present. We don't
> read MSR in failsafe mode, so they will fault. We are checking some
> MSRs but not all.

Fair enough.

> Also what will be default
> (struct pstate_funcs *)id->driver_data if the cpu model doesn't match.

Whoops... You're totally right, the patch I sent is broken! "id" must be a
valid pstate_funcs* pointer, or some other default methods must be provided.

And besides...

> I think better to add CPU model instead. We did that for SKX on user
> requests.

... I agree. Let's just add ICX to the list of explicitly supported CPUs.
I'll send a new patch doing that, please discard this one.


Giovanni


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-13 11:03     ` Srinivas Pandruvada
  2021-05-13 12:10       ` Giovanni Gherdovich
@ 2021-05-13 13:20       ` Giovanni Gherdovich
  2021-05-14 15:31         ` Doug Smythies
  1 sibling, 1 reply; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-13 13:20 UTC (permalink / raw)
  To: Rafael J . Wysocki, Viresh Kumar, Srinivas Pandruvada
  Cc: Len Brown, linux-pm, linux-kernel, Giovanni Gherdovich

Users may disable HWP in firmware, in which case intel_pstate wouldn't load
unless the CPU model is explicitly supported.

Add ICELAKE_X to the list of CPUs that can register intel_pstate while not
advertising the HWP capability. Without this change, an ICELAKE_X in no-HWP
mode could only use the acpi_cpufreq frequency scaling driver.

See also commit d8de7a44e11f ("cpufreq: intel_pstate: Add Skylake servers
support").

Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
---
This replaces https://lore.kernel.org/lkml/20210513075930.22657-1-ggherdovich@suse.cz

 drivers/cpufreq/intel_pstate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index f0401064d7aa..28c9733e0dce 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2087,6 +2087,7 @@ static const struct x86_cpu_id intel_pstate_cpu_ids[] = {
 	X86_MATCH(ATOM_GOLDMONT,	core_funcs),
 	X86_MATCH(ATOM_GOLDMONT_PLUS,	core_funcs),
 	X86_MATCH(SKYLAKE_X,		core_funcs),
+	X86_MATCH(ICELAKE_X,		core_funcs),
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, intel_pstate_cpu_ids);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-13 13:20       ` [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode Giovanni Gherdovich
@ 2021-05-14 15:31         ` Doug Smythies
  2021-05-14 20:33           ` Giovanni Gherdovich
  2021-05-17 15:26           ` Rafael J. Wysocki
  0 siblings, 2 replies; 23+ messages in thread
From: Doug Smythies @ 2021-05-14 15:31 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Rafael J . Wysocki, Viresh Kumar, Srinivas Pandruvada, Len Brown,
	Linux PM list, Linux Kernel Mailing List, dsmythies

Hi All,

Can I on-board to this patch or do you want me to submit another?
I want to add COMETLAKE (tested), as below:

... Doug

On Thu, May 13, 2021 at 6:21 AM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
>
> Users may disable HWP in firmware, in which case intel_pstate wouldn't load
> unless the CPU model is explicitly supported.
>
> Add ICELAKE_X to the list of CPUs that can register intel_pstate while not
> advertising the HWP capability. Without this change, an ICELAKE_X in no-HWP
> mode could only use the acpi_cpufreq frequency scaling driver.
>
> See also commit d8de7a44e11f ("cpufreq: intel_pstate: Add Skylake servers
> support").
>
> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> ---
> This replaces https://lore.kernel.org/lkml/20210513075930.22657-1-ggherdovich@suse.cz
>
>  drivers/cpufreq/intel_pstate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index f0401064d7aa..28c9733e0dce 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2087,6 +2087,7 @@ static const struct x86_cpu_id intel_pstate_cpu_ids[] = {
>         X86_MATCH(ATOM_GOLDMONT,        core_funcs),
>         X86_MATCH(ATOM_GOLDMONT_PLUS,   core_funcs),
>         X86_MATCH(SKYLAKE_X,            core_funcs),
> +       X86_MATCH(ICELAKE_X,            core_funcs),
   +       X86_MATCH(COMETLAKE,          core_funcs),
>         {}
>  };
>  MODULE_DEVICE_TABLE(x86cpu, intel_pstate_cpu_ids);
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-14 15:31         ` Doug Smythies
@ 2021-05-14 20:33           ` Giovanni Gherdovich
  2021-05-14 22:12             ` Doug Smythies
  2021-05-15  2:58             ` Srinivas Pandruvada
  2021-05-17 15:26           ` Rafael J. Wysocki
  1 sibling, 2 replies; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-14 20:33 UTC (permalink / raw)
  To: Doug Smythies
  Cc: Rafael J . Wysocki, Viresh Kumar, Srinivas Pandruvada, Len Brown,
	Linux PM list, Linux Kernel Mailing List

On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
> Hi All,
> 
> Can I on-board to this patch or do you want me to submit another?
> I want to add COMETLAKE (tested), as below:
> 
> ... Doug

Hello Doug!

Wait, why you don't want to use HWP? It's such a fantastic technology!

:) I'm just teasing you.

More seriously: 

when COMETLAKE is not in that list, can you confirm that if you go into the
BIOS config at boot, and disable HWP from there, then intel_pstate does *not* load?

Does it say "intel_pstate: CPU model not supported" in the dmesg log?

The control may be somewhere around "power mangement" in the BIOS config, and
may be called "Enable/disable Intel Speed Shift".

I'm asking because I've just checked on two Dell laptops, one Skylake and the
other Kabylake, and the menu is there in the BIOS config to disable HWP,
but if I disable it... nothing happens. "lscpu" shows all the hwp flags as usual:

    # lscpu | grep Flags | tr ' ' '\n' | grep hwp
    hwp
    hwp_notify
    hwp_act_window
    hwp_epp

and turbostat gives me:

    # turbostat -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
    cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)

Which is to say, on the Intel client machines I have, the firmware doesn't
seem to be able to hide HWP from the OS. Buggy BIOS? Maybe, the fact of the
matter is, I wouldn't need to add, say, KABYLAKE to that list, based on my
experience.

The other side of the issue is that, from my understanding, the
preferred/supported way to disable HWP is to boot with intel_pstate=no_hwp,
and that list is a sort of "known exceptions" that people really can't live
without (it's mostly server CPUs, and mostly because of unfortunate firmware
defaults). Otherwise you'd see the entire intel-family.h file in there.


Cheers,
Giovanni

> 
> On Thu, May 13, 2021 at 6:21 AM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
> > Users may disable HWP in firmware, in which case intel_pstate wouldn't load
> > unless the CPU model is explicitly supported.
> > 
> > Add ICELAKE_X to the list of CPUs that can register intel_pstate while not
> > advertising the HWP capability. Without this change, an ICELAKE_X in no-HWP
> > mode could only use the acpi_cpufreq frequency scaling driver.
> > 
> > See also commit d8de7a44e11f ("cpufreq: intel_pstate: Add Skylake servers
> > support").
> > 
> > Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> > ---
> > This replaces https://lore.kernel.org/lkml/20210513075930.22657-1-ggherdovich@suse.cz
> > 
> >  drivers/cpufreq/intel_pstate.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index f0401064d7aa..28c9733e0dce 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -2087,6 +2087,7 @@ static const struct x86_cpu_id intel_pstate_cpu_ids[] = {
> >         X86_MATCH(ATOM_GOLDMONT,        core_funcs),
> >         X86_MATCH(ATOM_GOLDMONT_PLUS,   core_funcs),
> >         X86_MATCH(SKYLAKE_X,            core_funcs),
> > +       X86_MATCH(ICELAKE_X,            core_funcs),
>    +       X86_MATCH(COMETLAKE,          core_funcs),
> >         {}
> >  };
> >  MODULE_DEVICE_TABLE(x86cpu, intel_pstate_cpu_ids);
> > --
> > 2.26.2
> > 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-14 20:33           ` Giovanni Gherdovich
@ 2021-05-14 22:12             ` Doug Smythies
  2021-09-07 15:45               ` Doug Smythies
  2021-05-15  2:58             ` Srinivas Pandruvada
  1 sibling, 1 reply; 23+ messages in thread
From: Doug Smythies @ 2021-05-14 22:12 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Rafael J . Wysocki, Viresh Kumar, Srinivas Pandruvada, Len Brown,
	Linux PM list, Linux Kernel Mailing List, dsmythies

On Fri, May 14, 2021 at 1:33 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
> On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
>
> > Can I on-board to this patch or do you want me to submit another?
> > I want to add COMETLAKE (tested), as below:
> >
> > ... Doug
>
> Hello Doug!

Hi Giovanni,
Thank you for your reply.
>
> Wait, why you don't want to use HWP? It's such a fantastic technology!
>
> :) I'm just teasing you.
>
> More seriously:
>
> when COMETLAKE is not in that list, can you confirm that if you go into the
> BIOS config at boot, and disable HWP from there, then intel_pstate does *not* load?

Yes, already tested before my original reply.

> Does it say "intel_pstate: CPU model not supported" in the dmesg log?

That I did not check, but if I boot now with an unmodified kernel
5.13-rc1 (i.e. without this patch):

[    0.369323] intel_pstate: CPU model not supported

> The control may be somewhere around "power mangement" in the BIOS config, and
> may be called "Enable/disable Intel Speed Shift".

Yes.

> I'm asking because I've just checked on two Dell laptops, one Skylake and the
> other Kabylake, and the menu is there in the BIOS config to disable HWP,
> but if I disable it... nothing happens. "lscpu" shows all the hwp flags as usual:

Motherboard here is ASUS PRIME Z490-A.
CPU: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz

>     # lscpu | grep Flags | tr ' ' '\n' | grep hwp
>     hwp
>     hwp_notify
>     hwp_act_window
>     hwp_epp

Here, for some reason I have to do it this way (sudo) or your command
doesn't work properly. Results herein confirmed by looking at the
"Flags" output manually without filtering:

intel_speed_shift = Disabled in BIOS:

doug@s19:~$ sudo lscpu | tr ' ' '\n' | grep hwp
doug@s19:~$

intel_speed_shift = Auto in BIOS

$ sudo lscpu | tr ' ' '\n' | grep hwp
hwp
hwp_notify
hwp_act_window
hwp_epp

> and turbostat gives me:
>
>     # turbostat -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
>     cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)

Here:

intel_speed_shift = Disabled in BIOS:

root@s19:/home/doug#
/home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
-Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
root@s19:/home/doug#

intel_speed_shift = Auto in BIOS (the default setting)

root@s19:/home/doug#
/home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
-Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)

or with "intel_pstate=no_hwp"

root@s19:/home/doug#
/home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
-Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
cpu0: MSR_PM_ENABLE: 0x00000000 (No-HWP)

> Which is to say, on the Intel client machines I have, the firmware doesn't
> seem to be able to hide HWP from the OS. Buggy BIOS? Maybe, the fact of the
> matter is, I wouldn't need to add, say, KABYLAKE to that list, based on my
> experience.

My experience (hardware) differs from yours with respect to this.

> The other side of the issue is that, from my understanding, the
> preferred/supported way to disable HWP is to boot with intel_pstate=no_hwp,
> and that list is a sort of "known exceptions" that people really can't live
> without (it's mostly server CPUs, and mostly because of unfortunate firmware
> defaults). Otherwise you'd see the entire intel-family.h file in there.

I'm not sure how to respond here. Yes, I'd expect to see a big list
here, and in the recently added TCC Offset thermal stuff and in the
recently added turbostat patches to deal with a TCC offset. I do not
understand doing things partially only. But that is a bigger/broader
subject than herein.

That said, yes, "intel_pstate=no_hwp" is what I normally do. And my
BIOS normally has "Intel Speed Shift = AUTO", which is the default.

... deleted the rest ...

... Doug

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-14 20:33           ` Giovanni Gherdovich
  2021-05-14 22:12             ` Doug Smythies
@ 2021-05-15  2:58             ` Srinivas Pandruvada
  1 sibling, 0 replies; 23+ messages in thread
From: Srinivas Pandruvada @ 2021-05-15  2:58 UTC (permalink / raw)
  To: Giovanni Gherdovich, Doug Smythies
  Cc: Rafael J . Wysocki, Viresh Kumar, Len Brown, Linux PM list,
	Linux Kernel Mailing List

On Fri, 2021-05-14 at 22:33 +0200, Giovanni Gherdovich wrote:
> On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
> > Hi All,
> > 
> > Can I on-board to this patch or do you want me to submit another?
> > I want to add COMETLAKE (tested), as below:
> > 
> > ... Doug
> 
> Hello Doug!
> 
> Wait, why you don't want to use HWP? It's such a fantastic
> technology!
> 
> :) I'm just teasing you.
> 
> More seriously: 
> 
> when COMETLAKE is not in that list, can you confirm that if you go
> into the
> BIOS config at boot, and disable HWP from there, then intel_pstate
> does *not* load?
> 
> Does it say "intel_pstate: CPU model not supported" in the dmesg log?
> 
> The control may be somewhere around "power mangement" in the BIOS
> config, and
> may be called "Enable/disable Intel Speed Shift".
> 
> I'm asking because I've just checked on two Dell laptops, one Skylake
> and the
> other Kabylake, and the menu is there in the BIOS config to disable
> HWP,
> but if I disable it... nothing happens. "lscpu" shows all the hwp
> flags as usual:
> 
>     # lscpu | grep Flags | tr ' ' '\n' | grep hwp
>     hwp
>     hwp_notify
>     hwp_act_window
>     hwp_epp
> 
> and turbostat gives me:
> 
>     # turbostat -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
>     cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)
> 
> Which is to say, on the Intel client machines I have, the firmware
> doesn't
> seem to be able to hide HWP from the OS. Buggy BIOS? Maybe, the fact
> of the
> matter is, I wouldn't need to add, say, KABYLAKE to that list, based
> on my
> experience.

When you disable in BIOS on these systems, it just hides HWP control
via ACPI CPC table. It doesn't disable HWP CPU feature.

Thanks,
Srinivas

> 
> The other side of the issue is that, from my understanding, the
> preferred/supported way to disable HWP is to boot with
> intel_pstate=no_hwp,
> and that list is a sort of "known exceptions" that people really
> can't live
> without (it's mostly server CPUs, and mostly because of unfortunate
> firmware
> defaults). Otherwise you'd see the entire intel-family.h file in
> there.
> 
> 
> Cheers,
> Giovanni
> 
> > 
> > On Thu, May 13, 2021 at 6:21 AM Giovanni Gherdovich <
> > ggherdovich@suse.cz> wrote:
> > > Users may disable HWP in firmware, in which case intel_pstate
> > > wouldn't load
> > > unless the CPU model is explicitly supported.
> > > 
> > > Add ICELAKE_X to the list of CPUs that can register intel_pstate
> > > while not
> > > advertising the HWP capability. Without this change, an ICELAKE_X
> > > in no-HWP
> > > mode could only use the acpi_cpufreq frequency scaling driver.
> > > 
> > > See also commit d8de7a44e11f ("cpufreq: intel_pstate: Add Skylake
> > > servers
> > > support").
> > > 
> > > Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> > > ---
> > > This replaces 
> > > https://lore.kernel.org/lkml/20210513075930.22657-1-ggherdovich@suse.cz
> > > 
> > >  drivers/cpufreq/intel_pstate.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/cpufreq/intel_pstate.c
> > > b/drivers/cpufreq/intel_pstate.c
> > > index f0401064d7aa..28c9733e0dce 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -2087,6 +2087,7 @@ static const struct x86_cpu_id
> > > intel_pstate_cpu_ids[] = {
> > >         X86_MATCH(ATOM_GOLDMONT,        core_funcs),
> > >         X86_MATCH(ATOM_GOLDMONT_PLUS,   core_funcs),
> > >         X86_MATCH(SKYLAKE_X,            core_funcs),
> > > +       X86_MATCH(ICELAKE_X,            core_funcs),
> >    +       X86_MATCH(COMETLAKE,          core_funcs),
> > >         {}
> > >  };
> > >  MODULE_DEVICE_TABLE(x86cpu, intel_pstate_cpu_ids);
> > > --
> > > 2.26.2
> > > 
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-14 15:31         ` Doug Smythies
  2021-05-14 20:33           ` Giovanni Gherdovich
@ 2021-05-17 15:26           ` Rafael J. Wysocki
  2021-05-18 11:33             ` Giovanni Gherdovich
  2021-05-18 12:34             ` [PATCH v3 1/2] " Giovanni Gherdovich
  1 sibling, 2 replies; 23+ messages in thread
From: Rafael J. Wysocki @ 2021-05-17 15:26 UTC (permalink / raw)
  To: Doug Smythies
  Cc: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar,
	Srinivas Pandruvada, Len Brown, Linux PM list,
	Linux Kernel Mailing List

On Fri, May 14, 2021 at 5:31 PM Doug Smythies <dsmythies@telus.net> wrote:
>
> Hi All,
>
> Can I on-board to this patch or do you want me to submit another?

Please send another one.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-17 15:26           ` Rafael J. Wysocki
@ 2021-05-18 11:33             ` Giovanni Gherdovich
  2021-05-19  5:37               ` Doug Smythies
  2021-05-18 12:34             ` [PATCH v3 1/2] " Giovanni Gherdovich
  1 sibling, 1 reply; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-18 11:33 UTC (permalink / raw)
  To: Rafael J. Wysocki, Doug Smythies
  Cc: Rafael J . Wysocki, Viresh Kumar, Srinivas Pandruvada, Len Brown,
	Linux PM list, Linux Kernel Mailing List

On Mon, 2021-05-17 at 17:26 +0200, Rafael J. Wysocki wrote:
> On Fri, May 14, 2021 at 5:31 PM Doug Smythies <dsmythies@telus.net> wrote:
> > Hi All,
> > 
> > Can I on-board to this patch or do you want me to submit another?
> 
> Please send another one.

Hello Rafael, Doug,

I can resend a series with two patches, one adding ICELAKE_X and the
second adding COMETLAKE to the intel_pstate_cpu_ids array.
I'll prepare it.


Giovanni


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v3 1/2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-17 15:26           ` Rafael J. Wysocki
  2021-05-18 11:33             ` Giovanni Gherdovich
@ 2021-05-18 12:34             ` Giovanni Gherdovich
  2021-05-18 12:34               ` [PATCH v3 2/2] cpufreq: intel_pstate: Add Cometlake " Giovanni Gherdovich
  2021-05-21 16:49               ` [PATCH v3 1/2] cpufreq: intel_pstate: Add Icelake servers " Rafael J. Wysocki
  1 sibling, 2 replies; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-18 12:34 UTC (permalink / raw)
  To: Rafael J . Wysocki, Viresh Kumar
  Cc: Srinivas Pandruvada, Doug Smythies, Len Brown, linux-pm,
	linux-kernel, Giovanni Gherdovich

Users may disable HWP in firmware, in which case intel_pstate wouldn't load
unless the CPU model is explicitly supported.

Add ICELAKE_X to the list of CPUs that can register intel_pstate while not
advertising the HWP capability. Without this change, an ICELAKE_X in no-HWP
mode could only use the acpi_cpufreq frequency scaling driver.

See also commit d8de7a44e11f ("cpufreq: intel_pstate: Add Skylake servers
support").

Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
---
This replaces https://lore.kernel.org/lkml/20210513132051.31465-1-ggherdovich@suse.cz

 drivers/cpufreq/intel_pstate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 0e69dffd5a76..9a93603acd3f 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2087,6 +2087,7 @@ static const struct x86_cpu_id intel_pstate_cpu_ids[] = {
 	X86_MATCH(ATOM_GOLDMONT,	core_funcs),
 	X86_MATCH(ATOM_GOLDMONT_PLUS,	core_funcs),
 	X86_MATCH(SKYLAKE_X,		core_funcs),
+	X86_MATCH(ICELAKE_X,		core_funcs),
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, intel_pstate_cpu_ids);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v3 2/2] cpufreq: intel_pstate: Add Cometlake support in no-HWP mode
  2021-05-18 12:34             ` [PATCH v3 1/2] " Giovanni Gherdovich
@ 2021-05-18 12:34               ` Giovanni Gherdovich
  2021-05-21 16:49               ` [PATCH v3 1/2] cpufreq: intel_pstate: Add Icelake servers " Rafael J. Wysocki
  1 sibling, 0 replies; 23+ messages in thread
From: Giovanni Gherdovich @ 2021-05-18 12:34 UTC (permalink / raw)
  To: Rafael J . Wysocki, Viresh Kumar
  Cc: Srinivas Pandruvada, Doug Smythies, Len Brown, linux-pm,
	linux-kernel, Giovanni Gherdovich

Users may disable HWP in firmware, in which case intel_pstate wouldn't load
unless the CPU model is explicitly supported.

See also commit d8de7a44e11f ("cpufreq: intel_pstate: Add Skylake servers
support").

Suggested-by: Doug Smythies <dsmythies@telus.net>
Tested-by: Doug Smythies <dsmythies@telus.net>
Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
---
 drivers/cpufreq/intel_pstate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 9a93603acd3f..a6d8e56c8e27 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2087,6 +2087,7 @@ static const struct x86_cpu_id intel_pstate_cpu_ids[] = {
 	X86_MATCH(ATOM_GOLDMONT,	core_funcs),
 	X86_MATCH(ATOM_GOLDMONT_PLUS,	core_funcs),
 	X86_MATCH(SKYLAKE_X,		core_funcs),
+	X86_MATCH(COMETLAKE,		core_funcs),
 	X86_MATCH(ICELAKE_X,		core_funcs),
 	{}
 };
-- 
2.26.2


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-18 11:33             ` Giovanni Gherdovich
@ 2021-05-19  5:37               ` Doug Smythies
  0 siblings, 0 replies; 23+ messages in thread
From: Doug Smythies @ 2021-05-19  5:37 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Rafael J. Wysocki, Rafael J . Wysocki, Viresh Kumar,
	Srinivas Pandruvada, Len Brown, Linux PM list,
	Linux Kernel Mailing List, dsmythies

On Tue, May 18, 2021 at 4:33 AM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
> On Mon, 2021-05-17 at 17:26 +0200, Rafael J. Wysocki wrote:
> > On Fri, May 14, 2021 at 5:31 PM Doug Smythies <dsmythies@telus.net> wrote:
> > > Hi All,
> > >
> > > Can I on-board to this patch or do you want me to submit another?
> >
> > Please send another one.
>
> Hello Rafael, Doug,
>
> I can resend a series with two patches, one adding ICELAKE_X and the
> second adding COMETLAKE to the intel_pstate_cpu_ids array.
> I'll prepare it.

That is very nice of you. Thanks.

... Doug

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v3 1/2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-18 12:34             ` [PATCH v3 1/2] " Giovanni Gherdovich
  2021-05-18 12:34               ` [PATCH v3 2/2] cpufreq: intel_pstate: Add Cometlake " Giovanni Gherdovich
@ 2021-05-21 16:49               ` Rafael J. Wysocki
  1 sibling, 0 replies; 23+ messages in thread
From: Rafael J. Wysocki @ 2021-05-21 16:49 UTC (permalink / raw)
  To: Giovanni Gherdovich
  Cc: Rafael J . Wysocki, Viresh Kumar, Srinivas Pandruvada,
	Doug Smythies, Len Brown, Linux PM, Linux Kernel Mailing List

On Tue, May 18, 2021 at 2:34 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
>
> Users may disable HWP in firmware, in which case intel_pstate wouldn't load
> unless the CPU model is explicitly supported.
>
> Add ICELAKE_X to the list of CPUs that can register intel_pstate while not
> advertising the HWP capability. Without this change, an ICELAKE_X in no-HWP
> mode could only use the acpi_cpufreq frequency scaling driver.
>
> See also commit d8de7a44e11f ("cpufreq: intel_pstate: Add Skylake servers
> support").
>
> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> ---
> This replaces https://lore.kernel.org/lkml/20210513132051.31465-1-ggherdovich@suse.cz
>
>  drivers/cpufreq/intel_pstate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 0e69dffd5a76..9a93603acd3f 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2087,6 +2087,7 @@ static const struct x86_cpu_id intel_pstate_cpu_ids[] = {
>         X86_MATCH(ATOM_GOLDMONT,        core_funcs),
>         X86_MATCH(ATOM_GOLDMONT_PLUS,   core_funcs),
>         X86_MATCH(SKYLAKE_X,            core_funcs),
> +       X86_MATCH(ICELAKE_X,            core_funcs),
>         {}
>  };
>  MODULE_DEVICE_TABLE(x86cpu, intel_pstate_cpu_ids);
> --

Both patches in the series applied as 5.14 material, thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-05-14 22:12             ` Doug Smythies
@ 2021-09-07 15:45               ` Doug Smythies
  2021-09-07 16:01                 ` Srinivas Pandruvada
  0 siblings, 1 reply; 23+ messages in thread
From: Doug Smythies @ 2021-09-07 15:45 UTC (permalink / raw)
  To: Giovanni Gherdovich, Rafael J . Wysocki, Srinivas Pandruvada
  Cc: Viresh Kumar, Len Brown, Linux PM list, Linux Kernel Mailing List

Hi all,

Recent ASUS BIOS updates have changed the default system
response for this old thread, rendering "intel_pstate=no_hwp" useless.

It also raises a question: If BIOS has forced HWP, then how do we
prevent the acpi-cpufreq driver from being used? Read on.

On Fri, May 14, 2021 at 3:12 PM Doug Smythies <dsmythies@telus.net> wrote:
>
> On Fri, May 14, 2021 at 1:33 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
> > On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
...
> >
> > when COMETLAKE is not in that list, can you confirm that if you go into the
> > BIOS config at boot, and disable HWP from there, then intel_pstate does *not* load?
>
> Yes, already tested before my original reply.
>
> > Does it say "intel_pstate: CPU model not supported" in the dmesg log?
>
> That I did not check, but if I boot now with an unmodified kernel
> 5.13-rc1 (i.e. without this patch):
>
> [    0.369323] intel_pstate: CPU model not supported
>
> > The control may be somewhere around "power mangement" in the BIOS config, and
> > may be called "Enable/disable Intel Speed Shift".
>
> Yes.
>
> > I'm asking because I've just checked on two Dell laptops, one Skylake and the
> > other Kabylake, and the menu is there in the BIOS config to disable HWP,
> > but if I disable it... nothing happens. "lscpu" shows all the hwp flags as usual:
>
> Motherboard here is ASUS PRIME Z490-A.
> CPU: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz
>
> >     # lscpu | grep Flags | tr ' ' '\n' | grep hwp
> >     hwp
> >     hwp_notify
> >     hwp_act_window
> >     hwp_epp
>
> Here, for some reason I have to do it this way (sudo) or your command
> doesn't work properly. Results herein confirmed by looking at the
> "Flags" output manually without filtering:
>
> intel_speed_shift = Disabled in BIOS:
>
> doug@s19:~$ sudo lscpu | tr ' ' '\n' | grep hwp
> doug@s19:~$
>
> intel_speed_shift = Auto in BIOS
>
> $ sudo lscpu | tr ' ' '\n' | grep hwp
> hwp
> hwp_notify
> hwp_act_window
> hwp_epp
>
> > and turbostat gives me:
> >
> >     # turbostat -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> >     cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)
>
> Here:
>
> intel_speed_shift = Disabled in BIOS:
>
> root@s19:/home/doug#
> /home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
> -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> root@s19:/home/doug#
>
> intel_speed_shift = Auto in BIOS (the default setting)
>
> root@s19:/home/doug#
> /home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
> -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)
>
> or with "intel_pstate=no_hwp"
>
> root@s19:/home/doug#
> /home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
> -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> cpu0: MSR_PM_ENABLE: 0x00000000 (No-HWP)
>
> > Which is to say, on the Intel client machines I have, the firmware doesn't
> > seem to be able to hide HWP from the OS. Buggy BIOS? Maybe, the fact of the
> > matter is, I wouldn't need to add, say, KABYLAKE to that list, based on my
> > experience.
>
> My experience (hardware) differs from yours with respect to this.
>
> > The other side of the issue is that, from my understanding, the
> > preferred/supported way to disable HWP is to boot with intel_pstate=no_hwp,

Previous correspondence was with BIOS version 1003. There have been 3 BIOS
releases since then (at least that I know of), 2103, 2201, 2301, and all of them
have changed the behaviour of the "Auto" setting for Intel Speed Shift
Technology BIOS setting, forcing it on upon transfer of control to the OS.

Where with "intel_pstate=no_hwp" one used to get 0 for MSR_PM_ENABLE
(0x770) they now get 1.

That was for my ASUS Z490-P Motherboard.
For my ASUS Z390-A Motherboard (actually a windows 10 computer now),
with an older BIOS, things still work properly for "intel_pstate=no_hwp", and
while there is a newer BIOS, I will not install it as it is also not possible to
rollback.

I have had an escalation in progress with ASUS about this for about a month,
and while they have been responsive we have communications/language
issues and have yet to even agree that there is an issue.

I have been working with this incomplete patch:

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index bb4549959b11..7dcc51ee56ea 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -3347,17 +3347,27 @@ device_initcall(intel_pstate_init);

 static int __init intel_pstate_setup(char *str)
 {
+       /* The following needs to be conditional on CPUID with EAX 6 */
+       /* CPUID.06H:EAX[bit 7], which I do not know how to do. */
+       /* Avoid doing checking and printing multiple times, */
+       /* which I do not know why it does. */
+       if(!force_load){
+               if(intel_pstate_hwp_is_enabled()){
+                       pr_info("HWP enabled by BIOS\n");
+                       force_load = 1;
+               }
+       }
        if (!str)
                return -EINVAL;

-       if (!strcmp(str, "disable"))
+       if (!strcmp(str, "disable") && !force_load)
                no_load = 1;
-       else if (!strcmp(str, "active"))
+       if (!strcmp(str, "active") && !no_load)
                default_driver = &intel_pstate;
        else if (!strcmp(str, "passive"))
                default_driver = &intel_cpufreq;

-       if (!strcmp(str, "no_hwp")) {
+       if (!strcmp(str, "no_hwp") && !force_load) {
                pr_info("HWP disabled\n");
                no_hwp = 1;
        }

Which has troubles when HWP has been disabled by BIOS, but otherwise works well.

HWP Disabled by BIOS:

doug@s19:~$ sudo dmesg | grep intel_pstate
[sudo] password for doug:
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=314 intel_pstate=active intel_pstate=no_hwp
msr.allow_writes=on cpuidle.governor=teo
[    0.000000]  intel_pstate_setup+0x24/0x151
[    0.000000]  intel_pstate_setup+0x24/0x151
[    0.000000] intel_pstate: HWP disabled
[    0.051278] Kernel command line:
BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=314 intel_pstate=active intel_pstate=no_hwp
msr.allow_writes=on cpuidle.governor=teo
[    0.393236] intel_pstate: Intel P-state driver initializing

HWP force enabled by BIOS, which is the only other option with the newer BIOS:

doug@s19:~$ sudo dmesg | grep intel_pstate
[sudo] password for doug:
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=314 intel_pstate=active intel_pstate=no_hwp
msr.allow_writes=on cpuidle.governor=teo
[    0.000000] intel_pstate: HWP enabled by BIOS
[    0.049205] Kernel command line:
BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=314 intel_pstate=active intel_pstate=no_hwp
msr.allow_writes=on cpuidle.governor=teo
[    0.370662] intel_pstate: Intel P-state driver initializing
[    0.371590] intel_pstate: HWP enabled

HWP forced by BIOS, disable intel_pstate:

doug@s19:~$ sudo dmesg | grep intel_pstate
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=300 intel_pstate=disable msr.allow_writes=on
cpuidle.governor=teo
[    0.000000] intel_pstate: HWP enabled by BIOS
[    0.049133] Kernel command line:
BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=300 intel_pstate=disable msr.allow_writes=on
cpuidle.governor=teo
[    0.370519] intel_pstate: Intel P-state driver initializing
[    0.371451] intel_pstate: HWP enabled
doug@s19:~$ cat /sys/devices/system/cpu/cpu5/cpufreq/scaling_driver
intel_pstate

HWP forced by BIOS, disable intel_pstate:, unpatched kernel 5.14:

doug@s19:~$ sudo dmesg | grep intel_pstate
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-stock
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=300 intel_pstate=disable msr.allow_writes=on
cpuidle.governor=teo
[    0.049108] Kernel command line:
BOOT_IMAGE=/boot/vmlinuz-5.14.0-stock
root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
consoleblank=300 intel_pstate=disable msr.allow_writes=on
cpuidle.governor=teo
doug@s19:~$ cat /sys/devices/system/cpu/cpu5/cpufreq/scaling_driver
acpi-cpufreq
doug@s19:~$ sudo rdmsr 0x770
1

Observe the driver ended up as acpi-cpufreq, but with HWP enabled.

... Doug

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-09-07 15:45               ` Doug Smythies
@ 2021-09-07 16:01                 ` Srinivas Pandruvada
  2021-09-07 20:16                   ` Doug Smythies
  0 siblings, 1 reply; 23+ messages in thread
From: Srinivas Pandruvada @ 2021-09-07 16:01 UTC (permalink / raw)
  To: Doug Smythies, Giovanni Gherdovich, Rafael J . Wysocki
  Cc: Viresh Kumar, Len Brown, Linux PM list, Linux Kernel Mailing List

Hi Doug,

On Tue, 2021-09-07 at 08:45 -0700, Doug Smythies wrote:
> Hi all,
> 
> Recent ASUS BIOS updates have changed the default system
> response for this old thread, rendering "intel_pstate=no_hwp"
> useless.
> 
> It also raises a question: If BIOS has forced HWP, then how do we
> prevent the acpi-cpufreq driver from being used? Read on.

Does BIOS has option to enable Intel speed shift with no legacy
support?
Then this option will not populate ACPI _PSS table.

> 
> On Fri, May 14, 2021 at 3:12 PM Doug Smythies <dsmythies@telus.net>
> wrote:
> > 
> > On Fri, May 14, 2021 at 1:33 PM Giovanni Gherdovich <  
> > ggherdovich@suse.cz> wrote:
> > > On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
> ...
> > > 
> > > when COMETLAKE is not in that list, can you confirm that if you
> > > go into the
> > > BIOS config at boot, and disable HWP from there, then
> > > intel_pstate does *not* load?
> > 
> > Yes, already tested before my original reply.
> > 
> > > Does it say "intel_pstate: CPU model not supported" in the dmesg
> > > log?
> > 
> > That I did not check, but if I boot now with an unmodified kernel
> > 5.13-rc1 (i.e. without this patch):
> > 
> > [    0.369323] intel_pstate: CPU model not supported
> > 
> > > The control may be somewhere around "power mangement" in the BIOS
> > > config, and
> > > may be called "Enable/disable Intel Speed Shift".
> > 
> > Yes.
> > 
> > > I'm asking because I've just checked on two Dell laptops, one
> > > Skylake and the
> > > other Kabylake, and the menu is there in the BIOS config to
> > > disable HWP,
> > > but if I disable it... nothing happens. "lscpu" shows all the hwp
> > > flags as usual:
> > 
> > Motherboard here is ASUS PRIME Z490-A.
> > CPU: Intel(R) Core(TM) i5-10600K CPU @ 4.10GHz
> > 
> > >     # lscpu | grep Flags | tr ' ' '\n' | grep hwp
> > >     hwp
> > >     hwp_notify
> > >     hwp_act_window
> > >     hwp_epp
> > 
> > Here, for some reason I have to do it this way (sudo) or your
> > command
> > doesn't work properly. Results herein confirmed by looking at the
> > "Flags" output manually without filtering:
> > 
> > intel_speed_shift = Disabled in BIOS:
> > 
> > doug@s19:~$ sudo lscpu | tr ' ' '\n' | grep hwp
> > doug@s19:~$
> > 
> > intel_speed_shift = Auto in BIOS
> > 
> > $ sudo lscpu | tr ' ' '\n' | grep hwp
> > hwp
> > hwp_notify
> > hwp_act_window
> > hwp_epp
> > 
> > > and turbostat gives me:
> > > 
> > >     # turbostat -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> > >     cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)
> > 
> > Here:
> > 
> > intel_speed_shift = Disabled in BIOS:
> > 
> > root@s19:/home/doug#
> > /home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
> > -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> > root@s19:/home/doug#
> > 
> > intel_speed_shift = Auto in BIOS (the default setting)
> > 
> > root@s19:/home/doug#
> > /home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
> > -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> > cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)
> > 
> > or with "intel_pstate=no_hwp"
> > 
> > root@s19:/home/doug#
> > /home/doug/temp-k-git/linux/tools/power/x86/turbostat/turbostat
> > -Summary -i 1 : 2>&1 | grep MSR_PM_ENABLE
> > cpu0: MSR_PM_ENABLE: 0x00000000 (No-HWP)
> > 
> > > Which is to say, on the Intel client machines I have, the
> > > firmware doesn't
> > > seem to be able to hide HWP from the OS. Buggy BIOS? Maybe, the
> > > fact of the
> > > matter is, I wouldn't need to add, say, KABYLAKE to that list,
> > > based on my
> > > experience.
> > 
> > My experience (hardware) differs from yours with respect to this.
> > 
> > > The other side of the issue is that, from my understanding, the
> > > preferred/supported way to disable HWP is to boot with
> > > intel_pstate=no_hwp,
> 
> Previous correspondence was with BIOS version 1003. There have been 3
> BIOS
> releases since then (at least that I know of), 2103, 2201, 2301, and
> all of them
> have changed the behaviour of the "Auto" setting for Intel Speed
> Shift
> Technology BIOS setting, forcing it on upon transfer of control to
> the OS.
> 
> Where with "intel_pstate=no_hwp" one used to get 0 for MSR_PM_ENABLE
> (0x770) they now get 1.

So they are forcing Out of band OOB mode. 
Does bit 8 or 18 in MSR 0x1aa is set? 

Thanks,
Srinivas

> 
> That was for my ASUS Z490-P Motherboard.
> For my ASUS Z390-A Motherboard (actually a windows 10 computer now),
> with an older BIOS, things still work properly for
> "intel_pstate=no_hwp", and
> while there is a newer BIOS, I will not install it as it is also not
> possible to
> rollback.
> 
> I have had an escalation in progress with ASUS about this for about a
> month,
> and while they have been responsive we have communications/language
> issues and have yet to even agree that there is an issue.
> 
> I have been working with this incomplete patch:
> 
> diff --git a/drivers/cpufreq/intel_pstate.c
> b/drivers/cpufreq/intel_pstate.c
> index bb4549959b11..7dcc51ee56ea 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -3347,17 +3347,27 @@ device_initcall(intel_pstate_init);
> 
>  static int __init intel_pstate_setup(char *str)
>  {
> +       /* The following needs to be conditional on CPUID with EAX 6
> */
> +       /* CPUID.06H:EAX[bit 7], which I do not know how to do. */
> +       /* Avoid doing checking and printing multiple times, */
> +       /* which I do not know why it does. */
> +       if(!force_load){
> +               if(intel_pstate_hwp_is_enabled()){
> +                       pr_info("HWP enabled by BIOS\n");
> +                       force_load = 1;
> +               }
> +       }
>         if (!str)
>                 return -EINVAL;
> 
> -       if (!strcmp(str, "disable"))
> +       if (!strcmp(str, "disable") && !force_load)
>                 no_load = 1;
> -       else if (!strcmp(str, "active"))
> +       if (!strcmp(str, "active") && !no_load)
>                 default_driver = &intel_pstate;
>         else if (!strcmp(str, "passive"))
>                 default_driver = &intel_cpufreq;
> 
> -       if (!strcmp(str, "no_hwp")) {
> +       if (!strcmp(str, "no_hwp") && !force_load) {
>                 pr_info("HWP disabled\n");
>                 no_hwp = 1;
>         }
> 
> Which has troubles when HWP has been disabled by BIOS, but otherwise
> works well.
> 
> HWP Disabled by BIOS:
> 
> doug@s19:~$ sudo dmesg | grep intel_pstate
> [sudo] password for doug:
> [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=314 intel_pstate=active intel_pstate=no_hwp
> msr.allow_writes=on cpuidle.governor=teo
> [    0.000000]  intel_pstate_setup+0x24/0x151
> [    0.000000]  intel_pstate_setup+0x24/0x151
> [    0.000000] intel_pstate: HWP disabled
> [    0.051278] Kernel command line:
> BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=314 intel_pstate=active intel_pstate=no_hwp
> msr.allow_writes=on cpuidle.governor=teo
> [    0.393236] intel_pstate: Intel P-state driver initializing
> 
> HWP force enabled by BIOS, which is the only other option with the
> newer BIOS:
> 
> doug@s19:~$ sudo dmesg | grep intel_pstate
> [sudo] password for doug:
> [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=314 intel_pstate=active intel_pstate=no_hwp
> msr.allow_writes=on cpuidle.governor=teo
> [    0.000000] intel_pstate: HWP enabled by BIOS
> [    0.049205] Kernel command line:
> BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=314 intel_pstate=active intel_pstate=no_hwp
> msr.allow_writes=on cpuidle.governor=teo
> [    0.370662] intel_pstate: Intel P-state driver initializing
> [    0.371590] intel_pstate: HWP enabled
> 
> HWP forced by BIOS, disable intel_pstate:
> 
> doug@s19:~$ sudo dmesg | grep intel_pstate
> [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=300 intel_pstate=disable msr.allow_writes=on
> cpuidle.governor=teo
> [    0.000000] intel_pstate: HWP enabled by BIOS
> [    0.049133] Kernel command line:
> BOOT_IMAGE=/boot/vmlinuz-5.14.0-ipstate4
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=300 intel_pstate=disable msr.allow_writes=on
> cpuidle.governor=teo
> [    0.370519] intel_pstate: Intel P-state driver initializing
> [    0.371451] intel_pstate: HWP enabled
> doug@s19:~$ cat /sys/devices/system/cpu/cpu5/cpufreq/scaling_driver
> intel_pstate
> 
> HWP forced by BIOS, disable intel_pstate:, unpatched kernel 5.14:
> 
> doug@s19:~$ sudo dmesg | grep intel_pstate
> [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.14.0-stock
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=300 intel_pstate=disable msr.allow_writes=on
> cpuidle.governor=teo
> [    0.049108] Kernel command line:
> BOOT_IMAGE=/boot/vmlinuz-5.14.0-stock
> root=UUID=0ac356c1-caa9-4c2e-8229-4408bd998dbd ro ipv6.disable=1
> consoleblank=300 intel_pstate=disable msr.allow_writes=on
> cpuidle.governor=teo
> doug@s19:~$ cat /sys/devices/system/cpu/cpu5/cpufreq/scaling_driver
> acpi-cpufreq
> doug@s19:~$ sudo rdmsr 0x770
> 1
> 
> Observe the driver ended up as acpi-cpufreq, but with HWP enabled.
> 
> ... Doug



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-09-07 16:01                 ` Srinivas Pandruvada
@ 2021-09-07 20:16                   ` Doug Smythies
  2021-09-08  2:04                     ` Srinivas Pandruvada
  0 siblings, 1 reply; 23+ messages in thread
From: Doug Smythies @ 2021-09-07 20:16 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar, Len Brown,
	Linux PM list, Linux Kernel Mailing List, dsmythies

Hi Srinivas, Thank you for your reply.

On Tue, Sep 7, 2021 at 9:01 AM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> On Tue, 2021-09-07 at 08:45 -0700, Doug Smythies wrote:
>>
> > Recent ASUS BIOS updates have changed the default system
> > response for this old thread, rendering "intel_pstate=no_hwp"
> > useless.
> >
> > It also raises a question: If BIOS has forced HWP, then how do we
> > prevent the acpi-cpufreq driver from being used? Read on.
>
> Does BIOS has option to enable Intel speed shift with no legacy
> support?
> Then this option will not populate ACPI _PSS table.

The option is there no matter what.
I have tried every variation of legacy or no legacy that
I can find. Currently:
Current boot mode:   UEFI Firmware mode
SecureBoot:          disabled

>
> >
> > On Fri, May 14, 2021 at 3:12 PM Doug Smythies <dsmythies@telus.net>
> > wrote:
> > >
> > > On Fri, May 14, 2021 at 1:33 PM Giovanni Gherdovich <
> > > ggherdovich@suse.cz> wrote:
> > > > On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
> > ...
> > > >
...
> > Previous correspondence was with BIOS version 1003. There have been 3
> > BIOS
> > releases since then (at least that I know of), 2103, 2201, 2301, and
> > all of them
> > have changed the behaviour of the "Auto" setting for Intel Speed
> > Shift
> > Technology BIOS setting, forcing it on upon transfer of control to
> > the OS.
> >
> > Where with "intel_pstate=no_hwp" one used to get 0 for MSR_PM_ENABLE
> > (0x770) they now get 1.
>
> So they are forcing Out of band OOB mode.
> Does bit 8 or 18 in MSR 0x1aa is set?

No.

Here is the output from my msr reader/decoder program.
Kernel 5.14 (unpatched).
intel_pstate=disable
BIOS setting "Auto" for Intel Speed Shift,
Driver: acpi-cpufreq

doug@s19:~$ sudo c/msr-decoder
How many CPUs?: 12
8.) 0x198: IA32_PERF_STATUS     : CPU 0-11 :  45 :  45 :  45 :  45 :
45 :  45 :  45 :  45 :  45 :  45 :  45 :  45 :
B.) 0x770: IA32_PM_ENABLE: 1 : HWP enable
1.) 0x19C: IA32_THERM_STATUS: 88450000
2.) 0x1AA: MSR_MISC_PWR_MGMT: 401CC0 EIST enabled Coordination enabled
OOB Bit 8 reset OOB Bit 18 reset
3.) 0x1B1: IA32_PACKAGE_THERM_STATUS: 88410000
4.) 0x64F: MSR_CORE_PERF_LIMIT_REASONS: 0
A.) 0x1FC: MSR_POWER_CTL: 3C005D : C1E disable : EEO disable : RHO disable
5.) 0x771: IA32_HWP_CAPABILITIES (performance): 10B2930 : high 48 :
guaranteed 41 : efficient 11 : lowest 1
6.) 0x774: IA32_HWP_REQUEST:    CPU 0-11 :
    raw: 80003001 : 80003001 : 80003001 : 80003001 : 80003001 :
80003001 : 80003001 : 80003001 : 80003001 : 80003001 : 80003001 :
80003001 :
    min:        1 :        1 :        1 :        1 :        1 :
1 :        1 :        1 :        1 :        1 :        1 :        1 :
    max:       48 :       48 :       48 :       48 :       48 :
48 :       48 :       48 :       48 :       48 :       48 :       48 :
    des:        0 :        0 :        0 :        0 :        0 :
0 :        0 :        0 :        0 :        0 :        0 :        0 :
    epp:      128 :      128 :      128 :      128 :      128 :
128 :      128 :      128 :      128 :      128 :      128 :      128
:
    act:        0 :        0 :        0 :        0 :        0 :
0 :        0 :        0 :        0 :        0 :        0 :        0 :
7.) 0x777: IA32_HWP_STATUS: 0 : high 0 : guaranteed 0 : efficient 0 : lowest 0

...

... Doug

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-09-07 20:16                   ` Doug Smythies
@ 2021-09-08  2:04                     ` Srinivas Pandruvada
  2021-09-08  3:43                       ` Doug Smythies
  0 siblings, 1 reply; 23+ messages in thread
From: Srinivas Pandruvada @ 2021-09-08  2:04 UTC (permalink / raw)
  To: Doug Smythies
  Cc: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar, Len Brown,
	Linux PM list, Linux Kernel Mailing List

On Tue, 2021-09-07 at 13:16 -0700, Doug Smythies wrote:
> Hi Srinivas, Thank you for your reply.
> 
> On Tue, Sep 7, 2021 at 9:01 AM Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > On Tue, 2021-09-07 at 08:45 -0700, Doug Smythies wrote:
> > > 
> > > Recent ASUS BIOS updates have changed the default system
> > > response for this old thread, rendering "intel_pstate=no_hwp"
> > > useless.
> > > 
> > > It also raises a question: If BIOS has forced HWP, then how do we
> > > prevent the acpi-cpufreq driver from being used? Read on.
> > 
> > Does BIOS has option to enable Intel speed shift with no legacy
> > support?
> > Then this option will not populate ACPI _PSS table.
> 
> The option is there no matter what.
> I have tried every variation of legacy or no legacy that
> I can find. Currently:
> Current boot mode:   UEFI Firmware mode
> SecureBoot:          disabled
> 
> > 
> > > 
> > > On Fri, May 14, 2021 at 3:12 PM Doug Smythies < 
> > > dsmythies@telus.net>
> > > wrote:
> > > > 
> > > > On Fri, May 14, 2021 at 1:33 PM Giovanni Gherdovich <
> > > > ggherdovich@suse.cz> wrote:
> > > > > On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
> > > ...
> > > > > 
> ...
> > > Previous correspondence was with BIOS version 1003. There have
> > > been 3
> > > BIOS
> > > releases since then (at least that I know of), 2103, 2201, 2301,
> > > and
> > > all of them
> > > have changed the behaviour of the "Auto" setting for Intel Speed
> > > Shift
> > > Technology BIOS setting, forcing it on upon transfer of control
> > > to
> > > the OS.
> > > 
> > > Where with "intel_pstate=no_hwp" one used to get 0 for
> > > MSR_PM_ENABLE
> > > (0x770) they now get 1.
> > 
> > So they are forcing Out of band OOB mode.
> > Does bit 8 or 18 in MSR 0x1aa is set?
> 
> No.

So there is no legacy path. I think you are working with their support.
In HWP mode does setting scaling min/max frequency has any impact?

Thanks,
Srinivas

> 
> Here is the output from my msr reader/decoder program.
> Kernel 5.14 (unpatched).
> intel_pstate=disable
> BIOS setting "Auto" for Intel Speed Shift,
> Driver: acpi-cpufreq
> 
> doug@s19:~$ sudo c/msr-decoder
> How many CPUs?: 12
> 8.) 0x198: IA32_PERF_STATUS     : CPU 0-11 :  45 :  45 :  45 :  45 :
> 45 :  45 :  45 :  45 :  45 :  45 :  45 :  45 :
> B.) 0x770: IA32_PM_ENABLE: 1 : HWP enable
> 1.) 0x19C: IA32_THERM_STATUS: 88450000
> 2.) 0x1AA: MSR_MISC_PWR_MGMT: 401CC0 EIST enabled Coordination
> enabled
> OOB Bit 8 reset OOB Bit 18 reset
> 3.) 0x1B1: IA32_PACKAGE_THERM_STATUS: 88410000
> 4.) 0x64F: MSR_CORE_PERF_LIMIT_REASONS: 0
> A.) 0x1FC: MSR_POWER_CTL: 3C005D : C1E disable : EEO disable : RHO
> disable
> 5.) 0x771: IA32_HWP_CAPABILITIES (performance): 10B2930 : high 48 :
> guaranteed 41 : efficient 11 : lowest 1
> 6.) 0x774: IA32_HWP_REQUEST:    CPU 0-11 :
>     raw: 80003001 : 80003001 : 80003001 : 80003001 : 80003001 :
> 80003001 : 80003001 : 80003001 : 80003001 : 80003001 : 80003001 :
> 80003001 :
>     min:        1 :        1 :        1 :        1 :        1 :
> 1 :        1 :        1 :        1 :        1 :        1 :        1 :
>     max:       48 :       48 :       48 :       48 :       48 :
> 48 :       48 :       48 :       48 :       48 :       48 :       48
> :
>     des:        0 :        0 :        0 :        0 :        0 :
> 0 :        0 :        0 :        0 :        0 :        0 :        0 :
>     epp:      128 :      128 :      128 :      128 :      128 :
> 128 :      128 :      128 :      128 :      128 :      128 :      128
> :
>     act:        0 :        0 :        0 :        0 :        0 :
> 0 :        0 :        0 :        0 :        0 :        0 :        0 :
> 7.) 0x777: IA32_HWP_STATUS: 0 : high 0 : guaranteed 0 : efficient 0 :
> lowest 0
> 
> ...
> 
> ... Doug



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-09-08  2:04                     ` Srinivas Pandruvada
@ 2021-09-08  3:43                       ` Doug Smythies
  2021-09-14 18:41                         ` Doug Smythies
  0 siblings, 1 reply; 23+ messages in thread
From: Doug Smythies @ 2021-09-08  3:43 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar, Len Brown,
	Linux PM list, Linux Kernel Mailing List, dsmythies

On Tue, Sep 7, 2021 at 7:04 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Tue, 2021-09-07 at 13:16 -0700, Doug Smythies wrote:
> > Hi Srinivas, Thank you for your reply.
> >
> > On Tue, Sep 7, 2021 at 9:01 AM Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com> wrote:
> > > On Tue, 2021-09-07 at 08:45 -0700, Doug Smythies wrote:
> > > >
> > > > Recent ASUS BIOS updates have changed the default system
> > > > response for this old thread, rendering "intel_pstate=no_hwp"
> > > > useless.
> > > >
> > > > It also raises a question: If BIOS has forced HWP, then how do we
> > > > prevent the acpi-cpufreq driver from being used? Read on.
> > >
> > > Does BIOS has option to enable Intel speed shift with no legacy
> > > support?
> > > Then this option will not populate ACPI _PSS table.
> >
> > The option is there no matter what.
> > I have tried every variation of legacy or no legacy that
> > I can find. Currently:
> > Current boot mode:   UEFI Firmware mode
> > SecureBoot:          disabled
> >
> > >
> > > >
> > > > On Fri, May 14, 2021 at 3:12 PM Doug Smythies <
> > > > dsmythies@telus.net>
> > > > wrote:
> > > > >
> > > > > On Fri, May 14, 2021 at 1:33 PM Giovanni Gherdovich <
> > > > > ggherdovich@suse.cz> wrote:
> > > > > > On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
> > > > ...
> > > > > >
> > ...
> > > > Previous correspondence was with BIOS version 1003. There have
> > > > been 3
> > > > BIOS
> > > > releases since then (at least that I know of), 2103, 2201, 2301,
> > > > and
> > > > all of them
> > > > have changed the behaviour of the "Auto" setting for Intel Speed
> > > > Shift
> > > > Technology BIOS setting, forcing it on upon transfer of control
> > > > to
> > > > the OS.
> > > >
> > > > Where with "intel_pstate=no_hwp" one used to get 0 for
> > > > MSR_PM_ENABLE
> > > > (0x770) they now get 1.
> > >
> > > So they are forcing Out of band OOB mode.
> > > Does bit 8 or 18 in MSR 0x1aa is set?
> >
> > No.
>
> So there is no legacy path. I think you are working with their support.

Yes, for almost a month now, with very little to show for it.
We'll see what happens. I did get a message this afternoon:

"Our GTSD is debugging the issue,.
When they have the result, they will directly update you."

> In HWP mode does setting scaling min/max frequency has any impact?

No. I wouldn't have expected it to, as the system is confused as to who
is in charge. The acpi-cpufreq driver thinks it is in charge, but HWP
thinks it is.

The intel_pstate driver works fine.

... Doug

>
> Thanks,
> Srinivas
>
> >
> > Here is the output from my msr reader/decoder program.
> > Kernel 5.14 (unpatched).
> > intel_pstate=disable
> > BIOS setting "Auto" for Intel Speed Shift,
> > Driver: acpi-cpufreq
> >
> > doug@s19:~$ sudo c/msr-decoder
> > How many CPUs?: 12
> > 8.) 0x198: IA32_PERF_STATUS     : CPU 0-11 :  45 :  45 :  45 :  45 :
> > 45 :  45 :  45 :  45 :  45 :  45 :  45 :  45 :
> > B.) 0x770: IA32_PM_ENABLE: 1 : HWP enable
> > 1.) 0x19C: IA32_THERM_STATUS: 88450000
> > 2.) 0x1AA: MSR_MISC_PWR_MGMT: 401CC0 EIST enabled Coordination
> > enabled
> > OOB Bit 8 reset OOB Bit 18 reset
> > 3.) 0x1B1: IA32_PACKAGE_THERM_STATUS: 88410000
> > 4.) 0x64F: MSR_CORE_PERF_LIMIT_REASONS: 0
> > A.) 0x1FC: MSR_POWER_CTL: 3C005D : C1E disable : EEO disable : RHO
> > disable
> > 5.) 0x771: IA32_HWP_CAPABILITIES (performance): 10B2930 : high 48 :
> > guaranteed 41 : efficient 11 : lowest 1
> > 6.) 0x774: IA32_HWP_REQUEST:    CPU 0-11 :
> >     raw: 80003001 : 80003001 : 80003001 : 80003001 : 80003001 :
> > 80003001 : 80003001 : 80003001 : 80003001 : 80003001 : 80003001 :
> > 80003001 :
> >     min:        1 :        1 :        1 :        1 :        1 :
> > 1 :        1 :        1 :        1 :        1 :        1 :        1 :
> >     max:       48 :       48 :       48 :       48 :       48 :
> > 48 :       48 :       48 :       48 :       48 :       48 :       48
> > :
> >     des:        0 :        0 :        0 :        0 :        0 :
> > 0 :        0 :        0 :        0 :        0 :        0 :        0 :
> >     epp:      128 :      128 :      128 :      128 :      128 :
> > 128 :      128 :      128 :      128 :      128 :      128 :      128
> > :
> >     act:        0 :        0 :        0 :        0 :        0 :
> > 0 :        0 :        0 :        0 :        0 :        0 :        0 :
> > 7.) 0x777: IA32_HWP_STATUS: 0 : high 0 : guaranteed 0 : efficient 0 :
> > lowest 0
> >
> > ...
> >
> > ... Doug
>
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-09-08  3:43                       ` Doug Smythies
@ 2021-09-14 18:41                         ` Doug Smythies
  2021-09-15  9:38                           ` Srinivas Pandruvada
  0 siblings, 1 reply; 23+ messages in thread
From: Doug Smythies @ 2021-09-14 18:41 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar, Len Brown,
	Linux PM list, Linux Kernel Mailing List, dsmythies

On Tue, Sep 7, 2021 at 8:43 PM Doug Smythies <dsmythies@telus.net> wrote:
> On Tue, Sep 7, 2021 at 7:04 PM Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > On Tue, 2021-09-07 at 13:16 -0700, Doug Smythies wrote:
> > > On Tue, Sep 7, 2021 at 9:01 AM Srinivas Pandruvada
> > > <srinivas.pandruvada@linux.intel.com> wrote:
> > > > On Tue, 2021-09-07 at 08:45 -0700, Doug Smythies wrote:
> > > > >
> > > > > Recent ASUS BIOS updates have changed the default system
> > > > > response for this old thread, rendering "intel_pstate=no_hwp"
> > > > > useless.
> > > > >
> > > > > It also raises a question: If BIOS has forced HWP, then how do we
> > > > > prevent the acpi-cpufreq driver from being used? Read on.
> > > >
> > > > Does BIOS has option to enable Intel speed shift with no legacy
> > > > support?
> > > > Then this option will not populate ACPI _PSS table.
> > >
> > > The option is there no matter what.
> > > I have tried every variation of legacy or no legacy that
> > > I can find. Currently:
> > > Current boot mode:   UEFI Firmware mode
> > > SecureBoot:          disabled
> > >
> > > >
> > > > >
> > > > > On Fri, May 14, 2021 at 3:12 PM Doug Smythies <
> > > > > dsmythies@telus.net>
> > > > > wrote:
> > > > > >
> > > > > > On Fri, May 14, 2021 at 1:33 PM Giovanni Gherdovich <
> > > > > > ggherdovich@suse.cz> wrote:
> > > > > > > On Fri, 2021-05-14 at 08:31 -0700, Doug Smythies wrote:
> > > > > ...
> > > > > > >
> > > ...
> > > > > Previous correspondence was with BIOS version 1003. There have
> > > > > been 3 BIOS
> > > > > releases since then (at least that I know of), 2103, 2201, 2301,
> > > > > and all of them have changed the behaviour
> > > > > of the "Auto" setting for Intel Speed Shift
> > > > > Technology BIOS setting, forcing it on upon transfer of control
> > > > > to the OS.
> > > > >
> > > > > Where with "intel_pstate=no_hwp" one used to get 0 for
> > > > > MSR_PM_ENABLE
> > > > > (0x770) they now get 1.
> > > >
> > > > So they are forcing Out of band OOB mode.
> > > > Does bit 8 or 18 in MSR 0x1aa is set?
> > >
> > > No.
> >
> > So there is no legacy path. I think you are working with their support.
>
> Yes, for almost a month now, with very little to show for it.
> We'll see what happens. I did get a message this afternoon:
>
> "Our GTSD is debugging the issue,.
> When they have the result, they will directly update you."
>
> > In HWP mode does setting scaling min/max frequency has any impact?
>
> No. I wouldn't have expected it to, as the system is confused as to who
> is in charge. The acpi-cpufreq driver thinks it is in charge, but HWP
> thinks it is.
>
> The intel_pstate driver works fine.

Hi Srinivas,

I heard back from ASUS, and they now confirm that they
did change the behaviour of the "Auto" setting in BIOS
version 2103. They say they did it to fix an issue with
ITB3.0, which I assume means Intel Turbo Boost 3.0.
I'll copy and paste the relevant portion of the email
below:

" I am in direct contact with the engineers.
Here is the result from their test:

In BIOS 2103,the “Auto” setting transfers control to
the OS with HWP available and enabled.

This is side effect to fix ITBM3.0 not work after HWP enabled.
We can remove this patch, but ITBM3.0 will not work when HWP enabled"

Are you familiar with this issue?

I want the original behaviour of the "Auto" setting, as it is
the only way for control to go to the OS with HWP
available but disabled.

... Doug

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode
  2021-09-14 18:41                         ` Doug Smythies
@ 2021-09-15  9:38                           ` Srinivas Pandruvada
  0 siblings, 0 replies; 23+ messages in thread
From: Srinivas Pandruvada @ 2021-09-15  9:38 UTC (permalink / raw)
  To: Doug Smythies
  Cc: Giovanni Gherdovich, Rafael J . Wysocki, Viresh Kumar, Len Brown,
	Linux PM list, Linux Kernel Mailing List

On Tue, 2021-09-14 at 11:41 -0700, Doug Smythies wrote:
> On Tue, Sep 7, 2021 at 8:43 PM Doug Smythies <dsmythies@telus.net>
> wrote:
> > 

[...]

> > The intel_pstate driver works fine.
> 
> Hi Srinivas,
Hi Doug,

> 
> I heard back from ASUS, and they now confirm that they
> did change the behaviour of the "Auto" setting in BIOS
> version 2103. They say they did it to fix an issue with
> ITB3.0, which I assume means Intel Turbo Boost 3.0.
> I'll copy and paste the relevant portion of the email
> below:
> 
> " I am in direct contact with the engineers.
> Here is the result from their test:
> 
> In BIOS 2103,the “Auto” setting transfers control to
> the OS with HWP available and enabled.
> 
> This is side effect to fix ITBM3.0 not work after HWP enabled.
> We can remove this patch, but ITBM3.0 will not work when HWP enabled"
> 
> Are you familiar with this issue?
No. I think they mean that they need to expose ACPI CPPC information.
It is possible they only expose ACPI CPPC information when HWP feature
is available. But not sure why it requires HWP enable also. We (on
Linux) depend on ACPI CPPC for turbo 3.0, but don't require HWP to be
enabled by default.


Thanks,
Srinivas



> 
> I want the original behaviour of the "Auto" setting, as it is
> the only way for control to go to the OS with HWP
> available but disabled.
> 
> ... Doug



^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-09-15  9:38 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-13  7:59 [PATCH] cpufreq: intel_pstate: Force intel_pstate to load when HWP disabled in firmware Giovanni Gherdovich
2021-05-13  9:24 ` Srinivas Pandruvada
2021-05-13 10:10   ` Giovanni Gherdovich
2021-05-13 11:03     ` Srinivas Pandruvada
2021-05-13 12:10       ` Giovanni Gherdovich
2021-05-13 13:20       ` [PATCH v2] cpufreq: intel_pstate: Add Icelake servers support in no-HWP mode Giovanni Gherdovich
2021-05-14 15:31         ` Doug Smythies
2021-05-14 20:33           ` Giovanni Gherdovich
2021-05-14 22:12             ` Doug Smythies
2021-09-07 15:45               ` Doug Smythies
2021-09-07 16:01                 ` Srinivas Pandruvada
2021-09-07 20:16                   ` Doug Smythies
2021-09-08  2:04                     ` Srinivas Pandruvada
2021-09-08  3:43                       ` Doug Smythies
2021-09-14 18:41                         ` Doug Smythies
2021-09-15  9:38                           ` Srinivas Pandruvada
2021-05-15  2:58             ` Srinivas Pandruvada
2021-05-17 15:26           ` Rafael J. Wysocki
2021-05-18 11:33             ` Giovanni Gherdovich
2021-05-19  5:37               ` Doug Smythies
2021-05-18 12:34             ` [PATCH v3 1/2] " Giovanni Gherdovich
2021-05-18 12:34               ` [PATCH v3 2/2] cpufreq: intel_pstate: Add Cometlake " Giovanni Gherdovich
2021-05-21 16:49               ` [PATCH v3 1/2] cpufreq: intel_pstate: Add Icelake servers " Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).