linux-kernel - RE: [PATCH net-next 07/13] net: macb: move HW IP alignment value to macb

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:
 <SJ2PR12MB8739A1E03E116F9D6A312EB99EA62@SJ2PR12MB8739.namprd12.prod.outlook.com>
Date: Wed, 26 Mar 2025 05:01:50 +0000
From: "Katakam, Harini" <harini.katakam@....com>
To: Andrew Lunn <andrew@...n.ch>, Théo Lebrun
	<theo.lebrun@...tlin.com>
CC: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Rob Herring
	<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
	<conor+dt@...nel.org>, Nicolas Ferre <nicolas.ferre@...rochip.com>, Claudiu
 Beznea <claudiu.beznea@...on.dev>, Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
	Alexandre Ghiti <alex@...ti.fr>, Samuel Holland <samuel.holland@...ive.com>,
	Richard Cochran <richardcochran@...il.com>, Russell King
	<linux@...linux.org.uk>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
	Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>, Gregory CLEMENT
	<gregory.clement@...tlin.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>, "linux-mips@...r.kernel.org"
	<linux-mips@...r.kernel.org>, Thomas Petazzoni
	<thomas.petazzoni@...tlin.com>, Tawfik Bayouk <tawfik.bayouk@...ileye.com>
Subject: RE: [PATCH net-next 07/13] net: macb: move HW IP alignment value to
 macb_config

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Theo,

> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Tuesday, March 25, 2025 12:06 AM
> To: Théo Lebrun <theo.lebrun@...tlin.com>
> Cc: Andrew Lunn <andrew+netdev@...n.ch>; David S. Miller
> <davem@...emloft.net>; Eric Dumazet <edumazet@...gle.com>; Jakub Kicinski
> <kuba@...nel.org>; Paolo Abeni <pabeni@...hat.com>; Rob Herring
> <robh@...nel.org>; Krzysztof Kozlowski <krzk+dt@...nel.org>; Conor Dooley
> <conor+dt@...nel.org>; Nicolas Ferre <nicolas.ferre@...rochip.com>; Claudiu
> Beznea <claudiu.beznea@...on.dev>; Paul Walmsley
> <paul.walmsley@...ive.com>; Palmer Dabbelt <palmer@...belt.com>; Albert Ou
> <aou@...s.berkeley.edu>; Alexandre Ghiti <alex@...ti.fr>; Samuel Holland
> <samuel.holland@...ive.com>; Richard Cochran <richardcochran@...il.com>;
> Russell King <linux@...linux.org.uk>; Thomas Bogendoerfer
> <tsbogend@...ha.franken.de>; Vladimir Kondratiev
> <vladimir.kondratiev@...ileye.com>; Gregory CLEMENT
> <gregory.clement@...tlin.com>; netdev@...r.kernel.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> riscv@...ts.infradead.org; linux-mips@...r.kernel.org; Thomas Petazzoni
> <thomas.petazzoni@...tlin.com>; Tawfik Bayouk <tawfik.bayouk@...ileye.com>
> Subject: Re: [PATCH net-next 07/13] net: macb: move HW IP alignment value to
> macb_config
>
> On Mon, Mar 24, 2025 at 06:49:05PM +0100, Théo Lebrun wrote:
> > Hello Andrew,
> >
> > On Fri Mar 21, 2025 at 10:06 PM CET, Andrew Lunn wrote:
> > > On Fri, Mar 21, 2025 at 08:09:38PM +0100, Théo Lebrun wrote:
> > >> The controller does IP alignment (two bytes).
> > >
> > > I'm a bit confused here. Is this hard coded, baked into the silicon?
> > > It will always do IP alignment? It cannot be turned off?
> >
> > Yes, the alignment is baked inside the silicon.
> > I looked but haven't seen any register to configure the alignment.
> >
> > Sorry the commit message isn't clear, it needs improvements.
> >
> > >>  skb_reserve(skb, NET_IP_ALIGN);
> > >
> > > Why not just replace this with
> > >
> > >         skb_reserve(skb, 2);
> >
> > On arm64, NET_IP_ALIGN=0. I don't have HW to test, but the current
> > code is telling us that the silicon doesn't do alignment on those:
>
> This is part of the confusion. You say the hardware does alignment, and then say it
> does not....
>
> >    skb = netdev_alloc_skb(...);
> >    paddr = dma_map_single(..., skb->data, ...);
> >    macb_set_addr(..., paddr);
> >
> >    // arm   => NET_IP_ALIGN=2 => silicon does alignment
> >    // arm64 => NET_IP_ALIGN=0 => silicon doesn't do alignment
> >    skb_reserve(skb, NET_IP_ALIGN);
> >
> > The platform we introduce is the first one where the silicon alignment
> > (0 bytes) is different from the NET_IP_ALIGN value (MIPS, 2 bytes).
>
> This is starting to make it clearer. So the first statement that the controller does IP
> alignment (two bytes) is not the full story. I would start there, explain the full story,
> otherwise readers get the wrong idea.
>
> > >>     Compatible             |  DTS folders              |  hw_ip_align
> > >>    ------------------------|---------------------------|----------------
> > >>    cdns,at91sam9260-macb   | arch/arm/                 | 2
> > >>    cdns,macb               | arch/{arm,riscv}/         | NET_IP_ALIGN
> > >>    cdns,np4-macb           | NULL                      | NET_IP_ALIGN
> > >>    cdns,pc302-gem          | NULL                      | NET_IP_ALIGN
> > >>    cdns,gem                | arch/{arm,arm64}/         | NET_IP_ALIGN
> > >>    cdns,sam9x60-macb       | arch/arm/                 | 2
> > >>    atmel,sama5d2-gem       | arch/arm/                 | 2
> > >>    atmel,sama5d29-gem      | arch/arm/                 | 2
> > >>    atmel,sama5d3-gem       | arch/arm/                 | 2
> > >>    atmel,sama5d3-macb      | arch/arm/                 | 2
> > >>    atmel,sama5d4-gem       | arch/arm/                 | 2
> > >>    cdns,at91rm9200-emac    | arch/arm/                 | 2
> > >>    cdns,emac               | arch/arm/                 | 2
> > >>    cdns,zynqmp-gem         | *same as xlnx,zynqmp-gem* | 0
> > >>    cdns,zynq-gem           | *same as xlnx,zynq-gem*   | 2
> > >>    sifive,fu540-c000-gem   | arch/riscv/               | 2
> > >>    microchip,mpfs-macb     | arch/riscv/               | 2
> > >>    microchip,sama7g5-gem   | arch/arm/                 | 2
> > >>    microchip,sama7g5-emac  | arch/arm/                 | 2
> > >>    xlnx,zynqmp-gem         | arch/arm64/               | 0
> > >>    xlnx,zynq-gem           | arch/arm/                 | 2
> > >>    xlnx,versal-gem         | NULL                      | NET_IP_ALIGN

Thanks for the patch. xlnx,versal-gem is arm64 and NET_IP_ALIGN is 0.

AFAIK, IP alignment is controlled by the register field " receive buffer offset "
in the NW config register. The only exception is when " gem_pbuf_rsc " i.e.
receive coalescing is enabled in the RTL in the IP. In that case, the Cadenc
specification states that these bits are ignored.
So to summarize, if RSC is not enabled (see bit 26 of designcfg_debug6),
then the current implementation works for all architectures i.e. these two
statements are in sync:
config |= MACB_BF(RBOF, NET_IP_ALIGN);  /* Make eth data aligned */
skb_reserve(skb, NET_IP_ALIGN);

Hope this helps simplify the patch (and also fill up the table that Andrew suggested)

Regards,
Harini

>
> I'm not sure this table is useful. What might be more interesting is
>
>      Compatible             |  architecture |  hw_ip_align | value of NET_IP_ALIGN
>
> We can then see if there are cases when the 3rd and 4th column differ.
>
>       Andrew