All of lore.kernel.org
 help / color / mirror / Atom feed
* MDIO Debug Interface
@ 2020-07-09 20:47 Tobias Waldekranz
  2020-07-09 22:18 ` Vladimir Oltean
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Tobias Waldekranz @ 2020-07-09 20:47 UTC (permalink / raw)
  To: netdev; +Cc: andrew, f.fainelli, hkallweit1

Hi netdev,

TL;DR: Is something like https://github.com/wkz/mdio-tools a good
idea?

The kernel does not, as far as I know, have a low-level debug
interface to MDIO devices. I.e. something equivalent to i2c-dev or
spi-dev for example. The closest thing I've found are the
SIOCG/SMIIREG ioctls, but they have several drawbacks:

1. "Write register" is not always exactly that. The kernel will try to
   be extra helpful and do things behind the scenes if it detects a
   write to the reset bit of a PHY for example.

2. Only one op per syscall. This means that is impossible to implement
   many operations in a safe manner. Something as simple as a
   read/mask/write cycle can race against an in-kernel driver.

3. Addressing is awkward since you don't address the MDIO bus
   directly, rather "the MDIO bus to which this netdev's PHY is
   connected". This makes it hard to talk to devices on buses to which
   no PHYs are connected, the typical case being Ethernet switches.

The kernel side of mdio-tools, mdio-netlink, tries to address these
problems by adding a GENL interface with which a user can interact
with an MDIO bus directly.

The user sends a program that the mdio-netlink module executes,
possibly emitting data back to the user. I.e. it implements a very
simple VM. Read/mask/write operations could be implemented by
dedicated commands, but when you start looking at more advanced things
like reading out the VLAN database of a switch you need to state and
branching.

FAQ:

- A VM just for MDIO, that seems ridiculous, no?

  It does. But if you want to support the complex kinds of operations
  that I'm looking for, without the kernel module having to be aware
  of every kind of MDIO device in the world, I haven't found an easier
  way.

- Why not use BPF?

  That could absolutely be one way forward, but the GENL approach was
  easy to build out-of-tree to prove the idea. Its not obvious how it
  would work though as BPF programs typically run async on some event
  (probe hit, packet received etc.) whereas this is a single execution
  on behalf of a user. So to what would the program be attached? The
  output path is also not straight forward, but it could be done with
  perf events i suppose.

My question is thus; do you think mdio-netlink, or something like it,
is a candidate for mainline?

Thank you

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 20:47 MDIO Debug Interface Tobias Waldekranz
@ 2020-07-09 22:18 ` Vladimir Oltean
  2020-07-09 22:48   ` Andrew Lunn
  2020-07-10  7:51   ` Tobias Waldekranz
  2020-07-09 22:36 ` Florian Fainelli
  2020-07-09 22:39 ` Andrew Lunn
  2 siblings, 2 replies; 15+ messages in thread
From: Vladimir Oltean @ 2020-07-09 22:18 UTC (permalink / raw)
  To: Tobias Waldekranz; +Cc: netdev, andrew, f.fainelli, hkallweit1

Hi Tobias,

