Re: [PATCH] rtl8187: fix regression on MIPS without coherent DMA

[Larry Finger <Larry.Finger@lwfinger.net>]

On 02/10/2014 03:38 PM, Stanislaw Gruszka wrote:
> This patch fixes regression caused by commit a16dad77634 "MIPS: Fix
> potencial corruption". That commit fixes one corruption scenario in
> cost of adding another one, which actually start to cause crashes
> on Yeeloong laptop when rtl8187 driver is used.
>
> For correct DMA read operation on machines without DMA coherence, kernel
> have to invalidate cache, such it will refill later with new data that
> device wrote to memory, when that data is needed to process. We can only
> invalidate full cache line. Hence when cache line includes both dma
> buffer and some other data (written in cache, but not yet in main
> memory), the other data can not hit memory due to invalidation. That
> happen on rtl8187 where struct rtl8187_priv fields are located just
> before and after small buffers that are passed to USB layer and DMA
> is performed on them.
>
> To fix the problem we align buffers and reserve space after them to make
> them match cache line.
>
> This patch does not resolve all possible MIPS problems entirely, for
> that we have to assure that we always map cache aligned buffers for DMA,
> what can be complex or even not possible. But patch fixes visible and
> reproducible regression and seems other possible corruptions do not
> happen in practice, since Yeeloong laptop works stable without rtl8187
> driver.
>
> Bug report:
> https://bugzilla.kernel.org/show_bug.cgi?id=54391
>
> Reported-by: Petr Pisar <petr.pisar@atlas.cz>
> Bisected-by: Tom Li <biergaizi2009@gmail.com>
> Reported-and-tested-by: Tom Li <biergaizi2009@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
> ---

Congratulations to all for sorting this out. It could not have been too easy.

The only effect I see on architectures with DMA coherence is that the private 
data area has grown a little. Certainly, my RTL8187B device still works on x86_64.

Acked-by: Larry Finger <Larry.Finger@lwfinger.next>

Larry

>   drivers/net/wireless/rtl818x/rtl8187/rtl8187.h |   10 ++++++++--
>   1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h b/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
> index 56aee06..a6ad79f 100644
> --- a/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
> +++ b/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
> @@ -15,6 +15,8 @@
>   #ifndef RTL8187_H
>   #define RTL8187_H
>
> +#include <linux/cache.h>
> +
>   #include "rtl818x.h"
>   #include "leds.h"
>
> @@ -139,7 +141,10 @@ struct rtl8187_priv {
>   	u8 aifsn[4];
>   	u8 rfkill_mask;
>   	struct {
> -		__le64 buf;
> +		union {
> +			__le64 buf;
> +			u8 dummy1[L1_CACHE_BYTES];
> +		} ____cacheline_aligned;
>   		struct sk_buff_head queue;
>   	} b_tx_status; /* This queue is used by both -b and non-b devices */
>   	struct mutex io_mutex;
> @@ -147,7 +152,8 @@ struct rtl8187_priv {
>   		u8 bits8;
>   		__le16 bits16;
>   		__le32 bits32;
> -	} *io_dmabuf;
> +		u8 dummy2[L1_CACHE_BYTES];
> +	} *io_dmabuf ____cacheline_aligned;
>   	bool rfkill_off;
>   	u16 seqno;
>   };
>