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: <e4e84937-7fb6-f715-b33c-4d34a598f9ee@machnikowski.net>
Date:   Wed, 11 Jan 2023 15:40:23 +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 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.

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.

Thanks,
Maciek

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