On Thu, Jul 09, 2020 at 10:47:54PM +0200, Tobias Waldekranz wrote:
> Hi netdev,
> 
> TL;DR: Is something like https://github.com/wkz/mdio-tools a good
> idea?
> 
> The kernel does not, as far as I know, have a low-level debug
> interface to MDIO devices. I.e. something equivalent to i2c-dev or
> spi-dev for example. The closest thing I've found are the
> SIOCG/SMIIREG ioctls, but they have several drawbacks:
> 
> 1. "Write register" is not always exactly that. The kernel will try to
>    be extra helpful and do things behind the scenes if it detects a
>    write to the reset bit of a PHY for example.
> 
> 2. Only one op per syscall. This means that is impossible to implement
>    many operations in a safe manner. Something as simple as a
>    read/mask/write cycle can race against an in-kernel driver.
> 
> 3. Addressing is awkward since you don't address the MDIO bus
>    directly, rather "the MDIO bus to which this netdev's PHY is
>    connected". This makes it hard to talk to devices on buses to which
>    no PHYs are connected, the typical case being Ethernet switches.
> 
> The kernel side of mdio-tools, mdio-netlink, tries to address these
> problems by adding a GENL interface with which a user can interact
> with an MDIO bus directly.
> 
> The user sends a program that the mdio-netlink module executes,
> possibly emitting data back to the user. I.e. it implements a very
> simple VM. Read/mask/write operations could be implemented by
> dedicated commands, but when you start looking at more advanced things
> like reading out the VLAN database of a switch you need to state and
> branching.
> 
> FAQ:
> 
> - A VM just for MDIO, that seems ridiculous, no?
> 
>   It does. But if you want to support the complex kinds of operations
>   that I'm looking for, without the kernel module having to be aware
>   of every kind of MDIO device in the world, I haven't found an easier
>   way.
> 
> - Why not use BPF?
> 
>   That could absolutely be one way forward, but the GENL approach was
>   easy to build out-of-tree to prove the idea. Its not obvious how it
>   would work though as BPF programs typically run async on some event
>   (probe hit, packet received etc.) whereas this is a single execution
>   on behalf of a user. So to what would the program be attached? The
>   output path is also not straight forward, but it could be done with
>   perf events i suppose.
> 
> My question is thus; do you think mdio-netlink, or something like it,
> is a candidate for mainline?
> 
> Thank you

I will let the PHY library maintainers comment about implementation
design choices made by mdio-netlink. However, I want to add a big "+1"
from my side for identifying the correct issues in the existing PHY
ioctls and doing something about it. I think the mainline kernel needs
this.
Please be aware that, if your mdio-netlink module, or something
equivalent to it, lands in mainline, QEMU/KVM is going to be one of its
users (for virtualizing an MDIO bus). So this is going to be more than
just for debugging.
And, while we're at it: context switches from a VM to a host are
expensive. And the PHY library polls around 5 MDIO registers per PHY
every second. It would be nice if your mdio-netlink module had some sort
of concept of "poll offload": just do the polling in the kernel side and
notify the user space only of a change.

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 20:47 MDIO Debug Interface Tobias Waldekranz
  2020-07-09 22:18 ` Vladimir Oltean
@ 2020-07-09 22:36 ` Florian Fainelli
  2020-07-10  8:29   ` Tobias Waldekranz
  2020-07-09 22:39 ` Andrew Lunn
  2 siblings, 1 reply; 15+ messages in thread
From: Florian Fainelli @ 2020-07-09 22:36 UTC (permalink / raw)
  To: Tobias Waldekranz, netdev; +Cc: andrew, hkallweit1, Vladimir Oltean



On 7/9/2020 1:47 PM, Tobias Waldekranz wrote:
> Hi netdev,
> 
> TL;DR: Is something like https://github.com/wkz/mdio-tools a good
> idea?
> 
> The kernel does not, as far as I know, have a low-level debug
> interface to MDIO devices. I.e. something equivalent to i2c-dev or
> spi-dev for example. The closest thing I've found are the
> SIOCG/SMIIREG ioctls, but they have several drawbacks:
> 
> 1. "Write register" is not always exactly that. The kernel will try to
>    be extra helpful and do things behind the scenes if it detects a
>    write to the reset bit of a PHY for example.
> 
> 2. Only one op per syscall. This means that is impossible to implement
>    many operations in a safe manner. Something as simple as a
>    read/mask/write cycle can race against an in-kernel driver.
> 
> 3. Addressing is awkward since you don't address the MDIO bus
>    directly, rather "the MDIO bus to which this netdev's PHY is
>    connected". This makes it hard to talk to devices on buses to which
>    no PHYs are connected, the typical case being Ethernet switches.
> 
> The kernel side of mdio-tools, mdio-netlink, tries to address these
> problems by adding a GENL interface with which a user can interact
> with an MDIO bus directly.
> 
> The user sends a program that the mdio-netlink module executes,
> possibly emitting data back to the user. I.e. it implements a very
> simple VM. Read/mask/write operations could be implemented by
> dedicated commands, but when you start looking at more advanced things
> like reading out the VLAN database of a switch you need to state and
> branching.
> 
> FAQ:
> 
> - A VM just for MDIO, that seems ridiculous, no?
> 
>   It does. But if you want to support the complex kinds of operations
>   that I'm looking for, without the kernel module having to be aware
>   of every kind of MDIO device in the world, I haven't found an easier
>   way.
> 
> - Why not use BPF?
> 
>   That could absolutely be one way forward, but the GENL approach was
>   easy to build out-of-tree to prove the idea. Its not obvious how it
>   would work though as BPF programs typically run async on some event
>   (probe hit, packet received etc.) whereas this is a single execution
>   on behalf of a user. So to what would the program be attached? The
>   output path is also not straight forward, but it could be done with
>   perf events i suppose.
> 
> My question is thus; do you think mdio-netlink, or something like it,
> is a candidate for mainline?

