[PATCH] cpufreq: intel_pstate: Enable HWP IO boost for all servers

[PATCH] cpufreq: intel_pstate: Enable HWP IO boost for all servers

[Srinivas Pandruvada]

The HWP IO boost results in slight improvements for IO performance on
both Ice Lake and Sapphire Rapid servers.

Currently there is a CPU model check for Skylake desktop and server along
with the ACPI PM profile for performance and enterprise servers to enable
IO boost.

Remove the CPU model check, so that all current server models enable HWP
IO boost by default.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index cb4beec27555..8edbc0856892 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2384,12 +2384,6 @@ static const struct x86_cpu_id intel_pstate_cpu_ee_disable_ids[] = {
 	{}
 };
 
-static const struct x86_cpu_id intel_pstate_hwp_boost_ids[] = {
-	X86_MATCH(SKYLAKE_X,		core_funcs),
-	X86_MATCH(SKYLAKE,		core_funcs),
-	{}
-};
-
 static int intel_pstate_init_cpu(unsigned int cpunum)
 {
 	struct cpudata *cpu;
@@ -2408,12 +2402,9 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
 		cpu->epp_default = -EINVAL;
 
 		if (hwp_active) {
-			const struct x86_cpu_id *id;
-
 			intel_pstate_hwp_enable(cpu);
 
-			id = x86_match_cpu(intel_pstate_hwp_boost_ids);
-			if (id && intel_pstate_acpi_pm_profile_server())
+			if (intel_pstate_acpi_pm_profile_server())
 				hwp_boost = true;
 		}
 	} else if (hwp_active) {
-- 
2.34.1

Re: [PATCH] cpufreq: intel_pstate: Enable HWP IO boost for all servers

[Rafael J. Wysocki]

On Fri, Mar 3, 2023 at 5:14 AM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> The HWP IO boost results in slight improvements for IO performance on
> both Ice Lake and Sapphire Rapid servers.
>
> Currently there is a CPU model check for Skylake desktop and server along
> with the ACPI PM profile for performance and enterprise servers to enable
> IO boost.
>
> Remove the CPU model check, so that all current server models enable HWP
> IO boost by default.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index cb4beec27555..8edbc0856892 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2384,12 +2384,6 @@ static const struct x86_cpu_id intel_pstate_cpu_ee_disable_ids[] = {
>         {}
>  };
>
> -static const struct x86_cpu_id intel_pstate_hwp_boost_ids[] = {
> -       X86_MATCH(SKYLAKE_X,            core_funcs),
> -       X86_MATCH(SKYLAKE,              core_funcs),
> -       {}
> -};
> -
>  static int intel_pstate_init_cpu(unsigned int cpunum)
>  {
>         struct cpudata *cpu;
> @@ -2408,12 +2402,9 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
>                 cpu->epp_default = -EINVAL;
>
>                 if (hwp_active) {
> -                       const struct x86_cpu_id *id;
> -
>                         intel_pstate_hwp_enable(cpu);
>
> -                       id = x86_match_cpu(intel_pstate_hwp_boost_ids);
> -                       if (id && intel_pstate_acpi_pm_profile_server())
> +                       if (intel_pstate_acpi_pm_profile_server())
>                                 hwp_boost = true;
>                 }
>         } else if (hwp_active) {
> --

Applied as 6.4 material, thanks!

Re: [PATCH] cpufreq: intel_pstate: Enable HWP IO boost for all servers

[Giovanni Gherdovich]

On Thu, 2023-03-02 at 20:14 -0800, Srinivas Pandruvada wrote:
> The HWP IO boost results in slight improvements for IO performance on
> both Ice Lake and Sapphire Rapid servers.
> 
> Currently there is a CPU model check for Skylake desktop and server along
> with the ACPI PM profile for performance and enterprise servers to enable
> IO boost.
> 
> Remove the CPU model check, so that all current server models enable HWP
> IO boost by default.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index cb4beec27555..8edbc0856892 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2384,12 +2384,6 @@ static const struct x86_cpu_id intel_pstate_cpu_ee_disable_ids[] = {
>  	{}
>  };
>  
> -static const struct x86_cpu_id intel_pstate_hwp_boost_ids[] = {
> -	X86_MATCH(SKYLAKE_X,		core_funcs),
> -	X86_MATCH(SKYLAKE,		core_funcs),
> -	{}
> -};
> -
>  static int intel_pstate_init_cpu(unsigned int cpunum)
>  {
>  	struct cpudata *cpu;
> @@ -2408,12 +2402,9 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
>  		cpu->epp_default = -EINVAL;
>  
>  		if (hwp_active) {
> -			const struct x86_cpu_id *id;
> -
>  			intel_pstate_hwp_enable(cpu);
>  
> -			id = x86_match_cpu(intel_pstate_hwp_boost_ids);
> -			if (id && intel_pstate_acpi_pm_profile_server())
> +			if (intel_pstate_acpi_pm_profile_server())
>  				hwp_boost = true;
>  		}
>  	} else if (hwp_active) {

Hello Srinivas,

Good catch. We've had HWP IO Boost in the kernel for a while now but we
weren't enabling on most of the modern CPUs... This fixes it.

One thing though: I've the impression that HWP IO Boost depends on having
per-core p-states -- otherwise you'll be boosting up and down the entire machine
instead of just the one core waking up from IO.
Enabling the feature on all machines with the ACPI PM server profile would
force it also where per-core p-states aren't available.

Would you agree with this assessment?
Do I correctly understand the reason why per-core p-states are needed here?

I remember you mentioned the the dependency on per-core p-states in the cover
letter from the HWP IO Boost submission in 2018
https://lore.kernel.org/lkml/20180605214242.62156-1-srinivas.pandruvada@linux.intel.com/

I think there's a tradeoff here. Up until this patch, we forgot to enable the
feature on four generations of Intel CPUs. That's a lot of lost performance;
thanks to this patch it won't happen ever again, because nobody will have to
update the model list at every new CPU release.

On the other side, there may be some penalty for machines that:
- have HWP
- don't have per-core p-states
I don't know how large that interesection is, or how big the penalty.


Giovanni

Re: [PATCH] cpufreq: intel_pstate: Enable HWP IO boost for all servers

[srinivas pandruvada]

On Mon, 2023-03-20 at 18:03 +0100, Giovanni Gherdovich wrote:
> On Thu, 2023-03-02 at 20:14 -0800, Srinivas Pandruvada wrote:
> 

...

Hi Giovanni,

> Hello Srinivas,
> 
> Good catch. We've had HWP IO Boost in the kernel for a while now but
> we
> weren't enabling on most of the modern CPUs... This fixes it.
> 
> One thing though: I've the impression that HWP IO Boost depends on
> having
> per-core p-states -- otherwise you'll be boosting up and down the
> entire machine
> instead of just the one core waking up from IO.
> Enabling the feature on all machines with the ACPI PM server profile
> would
> force it also where per-core p-states aren't available.

This feature only exists on HWP systems. There are no HWP systems
without per core P-states. Here we are enabling for performance and
enterprise servers only.


> 
> Would you agree with this assessment?
> Do I correctly understand the reason why per-core p-states are needed
> here?
This problem with IO will be more pronounced in per-core P-states
systems as it will not be influenced by other cores. But even if non
per-core systems if other cores are idle or lightly loaded, the same
problem can occur. But I don't have data on such systems.

> 
> I remember you mentioned the the dependency on per-core p-states in
> the cover
> letter from the HWP IO Boost submission in 2018
> https://lore.kernel.org/lkml/20180605214242.62156-1-srinivas.pandruvada@linux.intel.com/
> 
> I think there's a tradeoff here. Up until this patch, we forgot to
> enable the
> feature on four generations of Intel CPUs. That's a lot of lost
> performance;
> thanks to this patch it won't happen ever again, because nobody will
> have to
> update the model list at every new CPU release.
> 
CPU model updates always gets missed and also misses testing
opportunity.

> On the other side, there may be some penalty for machines that:
> - have HWP
> - don't have per-core p-states
> I don't know how large that interesection is, or how big the penalty.
> 

Thanks,
Srinivas



> 
> Giovanni
>