linux-kernel - Re: [PATCH 07/18] regmap: mmio: add config option to allow relaxed MMIO accesses

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:   Wed, 14 Oct 2020 13:12:49 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Adrian Ratiu <adrian.ratiu@...labora.com>
Cc:     Ezequiel Garcia <ezequiel@...labora.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Fruehberger Peter <Peter.Fruehberger@...bosch.com>,
        kuhanh.murugasen.krishnan@...el.com,
        Daniel Vetter <daniel@...ll.ch>, kernel@...labora.com,
        linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 07/18] regmap: mmio: add config option to allow relaxed
 MMIO accesses

On Wed, Oct 14, 2020 at 02:51:14PM +0300, Adrian Ratiu wrote:
> On Tue, 13 Oct 2020, Mark Brown <broonie@...nel.org> wrote:
> > On Mon, Oct 12, 2020 at 11:59:46PM +0300, Adrian Ratiu wrote:

> > > -	writeb(val, ctx->regs + reg); +	if (ctx->relaxed_mmio) +
> > > writeb_relaxed(val, ctx->regs + reg); +	else + writeb(val, ctx->regs
> > > + reg);

> > There is no point in doing a conditional operation on every I/O, it'd be
> > better to register a different set of ops when doing relaxed I/O.

> Indeed I have considered adding new functions but went with this solution
> because it's easier for the users to only have to define a "relaxed" config
> then test the regmap ctx as above.

It seems like you've taken this in a direction other than what  I was
thinking of here - defining separate ops doesn't mean we have to do
anything which has any impact on the interface seen by users.  The
regmap config is supplied at registration time, it's just as available
then as it is when doing I/O.

> Thinking a bit more about it, yes, it makes more sense to have dedicated
> ops: this way users don't have to be explicit about adding membarriers and
> can combine relaxed and non-relaxed more easily, so it's also a better API
> trade-off in addition to avoiding the conditional. Thanks!

I'm not sure what you're proposing here - it does seem useful to be able
to combine relaxed and non-relaxed I/O but that seems like it'd break
down the abstraction for regmap since tht's not really a concept other
buses are going to have?  Unless we provide an operation to switch by
setting flags or somethin possibly and integrate it with the cache
perhaps.  Could you be a bit more specific about what you were thinking
of here please?

> Question: Do you want me to split this patch from the series and send it
> separately just for the regmap subsystem to be easier to review / apply?

Sure.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)