Certainly, the current interface clearly has deficiencies and since
mdio_device instances were introduced, we should have an interface to
debug those from user-space ala i2c-dev or spidev.

Can you post the kernel code for review? Would you entertain having mdio
as an user-space command being part of ethtool for instance (just to
ease the distribution)?
-- 
Florian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 20:47 MDIO Debug Interface Tobias Waldekranz
  2020-07-09 22:18 ` Vladimir Oltean
  2020-07-09 22:36 ` Florian Fainelli
@ 2020-07-09 22:39 ` Andrew Lunn
  2020-07-09 22:57   ` Vladimir Oltean
  2020-07-10  8:51   ` Tobias Waldekranz
  2 siblings, 2 replies; 15+ messages in thread
From: Andrew Lunn @ 2020-07-09 22:39 UTC (permalink / raw)
  To: Tobias Waldekranz; +Cc: netdev, f.fainelli, hkallweit1

On Thu, Jul 09, 2020 at 10:47:54PM +0200, Tobias Waldekranz wrote:
> Hi netdev,
> 
> TL;DR: Is something like https://github.com/wkz/mdio-tools a good
> idea?
> 
> The kernel does not, as far as I know, have a low-level debug
> interface to MDIO devices. I.e. something equivalent to i2c-dev or
> spi-dev for example.

Hi Tobias

These APIs exist to allow user space drivers. I don't know how much
that happens now a days, there seems to be a lot of kernel space
drivers for SPI and I2C, but it is still possible to write user space
drivers.

We have never allowed user space drivers for MDIO devices. As a
result, we have pretty good kernel support for PHYs and quite a few L2
switches, and the numbers keep increasing.

But the API you are suggesting sounds like it becomes an easy way for
vendors to run their SDKs in user space, with a small bit of glue code
to this new API. That is something we should avoid.

It is a difficult trade off. Such an API as you suggest does allow for
nice debug tools for driver developers. And i have no problems with
such a tool existing, being out of tree for any developer to use. But
i'm not too happy with it being in mainline because i suspect it will
get abused by vendors.

Something i'm want to look at soon is dumping more of the internal
state of the mv88e6xxx switches. The full ATU and VTU, TCAM etc. I
think devlink region could work for this. And i think the ethtool -d
command could be made a lot better now we have a netlink API. The old
API assumed a single address space. It would be nice to support
multiple address spaces.

The advantage of these APIs is that they cannot be abused by vendors
to write user space drivers. But we can still have reasonably powerful
debug tools built on top of them.

       Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 22:18 ` Vladimir Oltean
@ 2020-07-09 22:48   ` Andrew Lunn
  2020-07-10  7:51   ` Tobias Waldekranz
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Lunn @ 2020-07-09 22:48 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: Tobias Waldekranz, netdev, f.fainelli, hkallweit1

> And, while we're at it: context switches from a VM to a host are
> expensive. And the PHY library polls around 5 MDIO registers per PHY
> every second.

Just wire up the interrupt. That stops all polling.

It would however be good to have details of what QEMU wants.

	Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 22:39 ` Andrew Lunn
@ 2020-07-09 22:57   ` Vladimir Oltean
  2020-07-09 23:20     ` Andrew Lunn
  2020-07-10  8:51   ` Tobias Waldekranz
  1 sibling, 1 reply; 15+ messages in thread
From: Vladimir Oltean @ 2020-07-09 22:57 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Tobias Waldekranz, netdev, f.fainelli, hkallweit1

