linux-kernel - Re: [PATCH 1/2] soc: qcom: rpmhpd: Use corner in power

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]

Date:   Thu, 8 Jul 2021 06:51:38 +0000
From:   Stephen Boyd <swboyd@...omium.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rajendra Nayak <rnayak@...eaurora.org>
Cc:     Andy Gross <agross@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] soc: qcom: rpmhpd: Use corner in power_off

Quoting Rajendra Nayak (2021-07-07 22:03:53)
>
>
> On 7/8/2021 10:05 AM, Bjorn Andersson wrote:
> > On Wed 07 Jul 19:21 CDT 2021, Stephen Boyd wrote:
> >
> >> Quoting Bjorn Andersson (2021-07-02 17:54:15)
> >>> rpmhpd_aggregate_corner() takes a corner as parameter, but in
> >>> rpmhpd_power_off() the code requests the level of the first corner
> >>> instead.
> >>>
> >>> In all (known) current cases the first corner has level 0, so this
> >>> change should be a nop, but in case that there's a power domain with a
> >>> non-zero lowest level this makes sure that rpmhpd_power_off() actually
> >>> requests the lowest level - which is the closest to "power off" we can
> >>> get.
> >>>
> >>> While touching the code, also skip the unnecessary zero-initialization
> >>> of "ret".
> >>>
> >>> Fixes: 279b7e8a62cc ("soc: qcom: rpmhpd: Add RPMh power domain driver")
> >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@...aro.org>
> >>> ---
> >>
> >> I think this is why qcom folks talk about "virtual corner" and "physical
> >> corner" because there's the one in command DB and the one in hardware.
> >
> > I think the driver uses "level" and "corner" to denote the two different
> > number spaces, so I think we're good...now that we after this patch
> > don't pass a "level" as "corner" during power_off ;)

Alright then nothing to do. Yay?

> >
> >> Maybe we should change rpmhpd_aggregate_corner() to call the argument
> >> 'vcorner'?
> >
> > So "virtual corner" is "corner" and "physical corner" is level? I.e. 256
> > is a "physical corner"?
>
> I haven't heard of anything called a 'physical corner'. These were always
> referred to as virtual corners, on older platforms it was just one contiguous
> number space, on newer ones we added another higher level sparse number space
> just for more fun :)
> Command DB refers to these as hlvl and vlvl, I haven;t yet figured out what their
> full forms are :/

Ah maybe I'm mixing up CPR terms with this stuff. I suspect hlvl is
"hardware level" and vlvl is "virtual level", but probably should have
been "software level".

As far as I remember, the command DB layer was stacked on top so that
they could insert more levels in between two levels in the hardware
number space and not have to change all the rpmh clients out there (of
which there could be many considering all the independent operating
systems running on the SoC). For example, [0, 128, 256] maps to [0, 1,
2] and then they realize they need to jam another level between 1 and 2
so they remap 256 to 3 so everyone keeps considering 256 as the previous
vlvl to clear the way for 2 to be reused as 198 or something like that.

I don't think this ever really changes after the device ships, but it
lets them decouple rpmh firmware updates from the rest of the system. As
long as they're kept as far apart in vlvl space as there are numbers in
hlvl space they can easily do this remap trick and hardware can treat it
as levels that bounce around physical voltages that are monotonically
increasing.