linux-kernel - Re: [PATCH v3 1/5] firmware: qcom_scm: provide a read-modify-write function

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: <CACRpkdYq96d=0hoBhNGYihVJbX-D4WLw3+GdQC5_NDXuBiaF7Q@mail.gmail.com>
Date:   Mon, 20 Mar 2023 09:54:15 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Bjorn Andersson <andersson@...nel.org>
Cc:     Mukesh Ojha <quic_mojha@...cinc.com>, agross@...nel.org,
        konrad.dybcio@...aro.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v3 1/5] firmware: qcom_scm: provide a read-modify-write function

On Mon, Mar 20, 2023 at 4:19 AM Bjorn Andersson <andersson@...nel.org> wrote:

> > This is starting to reimplement regmap.
> > In this case regmap_update_bits().
> >
> > What about just using regmap as accessor for these
> > registers instead?
> >
>
> I'm not sure it would be beneficial...

Me neither, I don't know the details, I just notice the similarity in the
accessors.

> The regmap interface provides a standardized representation of a block
> of registers, with the suitable accessors backing it. But in both cases
> touched upon in this series, the addressed registers are part of regions
> already handled by the kernel.
>
> So it wouldn't be suitable to create a regmap-abstraction for "a block
> of secure registers", at best that would give us two kinds of regmaps
> abstracting the same register block.

>From my viewpoint regmap does three things:
- Abstract one coherent region of registers under a shared lock, with
  nifty accessors (such as mask-and-set with regmap_update_bits())
- Maps access patterns/permissions and permissible access range
- Optionally cache the contents

The way I would use it if these secure registers are in the same range as a
bunch of non-secure ones is for sharing a lock and the regmap accessors.

I wouldn't worry about access patterns and such. That usecase (block
access to certain registers or bits) is partly overdesign in some cases
IMO.

If regmap abstraction isn't helpful overall then we shouldn't do it.

Yours,
Linus Walleij