Hi Andrew,

On Fri, Jul 10, 2020 at 12:39:36AM +0200, Andrew Lunn wrote:
> On Thu, Jul 09, 2020 at 10:47:54PM +0200, Tobias Waldekranz wrote:
> > Hi netdev,
> > 
> > TL;DR: Is something like https://github.com/wkz/mdio-tools a good
> > idea?
> > 
> > The kernel does not, as far as I know, have a low-level debug
> > interface to MDIO devices. I.e. something equivalent to i2c-dev or
> > spi-dev for example.
> 
> Hi Tobias
> 
> These APIs exist to allow user space drivers. I don't know how much
> that happens now a days, there seems to be a lot of kernel space
> drivers for SPI and I2C, but it is still possible to write user space
> drivers.
> 
> We have never allowed user space drivers for MDIO devices. As a
> result, we have pretty good kernel support for PHYs and quite a few L2
> switches, and the numbers keep increasing.
> 
> But the API you are suggesting sounds like it becomes an easy way for
> vendors to run their SDKs in user space, with a small bit of glue code
> to this new API. That is something we should avoid.
> 
> It is a difficult trade off. Such an API as you suggest does allow for
> nice debug tools for driver developers. And i have no problems with
> such a tool existing, being out of tree for any developer to use. But
> i'm not too happy with it being in mainline because i suspect it will
> get abused by vendors.
> 
> Something i'm want to look at soon is dumping more of the internal
> state of the mv88e6xxx switches. The full ATU and VTU, TCAM etc. I
> think devlink region could work for this. And i think the ethtool -d
> command could be made a lot better now we have a netlink API. The old
> API assumed a single address space. It would be nice to support
> multiple address spaces.
> 
> The advantage of these APIs is that they cannot be abused by vendors
> to write user space drivers. But we can still have reasonably powerful
> debug tools built on top of them.
> 
>        Andrew

Fear not, the lack of a mainline UAPI for MDIO access will not prevent
any vendor from adding a sysfs mdio_read and mdio_write, if they need it
for their user space SDK :)

The reverse also seems true: if there are things that only the kernel
can do, then there should be a kernel driver for that respective
MDIO/SPI/I2C device, regardless of whether there is also a raw UAPI
available. It is not unheard of for a user space solution to finally get
converted to a kernel implementation.

Virtualization is a reasonable use case in my opinion and it would need
something like this, for the guest kernel to have access to its PHY.

Whereas things like "devlink region" for the ATU, VTU, etc could be kept
more high-level than the interface Tobias is proposing. There is no
clash between the 2.

-Vladimir

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 22:57   ` Vladimir Oltean
@ 2020-07-09 23:20     ` Andrew Lunn
  2020-07-09 23:33       ` Vladimir Oltean
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Lunn @ 2020-07-09 23:20 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: Tobias Waldekranz, netdev, f.fainelli, hkallweit1

> Fear not, the lack of a mainline UAPI for MDIO access will not prevent
> any vendor from adding a sysfs mdio_read and mdio_write, if they need it
> for their user space SDK :)

They do. But it means you have to use their kernel, or at least their
kernel module. That will put off some people. But if they can claim it
works on any kernel after v5.9, it makes the marketing much easier.

Like i said, it is a trade off.

> Virtualization is a reasonable use case in my opinion and it would
> need something like this, for the guest kernel to have access to its
> PHY.

And i would like a better understanding of this use case. It seems odd
you are putting the driver for a PHY in the VM. Is the MAC driver also
in the VM? As you said, VM context switches are expensive, so it would
seem to make more sense to let the host drive the hardware, which it
can do without all these context switches, and export a much simpler
and efficient API to the VM.

    Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 23:20     ` Andrew Lunn
@ 2020-07-09 23:33       ` Vladimir Oltean
  2020-07-10  9:30         ` Tobias Waldekranz
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Oltean @ 2020-07-09 23:33 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Tobias Waldekranz, netdev, f.fainelli, hkallweit1

