Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented

[Stefano Stabellini <sstabellini@kernel.org>]

On Tue, 28 Mar 2017, Oleksandr Andrushchenko wrote:
> Hi, Stefano!
> 
> On 03/27/2017 11:23 PM, Stefano Stabellini wrote:
> > Hello Oleksandr,
> > 
> > Just to clarify, you are talking about dma_to_phys/phys_to_dma in Linux
> > (not in Xen), right?
> I am talking about Linux, sorry I was not clear
> > Drivers shouldn't use those functions directly (see the comment in
> > arch/arm64/include/asm/dma-mapping.h), they should call the appropriate
> > dma_map_ops functions instead.
> Yes, you are correct and I know about this and do not call
> dma_to_phys/phys_to_dma directly
> >   The dma_map_ops functions should do the
> > right thing on Xen, even for pages where pfn != mfn, thanks to the
> > swiotlb-xen (drivers/xen/swiotlb-xen.c). Specifically, you can see the
> > special case for pfn != mfn here (see the "local" variable):
> > 
> > arch/arm/include/asm/xen/page-coherent.h:xen_dma_map_page
> Yes, the scenarios with pfn != mfn we had so
> far are all working correct
> > 
> > So, why are you calling dma_to_phys and phys_to_dma instead of the
> > dma_map_ops functions?
> Ok, let me give you an example of failing scenario which
> was not used before this by any backend (this is from
> my work on implementing zero copy for DRM drivers):
> 
> 1. Create sg table from pages:
> sg_alloc_table_from_pages(sgt, PFN_0, ...);
> map it and get dev_bus_addr - at this stage sg table is
> perfectly correct and properly mapped,
> dev_bus_addr == (MFN_u << PAGE_SHIFT)

Let me get this straight: one of the pages passed to
sg_alloc_table_from_pages is actually a foreign page (pfn != mfn), is
that right?

And by "map", you mean dma_get_sgtable_attrs is called on it, right?


> 2. Duplicate it:
> dma_get_sgtable(..., sgt, ... dev_bus_addr,...);

Yeah, if one of the pages passed to sg_alloc_table_from_pages is
foreign, as Andrii pointed out, dma_get_sgtable
(xen_swiotlb_get_sgtable) doesn't actually work.

Is it legitimate that one of those pages is foreign or is it a mistake?
If it is a mistake, you could fix it.  Otherwise, if the use of
sg_alloc_table_from_pages or the following call to dma_get_sgtable are
part of your code, I suggest you work-around the problem by avoiding
the dma_get_sgtable call altogether. Don't use the sg_ dma api, use the
regular dma api instead.


Unfortunately, if the dma_get_sgtable is part of existing code, then we
have a problem. In that case, could you point me to the code that call
dma_get_sgtable?

There is no easy way to make it work on Xen: virt_to_phys doesn't work
on ARM and dma_to_phys doesn't work on Xen. We could implement
xen_swiotlb_get_sgtable correctly if we had the physical address of the
page, because we could easily find out if the page is local or foreign
with a pfn != mfn check (similar to the one in
include/xen/arm/page-coherent.h:xen_dma_map_page).

If the page is local, then we would do what we do today. If the page is
foreign, then we would need to do something like:

    int ret = sg_alloc_table(sgt, 1, GFP_KERNEL);

	if (!ret) {
		sg_set_page(sgt->sgl, phys_to_page(phys), PAGE_ALIGN(size), 0);
		sgt->sgl->dma_address = dev_addr;
	}


Now the question is, how do we get the pseudo-physical address of the
page? We could parse the stage1 page table entry of the kernel, or we
could use the "at" instruction (see for example gva_to_ipa_par in xen).
It is a bit rough, but I cannot think of anything else.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel