| 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
| ||
|
Message-ID: <DM6PR11MB4657E9C7F64FC9DF713696059BFC9@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Wed, 11 Jan 2023 15:30:37 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Maciek Machnikowski <maciek@...hnikowski.net>,
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
>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.
>
>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.
BR,
Arkadiusz
>Thanks,
>Maciek
Powered by blists - more mailing lists