On Fri, Jul 10, 2020 at 01:20:35AM +0200, Andrew Lunn wrote:
> > Virtualization is a reasonable use case in my opinion and it would
> > need something like this, for the guest kernel to have access to its
> > PHY.
> 
> And i would like a better understanding of this use case. It seems odd
> you are putting the driver for a PHY in the VM. Is the MAC driver also
> in the VM? As you said, VM context switches are expensive, so it would
> seem to make more sense to let the host drive the hardware, which it
> can do without all these context switches, and export a much simpler
> and efficient API to the VM.
> 
>     Andrew

The MAC driver is also in the VM, yes, and the VM can be given
pass-through access to the MAC register map. But the PHY is on a shared
bus. It is not desirable to give the VM access to the entire MDIO
controller's register map, for a number of reasons which I'm sure I
don't need to enumerate. So QEMU should be instructed which PHY should
each network interface use and on which bus, and it should fix-up the
device tree of the guest with a phy-handle to a PHY on a
para-virtualized MDIO bus, guest-side driver of which is going to be
accessing the real MDIO bus through this UAPI which we're talking about.
Then the host-side MDIO driver can ensure serialization of MDIO
accesses, etc etc.

It's been a while since I looked at this, so I may be lacking some of
the technical details, and brushing them up is definitely not something
for today.

-Vladimir

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 22:18 ` Vladimir Oltean
  2020-07-09 22:48   ` Andrew Lunn
@ 2020-07-10  7:51   ` Tobias Waldekranz
  1 sibling, 0 replies; 15+ messages in thread
From: Tobias Waldekranz @ 2020-07-10  7:51 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: netdev, andrew, f.fainelli, hkallweit1

On Fri Jul 10, 2020 at 3:18 AM CEST, Vladimir Oltean wrote:
> I will let the PHY library maintainers comment about implementation
> design choices made by mdio-netlink. However, I want to add a big "+1"
> from my side for identifying the correct issues in the existing PHY
> ioctls and doing something about it. I think the mainline kernel needs
> this.
> Please be aware that, if your mdio-netlink module, or something
> equivalent to it, lands in mainline, QEMU/KVM is going to be one of its
> users (for virtualizing an MDIO bus). So this is going to be more than
> just for debugging.
> And, while we're at it: context switches from a VM to a host are
> expensive. And the PHY library polls around 5 MDIO registers per PHY
> every second. It would be nice if your mdio-netlink module had some sort
> of concept of "poll offload": just do the polling in the kernel side and
> notify the user space only of a change.

The current flow is:

1. User: Send program to kernel.
2. Kernel: Verify program.
3. Kernel: Lock bus.
4. Kernel: Execute program.
5. Kernel: Unlock bus.
6. User: Read back status, including the output buffer.

(3, 5) is what allows for doing complex operations in a race free
manner. (4) is capped with a timeout to make sure that userspace can't
monopolize the bus. A "poll offload" would have to yield (i.e. unlock)
the bus in between poll cycles. Certainly doable, but it complicates
the model a bit.

> Thanks,
> -Vladimir


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 22:36 ` Florian Fainelli
@ 2020-07-10  8:29   ` Tobias Waldekranz
  0 siblings, 0 replies; 15+ messages in thread
From: Tobias Waldekranz @ 2020-07-10  8:29 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: andrew, hkallweit1, Vladimir Oltean

On Thu Jul 9, 2020 at 5:36 PM CEST, Florian Fainelli wrote:
> Certainly, the current interface clearly has deficiencies and since
> mdio_device instances were introduced, we should have an interface to
> debug those from user-space ala i2c-dev or spidev.
>
> Can you post the kernel code for review? Would you entertain having mdio

Certainly. I just linked to the repo to show how the userspace part
would look in combination with the netlink interface, and to see if I
was in ice-cube-in-hell territory or slightly better :)

> as an user-space command being part of ethtool for instance (just to
> ease the distribution)?

If by "ethtool" you mean the project, then yes. But I think it should
be a separate binary as ethtool is very interface centric.

We might not want to restrict ourselves to a single tool either. A
binary that can do basic register read/write seems like a good fit for
shipping alongside ethtool. But a tool to read/write the LinkCrypt
registers of a Marvell PHY, as an example, might be too specific and
better managed in a separate repo.

> --
> Florian


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 22:39 ` Andrew Lunn
  2020-07-09 22:57   ` Vladimir Oltean
@ 2020-07-10  8:51   ` Tobias Waldekranz
  1 sibling, 0 replies; 15+ messages in thread
