[PATCH net-next v7 10/14] net: lan966x: add PCIe FDMA support

[Daniel Machon]

> On Wed, Sep 23, 2026 at 10:45:38AM +0200, Daniel Machon wrote:
> 
> ...
> 
> > > [Severity: Medium]
> > > Is this path expected to work on a big-endian host?
> > >
> > > Until now the descriptor producer and the FDMA engine were both on the
> > > little-endian SoC, so the native-endian descriptor layout in
> > > drivers/net/ethernet/microchip/fdma/fdma_api.h was fine:
> > >
> > >   struct fdma_db {
> > >         u64 dataptr;
> > >         u64 status;
> > >   };
> > >
> > >   struct fdma_dcb {
> > >         u64 nextptr;
> > >         u64 info;
> > >         struct fdma_db db[FDMA_DB_MAX];
> > >   };
> > >
> > > With PCIe FDMA the descriptors live in host memory and are fetched by the
> > > endpoint, so the host and the engine can disagree on byte order. The
> > > translated addresses written here, and the status/info words written by
> > > __fdma_db_add()/__fdma_dcb_add(), all go out in host byte order, and the
> > > HW-written DONE/BLOCKL fields are read back the same way in
> > > lan966x_fdma_pci_get_next_dcb() and lan966x_fdma_pci_rx_check_frame().
> > >
> > > CONFIG_MCHP_LAN966X_PCI in drivers/misc/Kconfig only has
> > >
> > >   depends on PCI
> > >   depends on OF_OVERLAY
> > >
> > > so a big-endian host (e.g. powerpc) can select it. On such a host, would
> > > the engine see byte-swapped nextptr/dataptr and length fields, i.e. would
> > > RX/TX not work at all? Should the descriptor fields become explicit
> > > little-endian types with accessors at the hardware boundary, or should the
> > > Kconfig entry gain a little-endian dependency?
> >
> > Hmm. Never tested on PowerPC, not this driver nor the DT overlay mechanism it
> > relies on for PCI deployment, which has only been exercised on ARM and x86 so
> > far (AFAICT) [1].
> >
> > [1] https://lore.kernel.org/all/20240430183301.46568e35@bootlin.com/
> 
> I don't think that the baseline expectation is that drivers are tested
> on big-endian systems, but rather that reasonable effort is made to ensure
> that byte-order usage is correct.
> 
> In this case I suggest that will involve using __le64 instead of u64,
> coupled with calling le64_to_cpu() and cpu_to_le64() as appropriate.
> 
> Sparse can aid with getting this right.

Ack.

I am a little reluctant, when I cant really test it properly. Sashiko gives
another solution - adding a LE dependency to the Kconfig of the misc PCI driver,
maybe that'll do?

> 
> ...
> 
> > > [Severity: Medium]
> > > Is rx_bytes meant to count the frame as it arrived on the wire?
> > >
> > > By the time this runs, skb->len has already been reduced twice: the
> > > skb_trim() above drops ETH_FCS_LEN, and eth_type_trans() pulls the
> > > MAC header out of the linear region, so skb->len is short by at least
> > > ETH_HLEN. If lan966x_hw_offload() ends up untagging a VLAN header,
> > > that is another four bytes gone. So every packet delivered through
> > > lan966x_fdma_pci_rx_get_frame() undercounts rx_bytes by 14 bytes or
> > > more, which is visible to userspace via ip -s link.
> > >
> > > The frame length is available before any of that surgery happens --
> > > data_len from FDMA_DCB_STATUS_BLOCKL(db->status), or skb->len right
> > > after the skb_pull(skb, IFH_LEN_BYTES) -- so accounting could be done
> > > there instead.
> > >
> > > I realise this mirrors what the existing register/page path in
> > > lan966x_fdma.c does, so if the intent is to keep the two backends
> > > byte-for-byte consistent, please say so; otherwise it would be good
> > > not to copy the miscount into the new file.
> >
> > Not only lan966x, but sparx5 and lan969x does the exact same thing, increasing
> > rx_bytes after headers are pulled.  The undercount is real, but not visible to
> > userspace. Both implementations (platform and PCI) read hardware counters directly
> 
> I'm a little unsure, but if it's consistent then I guess that is ok.
> It's an old interface anyway.