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

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]

Date:   Sun, 19 Feb 2017 11:36:51 +0100
From:   Lino Sanfilippo <LinoSanfilippo@....de>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     Pavel Belous <Pavel.Belous@...antia.com>,
        "David S . Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Simon Edelhaus <Simon.Edelhaus@...antia.com>,
        Alexey Andriyanov <Alexey.Andriyanov@...antia.com>
Subject: 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
>> Sent: 16 February 2017 16:02
> ...
>> 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@....de>
Suggested-by: David Laight <David.Laight@...LAB.COM>
---
 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