Re: [PATCH 3/5] drm: add ARM64 flush implementations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24/01/18 12:36, Russell King - ARM Linux wrote:
On Wed, Jan 24, 2018 at 12:00:59PM +0000, Robin Murphy wrote:
On 24/01/18 02:56, Gurchetan Singh wrote:
This patch uses the __dma_map_area function to flush the cache
on ARM64.

v2: Don't use DMA API, call functions directly (Daniel)

Signed-off-by: Gurchetan Singh <gurchetansingh@xxxxxxxxxxxx>
---
  drivers/gpu/drm/drm_cache.c | 13 +++++++++++++
  1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 5124582451c6..250cdfbb711f 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -159,6 +159,12 @@ drm_flush_pages(struct page *pages[], unsigned long num_pages)
  	for (i = 0; i < num_pages; i++)
  		drm_cache_maint_page(pages[i], 0, PAGE_SIZE, DMA_TO_DEVICE,
  				     dmac_map_area);
+#elif defined(CONFIG_ARM64)
+	unsigned long i;
+
+	for (i = 0; i < num_pages; i++)
+		__dma_map_area(phys_to_virt(page_to_phys(pages[i])), PAGE_SIZE,
+			       DMA_TO_DEVICE);

Note that this is not exactly equivalent to clflush - if it's at all
possible for a non-coherent GPU to write back to these pages and someone
somewhere expects to be able to read the updated data from a CPU, that's
going to be subtly broken.

This also breaks building DRM as a module. And I doubt that either of the
two easy solutions to that are going to fly with the respective
maintainers...

Given all the bodging around which seems to happen in DRM/ION/etc., it would
be really nice to pin down what exactly the shortcomings of the DMA API are
for these use-cases, and extend it to address them properly.

Aren't the cache flushing instructions available from userspace on
ARM64?  (I've certainly used them in my spectre/meltdown PoC).  If
userspace wants to flush before reading a GPU buffer that has been
writtent to, why can't it use the already available cache maintenance
instructions?  It's likely to be quicker, and it can target just the
area of the buffer it wants to read.

Indeed, whilst the main reason we enable EL0 cache maintenance access is for JITs cleaning to PoU, it inherently makes PoC operations available as well. The semantics of drm_clflush_*() weren't immediately obvious, but if it is purely a sync point around userspace reading/writing GPU buffers directly, then what you suggest does sound ideal (and __dma_map_area is definitely wrong for the read case as above).


32-bit ARM is a different matter, and I'll reply separately.


Thanks,
Robin.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux