[PATCH 0/3] cpufreq/amd-pstate: A set of fixes

[PATCH 0/3] cpufreq/amd-pstate: A set of fixes

[Gautham R. Shenoy]

Hello Rafael, Viresh,

This series contains three fixes for the amd-pstate driver for 6.11.
Could you please include it in your tree?

These patches are based on linux-pm/master with the top commit
7c626ce4bae1 ("Linux 6.11-rc3").

There are three patches,

1. To fix an uninitialized variable in amd_pstate_cpu_boost_update()
   from Dan Carpenter:
   https://lore.kernel.org/lkml/7ff53543-6c04-48a0-8d99-7dc010b93b3a@stanley.mountain/

2. Use topology_logical_package_id() instead of
   topology_logical_die_id() definition of the later function has
   changed on some AMD processors since the inclusion of the CPUID
   0x80000026 parser:
   https://lore.kernel.org/lkml/20240801124509.3650-1-Dhananjay.Ugwekar@amd.com/

3. Remove a warning for the absence fo X86_FEATURE_CPPC on Zen1 and
   Zen2 since they don't have the feature defined. This fixes the
   regression reported by David Wang:
   https://lore.kernel.org/lkml/20240730140111.4491-1-00107082@163.com/

Thanks and Regards
gautham.

Dan Carpenter (1):
  cpufreq: amd-pstate: Fix uninitialized variable in
    amd_pstate_cpu_boost_update()

Gautham R. Shenoy (2):
  cpufreq/amd-pstate: Use topology_logical_package_id() instead of
    logical_die_id()
  cpufreq/amd-pstate: Remove warning for X86_FEATURE_CPPC on Zen1 and
    Zen2

 drivers/cpufreq/amd-pstate.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH 1/3] cpufreq: amd-pstate: Fix uninitialized variable in amd_pstate_cpu_boost_update()

[Gautham R. Shenoy]

From: Dan Carpenter <dan.carpenter@linaro.org>

Smatch complains that "ret" could be uninitialized:

  drivers/cpufreq/amd-pstate.c:734 amd_pstate_cpu_boost_update()
  error: uninitialized symbol 'ret'.

This seems like it probably is a real issue.  Initialize "ret" to zero to
be safe.

Fixes: c8c68c38b56f ("cpufreq: amd-pstate: initialize core precision boost state")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Perry Yuan <perry.yuan@amd.com>
Acked-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Link: https://lore.kernel.org/lkml/7ff53543-6c04-48a0-8d99-7dc010b93b3a@stanley.mountain/T/
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 68c616b572f2..358bd88cd0c5 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -692,7 +692,7 @@ static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on)
 	struct amd_cpudata *cpudata = policy->driver_data;
 	struct cppc_perf_ctrls perf_ctrls;
 	u32 highest_perf, nominal_perf, nominal_freq, max_freq;
-	int ret;
+	int ret = 0;
 
 	highest_perf = READ_ONCE(cpudata->highest_perf);
 	nominal_perf = READ_ONCE(cpudata->nominal_perf);
-- 
2.34.1

[PATCH 2/3] cpufreq/amd-pstate: Use topology_logical_package_id() instead of logical_die_id()

[Gautham R. Shenoy]

