linux-kernel - Re: [PATCH v2 1/2] cpufreq: CPPC: keep target core awake when reading its cpufreq rate

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] [day] [month] [year] [list]

Message-ID: <9025b93a-faec-bc84-00c4-34a3b1c01b35@huawei.com>
Date:   Fri, 26 May 2023 16:58:45 +0800
From:   Zeng Heng <zengheng4@...wei.com>
To:     Pierre Gondois <pierre.gondois@....com>,
        Ionela Voinescu <Ionela.Voinescu@....com>, <sumitg@...dia.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>,
        <wangxiongfeng2@...wei.com>, <xiexiuqi@...wei.com>,
        <liwei391@...wei.com>, <linux-acpi@...r.kernel.org>,
        <lenb@...nel.org>, <viresh.kumar@...aro.org>, <rafael@...nel.org>,
        Yang Shi <yang@...amperecomputing.com>
Subject: Re: [PATCH v2 1/2] cpufreq: CPPC: keep target core awake when reading
 its cpufreq rate


在 2023/5/17 16:17, Pierre Gondois 写道:
> +Ionela, Sumit, Yang,
>
> Hello Zeng,
>
> I think solutions around related issues were suggested at:
>
> [1] https://lore.kernel.org/all/20230418113459.12860-7-sumitg@nvidia.com/
> [2] 
> https://lore.kernel.org/all/20230328193846.8757-1-yang@os.amperecomputing.com/
> [3] https://lore.kernel.org/all/ZEl1Fms%2FJmdEZsVn@arm.com/
>
> About this patch, it seems to mean that CPPC counters of CPUx are always
> accessed from CPUx, even when they are not AMUs. For instance CPPC
> counters could be memory mapped and accessible from any CPU.
> cpu_has_amu_feat() should allow to probe if a CPU uses AMUs or not,
> and [2] had an implementation using it.
>
> Another comment about PATCH 2/2 is that if the counters are accessed
> through FFH, arm64 version of cpc_read_ffh() is calling
> counters_read_on_cpu(), and a comment in counters_read_on_cpu() seems
> to specify the function must be called with interrupt enabled.
>
> I think the best solution so far was the one at [3], suggested by Ionela,
> but it doesn't seem to solve your issue. Indeed, it is not checked 
> whether
> the counters are AMU counters and that they must be remotely read (to
> have the CPU awake),
>
> Regards,
> Pierre


Here is segment I picked from patch[3], and there is a modification is 
deserved to be discussed:

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

@@ -1336,8 +1361,25 @@ int cppc_get_perf_ctrs(int cpunum, struct 
cppc_perf_fb_ctrs *perf_fb_ctrs)
          }
      }

-    cpc_read(cpunum, delivered_reg, &delivered);
-    cpc_read(cpunum, reference_reg, &reference);
+    ctrs.cpunum = cpunum;
+    ctrs.delivered_reg = delivered_reg;
+    ctrs.reference_reg = reference_reg;
+    ctrs.delivered = &delivered;
+    ctrs.reference = &reference;
+

+    if (CPC_IN_FFH(delivered_reg) && CPC_IN_FFH(reference_reg)) {

Here we call cpu_has_amu_feat() as precondition (like Sumit's mail 
mentions), which could be compatible with

any SoCs with AMU that could be accessible via sys-register and system 
memory both.


+        ret = smp_call_on_cpu(cpunum, cppc_get_cycle_ctrs, &ctrs, false);
+    } else {
+        if (CPC_IN_SYSTEM_MEMORY(delivered_reg) &&
+            CPC_IN_SYSTEM_MEMORY(reference_reg)) {
+            local_irq_save(flags);
+            cppc_get_cycle_ctrs(&ctrs);
+            local_irq_restore(flags);
+        } else {
+            cppc_get_cycle_ctrs(&ctrs);
+        }
+    }
+
      cpc_read(cpunum, ref_perf_reg, &ref_perf);

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

If there were a new version patch released, please loop me at that time.

Thanks a lot in advance.


B.R.,

Zeng Heng