netdev - RE: [PATCH V4 net 3/5] net: stmmac: fix dma physical address of descriptor when display ring

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: <DB8PR04MB679566B923A571F8E8384278E6809@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date:   Tue, 23 Feb 2021 07:10:38 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "peppe.cavallaro@...com" <peppe.cavallaro@...com>,
        "alexandre.torgue@...com" <alexandre.torgue@...com>,
        "joabreu@...opsys.com" <joabreu@...opsys.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH V4 net 3/5] net: stmmac: fix dma physical address of
 descriptor when display ring


> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: 2021年2月23日 3:46
> To: Joakim Zhang <qiangqing.zhang@....com>
> Cc: peppe.cavallaro@...com; alexandre.torgue@...com;
> joabreu@...opsys.com; davem@...emloft.net; netdev@...r.kernel.org;
> dl-linux-imx <linux-imx@....com>
> Subject: Re: [PATCH V4 net 3/5] net: stmmac: fix dma physical address of
> descriptor when display ring
> 
> On Sat, 20 Feb 2021 07:43:33 +0000 Joakim Zhang wrote:
> > > >  	pr_info("%s descriptor ring:\n", rx ? "RX" : "TX");
> > > >
> > > >  	for (i = 0; i < size; i++) {
> > > > -		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
> > > > -			i, (unsigned int)virt_to_phys(p),
> > > > +		pr_info("%03d [0x%llx]: 0x%x 0x%x 0x%x 0x%x\n",
> > > > +			i, (unsigned long long)(dma_rx_phy + i * desc_size),
> > > >  			le32_to_cpu(p->des0), le32_to_cpu(p->des1),
> > > >  			le32_to_cpu(p->des2), le32_to_cpu(p->des3));
> > > >  		p++;
> > >
> > > Why do you pass the desc_size in? The virt memory pointer is
> > > incremented by
> > > sizeof(*p) surely
> > >
> > > 	dma_addr + i * sizeof(*p)
> >
> > I think we can't use sizeof(*p), as when display descriptor, only do "
> > struct dma_desc *p = (struct dma_desc *)head;", but driver can pass
> > "struct dma_desc", " struct dma_edesc" or " struct dma_extended_desc",
> 
> Looks like some of the functions you change already try to pick the right type.
> Which one is problematic?

Yes, some functions have picked the right type:
drivers/net/ethernet/stmicro/stmmac/enh_desc.c -> enh_desc_display_ring()
drivers/net/ethernet/stmicro/stmmac/norm_desc.c -> ndesc_display_ring()

the problematic one is:
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c -> dwmac4_display_ring()

Since the callback format is the same for them, and used from stmmac_main.c in the same way.
drivers/net/ethernet/stmicro/stmmac/hwif.h -> void (*display_ring)(void *head, unsigned int size, bool rx);

So I decide to modify them as a whole to avoid separate them as different format which would introduce more redundant code. Is it reasonable?

> > so it's necessary to pass desc_size to compatible all cases.
> 
> But you still increment the the VMA pointer ('p' in the quote above) but it's size,
> so how is that correct if the DMA addr needs a special size increment?

Yes, you are right. It indeed a problem. Seems dwmac4_display_ring() function has not supported different desc format well.
DMA phy address is just one of its problem. I will fix it together. Thanks.

Best Regards,
Joakim Zhang