Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   <ansuelsmth@gmail.com>
To:     "'Sudeep Holla'" <sudeep.holla@arm.com>
Cc:     "'Rob Herring'" <robh+dt@kernel.org>,
        "'Andy Gross'" <agross@kernel.org>,
        "'Bjorn Andersson'" <bjorn.andersson@linaro.org>,
        "'MyungJoo Ham'" <myungjoo.ham@samsung.com>,
        "'Kyungmin Park'" <kyungmin.park@samsung.com>,
        "'Chanwoo Choi'" <cw00.choi@samsung.com>,
        <linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-pm@vger.kernel.org>
References: <20200929162926.139-1-ansuelsmth@gmail.com> <20200930092954.GA7125@bogus>
In-Reply-To: <20200930092954.GA7125@bogus>
Subject: RE: [PATCH v2 1/2] devfreq: qcom: Add L2 Krait Cache devfreq scaling driver
Date:   Wed, 30 Sep 2020 13:56:01 +0200
Message-ID: <006c01d69720$abd3f230$037bd690$@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain;
        charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Language: it
Thread-Index: AQHRHCX89vsNM0cZY8BINv1SaJtjIwLCr4YDqXXwJVA=
Precedence: bulk

> Subject: Re: [PATCH v2 1/2] devfreq: qcom: Add L2 Krait Cache devfreq
> scaling driver
> 
> On Tue, Sep 29, 2020 at 06:29:24PM +0200, Ansuel Smith wrote:
> > Qcom L2 Krait CPUs use the generic cpufreq-dt driver and doesn't
actually
> > scale the Cache frequency when the CPU frequency is changed. This
> > devfreq driver register with the cpu notifier and scale the Cache
> > based on the max Freq across all core as the CPU cache is shared across
> > all of them. If provided this also scale the voltage of the regulator
> > attached to the CPU cache. The scaling logic is based on the CPU freq
> > and the 3 scaling interval are set by the device dts.
> >
> 
> I have raised this concern before. I am worried this kind of independent
> CPU and cache frequency controls make way for clkscrew kind of attacks.
> Why can't the clocks be made parent/child or secondary and automatically
> updated when CPU clocks are changed.
> 
> --
> Regards,
> Sudeep

I don't think I understand this fully. Anyway about the clkscrew attack, the
range are set on the dts so unless someone actually wants to have a
vulnerable
system, the range can't be changes at runtime. The devfreq governor is set
to
immutable and can't be changes AFAIK.

About 'automatically updated when CPU changes', the cache is shared across 2
core and they scale independently. We can be in situation where one cpu is
at
max and one at idle freq and the cache is set to idle. To fix this at every
change
the clk should find the max value and I think this would make all the clk
scaling
very slow. If you have any suggestion on how I can implement this better,
I'm
more than happy to address them. For now, the lack of this kind of cache
scale,
make the system really slow since by default the init of the cpu and cache
clks
put them at the lowest frequency and nobody changes that. (we have cpufreq
scaling support but the cache is never actually scaled)