Re: [PATCH v5 02/19] cpufreq/amd-pstate: Show a warning when a CPU fails to setup

[Paul Menzel]

Dear Mario,


Thank you for the patch.

Am 26.02.25 um 08:49 schrieb Mario Limonciello:
> From: Mario Limonciello <mario.limonciello@xxxxxxx>
>
> I came across a system that MSR_AMD_CPPC_CAP1 for some CPUs isn't
> populated.  This is an unexpected behavior that is most likely a
> BIOS bug. In the event it happens I'd like users to report bugs
> to properly root cause and get this fixed.
>
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@xxxxxxx>
> Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@xxxxxxx>
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
>   drivers/cpufreq/amd-pstate.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index a093389a8fe3e..1b98f5d76894d 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1034,6 +1034,7 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>   free_cpudata2:
>   	freq_qos_remove_request(&cpudata->req[0]);
>   free_cpudata1:
> +	pr_warn("Failed to initialize CPU %d: %d\n", policy->cpu, ret);

From a user/operator point of view, having a recommended action in the 
log message would help a lot, as I am not able to judge the 
consequences, and where to go to. So, I’d propose:

Failed to initialize CPU %d: %d. This is likely a firmware error, and 
should be reported to the vendor.

The Linux kernel also has some macros. From `include/linux/printk.h`:

```
/*
 * FW_BUG
 * Add this to a message where you are sure the firmware is buggy or 
behaves
 * really stupid or out of spec. Be aware that the responsible BIOS 
developer
 * should be able to fix this issue or at least get a concrete idea of the
 * problem by reading your message without the need of looking at the 
kernel
 * code.
 *
 * Use it for definite and high priority BIOS bugs.
 *
 * FW_WARN
 * Use it for not that clear (e.g. could the kernel messed up things 
already?)
 * and medium priority BIOS bugs.
 *
 * FW_INFO
 * Use this one if you want to tell the user or vendor about something
 * suspicious, but generally harmless related to the firmware.
 *
 * Use it for information or very low priority BIOS bugs.
 */
#define FW_BUG          "[Firmware Bug]: "
#define FW_WARN         "[Firmware Warn]: "
#define FW_INFO         "[Firmware Info]: "
```

For ACPI:

    drivers/acpi/acpica/acutils.h:#define ACPI_MSG_BIOS_ERROR 
"Firmware Error (ACPI): "

>   	kfree(cpudata);
>   	return ret;
>   }
> @@ -1527,6 +1528,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>   	return 0;
>   
>   free_cpudata1:
> +	pr_warn("Failed to initialize CPU %d: %d\n", policy->cpu, ret);
>   	kfree(cpudata);
>   	return ret;
>   }


Kind regards,

Paul