Re: [Bug] VCHIQ functional test broken

[Rabin Vincent]

On Thu, Apr 13, 2017 at 11:29:15PM +0100, Russell King - ARM Linux wrote:
> > 00a19f3e25c0c40e0ec77f52d4841d23ad269169 is the first bad commit
> > commit 00a19f3e25c0c40e0ec77f52d4841d23ad269169
> > Author: Rabin Vincent 
> > Date:   Tue Nov 8 09:21:19 2016 +0100
> > 
> > ARM: 8627/1: avoid cache flushing in flush_dcache_page()
> > 
> > When the data cache is PIPT or VIPT non-aliasing, and cache operations
> > are broadcast by the hardware, we can always postpone the flush in
> > flush_dcache_page().  A similar change was done for ARM64 in commit
> > b5b6c9e9149d ("arm64: Avoid cache flushing in flush_dcache_page()").
> > 
> > Reviewed-by: Catalin Marinas 
> > Signed-off-by: Rabin Vincent 
> > Signed-off-by: Russell King 
> > 
> > It seems that staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm
> > relies on the behavior of flush_dcache_page before this patch get
> > applied. Any advices to fix this issues are appreciated.
> 
> Any ideas why this causes a problem for this driver?  From what I can see,
> it doesn't make use of flush_dcache_page().

The driver's create_pagelist() uses get_free_pages(), and
get_free_pages() calls flush_dcache_page().

The problem is that the driver fails to flush the pages which it
acquires via get_free_pages().  It's clear that the driver needs to do
it, since there is a flush in the is_vmalloc_addr() path in the same
function.  The driver probably worked earlier because of the unecessary
flush in flush_dcache_page() which existed before this patch, but the
purpose of that flush was not DMA coherency and it was never guaranteed
that it would flush all the way to the point that devices could see the
data.

See radeon_ttm_tt_pin_userptr() in drivers/gpu/drm/radeon/radeon_ttm.c
for an example of how a driver can ensure cache coherency using the DMA
mapping API if it intends to DMA from/to pages acquired by
get_free_pages().

The rest of the driver should also be converted to the DMA mapping API
instead of abusing the API's private functions (dmac_map_area etc.)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Bug] VCHIQ functional test broken

[Rabin Vincent]

On Thu, Apr 20, 2017 at 11:27:38AM -0700, Eric Anholt wrote:
> I'm confused by what you're saying here.  The driver has already been
> converted to not use dmac_map_area (commit
> cf9caf1929882b66922aee698e99e6c8f357bee5), and uses dma_map_sg instead,
> matching the radeon driver you give as a model as far as I can see.
> That commit is in v4.11-rc6 from Stefan's regression report.

Right.  Sorry.  I must have had an old tag checked out when I looked at
the driver earlier.  The DMA API usage in the driver in v4.11-rc6 and
current master looks fine, except for one thing:

The flush in flush_dcache_page() (from get_user_pages()) was done with a
v6_flush_kern_dcache_page() which always did a clean+invalidate while
the DMA API only does what is required by the direction, which is only a
invalidate for DMA_FROM_DEVICE.  Since the driver calls dma_from_sg() on
the entire page, even if userspace sent in an offset into the page,
unrelated data in userspace may be thrown away.

Does changing the dma API calls to always use DMA_BIDIRECTIONAL make the
test pass?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel