Re: [PATCH v3] cpufreq: amd-pstate-ut: bail out early on non-AMD platform

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

Hi Qianheng,

Thank you for your patience. It looks good to me overall, just a few
nits inline.

On 7/16/2026 4:51 PM, Qianheng Peng wrote:
> The crash issue may occur when modprobe amd_pstate_ut on intel platform.
> 

It is better to start with:

"Loading the amd-pstate-ut module on a non-AMD platform triggers a NULL
pointer dereference:"

or

"A kernel NULL pointer dereference occurs when loading the amd-pstate-ut
module on a non-AMD (e.g. Intel) platform"

>     amd_pstate_ut: 1    amd_pstate_ut_acpi_cpc_valid  success!
>     amd_pstate_ut: 2    amd_pstate_ut_check_enabled   success!
>     BUG: kernel NULL pointer dereference, address: 0000000000000080
>     #PF: supervisor read access in kernel mode
>     #PF: error_code(0x0000) - not-present page
>     PGD 0 P4D 0
>     Oops: 0000 [#1] SMP NOPTI
>     CPU: 0 PID: 20300 Comm: modprobe
>     Kdump: loaded Tainted: G O 6.6.0-0010.rc1.ctl4.x86_64 #1
>     Hardware name: FiberHome R2200 V5/Xeon Boards, BIOS 3.1a 02/24/2020
>     RIP: 0010:amd_pstate_ut_check_perf+0x141/0x280 [amd_pstate_ut]
>     Call Trace:
>      <TASK>
>      amd_pstate_ut_init+0x1b/0xff0 [amd_pstate_ut]
>      ? __pfx_amd_pstate_ut_init+0x10/0x10 [amd_pstate_ut]
>      do_one_initcall+0x42/0x2e0
>      ? kmalloc_trace+0x26/0x90
>      do_init_module+0x60/0x240
>      __se_sys_init_module+0x185/0x1c0
>      do_syscall_64+0x62/0x190
>      entry_SYSCALL_64_after_hwframe+0x76/0x7e
>      </TASK>
> 
> Add state detection to amd pstate driver to prevent amd_pstate_ut driver

This change fixes the amd-pstate-ut module rather than the amd-pstate
driver itself. Maybe something like:

"Add driver-state detection to the amd-pstate-ut module ..."

> from testing on non-AMD platforms.

It also covers the AMD_PSTATE_DISABLE case but not only non-AMD platforms.

And likewise, the subject may be changed as well, for example:

"cpufreq: amd-pstate-ut: Bail out early if the amd-pstate driver is not
running"

or

"cpufreq: amd-pstate-ut: Skip tests when amd-pstate driver is not
active"


> 
> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
> Suggested-by: Li Xiong <xiongl24@chinatelecom.cn>
> Suggested-by: Xibo Wang <wangxb12@chinatelecom.cn>
> Signed-off-by: Qianheng Peng <pengqh1@chinatelecom.cn>
> 
> ---
> v3:
>    - Remove unnecessary NULL check
>    - Add detection to driver state instead of driver name
> 
> v2: https://lore.kernel.org/all/1784108124-19988-1-git-send-email-pengqh1@chinatelecom.cn
>    - Print policy->cpu instead of policy->kboj.name
>    - Add cpufreq driver name detection to amd_pstate_ut
> 
> v1: https://lore.kernel.org/all/1784015941-26535-1-git-send-email-pengqh1@chinatelecom.cn
> ---
>   drivers/cpufreq/amd-pstate-ut.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index 735b29f..2142838 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -560,6 +560,11 @@ static int amd_pstate_ut_check_freq_attrs(u32 index)
>   static int __init amd_pstate_ut_init(void)
>   {
>   	u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
> +	enum amd_pstate_mode mode = amd_pstate_get_status();
> +
> +	/* don't test if no running amd-pstate driver */

How about:

"/* Do not run tests when the amd-pstate driver is not active. */"

> +	if (mode == AMD_PSTATE_UNDEFINED || mode == AMD_PSTATE_DISABLE)
> +		return -EOPNOTSUPP;
>   
>   	for (i = 0; i < arr_size; i++) {
>   		int ret;


Anyone, please feel free to correct or ignore any of the nits above.

Regardless, the patch code looks good to me.

Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>


-- 
Thx and BRs,
Zhongqiu Han