| Message ID | 20230911092306.2132794-1-srasheed@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] octeon_ep: fix tx dma unmap len values in SG | expand |
On Mon, Sep 11, 2023 at 02:23:06AM -0700, Shinas Rasheed wrote: > Lengths of SG pointers are in big-endian > > Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support") > Signed-off-by: Shinas Rasheed <srasheed@marvell.com> > --- > drivers/net/ethernet/marvell/octeon_ep/octep_tx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c > index 5a520d37bea0..7e99486c274b 100644 > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c > @@ -69,12 +69,12 @@ int octep_iq_process_completions(struct octep_iq *iq, u16 budget) > compl_sg++; > > dma_unmap_single(iq->dev, tx_buffer->sglist[0].dma_ptr[0], > - tx_buffer->sglist[0].len[0], DMA_TO_DEVICE); > + tx_buffer->sglist[0].len[3], DMA_TO_DEVICE); > > i = 1; /* entry 0 is main skb, unmapped above */ > while (frags--) { > dma_unmap_page(iq->dev, tx_buffer->sglist[i >> 2].dma_ptr[i & 3], > - tx_buffer->sglist[i >> 2].len[i & 3], DMA_TO_DEVICE); > + tx_buffer->sglist[i >> 2].len[3 - (i & 3)], DMA_TO_DEVICE); > i++; > } Hi Shinas, is this change also needed in octep_iq_process_completions() ? The code there looks rather similar.
Hi Simon, This change is required in octep_iq_process_completions, as given in the patch, since the scatter gather pointer lengths arrive as big-endian in hardware.
On Tue, 2023-09-12 at 00:04 -0700, Shinas Rasheed wrote: > This change is required in octep_iq_process_completions, as given in the patch, > since the scatter gather pointer lengths arrive as big-endian in hardware. I guess Simon intended asking about octep_iq_free_pending(), and AFAICT your reply confirm that the change is required there, too. Additionally the changelog really need to be expanded. I don't understand how this change relates to endianess: if the ring format is big endian I expect some be16_to_cpu(len) instead of complement-to-4 of indexes. Please clarify and expand the changelog, thanks! Paolo
On Tue, Sep 12, 2023 at 06:37:46AM +0000, Shinas Rasheed wrote: > Hi Simon, > > This change is required in octep_iq_process_completions, as given in the patch, since the scatter gather pointer lengths arrive as big-endian in hardware. Hi, yes, I see that. And sorry for asking such a silly question. But what I meant to ask is, if the change is also needed in octep_iq_free_pending()?
Hi Paolo, Hi Simon, From: Paolo Abeni <pabeni@redhat.com> Sent: Tuesday, September 12, 2023 2:15 PM To: Shinas Rasheed <srasheed@marvell.com>; horms@kernel.org <horms@kernel.org> Cc: Abhijit Ayarekar <aayarekar@marvell.com>; davem@davemloft.net <davem@davemloft.net>; edumazet@google.com <edumazet@google.com>; egallen@redhat.com <egallen@redhat.com>; Haseeb Gani <hgani@marvell.com>; kuba@kernel.org <kuba@kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; mschmidt@redhat.com <mschmidt@redhat.com>; netdev@vger.kernel.org <netdev@vger.kernel.org>; Satananda Burla <sburla@marvell.com>; Sathesh B Edara <sedara@marvell.com>; Veerasenareddy Burru <vburru@marvell.com>; Vimlesh Kumar <vimleshk@marvell.com> Subject: [EXT] Re: [net PATCH] octeon_ep: fix tx dma unmap len values in SG External Email ---------------------------------------------------------------------- >I guess Simon intended asking about octep_iq_free_pending(), and AFAICT your reply confirm that the change is required there, too. You are correct in that the change is also required in octep_iq_free_pending as well. Thanks for pointing that out! I will submit another version of this patchset including that. >Additionally the changelog really need to be expanded. I don't understand how this change relates to endianess: if the ring format is big endian I expect some be16_to_cpu(len) instead of complement-to-4 of indexes. The bytes are in itself not big endian, but rather the each of the 16 bytes are kept in memory in a kind of big-endian order. Apologizing for the confusion. 63 48 47 32 31 16 15 0 | Len0 | Len1 | Len2 | Len3 | I shall provide an ascii figure like above in the code to explain and also update the changelog accordingly. Thanks for your time!
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c index 5a520d37bea0..7e99486c274b 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c @@ -69,12 +69,12 @@ int octep_iq_process_completions(struct octep_iq *iq, u16 budget) compl_sg++; dma_unmap_single(iq->dev, tx_buffer->sglist[0].dma_ptr[0], - tx_buffer->sglist[0].len[0], DMA_TO_DEVICE); + tx_buffer->sglist[0].len[3], DMA_TO_DEVICE); i = 1; /* entry 0 is main skb, unmapped above */ while (frags--) { dma_unmap_page(iq->dev, tx_buffer->sglist[i >> 2].dma_ptr[i & 3], - tx_buffer->sglist[i >> 2].len[i & 3], DMA_TO_DEVICE); + tx_buffer->sglist[i >> 2].len[3 - (i & 3)], DMA_TO_DEVICE); i++; }
Lengths of SG pointers are in big-endian Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support") Signed-off-by: Shinas Rasheed <srasheed@marvell.com> --- drivers/net/ethernet/marvell/octeon_ep/octep_tx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)