From: Tobias Waldekranz @ 2020-07-10  8:51 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, f.fainelli, hkallweit1

On Fri Jul 10, 2020 at 2:39 AM CEST, Andrew Lunn wrote:
> On Thu, Jul 09, 2020 at 10:47:54PM +0200, Tobias Waldekranz wrote:
> > Hi netdev,
> > 
> > TL;DR: Is something like https://github.com/wkz/mdio-tools a good
> > idea?
> > 
> > The kernel does not, as far as I know, have a low-level debug
> > interface to MDIO devices. I.e. something equivalent to i2c-dev or
> > spi-dev for example.
>
> Hi Tobias
>
> These APIs exist to allow user space drivers. I don't know how much
> that happens now a days, there seems to be a lot of kernel space
> drivers for SPI and I2C, but it is still possible to write user space
> drivers.
>
> We have never allowed user space drivers for MDIO devices. As a
> result, we have pretty good kernel support for PHYs and quite a few L2
> switches, and the numbers keep increasing.

I'd be hesitant to claim any causality between those two statements
though. The way I see it, userspace drivers make sense for
"leaf-devices" i.e. devices which can be used by applications
directly. PHYs are not leaf-devices as they're intimately tied to a
netdev. Switches are doable as leaf-devices, which is why we have
vendor SDKs, but as the plethora of switchdev-ready applications grows
any single vendor won't be able to keep up with the community.

It is not the stick but the carrots that will kill the vendor SDKs.

> But the API you are suggesting sounds like it becomes an easy way for
> vendors to run their SDKs in user space, with a small bit of glue code
> to this new API. That is something we should avoid.
>
> It is a difficult trade off. Such an API as you suggest does allow for
> nice debug tools for driver developers. And i have no problems with
> such a tool existing, being out of tree for any developer to use. But
> i'm not too happy with it being in mainline because i suspect it will
> get abused by vendors.

Five years ago I would have signed on to that. No vendor had even
heard of switchdev and you were laughed out of the room for suggesting
they take that route. These days, they'll typically show switchdev
support as a target on marketing slides etc. Their primary target is
still their own SDK (which makes sense since that's where most of
their customers are), but they see the writing on the wall.

> Something i'm want to look at soon is dumping more of the internal
> state of the mv88e6xxx switches. The full ATU and VTU, TCAM etc. I
> think devlink region could work for this. And i think the ethtool -d
> command could be made a lot better now we have a netlink API. The old
> API assumed a single address space. It would be nice to support
> multiple address spaces.

Yes! I really like this part of devlink as well. I see it as a great
way to add production safe ways of extracting debug information.

> The advantage of these APIs is that they cannot be abused by vendors
> to write user space drivers. But we can still have reasonably powerful
> debug tools built on top of them.

Agreed. The drawback is that they are really only geared towards
non-destructive debugging. Sometimes, especially during development,
that is not enough.

