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]

Date:   Thu, 23 Jun 2022 00:05:46 +0800
From:   Dong Aisheng <dongas86@...il.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Aisheng Dong <aisheng.dong@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "l.stach@...gutronix.de" <l.stach@...gutronix.de>,
        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 8:36 PM Mark Brown <broonie@...nel.org> wrote:
>
> On Wed, Jun 22, 2022 at 06:12:49PM +0800, Dong Aisheng wrote:
>
> > NOTE:  i didn't fix _regmap_write() as i.MX controls regmap write well in driver
> > with power enabled first, so don't have issues in reality.
>
> I can't tell what you think the problem is with _regmap_write()?
>

Because from what I see,  _regmap_write() seems still can write to HW
register even with cache_only mode set theoretically.

> > It can be fixed in a separate patch later if needed.
> > You may check if it's as your expected solution.
>
> > For syscon, I still have no idea how to fix it if I can't disable it.
> >
> > diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
> > index 2eaffd3224c9..da1702fd57cc 100644
> > --- a/drivers/base/regmap/regcache.c
> > +++ b/drivers/base/regmap/regcache.c
> > @@ -495,7 +495,7 @@ EXPORT_SYMBOL_GPL(regcache_drop_region);
> >  void regcache_cache_only(struct regmap *map, bool enable)
> >  {
> >         map->lock(map->lock_arg);
> > -       WARN_ON(map->cache_bypass && enable);
> > +//     WARN_ON(map->cache_bypass && enable);
> >         map->cache_only = enable;
> >         trace_regmap_cache_only(map, enable);
> >         map->unlock(map->lock_arg);
>
> What is the purpose of this change?  Why would the combination of cache
> only and bypass modes work be a good idea, and how should things behave
> in that case?
>

Because without this change, there will be a kernel dump caused by
WARN_ON when drivers call regcache_cache_only(map, true) after power
is off. There's no cache used in the imx blkctl driver. So map->cache_bypass
is default to true.

I did this by following your previous suggestion as follows:
"You don't actually need to have a cache to use cache only mode, if there
are no cached registers then you'll just get -EBUSY on any access to the
registers but that's hopefully fine since at the minute things will just
fall over anyway.  You shouldn't even need to flag registers as volatile
if there's no cache since it's not really relevant without a cache."

Am I misunderstanding your suggestions?

Regards
Aisheng