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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c08622be-a054-5204-1875-b54f77626bff@machnikowski.net>
Date:   Wed, 11 Jan 2023 16:54:13 +0100
From:   Maciek Machnikowski <maciek@...hnikowski.net>
To:     "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>,
        Jiri Pirko <jiri@...nulli.us>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        '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 1/11/2023 4:30 PM, Kubalewski, Arkadiusz wrote:
>> From: Maciek Machnikowski <maciek@...hnikowski.net>
>> Sent: Wednesday, January 11, 2023 3:40 PM
>> To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@...el.com>; Jiri Pirko
>> <jiri@...nulli.us>
>>
>>
>> On 1/11/2023 3:17 PM, Kubalewski, Arkadiusz wrote:
>>>> From: Maciek Machnikowski <maciek@...hnikowski.net>
>>>> Sent: Tuesday, January 10, 2023 3:59 PM
>>>> To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@...el.com>; Jiri Pirko
>>>> <jiri@...nulli.us>
>>>>
>>>> On 1/10/2023 11:54 AM, Kubalewski, Arkadiusz wrote:
>>>>>> From: Jiri Pirko <jiri@...nulli.us>
>>>>>> Sent: Monday, January 9, 2023 5:30 PM
>>>>>>>
>>>>>>> Hi guys,
>>>>>>>
>>>>>>> We have been trying to figure out feasibility of new approach
>> proposed
>>>> on
>>>>>> our
>>>>>>> latest meeting - to have a single object which encapsulates multiple
>>>>>> DPLLs.
>>>>>>>
>>>>>>> Please consider following example:
>>>>>>>
>>>>>>> Shared common inputs:
>>>>>>> i0 - GPS  / external
>>>>>>> i1 - SMA1 / external
>>>>>>> i2 - SMA2 / external
>>>>>>> i3 - MUX0 / clk recovered from PHY0.X driven by MAC0
>>>>>>> i4 - MUX1 / clk recovered from PHY1.X driven by MAC1
>>>>>>>
>>>>>>> +---------------------------------------------------------+
>>>>>>> | Channel A / FW0             +---+                       |
>>>>>>> |                         i0--|   |                       |
>>>>>>> |         +---+               |   |                       |
>>>>>>> | PHY0.0--|   |           i1--| D |                       |
>>>>>>> |         |   |               | P |                       |
>>>>>>> | PHY0.1--| M |           i2--| L |   +---+   +--------+  |
>>>>>>> |         | U |               | L |---|   |---| PHY0.0 |--|
>>>>>>> | PHY0.2--| X |-+---------i3--| 0 |   |   |   +--------+  |
>>>>>>> |         | 0 | |+------+     |   |---| M |---| PHY0.1 |--|
>>>>>>> | ...   --|   | || MUX1 |-i4--|   |   | A |   +--------+  |
>>>>>>> |         |   | |+------+     +---+   | C |---| PHY0.2 |--|
>>>>>>> | PHY0.7--|   | |         i0--|   |   | 0 |   +--------+  |
>>>>>>> |         +---+ |             |   |---|   |---| ...    |--|
>>>>>>> |               |         i1--| D |   |   |   +--------+  |
>>>>>>> |               |             | P |---|   |---| PHY0.7 |--|
>>>>>>> |               |         i2--| L |   +---+   +--------+  |
>>>>>>> |               |             | L |                       |
>>>>>>> |               \---------i3--| 1 |                       |
>>>>>>> |                +------+     |   |                       |
>>>>>>> |                | MUX1 |-i4--|   |                       |
>>>>>>> |                +------+     +---+                       |
>>>>>>> +---------------------------------------------------------+
>>>>>>> | Channel B / FW1             +---+                       |
>>>>>>> |                         i0--|   |                       |
>>>>>>> |                             |   |                       |
>>>>>>> |                         i1--| D |                       |
>>>>>>> |         +---+               | P |                       |
>>>>>>> | PHY1.0--|   |           i2--| L |   +---+   +--------+  |
>>>>>>> |         |   |  +------+     | L |---|   |---| PHY1.0 |--|
>>>>>>> | PHY1.1--| M |  | MUX0 |-i3--| 0 |   |   |   +--------+  |
>>>>>>> |         | U |  +------+     |   |---| M |---| PHY1.1 |--|
>>>>>>> | PHY1.2--| X |-+---------i4--|   |   | A |   +--------+  |
>>>>>>> |         | 1 | |             +---+   | C |---| PHY1.2 |--|
>>>>>>> | ...   --|   | |         i0--|   |   | 1 |   +--------+  |
>>>>>>> |         |   | |             |   |---|   |---| ...    |--|
>>>>>>> | PHY1.7--|   | |         i1--| D |   |   |   +--------+  |
>>>>>>> |         +---+ |             | P |---|   |---| PHY1.7 |--|
>>>>>>> |               |         i2--| L |   +---+   +--------+  |
>>>>>>> |               |+------+     | L |                       |
>>>>>>> |               || MUX0 |-i3--| 1 |                       |
>>>>>>> |               |+------+     |   |                       |
>>>>>>> |               \---------i4--|   |                       |
>>>>>>> |                             +---+                       |
>>>>>>> +---------------------------------------------------------+
>>>>>>
>>>>>> What is "a channel" here? Are these 2 channels part of the same
>> physival
>>>>>> chip? Could you add the synchronizer chip/device entities to your
>>>> drawing?
>>>>>>
>>>>>
>>>>> No.
>>>>> A "Synchronization Channel" on a switch would allow to separate groups
>>>>> of physical ports. Each channel/group has own "Synchronizer Chip",
>> which
>>>> is
>>>>> used to drive PHY clocks of that group.
>>>>>
>>>>> "Synchronizer chip" would be the 2 DPLLs on old draw, something like
>>>> this:
>>>>> +--------------------------------------------------------------+
>>>>> | Channel A / FW0        +-------------+   +---+   +--------+  |
>>>>> |                    i0--|Synchronizer0|---|   |---| PHY0.0 |--|
>>>>> |         +---+          |             |   |   |   +--------+  |
>>>>> | PHY0.0--|   |      i1--|             |---| M |---| PHY0.1 |--|
>>>>> |         |   |          | +-----+     |   | A |   +--------+  |
>>>>> | PHY0.1--| M |      i2--| |DPLL0|     |   | C |---| PHY0.2 |--|
>>>>> |         | U |          | +-----+     |   | 0 |   +--------+  |
>>>>> | PHY0.2--| X |--+---i3--| +-----+     |---|   |---| ...    |--|
>>>>> |         | 0 |  |       | |DPLL1|     |   |   |   +--------+  |
>>>>> | ...   --|   |  | /-i4--| +-----+     |---|   |---| PHY0.7 |--|
>>>>> |         |   |  | |     +-------------+   +---+   +--------+  |
>>>>> | PHY0.7--|   |  | |                                           |
>>>>> |         +---+  | |                                           |
>>>>> +----------------|-|-------------------------------------------+
>>>>> | Channel B / FW1| |     +-------------+   +---+   +--------+  |
>>>>> |                | | i0--|Synchronizer1|---|   |---| PHY1.0 |--|
>>>>> |         +---+  | |     |             |   |   |   +--------+  |
>>>>> | PHY1.0--|   |  | | i1--|             |---| M |---| PHY1.1 |--|
>>>>> |         |   |  | |     | +-----+     |   | A |   +--------+  |
>>>>> | PHY1.1--| M |  | | i2--| |DPLL0|     |   | C |---| PHY1.2 |--|
>>>>> |         | U |  | |     | +-----+     |   | 1 |   +--------+  |
>>>>> | PHY1.2--| X |  \-|-i3--| +-----+     |---|   |---| ...    |--|
>>>>> |         | 1 |    |     | |DPLL1|     |   |   |   +--------+  |
>>>>> | ...   --|   |----+-i4--| +-----+     |---|   |---| PHY1.7 |--|
>>>>> |         |   |          +-------------+   +---+   +--------+  |
>>>>> | PHY1.7--|   |                                                |
>>>>> |         +---+                                                |
>>>>> +--------------------------------------------------------------+
>>>>> Also, please keep in mind that is an example, there could be easily 4
>>>>> (or more) channels wired similarly.
>>>>>
>>>>
>>>>
>>>> Hi,
>>>>
>>>> This model tries to put too much into the synchronizer subsystem. The
>>>> synchronizer device should only model inputs, DPLLs and outputs.
>>>>
>>>> The PHY lane to Synchronizer input muxing should be done in the
>>>> PHY/netdev subsystem. That's why I wanted to start with the full model
>>>> to specifically address this topic.
>>>>
>>>> The netdev should have an assigned list of Synchronizer inputs that it
>>>> can recover its SyncE clocks into. It can be done by having a connection
>>>> between the synchronizer input object(s) and the netdev, just like the
>>>> netdev is connected to PHC clocks in the PHC subsystem. This is the
>>>> model I initially presented about a year ago for solving this specific
>>>> issue.
>>>>
>>>> Analogically, the netdev will be connected to a given output, however
>>>> changing anything in the physical clock configuration sounds dangerous.
>>>>
>>>> Does that sound reasonable?
>>>>
>>>> Regards
>>>> Maciek
>>>
>>> It sounds reasonable to some point.
>>> You have mentioned list of Synchronizer inputs. If there is a list of
>> inputs
>>> it means it was created somewhere. I assume dpll subsystem? If so you
>> would
>>> like to export that list out of dpll subsystem, thus other entities would
>> need
>>> to find such list, then find particular source and somehow register with
>> it.
>>> All of this was proposed as part of netdev, I don't see any benefit in
>> having
>>> this parts separated from dpll, as only dpll would use it, right?
>>> The same behavior is now provided by the MUX type pin, enclosed within
>> dpll
>>> subsystem.
>>>
>>> BR,
>>> Arkadiusz
>>
>> The synchronizer object should expose the list of inputs that represent
>> possible sources of a given chip. The list will be the same for all
>> DPLLs used by the same device, so it can be a single set of sources
>> linked to multiple DPLLs inside the package. A netdev can then point to
>> a given input of a synchronizer that it's connected to.
>> The phy lane->recovered clock (or directly a synchronizer input) muxing
>> should stay in the netdev subsystem, or in the PHY driver.
>>
>> The reason, and benefit, of such split is when you create a board with a
>> netdev X and a synchronizer Y that is not instantiated by the same
>> driver. In this scenario you'd get the ice driver to instantiate
>> connections and the DPLL vendor's driver for the synchronizer. In such
>> case the netdev driver will simply send a netlink message to the
>> input/source with a requested configuration, such as expected frequency,
>> and everything from this point can be handled by a completely different
>> driver creating clean and logical split.
>>
>> If we mix the phy lanes into the DPLL subsystem it'll get very
>> challenging to add PHY lanes to the existing synchronizer exposed by a
>> different driver.
> 
> This is possible right now:
> 1. obtain a dpll object:
> struct dpll_device *dpll_device_get_by_clock_id(u64 clock_id,
> 						enum dpll_type type, u8 idx);
> 2. register new pin with muxed type pin:
> int dpll_muxed_pin_register(struct dpll_device *dpll,
> 			    const char *parent_pin_description,
> 			    struct dpll_pin *pin,
> 			    struct dpll_pin_ops *ops, void *priv);
> 
> To find dpll driver must know clock_id, type of dpll and its index given
> when dpll was registered.
> To register a pin, parent_pin_description of MUX type pin given on registering
> it with dpll device.

That would mean you need to repeat this process for all the DPLLs that
are co-packaged in a single synchronizer. Some chips have up to 8 DPLLs,
so you'd need to register number of phy lanes x number of DPLL times,
say 8x8 = 64 times - that's simply too messy in the long term.

>> Exporting and link between the synchronizer and the netdev is still a
>> must no matter which way we go. And IMO it's best to link netdev to
>> synchronizer sources, as that's the most natural way.
>>
> 
> The link is now just information for userspace Linux network interface index
> in DPLLA_PIN_NETIFINDEX attribute.

That's not the right way. We need to know:
- which DPLL pins/sources are driven by which netdev (to be able to
  identify a source of frequency that is currently driving a given DPLL)
- which DPLL generates a frequency for a given netdev (to know which
  DPLL to check for a specific netdev)

So we need to have 2 connections.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