Re: [PATCH net-next 11/13] net: ethernet: aquantia: Refactoring buffers copying.

Hi,

On 16.02.2017 17:37, David Laight wrote:

From: Lino Sanfilippo

quoted

Sent: 16 February 2017 16:02

...

quoted

I was referring to the copy of tx descriptors, not the frames/fragments itself.
I wrote "tx buffers" because in this driver a descriptor is represented as
 a struct "aq_ring_buff_s". I cannot see a reason why this descriptor copy
 should be necessary.

Not unless the descriptor memory is on the card.

Actually it might be worse than that.
The transmit descriptor probably has an 'owner' bit.
The write of the word containing that bit must be last and
preceded by the appropriate barrier (probably dma_wmb()).
Another barrier is needed before the 'doorbell' io write that kicks
the hardware.

After taking a look at the concerning code parts, I can say that there is
at least no barrier to ensure dma flush before the doorbell.
What about the patch below to fix this?

Regards,
Lino

Subject: [PATCH] net: aquantia: add memory barrier before tx descriptors are
 passed to the hardware

Add a memory barrier to ensure that tx descriptors are written completely
before the descriptors are passed to the hardware.

Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Suggested-by: David Laight <redacted>
---
 drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
index 3de651a..c3d73e7 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c

@@ -526,6 +526,8 @@ void reg_tx_dma_desc_base_addressmswset(struct aq_hw_s *aq_hw,
 void reg_tx_dma_desc_tail_ptr_set(struct aq_hw_s *aq_hw,
 				  u32 tx_dma_desc_tail_ptr, u32 descriptor)
 {
+	/* make sure writes to dma descriptors are completed */
+	wmb();
 	aq_hw_write_reg(aq_hw, tx_dma_desc_tail_ptr_adr(descriptor),
 			tx_dma_desc_tail_ptr);
 }

-- 
2.7.4