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: <7e86cdbe-f16c-4fe8-92c5-e6fb89f49811@huawei.com>
Date: Mon, 26 Jan 2026 16:10:49 +0800
From: "zhenglifeng (A)" <zhenglifeng1@...wei.com>
To: Sumit Gupta <sumitg@...dia.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>
Subject: Re: [PATCH v6 5/9] ACPI: CPPC: Extend cppc_set_epp_perf() for
 FFH/SystemMemory

On 2026/1/25 4:08, Sumit Gupta wrote:
> 
> On 22/01/26 14:48, zhenglifeng (A) wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 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?

> 
> 
> Thank you,
> Sumit Gupta
> 
> 
> 
>>>        } else {
>>>                ret = -ENOTSUPP;
>>> -             pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
>>> +             pr_debug("_CPC in PCC/FFH/SystemMemory are not supported\n");
>>>        }
>>>
>>>        return ret;