Re: [PATCH net-next] stmmac: align RX buffers

[Marc Zyngier <maz@kernel.org>]

On Tue, 17 Aug 2021 01:01:57 +0100,
Matteo Croce <mcroce@linux.microsoft.com> wrote:
> 
> On Mon, 16 Aug 2021 08:12:08 -0700
> Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > On Thu, 12 Aug 2021 12:05:38 +0100 Marc Zyngier wrote:
> > > > A possible fix, which takes in account also the XDP headroom for
> > > > stmmac_rx_buf1_len() only could be (only compile tested, I don't
> > > > have the hardware now):  
> > > 
> > > However, this doesn't fix my issue. I still get all sort of
> > > corruption. Probably stmmac_rx_buf2_len() also need adjusting (it
> > > has a similar logic as its buf1 counterpart...)
> > > 
> > > Unless you can fix it very quickly, and given that we're towards the
> > > end of the cycle, I'd be more comfortable if we reverted this patch.
> > 
> > Any luck investigating this one? The rc6 announcement sounds like
> > there may not be that many more rc releases for 5.14.
> 
> Hi Jackub.
> 
> Unfortunately I have only a device with stmmac, and it works fine with
> the patch. It seems that not all hardware suffers from this issue.
> 
> Also, using NET_IP_ALIGN on RX is a common pattern, I think that any
> ethernet device is doing the same to align the IPv4 header.
> 
> Anyway, I asked for two tests on the affected device:
> 1. Change NET_IP_ALIGN with 8, to see if the DMA has problems in
>    receiving to a non word aligned address
> 2. load a nop XDP program (I provided one), to see if the problem is
>    already there when XDP is used

Catching up on email, as I was away earlier this week. I'm yet to test
these two things (hopefully by the end of the day), but I just gave
the patch below a go, just in case...

> 
> I doubt that changing also stmmac_rx_buf2_len would help,
> but it's worth a try, here is a patch:
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 7b8404a21544..73d1f0ec66ff 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -93,7 +93,7 @@ static int tc = TC_DEFAULT;
>  module_param(tc, int, 0644);
>  MODULE_PARM_DESC(tc, "DMA threshold control value");
>  
> -#define	DEFAULT_BUFSIZE	1536
> +#define	DEFAULT_BUFSIZE	1536 + XDP_PACKET_HEADROOM + NET_IP_ALIGN
>  static int buf_sz = DEFAULT_BUFSIZE;
>  module_param(buf_sz, int, 0644);
>  MODULE_PARM_DESC(buf_sz, "DMA buffer size");
> @@ -4508,12 +4508,12 @@ static unsigned int stmmac_rx_buf1_len(struct stmmac_priv *priv,
>  
>  	/* First descriptor, not last descriptor and not split header */
>  	if (status & rx_not_ls)
> -		return priv->dma_buf_sz;
> +		return priv->dma_buf_sz - stmmac_rx_offset(priv);
>  
>  	plen = stmmac_get_rx_frame_len(priv, p, coe);
>  
>  	/* First descriptor and last descriptor and not split header */
> -	return min_t(unsigned int, priv->dma_buf_sz, plen);
> +	return min_t(unsigned int, priv->dma_buf_sz - stmmac_rx_offset(priv), plen);
>  }
>  
>  static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv,
> @@ -4529,12 +4529,12 @@ static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv,
>  
>  	/* Not last descriptor */
>  	if (status & rx_not_ls)
> -		return priv->dma_buf_sz;
> +		return priv->dma_buf_sz - stmmac_rx_offset(priv);
>  
>  	plen = stmmac_get_rx_frame_len(priv, p, coe);
>  
>  	/* Last descriptor */
> -	return plen - len;
> +	return plen - len - stmmac_rx_offset(priv);
>  }
>  
>  static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,

This patch results in a cosmic explosion, as it tries to invalidate a
range of VAs that cannot possibly be allocated to the DMA. Not really
what was expected.

Thanks,

	M.

