linux-kernel - Re: [PATCH RFC 1/2] regmap: add option to disable debugfs

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: <YrMKINPtKfmnxLep@sirena.org.uk>
Date:   Wed, 22 Jun 2022 13:25:04 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Lucas Stach <l.stach@...gutronix.de>
Cc:     Aisheng Dong <aisheng.dong@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dongas86@...il.com" <dongas86@...il.com>,
        Peng Fan <peng.fan@....com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>
Subject: Re: [PATCH RFC 1/2] regmap: add option to disable debugfs

On Wed, Jun 22, 2022 at 10:08:10AM +0200, Lucas Stach wrote:
> Am Dienstag, dem 21.06.2022 um 18:16 +0000 schrieb Aisheng Dong:

> > 1. There's a warning dump if using cache_only without cache
> > void regcache_cache_only(struct regmap *map, bool enable)
> > {
> >         map->lock(map->lock_arg);
> >         WARN_ON(map->cache_bypass && enable);
> >         ...
> > }
> > 2. It seems _regmap_write() did not handle cache_only case well
> > without cache.
> > It may still writes HW even for cache_only mode?
> > 
> > I guess we may need fix those two issues first before we can safely
> > use it?

> Why would you write to a cache only regmap. The debugfs interface only
> allows to dump the registers, no?

One of the use cases is where you've got settings that can be changed
while the device is idle and just map those directly onto the registers,
syncing the cache to the device when it gets powered up for actual use.
This can only be done when there's a cache, and won't apply to a lot of
devices.

There is debugfs code to do writes but you have to modify the kernel to
enable it, there's no config option for it upstream.

> I think it wouldn't be too hard to fix this for the blk-ctrl drivers.
> I'll give it a try today.

The other thing is that even with the bodge to just turn debugfs
presumably any case where the driver would try to write to a powered off
device is still a bug that needs fixing anyway - having a regmap in
cache only mode will translate it into a warning rather than a write
that goes AWOL or otherwise fails.

> > > That's a different thing, that's due to us naming the directory
> > > after the struct
> > > device but syscons being created before we have that struct device
> > > available.

> > Yes, but syscon has the same issue that the system may hang if dump
> > registers
> > through debugfs without power on.
> > How would you suggest for this case as syscon is also a common
> > driver?

> This is a general issue. If something uses a syscon that is inside a
> power-domain where the runtime PM is controlled by some other entity,
> how is it safe to use the syscon at all? Every access might end up
> locking up the system. So those syscons really need to learn some kind
> of runtime PM handling.

Right.  If you can interact with the device safely there's no need to
put it into cache only mode and you don't need to worry about managing
things (this should be the normal case for system controllers).  If you
can't interact with the device without powering it up then you still
have to worry about doing that regardless of what regmap is doing, if
anything I'd guess the warnings from regmap might be easier to debug
than the hardware misbehaviour.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

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