linux-kernel - Re: [PATCH v6 5/9] ACPI: CPPC: Extend cppc_set_epp

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: <abf9889b-d5e7-4c4b-b166-a5dde2425ab8@nvidia.com>
Date: Tue, 27 Jan 2026 16:47:51 +0530
From: Sumit Gupta <sumitg@...dia.com>
To: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
Cc: rafael@...nel.org, viresh.kumar@...aro.org, pierre.gondois@....com,
 ionela.voinescu@....com, lenb@...nel.org, robert.moore@...el.com,
 corbet@....net, rdunlap@...radead.org, ray.huang@....com,
 gautham.shenoy@....com, mario.limonciello@....com, perry.yuan@....com,
 zhanjie9@...ilicon.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,
 nhartman@...dia.com, bbasu@...dia.com, sumitg@...dia.com
Subject: Re: [PATCH v6 5/9] ACPI: CPPC: Extend cppc_set_epp_perf() for
 FFH/SystemMemory


>>> On 2026/1/20 22:56, Sumit Gupta wrote:
>>>> Extend cppc_set_epp_perf() to write both auto_sel and energy_perf
>>>> registers when they are in FFH or SystemMemory address space.
>>>>
>>>> This keeps the behavior consistent with PCC case where both registers
>>>> are already updated together, but was missing for FFH/SystemMemory.
>>>>
>>>> Signed-off-by: Sumit Gupta <sumitg@...dia.com>
>>>> ---
>>>>    drivers/acpi/cppc_acpi.c | 24 +++++++++++++++++++++---
>>>>    1 file changed, 21 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>>> index de35aeb07833..45c6bd6ec24b 100644
>>>> --- a/drivers/acpi/cppc_acpi.c
>>>> +++ b/drivers/acpi/cppc_acpi.c
>>>> @@ -1562,6 +1562,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_ffh_sysmem;
>>>> +     bool epp_ffh_sysmem;
>>>>         int ret;
>>>>
>>>>         if (!cpc_desc) {
>>>> @@ -1572,6 +1574,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_ffh_sysmem = CPC_SUPPORTED(epp_set_reg) &&
>>>> +             (CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
>>>> +     autosel_ffh_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);
>>>> @@ -1597,11 +1604,22 @@ 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)) {
>>>> -             ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
>>>> +                (epp_ffh_sysmem || autosel_ffh_sysmem)) {
>>>> +             if (autosel_ffh_sysmem) {
>>>> +                     ret = cpc_write(cpu, auto_sel_reg, enable);
>>>> +                     if (ret)
>>>> +                             return ret;
>>>> +             }
>>>> +
>>>> +             if (epp_ffh_sysmem) {
>>>> +                     ret = cpc_write(cpu, epp_set_reg,
>>>> +                                     perf_ctrls->energy_perf);
>>>> +                     if (ret)
>>>> +                             return ret;
>>>> +             }
>>> Don't know if such a scenario exists, but if one of them is in PCC and the
>>> other is in FFH or system memory, only the one in PCC will be updated
>>> based on your modifications.
>> The current code handles mixed cases correctly.
>> When either register is in PCC, the first if block executes and calls
>> cpc_write() for both registers. The cpc_write() internally handles
>> each register's type (PCC, FFH, or SystemMemory)
> Yes, I was wrong.
>
> According to the first if block, cpc_wite() is OK to be called for a
> register not in PCC. So it looks like this 'else if' is unnecessary. Only
> CPC_SUPPORTED is needed to be checked before calling cpc_write(), isn't it?

Yes, Once 'osc_cpc_flexible_adr_space_confirmed' is removed,
cppc_set_epp_perf() can be simplified to just call cpc_write() for
supported registers and only do PCC handling when needed.
As Pierre suggested [1], I will send a separate patch set for this
cleanup after the current patch set.
  [1] 
https://lore.kernel.org/all/c3fd7249-3cba-43e9-85c6-eadd711c0527@nvidia.com/

Thank you,
Sumit Gupta

....