linux-kernel - Re: [PATCH v2 1/5] net: cadence: macb: Set upper 32bits of DMA ring buffer

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] [day] [month] [year] [list]

Message-Id: <DCPA2BR78XM8.HWKZZ8WQF3S8@bootlin.com>
Date: Wed, 10 Sep 2025 18:57:23 +0200
From: Théo Lebrun <theo.lebrun@...tlin.com>
To: "Nicolas Ferre" <nicolas.ferre@...rochip.com>, "Jakub Kicinski"
 <kuba@...nel.org>, "Stanimir Varbanov" <svarbanov@...e.de>
Cc: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
 <linux-rpi-kernel@...ts.infradead.org>, "Broadcom internal kernel review
 list" <bcm-kernel-feedback-list@...adcom.com>, "Andrew Lunn"
 <andrew+netdev@...n.ch>, "David S . Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, "Paolo Abeni" <pabeni@...hat.com>, "Rob
 Herring" <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>,
 "Conor Dooley" <conor+dt@...nel.org>, "Florian Fainelli"
 <florian.fainelli@...adcom.com>, "Andrea della Porta"
 <andrea.porta@...e.com>, "Claudiu Beznea" <claudiu.beznea@...on.dev>, "Phil
 Elwell" <phil@...pberrypi.com>, "Jonathan Bell" <jonathan@...pberrypi.com>,
 "Dave Stevenson" <dave.stevenson@...pberrypi.com>,
 <stable@...r.kernel.org>, "Andrew Lunn" <andrew@...n.ch>
Subject: Re: [PATCH v2 1/5] net: cadence: macb: Set upper 32bits of DMA ring
 buffer

Hello Nicolas, Jakub, Stanimir,

On Tue Aug 26, 2025 at 11:14 AM CEST, Nicolas Ferre wrote:
> On 26/08/2025 at 01:53, Jakub Kicinski wrote:
>> On Fri, 22 Aug 2025 12:34:36 +0300 Stanimir Varbanov wrote:
>>> In case of rx queue reset and 64bit capable hardware, set the upper
>>> 32bits of DMA ring buffer address.
>>>
>>> Cc: stable@...r.kernel.org # v4.6+
>>> Fixes: 9ba723b081a2 ("net: macb: remove BUG_ON() and reset the queue to handle RX errors")
>>> Credits-to: Phil Elwell <phil@...pberrypi.com>
>>> Credits-to: Jonathan Bell <jonathan@...pberrypi.com>
>>> Signed-off-by: Stanimir Varbanov <svarbanov@...e.de>
>>> Reviewed-by: Andrew Lunn <andrew@...n.ch>
>> 
>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>>> index ce95fad8cedd..36717e7e5811 100644
>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>> @@ -1634,7 +1634,11 @@ static int macb_rx(struct macb_queue *queue, struct napi_struct *napi,
>>>                macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
>>>
>>>                macb_init_rx_ring(queue);
>>> -             queue_writel(queue, RBQP, queue->rx_ring_dma);
>>> +             queue_writel(queue, RBQP, lower_32_bits(queue->rx_ring_dma));
>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>> +             if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>>> +                     macb_writel(bp, RBQPH, upper_32_bits(queue->rx_ring_dma));
>>> +#endif
>>>
>>>                macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
>>>
>> 
>> Looks like a subset of Théo Lebrun's work:
>> https://lore.kernel.org/all/20250820-macb-fixes-v4-0-23c399429164@bootlin.com/
>> let's wait for his patches to get merged instead?
>
> Yes, we can certainly wait. As RBOPH changes by Théo are key, they will 
> probably remove the need for this fix altogether: but I count on you 
> Stanimir to monitor that (as I don't have a 64 bit capable platform at 
> hand).

I when looking for where this patch came from.
Commit in the raspberrypi downstream kernel:
https://github.com/raspberrypi/linux/commit/e45c98decbb16e58a79c7ec6fbe4374320e814f1

It is somewhat unreadable; the only part that seems related is the:

> net: macb: Several patches for RP1
> 64-bit RX fix

 - Is there any MACB hardware (not GEM) that uses 64-bit DMA
   descriptors? What platforms? RPi maybe?

 - Assuming such a platform exists, the next question is why does
   macb_rx() need to reinit RBQPH/0x04D4. It reinits RBQP/0x0018
   because it is the buffer pointer and increments as buffers get used.

   To reinit RBQPH would be for the case of the increment overflowing
   into the upper 32-bits. Sounds like a reasonable fix (for a really
   rare bug) if that hardware actually exists.

   This wouldn't be needed on GEM because RBQPH is shared across queues.
   So of course RBQPH would not increment with the buffer pointer.

If this patch is needed (does HW exist?), then my series doesn't address
it. I can take the patch in a potential V6 if you want. V5 got posted
today [0].

[0]: https://lore.kernel.org/lkml/20250910-macb-fixes-v5-0-f413a3601ce4@bootlin.com/

Thanks,
Have a nice day,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com