linux-kernel - Re: [PATCH v8 6/8] nvmem: core: Rework layouts to become platform devices

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: <20230807113321.0f0d46d8@xps-13>
Date:   Mon, 7 Aug 2023 11:33:21 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Robert Marko <robert.marko@...tura.hr>,
        Luka Perkov <luka.perkov@...tura.hr>,
        Michael Walle <michael@...le.cc>, linux-kernel@...r.kernel.org,
        Randy Dunlap <rdunlap@...radead.org>,
        Chen-Yu Tsai <wenst@...omium.org>,
        Daniel Golle <daniel@...rotopia.org>,
        Rafał Miłecki <rafal@...ecki.pl>
Subject: Re: [PATCH v8 6/8] nvmem: core: Rework layouts to become platform
 devices

Hi Greg,

gregkh@...uxfoundation.org wrote on Mon, 7 Aug 2023 11:05:48 +0200:

> On Mon, Aug 07, 2023 at 10:24:17AM +0200, Miquel Raynal wrote:
> > Current layout support was initially written without modules support in
> > mind. When the requirement for module support rose, the existing base
> > was improved to adopt modularization support, but kind of a design flaw
> > was introduced. With the existing implementation, when a storage device
> > registers into NVMEM, the core tries to hook a layout (if any) and
> > populates its cells immediately. This means, if the hardware description
> > expects a layout to be hooked up, but no driver was provided for that,
> > the storage medium will fail to probe and try later from
> > scratch. Technically, the layouts are more like a "plus" and, even we
> > consider that the hardware description shall be correct, we could still
> > probe the storage device (especially if it contains the rootfs).
> > 
> > One way to overcome this situation is to consider the layouts as
> > devices, and leverage the existing notifier mechanism. When a new NVMEM
> > device is registered, we can:
> > - populate its nvmem-layout child, if any
> > - try to modprobe the relevant driver, if relevant
> > - try to hook the NVMEM device with a layout in the notifier
> > And when a new layout is registered:
> > - try to hook all the existing NVMEM devices which are not yet hooked to
> >   a layout with the new layout
> > This way, there is no strong order to enforce, any NVMEM device creation
> > or NVMEM layout driver insertion will be observed as a new event which
> > may lead to the creation of additional cells, without disturbing the
> > probes with costly (and sometimes endless) deferrals.  
> 
> This is good, but why are you using a platform device here?  Is it a
> real platform device, or just a "fake" one you created?  If a fake one,
> please don't do that, use a real device, or a virtual device.  Platform
> devices should ONLY represent actual, real, platform devices (i.e. ones
> descibed by the firmware).

NVMEM layouts have their own firmware node (DT only in this case), so I
guess we are in the "real device" situation here? But there is no
specific hardware in the physical world, so I don't know in which class
they should preferably be. I thought platform devices would be a
helpful abstraction.

With this additional information, in case you advise for using virtual
devices, would you mind being more specific on what you expect? What
should be used instead of platform devices?

> Sorry but I couldn't answer this question by looking at this patch, the
> device creation path isn't exactly obvious :)

I would expect this to be handled by the of_platform_populate() call,
but perhaps you were looking for something else.

Thanks,
Miquèl