> Andrew


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-09 23:33       ` Vladimir Oltean
@ 2020-07-10  9:30         ` Tobias Waldekranz
  2020-07-10  9:45           ` Vladimir Oltean
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Waldekranz @ 2020-07-10  9:30 UTC (permalink / raw)
  To: Vladimir Oltean, Andrew Lunn
  Cc: Tobias Waldekranz, netdev, f.fainelli, hkallweit1

On Fri Jul 10, 2020 at 4:33 AM CEST, Vladimir Oltean wrote:
> On Fri, Jul 10, 2020 at 01:20:35AM +0200, Andrew Lunn wrote:
> > > Virtualization is a reasonable use case in my opinion and it would
> > > need something like this, for the guest kernel to have access to its
> > > PHY.
> > 
> > And i would like a better understanding of this use case. It seems odd
> > you are putting the driver for a PHY in the VM. Is the MAC driver also
> > in the VM? As you said, VM context switches are expensive, so it would
> > seem to make more sense to let the host drive the hardware, which it
> > can do without all these context switches, and export a much simpler
> > and efficient API to the VM.
> > 
> >     Andrew
>
> The MAC driver is also in the VM, yes, and the VM can be given
> pass-through access to the MAC register map. But the PHY is on a shared
> bus. It is not desirable to give the VM access to the entire MDIO
> controller's register map, for a number of reasons which I'm sure I
> don't need to enumerate. So QEMU should be instructed which PHY should
> each network interface use and on which bus, and it should fix-up the
> device tree of the guest with a phy-handle to a PHY on a
> para-virtualized MDIO bus, guest-side driver of which is going to be
> accessing the real MDIO bus through this UAPI which we're talking about.
> Then the host-side MDIO driver can ensure serialization of MDIO
> accesses, etc etc.
>
> It's been a while since I looked at this, so I may be lacking some of
> the technical details, and brushing them up is definitely not something
> for today.
>
> -Vladimir

Have you considered the case where the PHY is only a slice of a quad-
or octal PHY?

I know that on at least some of these chips, all slices have access to
global registers that can have destructive effects on neighboring
slices. That could make it very difficult to implement a secure
solution using this architecture.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-10  9:30         ` Tobias Waldekranz
@ 2020-07-10  9:45           ` Vladimir Oltean
  2020-07-10 13:35             ` Andrew Lunn
  0 siblings, 1 reply; 15+ messages in thread
From: Vladimir Oltean @ 2020-07-10  9:45 UTC (permalink / raw)
  To: Tobias Waldekranz; +Cc: Andrew Lunn, netdev, f.fainelli, hkallweit1

Hi Tobias,

On Fri, Jul 10, 2020 at 11:30:21AM +0200, Tobias Waldekranz wrote:
> On Fri Jul 10, 2020 at 4:33 AM CEST, Vladimir Oltean wrote:
> > On Fri, Jul 10, 2020 at 01:20:35AM +0200, Andrew Lunn wrote:
> > > > Virtualization is a reasonable use case in my opinion and it would
> > > > need something like this, for the guest kernel to have access to its
> > > > PHY.
> > > 
> > > And i would like a better understanding of this use case. It seems odd
> > > you are putting the driver for a PHY in the VM. Is the MAC driver also
> > > in the VM? As you said, VM context switches are expensive, so it would
> > > seem to make more sense to let the host drive the hardware, which it
> > > can do without all these context switches, and export a much simpler
> > > and efficient API to the VM.
> > > 
> > >     Andrew
> >
> > The MAC driver is also in the VM, yes, and the VM can be given
> > pass-through access to the MAC register map. But the PHY is on a shared
> > bus. It is not desirable to give the VM access to the entire MDIO
> > controller's register map, for a number of reasons which I'm sure I
> > don't need to enumerate. So QEMU should be instructed which PHY should
> > each network interface use and on which bus, and it should fix-up the
> > device tree of the guest with a phy-handle to a PHY on a
> > para-virtualized MDIO bus, guest-side driver of which is going to be
> > accessing the real MDIO bus through this UAPI which we're talking about.
> > Then the host-side MDIO driver can ensure serialization of MDIO
> > accesses, etc etc.
> >
> > It's been a while since I looked at this, so I may be lacking some of
> > the technical details, and brushing them up is definitely not something
> > for today.
> >
> > -Vladimir
> 
> Have you considered the case where the PHY is only a slice of a quad-
> or octal PHY?
> 
> I know that on at least some of these chips, all slices have access to
> global registers that can have destructive effects on neighboring
> slices. That could make it very difficult to implement a secure
> solution using this architecture.

You raise a good point. Some quad PHYs have a sane design which is
easier to virtualize and some have a less sane design.

In principle there is nothing in this para-virtualization design that
would preclude a quirky quad PHY from being accessed in a
virtualization-safe mode. The main use case for PHY access in a VM is
for detecting when the link went down. Worst case, the QEMU host-side
driver could lie about the PHY ID, and could only expose the clause 22
subset of registers that could make it compatible with genphy. I don't
think this changes the overall approach about how MDIO devices would be
virtualized with QEMU.

