netdev - Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API

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: <20221206184740.28cb7627@kernel.org>
Date:   Tue, 6 Dec 2022 18:47:40 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>,
        Vadim Fedorenko <vfedorenko@...ek.ru>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Paolo Abeni <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>
Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API

On Fri, 2 Dec 2022 17:12:06 +0100 Jiri Pirko wrote:
> >But this is only doable with assumption, that the board is internally capable
> >of such internal board level communication, which in case of separated
> >firmwares handling multiple dplls might not be the case, or it would require
> >to have some other sw component feel that gap.  
> 
> Yep, you have the knowledge of sharing inside the driver, so you should
> do it there. For multiple instances, use in-driver notifier for example.

No, complexity in the drivers is not a good idea. The core should cover
the complexity and let the drivers be simple.

> >For complex boards with multiple dplls/sync channels, multiple ports,
> >multiple firmware instances, it seems to be complicated to share a pin if
> >each driver would have own copy and should notify all the other about changes.
> >
> >To summarize, that is certainly true, shared pins idea complicates stuff
> >inside of dpll subsystem.
> >But at the same time it removes complexity from all the drivers which would use  
> 
> There are currently 3 drivers for dpll I know of. This in ptp_ocp and
> mlx5 there is no concept of sharing pins. You you are talking about a
> single driver.
> 
> What I'm trying to say is, looking at the code, the pin sharing,
> references and locking makes things uncomfortably complex. You are so
> far the only driver to need this, do it internally. If in the future
> other driver appears, this code would be eventually pushed into dpll
> core. No impact on UAPI from what I see. Please keep things as simple as
> possible.

But the pin is shared for one driver. Who cares if it's not shared in
another. The user space must be able to reason about the constraints.

You are suggesting drivers to magically flip state in core objects
because of some hidden dependencies?!

> >it and is easier for the userspace due to common identification of pins.  
> 
> By identification, you mean "description" right? I see no problem of 2
> instances have the same pin "description"/label.
>
> >This solution scales up without any additional complexity in the driver,
> >and without any need for internal per-board communication channels.
> >
> >Not sure if this is good or bad, but with current version, both approaches are
> >possible, so it pretty much depending on the driver to initialize dplls with
> >separated pin objects as you have suggested (and take its complexity into
> >driver) or just share them.
> >  
> >>
> >>3) I don't like the concept of muxed pins and hierarchies of pins. Why
> >>   does user care? If pin is muxed, the rest of the pins related to this
> >>   one should be in state disabled/disconnected. The user only cares
> >>   about to see which pins are related to each other. It can be easily
> >>   exposed by "muxid" like this:
> >>   pin 1
> >>   pin 2
> >>   pin 3 muxid 100
> >>   pin 4 muxid 100
> >>   pin 5 muxid 101
> >>   pin 6 muxid 101
> >>   In this example pins 3,4 and 5,6 are muxed, therefore the user knows
> >>   if he connects one, the other one gets disconnected (or will have to
> >>   disconnect the first one explicitly first).
> >
> >Currently DPLLA_PIN_PARENT_IDX is doing the same thing as you described, it
> >groups MUXed pins, the parent pin index here was most straightforward to me,  
> 
> There is a big difference if we model flat list of pins with a set of
> attributes for each, comparing to a tree of pins, some acting as leaf,
> node and root. Do we really need such complexicity? What value does it
> bring to the user to expose this?

The fact that you can't auto select from devices behind muxes.
The HW topology is of material importance to user space.
How many times does Arkadiusz have to explain this :|