linux-kernel - Re: [PATCH v3 3/8] ACPI: CPPC: extend APIs to support auto

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: <acb19233-1ab1-4c6e-a150-5657daa50846@nvidia.com>
Date: Fri, 24 Oct 2025 18:42:38 +0530
From: Sumit Gupta <sumitg@...dia.com>
To: Ionela Voinescu <ionela.voinescu@....com>
Cc: rafael@...nel.org, viresh.kumar@...aro.org, lenb@...nel.org,
 robert.moore@...el.com, corbet@....net, pierre.gondois@....com,
 zhenglifeng1@...wei.com, rdunlap@...radead.org, ray.huang@....com,
 gautham.shenoy@....com, mario.limonciello@....com, perry.yuan@....com,
 linux-pm@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-doc@...r.kernel.org, acpica-devel@...ts.linux.dev,
 linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org,
 treding@...dia.com, jonathanh@...dia.com, vsethi@...dia.com,
 ksitaraman@...dia.com, sanjayc@...dia.com, bbasu@...dia.com,
 sumitg@...dia.com
Subject: Re: [PATCH v3 3/8] ACPI: CPPC: extend APIs to support auto_sel and
 epp

Hi Ionela,

Thank you for the comments.


On 22/10/25 14:42, Ionela Voinescu wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Sumit,
>
> On Wednesday 01 Oct 2025 at 20:30:59 (+0530), Sumit Gupta wrote:
>> - Add auto_sel read support in cppc_get_perf_caps().
>> - Add write of both auto_sel and energy_perf in cppc_set_epp_perf().
>> - Remove redundant energy_perf field from 'struct cppc_perf_caps' as
>>    the same is available in 'struct cppc_perf_ctrls' which is used.
>>
>> Signed-off-by: Sumit Gupta <sumitg@...dia.com>
>> ---
> [..]
>> @@ -1555,6 +1559,8 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>        struct cpc_register_resource *auto_sel_reg;
>>        struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>>        struct cppc_pcc_data *pcc_ss_data = NULL;
>> +     bool autosel_support_in_ffh_or_sysmem;
>> +     bool epp_support_in_ffh_or_sysmem;
>>        int ret;
>>
>>        if (!cpc_desc) {
>> @@ -1565,6 +1571,11 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>        auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
>>        epp_set_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
>>
>> +     epp_support_in_ffh_or_sysmem = CPC_SUPPORTED(epp_set_reg) &&
>> +                             (CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
>> +     autosel_support_in_ffh_or_sysmem = CPC_SUPPORTED(auto_sel_reg) &&
>> +                             (CPC_IN_FFH(auto_sel_reg) || CPC_IN_SYSTEM_MEMORY(auto_sel_reg));
>> +
>>        if (CPC_IN_PCC(epp_set_reg) || CPC_IN_PCC(auto_sel_reg)) {
>>                if (pcc_ss_id < 0) {
>>                        pr_debug("Invalid pcc_ss_id for CPU:%d\n", cpu);
>> @@ -1590,8 +1601,19 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>                ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
>>                up_write(&pcc_ss_data->pcc_lock);
>>        } else if (osc_cpc_flexible_adr_space_confirmed &&
>> -                CPC_SUPPORTED(epp_set_reg) && CPC_IN_FFH(epp_set_reg)) {
>> +                epp_support_in_ffh_or_sysmem && autosel_support_in_ffh_or_sysmem) {
> Isn't this problematic for when auto-select is an integer set to 1 or it's
> not present at all? In those cases the EPP register won't be written and
> -ENOTSUPP will be returned.
>
> I suppose for the case when auto-select is not present at all in _CPC
> (it's an optional attribute) it's not very clear what one should do
> regarding writing the EPP register. The specification mentions that
> "Writes to this register only have meaning when Autonomous Selection
> is enabled.". From my perspective the OS should not block writes to the
> EPP register for this case, as autonomous selection might still be
> enabled but not exposed to the OS.
>
> Thanks,
> Ionela.

Will change in v4 to write them independently as below.

-------------
         } else if (osc_cpc_flexible_adr_space_confirmed) {
                 if (!epp_support_in_ffh_or_sysmem && 
!autosel_support_in_ffh_or_sysmem) {
                         ret = -ENOTSUPP;
                 } else {
                         if (autosel_support_in_ffh_or_sysmem) {
                                 ret = cpc_write(cpu, auto_sel_reg, enable);
                                 if (ret)
                                         return ret;
                         }

                         if (epp_support_in_ffh_or_sysmem) {
                                 ret = cpc_write(cpu, epp_set_reg, 
perf_ctrls->energy_perf);
                                 if (ret)
                                         return ret;
                         }
                 }
         } else {
                 ret = -ENOTSUPP;
         }

         if (ret == -ENOTSUPP)
                 pr_debug("_CPC in PCC and _CPC in FFH are not 
supported\n");

-------------

Thank you,
Sumit Gupta


>> +             ret = cpc_write(cpu, auto_sel_reg, enable);
>> +             if (ret) {
>> +                     pr_debug("Failed to write auto_sel=%d for CPU:%d\n", enable, cpu);
>> +                     return ret;
>> +             }
>> +
>>                ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
>> +             if (ret) {
>> +                     pr_debug("Failed to write energy_perf=%u for CPU:%d\n",
>> +                              perf_ctrls->energy_perf, cpu);
>> +                     return ret;
>> +             }
>>        } else {
>>                ret = -ENOTSUPP;
>>                pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
> [..]