On Wed, Nov 12, 2014 at 01:39:05PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> dma_alloc_coherent() returns a kernel virtual address that is part of
> the linear range. Passing such an address to virt_to_page() is illegal
> on non-coherent architectures. This causes the kernel to oops on 64-bit
> ARM because the struct page * obtained from virt_to_page() points to
> unmapped memory.
Oh! That is not good!
>
> This commit fixes this by using phys_to_page() since we get a physical
> address from dma_alloc_coherent(). Note that this is not a proper fix
> because if an IOMMU is set up to translate addresses for the GPU this
> address will be an I/O virtual address rather than a physical one. The
> proper fix probably involves not getting a pointer to the struct page
> in the first place, but that would be a much more intrusive change, if
> at all possible.
What type of caching types are there on ARM? We use the 'struct page'
on the set_pages_to_[wc|uc|wb] but all of those are X86 specfic.
But I think you could by passing the 'struct dma_page' instead
of 'struct page' (and the array uses) around. That should solve
the touching of 'struct page' and we can treat it as an opaque
type.
>
> Until that time, this temporary fix will allow TTM to work on 32-bit
> and 64-bit ARM as well, provided that no IOMMU translations are enabled
> for the GPU.
Is there a way to query the 'struct device' to see if the IOMMU translation
is enabled/disabled for said device?
Now your patch looks to get the 'struct page' by doing some form of
translation. Could you explain to me which type of memory have a 'struct page'
and which ones do not ?
It is OK if you explain this in nauseating details :-)
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> Arnd, I realize that this isn't a proper fix according to what we discussed on
> IRC yesterday, but I can't see a way to remove access to the pages array that
> would be as simple as this. I've marked this as RFC in the hope that it will
> trigger some discussion that will lead to a proper solution.
>
> drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index c96db433f8af..d7993985752c 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -343,7 +343,11 @@ static struct dma_page *__ttm_dma_alloc_page(struct dma_pool *pool)
> &d_page->dma,
> pool->gfp_flags);
> if (d_page->vaddr)
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> + d_page->p = phys_to_page(d_page->dma);
> +#else
> d_page->p = virt_to_page(d_page->vaddr);
> +#endif
> else {
> kfree(d_page);
> d_page = NULL;
> --
> 2.1.3
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