[   19.587499] Unable to handle kernel write to read-only memory at virtual address ffff000088f89000
[   19.596375] Mem abort info:
[   19.599165]   ESR = 0x9600014f
[   19.602215]   EC = 0x25: DABT (current EL), IL = 32 bits
[   19.607519]   SET = 0, FnV = 0
[   19.610568]   EA = 0, S1PTW = 0
[   19.613703]   FSC = 0x0f: level 3 permission fault
[   19.618487] Data abort info:
[   19.621362]   ISV = 0, ISS = 0x0000014f
[   19.625192]   CM = 1, WnR = 1
[   19.628154] swapper pgtable: 4k pages, 48-bit VAs, pgdp=00000000fa7b2000
[   19.634844] [ffff000088f89000] pgd=18000002771f7003, p4d=18000002771f7003, pud=1800000275ffd003, pmd=1800000275fb5003, pte=0060000108f89f87
[   19.647358] Internal error: Oops: 9600014f [#1] PREEMPT SMP
[   19.652920] Modules linked in: nls_ascii(E) nls_cp437(E) vfat(E) fat(E) tegra_drm(E) cec(E) drm_kms_helper(E) evdev(E) snd_hda_codec_hdmi(E) snd_hda_tegra(E) snd_hda_codec(E) aes_ce_blk(E) crypto_simd(E) snd_hda_core(E) cryptd(E) snd_hwdep(E) at24(E) aes_ce_cipher(E) snd_pcm(E) ghash_ce(E) gf128mul(E) snd_timer(E) sha2_ce(E) sha256_arm64(E) sha1_ce(E) snd(E) soundcore(E) tegra_bpmp_thermal(E) efi_pstore(E) tegra_xudc(E) udc_core(E) host1x(E) ina3221(E) fuse(E) drm(E) configfs(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) nvme(E) nvme_core(E) t10_pi(E) broadcom(E) bcm_phy_lib(E) ahci_tegra(E) libahci_platform(E) gpio_keys(E) libahci(E) sdhci_tegra(E) dwmac_dwc_qos_eth(E) stmmac_platform(E) libata(E) stmmac(E) sdhci_pltfm(E) pcs_xpcs(E) max77620_regulator(E) xhci_tegra(E) phylink(E) of_mdio(E) cqhci(E) scsi_mod(E) fixed_phy(E) sdhci(E) fwnode_mdio(E) libphy(E)
[   19.729311] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G S          E     5.14.0-rc6-dirty #4155
[   19.737734] Hardware name:  , BIOS 2021.04-rc2-00042-g2dddc1bb29 02/18/2021
[   19.744679] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
[   19.750673] pc : dcache_inval_poc+0x40/0x58
[   19.754852] lr : arch_sync_dma_for_cpu+0x2c/0x40
[   19.759460] sp : ffff800010003be0
[   19.762765] x29: ffff800010003be0 x28: ffff000085338000 x27: 0000000000000028
[   19.769889] x26: ffff000080a8fc00 x25: ffff000085338028 x24: 0000000000000000
[   19.777012] x23: 0000000000000002 x22: 0000000000000002 x21: 00000000ffffffbc
[   19.784135] x20: 0000000108f43000 x19: ffff000080223010 x18: 0000000000000000
[   19.791258] x17: ffff8001e3815000 x16: ffff800010004000 x15: 0000000000004000
[   19.798380] x14: 0000000000000000 x13: 0000000080000000 x12: 0000000000000001
[   19.805504] x11: 0000000000000004 x10: 0000000000000008 x9 : ffff8000109b9a2c
[   19.812626] x8 : 0000000000000000 x7 : 0000000000000001 x6 : ffff000088246500
[   19.819748] x5 : fffffffffffff000 x4 : 0003000108f40000 x3 : 000000000000003f
[   19.826869] x2 : 0000000000000040 x1 : ffff000188f42f80 x0 : ffff000088f89000
[   19.833994] Call trace:
[   19.836434]  dcache_inval_poc+0x40/0x58
[   19.840262]  iommu_dma_sync_single_for_cpu+0xdc/0xe0
[   19.845219]  dma_sync_single_for_cpu+0x3c/0x124
[   19.849742]  stmmac_rx+0x448/0x930 [stmmac]
[   19.853937]  stmmac_napi_poll_rx+0x4c/0xec [stmmac]
[   19.858818]  __napi_poll+0x40/0x210
[   19.862300]  net_rx_action+0x304/0x370
[   19.866041]  __do_softirq+0x130/0x3d8
[   19.869695]  __irq_exit_rcu+0x100/0x110
[   19.873525]  irq_exit+0x1c/0x30
[   19.876658]  handle_domain_irq+0x70/0x9c
[   19.880573]  gic_handle_irq+0x58/0xe0
[   19.884225]  call_on_irq_stack+0x2c/0x54
[   19.888138]  do_interrupt_handler+0x5c/0x70
[   19.892313]  el1_interrupt+0x30/0x70
[   19.895881]  el1h_64_irq_handler+0x18/0x24
[   19.899970]  el1h_64_irq+0x78/0x7c
[   19.903361]  arch_cpu_idle+0x18/0x3c
[   19.906930]  default_idle_call+0x4c/0x1d4
[   19.910931]  cpuidle_idle_call+0x168/0x1e0
[   19.915019]  do_idle+0xb8/0x110
[   19.918152]  cpu_startup_entry+0x30/0x8c
[   19.922065]  rest_init+0xf0/0x100
[   19.925372]  arch_call_rest_init+0x1c/0x28
[   19.929463]  start_kernel+0x4a0/0x4d8
[   19.933114]  __primary_switched+0xc0/0xc8
[   19.937118] Code: 8a230000 54000060 d50b7e20 14000002 (d5087620) 
[   19.943204] ---[ end trace f26fd0d14eea9a7d ]---
[   19.947811] Kernel panic - not syncing: Oops: Fatal exception in interrupt
[   19.954670] SMP: stopping secondary CPUs
[   19.958592] Kernel Offset: 0x1a0000 from 0xffff800010000000
[   19.964151] PHYS_OFFSET: 0x80000000
[   19.967630] CPU features: 0x10000231,00000846
[   19.971976] Memory Limit: none
[   19.975026] ---[ end Kernel panic - not syncing: Oops: Fatal exception in interrupt ]---


-- 
Without deviation from the norm, progress is not possible.