After the commit 63edbaa48a57 ("x86/cpu/topology: Add support for the
AMD 0x80000026 leaf"), the topolgy_logical_die_id() function returns
the logical Core Chiplet Die (CCD) ID instead of the logical socket
ID.

Since this is currently used to set MSR_AMD_CPPC_ENABLE, which needs
to be set on any one of the threads of the socket, it is prudent to
use topology_logical_package_id() in place of
topology_logical_die_id().

Fixes: 63edbaa48a57 ("x86/cpu/topology: Add support for the AMD 0x80000026 leaf")
cc: stable@vger.kernel.org # 6.10
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Tested-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
Link: https://lore.kernel.org/lkml/20240801124509.3650-1-Dhananjay.Ugwekar@amd.com/
Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 358bd88cd0c5..89bda7a2bb8d 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -321,7 +321,7 @@ static inline int pstate_enable(bool enable)
 		return 0;
 
 	for_each_present_cpu(cpu) {
-		unsigned long logical_id = topology_logical_die_id(cpu);
+		unsigned long logical_id = topology_logical_package_id(cpu);
 
 		if (test_bit(logical_id, &logical_proc_id_mask))
 			continue;
-- 
2.34.1

[PATCH 3/3] cpufreq/amd-pstate: Remove warning for X86_FEATURE_CPPC on Zen1 and Zen2

[Gautham R. Shenoy]

commit bff7d13c190a ("cpufreq: amd-pstate: add debug message while
CPPC is supported and disabled by SBIOS") issues a warning on plaforms
where the X86_FEATURE_CPPC is expected to be enabled, but is not due
to it being disabled in the BIOS.

This feature bit corresponds to CPUID 0x80000008.ebx[27] which is a
reserved bit on the Zen1 and Zen2 platforms, and is expected to be
cleared on these platforms. Thus printing the warning message for Zen1
and Zen2 models when X86_FEATURE_CPPC is incorrect. Fix this.

Fixes: bff7d13c190a ("cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS")
Reported-by: David Wang <00107082@163.com>
Closes: https://lore.kernel.org/lkml/20240730140111.4491-1-00107082@163.com/
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 89bda7a2bb8d..66002718397c 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1841,10 +1841,8 @@ static bool amd_cppc_supported(void)
 	 * the code is added for debugging purposes.
 	 */
 	if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
-		if (cpu_feature_enabled(X86_FEATURE_ZEN1) || cpu_feature_enabled(X86_FEATURE_ZEN2)) {
-			if (c->x86_model > 0x60 && c->x86_model < 0xaf)
-				warn = true;
-		} else if (cpu_feature_enabled(X86_FEATURE_ZEN3) || cpu_feature_enabled(X86_FEATURE_ZEN4)) {
+		if (cpu_feature_enabled(X86_FEATURE_ZEN3) ||
+		    cpu_feature_enabled(X86_FEATURE_ZEN4)) {
 			if ((c->x86_model > 0x10 && c->x86_model < 0x1F) ||
 					(c->x86_model > 0x40 && c->x86_model < 0xaf))
 				warn = true;
-- 
2.34.1

Re: [PATCH 3/3] cpufreq/amd-pstate: Remove warning for X86_FEATURE_CPPC on Zen1 and Zen2

[Xiaojian Du]

Hi Gautham,

On 2024/8/13 17:54, Gautham R. Shenoy wrote:
> ...
>
> This feature bit corresponds to CPUID 0x80000008.ebx[27] which is a
> reserved bit on the Zen1 and Zen2 platforms, and is expected to be
> cleared on these platforms. Thus printing the warning message for Zen1
> and Zen2 models when X86_FEATURE_CPPC is incorrect. Fix this.
>
> ...
>   	if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
> -		if (cpu_feature_enabled(X86_FEATURE_ZEN1) || cpu_feature_enabled(X86_FEATURE_ZEN2)) {
> -			if (c->x86_model > 0x60 && c->x86_model < 0xaf)
> -				warn = true;

Some models of ZEN2 APU/CPU require this warning info, like Renoir 
(Ryzen 7 4800H mobile APU/4750G desktop APU,
Ryzen 5 4600 desktop CPU), Lucienne (Ryzen 5 5500U mobile APU) and 
Aerith (APU of Steam Deck console).

So it has to use model ID to narrow down the coverage.

For ZEN1 APU/CPU, this warning can be removed completely, because ZEN1 
doesn't support CPPC.

Thanks,
Xiaojian

> -		} else if (cpu_feature_enabled(X86_FEATURE_ZEN3) || cpu_feature_enabled(X86_FEATURE_ZEN4)) {
> +		if (cpu_feature_enabled(X86_FEATURE_ZEN3) ||
> +		    cpu_feature_enabled(X86_FEATURE_ZEN4)) {
>   			if ((c->x86_model > 0x10 && c->x86_model < 0x1F) ||
>   					(c->x86_model > 0x40 && c->x86_model < 0xaf))
>   				warn = true;

Re: [PATCH 3/3] cpufreq/amd-pstate: Remove warning for X86_FEATURE_CPPC on Zen1 and Zen2

[Gautham R. Shenoy]

Hello Xiaojian,

On Wed, Aug 14, 2024 at 05:29:04PM +0800, Xiaojian Du wrote:
> Hi Gautham,
> 
> On 2024/8/13 17:54, Gautham R. Shenoy wrote:
> > ...
> > 
> > This feature bit corresponds to CPUID 0x80000008.ebx[27] which is a
> > reserved bit on the Zen1 and Zen2 platforms, and is expected to be
> > cleared on these platforms. Thus printing the warning message for Zen1
> > and Zen2 models when X86_FEATURE_CPPC is incorrect. Fix this.
> > 
> > ...
> >   	if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
> > -		if (cpu_feature_enabled(X86_FEATURE_ZEN1) || cpu_feature_enabled(X86_FEATURE_ZEN2)) {
> > -			if (c->x86_model > 0x60 && c->x86_model < 0xaf)
> > -				warn = true;
> 
> Some models of ZEN2 APU/CPU require this warning info, like Renoir (Ryzen 7
> 4800H mobile APU/4750G desktop APU,
> Ryzen 5 4600 desktop CPU), Lucienne (Ryzen 5 5500U mobile APU) and Aerith
> (APU of Steam Deck console).
> 
> So it has to use model ID to narrow down the coverage.
> 

I checked the publicly available PPRs of the Family 17h models 0x60
(Renoir) [1] and Family 17h model 0x71 (Matisse) [2]. In both these
PPRs, CPUID 0x80000008 EBX[27] is a reserved bit.

In fact, David reported this issue on Matisse system.

I am happy to retain the warnings for specific models which are known
to have support for the CPPC MSRs (which is what CPUID 0x80000008
EBX[27] advertises). Could you please share the model numbers of those
that you are aware of?


[1] PPR Family 17h Model 0x60 : https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/55922-A1-PUB.zip
[2] PPR Family 17h Model 0x71 : https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/56176_ppr_Family_17h_Model_71h_B0_pub_Rev_3_06.zip

> For ZEN1 APU/CPU, this warning can be removed completely, because ZEN1
> doesn't support CPPC.

Agreed.

> 
> Thanks,
> Xiaojian

--
Thanks and Regards
gautham.

Re: [PATCH 0/3] cpufreq/amd-pstate: A set of fixes

[Mario Limonciello]

On 8/13/2024 04:51, Gautham R. Shenoy wrote:
> Hello Rafael, Viresh,
> 
> This series contains three fixes for the amd-pstate driver for 6.11.
> Could you please include it in your tree?
> 
> These patches are based on linux-pm/master with the top commit
> 7c626ce4bae1 ("Linux 6.11-rc3").
> 
> There are three patches,
> 
> 1. To fix an uninitialized variable in amd_pstate_cpu_boost_update()
>     from Dan Carpenter:
>     https://lore.kernel.org/lkml/7ff53543-6c04-48a0-8d99-7dc010b93b3a@stanley.mountain/
> 
> 2. Use topology_logical_package_id() instead of
>     topology_logical_die_id() definition of the later function has
>     changed on some AMD processors since the inclusion of the CPUID
>     0x80000026 parser:
>     https://lore.kernel.org/lkml/20240801124509.3650-1-Dhananjay.Ugwekar@amd.com/
> 
> 3. Remove a warning for the absence fo X86_FEATURE_CPPC on Zen1 and
>     Zen2 since they don't have the feature defined. This fixes the
>     regression reported by David Wang:
>     https://lore.kernel.org/lkml/20240730140111.4491-1-00107082@163.com/
> 
> Thanks and Regards
> gautham.
> 
> Dan Carpenter (1):
>    cpufreq: amd-pstate: Fix uninitialized variable in
>      amd_pstate_cpu_boost_update()
> 
> Gautham R. Shenoy (2):
>    cpufreq/amd-pstate: Use topology_logical_package_id() instead of
>      logical_die_id()
>    cpufreq/amd-pstate: Remove warning for X86_FEATURE_CPPC on Zen1 and
>      Zen2
> 
>   drivers/cpufreq/amd-pstate.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 

Thanks for handling these.  I'm back now, I'm digging through my inbox.
Assuming Rafael didn't already pull these I'll batch these into my 
amd-pstate branch for the robots to bang on and then send a PR after I 
go through everything else that happened.

Re: [PATCH 0/3] cpufreq/amd-pstate: A set of fixes

[Gautham R. Shenoy]

Hello Mario,


On Thu, Aug 22, 2024 at 03:41:30PM -0500, Mario Limonciello wrote:
> On 8/13/2024 04:51, Gautham R. Shenoy wrote:
> > Hello Rafael, Viresh,
> > 
> > This series contains three fixes for the amd-pstate driver for 6.11.
> > Could you please include it in your tree?
> > 
> > These patches are based on linux-pm/master with the top commit
> > 7c626ce4bae1 ("Linux 6.11-rc3").
> > 
> > There are three patches,
> > 
> > 1. To fix an uninitialized variable in amd_pstate_cpu_boost_update()
> >     from Dan Carpenter:
> >     https://lore.kernel.org/lkml/7ff53543-6c04-48a0-8d99-7dc010b93b3a@stanley.mountain/
> > 
> > 2. Use topology_logical_package_id() instead of
> >     topology_logical_die_id() definition of the later function has
> >     changed on some AMD processors since the inclusion of the CPUID
> >     0x80000026 parser:
> >     https://lore.kernel.org/lkml/20240801124509.3650-1-Dhananjay.Ugwekar@amd.com/
> > 
> > 3. Remove a warning for the absence fo X86_FEATURE_CPPC on Zen1 and
> >     Zen2 since they don't have the feature defined. This fixes the
> >     regression reported by David Wang:
> >     https://lore.kernel.org/lkml/20240730140111.4491-1-00107082@163.com/

> > 
> > Thanks and Regards
> > gautham.
> > 
> > Dan Carpenter (1):
> >    cpufreq: amd-pstate: Fix uninitialized variable in
> >      amd_pstate_cpu_boost_update()
> > 
> > Gautham R. Shenoy (2):
> >    cpufreq/amd-pstate: Use topology_logical_package_id() instead of
> >      logical_die_id()
> >    cpufreq/amd-pstate: Remove warning for X86_FEATURE_CPPC on Zen1 and
> >      Zen2
> > 
> >   drivers/cpufreq/amd-pstate.c | 10 ++++------
> >   1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> 
> Thanks for handling these.  I'm back now, I'm digging through my inbox.
> Assuming Rafael didn't already pull these I'll batch these into my
> amd-pstate branch for the robots to bang on and then send a PR after I go
> through everything else that happened.

Thank you Mario. I will be sending an updated version to Patch 3
"Remove warning for X86_FEATURE_CPPC on Zen1 and Zen2" to incorporate
feedback from Xiaojian.

Please merge the Patches 1 and 2 from this series.

--
Thanks and Regards
gautham.

Re: [PATCH 0/3] cpufreq/amd-pstate: A set of fixes

[Mario Limonciello]

On 8/22/2024 23:13, Gautham R. Shenoy wrote:
> Hello Mario,
> 
> 
> On Thu, Aug 22, 2024 at 03:41:30PM -0500, Mario Limonciello wrote:
>> On 8/13/2024 04:51, Gautham R. Shenoy wrote:
>>> Hello Rafael, Viresh,
>>>
>>> This series contains three fixes for the amd-pstate driver for 6.11.
>>> Could you please include it in your tree?
>>>
>>> These patches are based on linux-pm/master with the top commit
>>> 7c626ce4bae1 ("Linux 6.11-rc3").
>>>
>>> There are three patches,
>>>
>>> 1. To fix an uninitialized variable in amd_pstate_cpu_boost_update()
>>>      from Dan Carpenter:
>>>      https://lore.kernel.org/lkml/7ff53543-6c04-48a0-8d99-7dc010b93b3a@stanley.mountain/
>>>
>>> 2. Use topology_logical_package_id() instead of
>>>      topology_logical_die_id() definition of the later function has
>>>      changed on some AMD processors since the inclusion of the CPUID
>>>      0x80000026 parser:
>>>      https://lore.kernel.org/lkml/20240801124509.3650-1-Dhananjay.Ugwekar@amd.com/
>>>
>>> 3. Remove a warning for the absence fo X86_FEATURE_CPPC on Zen1 and
>>>      Zen2 since they don't have the feature defined. This fixes the
>>>      regression reported by David Wang:
>>>      https://lore.kernel.org/lkml/20240730140111.4491-1-00107082@163.com/
> 
>>>
>>> Thanks and Regards
>>> gautham.
>>>
>>> Dan Carpenter (1):
>>>     cpufreq: amd-pstate: Fix uninitialized variable in
>>>       amd_pstate_cpu_boost_update()
>>>
>>> Gautham R. Shenoy (2):
>>>     cpufreq/amd-pstate: Use topology_logical_package_id() instead of
>>>       logical_die_id()
>>>     cpufreq/amd-pstate: Remove warning for X86_FEATURE_CPPC on Zen1 and
>>>       Zen2
>>>
>>>    drivers/cpufreq/amd-pstate.c | 10 ++++------
>>>    1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>
>> Thanks for handling these.  I'm back now, I'm digging through my inbox.
>> Assuming Rafael didn't already pull these I'll batch these into my
>> amd-pstate branch for the robots to bang on and then send a PR after I go
>> through everything else that happened.
> 
> Thank you Mario. I will be sending an updated version to Patch 3
> "Remove warning for X86_FEATURE_CPPC on Zen1 and Zen2" to incorporate
> feedback from Xiaojian.
> 
> Please merge the Patches 1 and 2 from this series.
> 

Thanks, they're staged with the other fixes that were on the list.
Will wait for your updated patch 3 for the fixes PR.