Thanks,
-Vladimir

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-10  9:45           ` Vladimir Oltean
@ 2020-07-10 13:35             ` Andrew Lunn
  2020-07-10 15:47               ` Ioana Ciornei
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Lunn @ 2020-07-10 13:35 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: Tobias Waldekranz, netdev, f.fainelli, hkallweit1

> In principle there is nothing in this para-virtualization design that
> would preclude a quirky quad PHY from being accessed in a
> virtualization-safe mode. The main use case for PHY access in a VM is
> for detecting when the link went down. Worst case, the QEMU host-side
> driver could lie about the PHY ID, and could only expose the clause 22
> subset of registers that could make it compatible with genphy. I don't
> think this changes the overall approach about how MDIO devices would be
> virtualized with QEMU.

A more generic solution might be to fully virtualize the PHY. Let the
host kernel drive the PHY, and QEMU can use /sys/bus/mdio_bus/devices,
and uevents sent to user space. Ioana already added support for a PHY
not bound to a MAC in phylink. You would need to add a UAPI for
start/stop, and maybe a couple more operations, and probably export a
bit more information.

This would then solve the quad PHY situation, and any other odd
setups. And all the VM would require is genphy, keeping it simple.

	Andrew


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: MDIO Debug Interface
  2020-07-10 13:35             ` Andrew Lunn
@ 2020-07-10 15:47               ` Ioana Ciornei
  0 siblings, 0 replies; 15+ messages in thread
From: Ioana Ciornei @ 2020-07-10 15:47 UTC (permalink / raw)
  To: Andrew Lunn, Vladimir Oltean
  Cc: Tobias Waldekranz, netdev, f.fainelli, hkallweit1

On 7/10/20 4:35 PM, Andrew Lunn wrote:
>> In principle there is nothing in this para-virtualization design that
>> would preclude a quirky quad PHY from being accessed in a
>> virtualization-safe mode. The main use case for PHY access in a VM is
>> for detecting when the link went down. Worst case, the QEMU host-side
>> driver could lie about the PHY ID, and could only expose the clause 22
>> subset of registers that could make it compatible with genphy. I don't
>> think this changes the overall approach about how MDIO devices would be
>> virtualized with QEMU.
> 
> A more generic solution might be to fully virtualize the PHY. Let the
> host kernel drive the PHY, and QEMU can use /sys/bus/mdio_bus/devices,
> and uevents sent to user space. Ioana already added support for a PHY
> not bound to a MAC in phylink. You would need to add a UAPI for
> start/stop, and maybe a couple more operations, and probably export a
> bit more information.


You would still need a struct device to bind to that PHY and I am not 
sure what device that might be since the userspace cannot provide one.

> 
> This would then solve the quad PHY situation, and any other odd
> setups. And all the VM would require is genphy, keeping it simple.
> 
> 	Andrew
> 
> 

How would the genphy driver work if there is no MDIO register map for it 
to access? This suggestion seems something in between a software PHY and 
hardware PHY. Also, it would work only on PHYs and not any other device 
accessible over an MDIO bus (so you couldn't assign a DSA switch to a VM).

Coming back to a point that you made earlier, even as we speak, with an 
upstream kernel you can still have an userspace application accessing 
devices on a MDIO bus. Since the MDIO controller is memory mapped you 
just need some devmem commands wrapped-up in a nice script, no need for 
a vendor patch over an upstream kernel for this.

Ioana

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-07-10 15:47 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 20:47 MDIO Debug Interface Tobias Waldekranz
2020-07-09 22:18 ` Vladimir Oltean
2020-07-09 22:48   ` Andrew Lunn
2020-07-10  7:51   ` Tobias Waldekranz
2020-07-09 22:36 ` Florian Fainelli
2020-07-10  8:29   ` Tobias Waldekranz
2020-07-09 22:39 ` Andrew Lunn
2020-07-09 22:57   ` Vladimir Oltean
2020-07-09 23:20     ` Andrew Lunn
2020-07-09 23:33       ` Vladimir Oltean
2020-07-10  9:30         ` Tobias Waldekranz
2020-07-10  9:45           ` Vladimir Oltean
2020-07-10 13:35             ` Andrew Lunn
2020-07-10 15:47               ` Ioana Ciornei
2020-07-10  8:51   ` Tobias Waldekranz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.