linux-kernel - Re: [PATCH 01/16] Blackfin SPI Driver: ensure cache coherency before doing DMA

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]

Date:	Thu, 20 Nov 2008 12:24:15 -0800
From:	David Brownell <david-b@...bell.net>
To:	Bryan Wu <cooloney@...nel.org>
Cc:	spi-devel-general@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org,
	Vitja Makarov <vitja.makarov@...il.com>
Subject: Re: [PATCH 01/16] Blackfin SPI Driver: ensure cache coherency before doing DMA

On Monday 17 November 2008, Bryan Wu wrote:
>                         /* set transfer mode, and enable SPI */
>                         dev_dbg(&drv_data->pdev->dev, "doing DMA in.\n");
>  
> +                       /* invalidate caches, if needed */
> +                       if (bfin_addr_dcachable((unsigned long) drv_data->rx))
> +                               invalidate_dcache_range((unsigned long) drv_data->rx,
> +                                                       (unsigned long) (drv_data->rx +
> +                                                       drv_data->len));

Could you explain why you're not using dma_map_*() calls
or the rx_dma (and tx_dma) addresses the caller may pass?

As a rule, you should use the standard kernel interfaces
for such stuff instead of platform-specific calls like
those two.  There are a LOT more developers who can find
and fix bugs that way.


Also, this patch affects the "not full duplex" branch of
this routine.  DMA here seems unusually convoluted ... but
if you didn't invalidate the cache (RX path) before
flushing it (TX path) and instead did it the other way
aroound, would you actually *need* separate branches?


> +
>                         /* clear tx reg soformer data is not shifted out */
>                         write_TDBR(drv_data, 0xFFFF);
>  


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/