linux-kernel - Re: [PATCH v4 09/19] cpufreq/amd-pstate-ut: Drop SUCCESS and FAIL enums

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ac22d9f-484f-447c-8113-d6a698b0bc73@amd.com>
Date: Tue, 25 Feb 2025 09:37:13 +0530
From: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
To: Mario Limonciello <superm1@...nel.org>,
 "Gautham R . Shenoy" <gautham.shenoy@....com>,
 Perry Yuan <perry.yuan@....com>
Cc: "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
 <linux-kernel@...r.kernel.org>,
 "open list:CPU FREQUENCY SCALING FRAMEWORK" <linux-pm@...r.kernel.org>,
 Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH v4 09/19] cpufreq/amd-pstate-ut: Drop SUCCESS and FAIL
 enums

On 2/25/2025 5:35 AM, Mario Limonciello wrote:
> On 2/24/2025 00:05, Dhananjay Ugwekar wrote:
>> On 2/20/2025 2:32 AM, Mario Limonciello wrote:
>>> From: Mario Limonciello <mario.limonciello@....com>
>>>
>>> Enums are effectively used as a boolean and don't show
>>> the return value of the failing call.
>>>
>>> Instead of using enums switch to returning the actual return
>>> code from the unit test.
>>>
>>
>> One query below, apart from that LGTM,
>>
>> Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@....com>
> 
> Thanks!
> 
>>
>>> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@....com>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>>> ---
>>>   drivers/cpufreq/amd-pstate-ut.c | 143 ++++++++++++--------------------
>>>   1 file changed, 55 insertions(+), 88 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
>>> index 0f0b867e271cc..028527a0019ca 100644
>>> --- a/drivers/cpufreq/amd-pstate-ut.c
>>> +++ b/drivers/cpufreq/amd-pstate-ut.c
>>> @@ -32,30 +32,20 @@
>>>     #include "amd-pstate.h"
>>>   -/*
>>> - * Abbreviations:
>>> - * amd_pstate_ut: used as a shortform for AMD P-State unit test.
>>> - * It helps to keep variable names smaller, simpler
>>> - */
>>> -enum amd_pstate_ut_result {
>>> -    AMD_PSTATE_UT_RESULT_PASS,
>>> -    AMD_PSTATE_UT_RESULT_FAIL,
>>> -};
>>>     struct amd_pstate_ut_struct {
>>>       const char *name;
>>> -    void (*func)(u32 index);
>>> -    enum amd_pstate_ut_result result;
>>> +    int (*func)(u32 index);
>>>   };
>>>     /*
>>>    * Kernel module for testing the AMD P-State unit test
>>>    */
>>> -static void amd_pstate_ut_acpi_cpc_valid(u32 index);
>>> -static void amd_pstate_ut_check_enabled(u32 index);
>>> -static void amd_pstate_ut_check_perf(u32 index);
>>> -static void amd_pstate_ut_check_freq(u32 index);
>>> -static void amd_pstate_ut_check_driver(u32 index);
>>> +static int amd_pstate_ut_acpi_cpc_valid(u32 index);
>>> +static int amd_pstate_ut_check_enabled(u32 index);
>>> +static int amd_pstate_ut_check_perf(u32 index);
>>> +static int amd_pstate_ut_check_freq(u32 index);
>>> +static int amd_pstate_ut_check_driver(u32 index);
>>>     static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
>>>       {"amd_pstate_ut_acpi_cpc_valid",   amd_pstate_ut_acpi_cpc_valid   },
>>> @@ -78,51 +68,46 @@ static bool get_shared_mem(void)
>>>   /*
>>>    * check the _CPC object is present in SBIOS.
>>>    */
>>> -static void amd_pstate_ut_acpi_cpc_valid(u32 index)
>>> +static int amd_pstate_ut_acpi_cpc_valid(u32 index)
>>>   {
>>> -    if (acpi_cpc_valid())
>>> -        amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
>>> -    else {
>>> -        amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>>> +    if (!acpi_cpc_valid()) {
>>>           pr_err("%s the _CPC object is not present in SBIOS!\n", __func__);
>>> +        return -EINVAL;
>>>       }
>>> +
>>> +    return 0;
>>>   }
>>>   -static void amd_pstate_ut_pstate_enable(u32 index)
>>> +/*
>>> + * check if amd pstate is enabled
>>> + */
>>> +static int amd_pstate_ut_check_enabled(u32 index)
>>>   {
>>> -    int ret = 0;
>>>       u64 cppc_enable = 0;
>>> +    int ret;
>>> +
>>> +    if (get_shared_mem())
>>> +        return 0;
>>
>> What do you think about adding a "cppc_get_enable()" function in acpi_cppc.c so that we can
>> run this check for shared mem systems as well ?
>>
> 
> I think it's a good idea.  Would you mind working that out for after this series lands?

Sure, I'll take it up.

> 
>> Thanks,
>> Dhananjay