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

Re: [Xen-devel] arm64: dma_to_phys/phys_to_dma need to be properly implemented



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)

2. Duplicate it:
dma_get_sgtable(..., sgt, ... dev_bus_addr,...);
                              ^^^^^^^^^^^^
which finally lands at
__swiotlb_get_sgtable(..., dma_addr_t handle,...)
{
    ...
    sg_set_page(..., phys_to_page(dma_to_phys(dev, handle)),...);
                                  ^^^^^^^^^^^      ^^^^^^
    ...
}
At this stage sg table's page_link points to bad page, because
of linear translation of dma_to_phys, e.g. paddr == dev_bus_addr

3. Map the duplicated sg table:
dma_map_sg(..., sgt->sgl,...);
    xen_swiotlb_map_sg_attrs(..., struct scatterlist *sgl,...)
    {
        phys_addr_t paddr = sg_phys(sg);
                            ^^^^^^^
        dma_addr_t dev_addr = xen_phys_to_bus(paddr);
/*
the translation (sg_phys) above does:
        page_to_phys(...sg->page_link & ~0x3...) + ...;
so after this step we have WRONG paddr and correct dev_addr (because
wrong PFN is not found in P2M table, so it is used as is
*/
4. Now map segments of the table:
__swiotlb_map_page(..., struct page *page,...)
{
    ...
    dev_addr = swiotlb_map_page(dev, page, offset, size, dir, attrs);
    if (!is_device_dma_coherent(dev))
        __dma_map_area(phys_to_virt(dma_to_phys(dev, dev_addr)),...);
                                    ^^^^^^^^^^^      ^^^^^^^^
    ...
}

5. Crash happens at
__dma_map_area(phys_to_virt(dma_to_phys(dev, dev_addr)),...);
with
Unable to handle kernel paging request at virtual address
because dev_addr was NOT translated to correct paddr by dma_to_phys

Hope, this better explains why I am talking about dma_to_phys

So, I'm curious if we need to change dma_to_phys/phys_to_dma
or hide this translation in Xen specific code.
Another question is that for phys_to_dma we have __pfn_to_mfn,
but there is no reverse translation , e.g. __mfn_to_pfn
Cheers,

Stefano
Thank you,
Oleksandr
On Mon, 27 Mar 2017, Oleksandr Andrushchenko wrote:
Hi, all!

First of all this could be a generic ARM64 question, but
only(?) Xen users will suffer.

At the moment for ARM64 dma_to_phys/phys_to_dma both assume that
MFN == PFN and though those do not do MFN <-> PFN conversion.
In case if MFN is mapped from other domain, then
MFN != PFN and the above assumption is not true any more and
proper MFN to PFN and vice versa must be implemented.

Use-case:
1. Get gref_u in DomU (PFN_u/MFN_u) and pass to Dom0
2. Balloon out a page PFN_0
3. Map gref_u onto PFN_0
4. Now PFN_0 is mapped to MFN_u and PFN_0 != MFN_u

If pair PFN_0/MFN_u is about to be used for example for
DMA sg table (which is my case), then conversion
dma_to_phys(MFN_u << PAGE_SHIFT) results in wrong PFN_bad,
not usable by CPU anymore because PFN_0 is expected instead

Is there any reason for ARM64's dma_to_phys/phys_to_dma to be
not implemented?

Thank you,

Oleksandr


On 03/23/2017 05:40 PM, Oleksandr Andrushchenko wrote:
Hi, all!

I am trying to implement a zero-copy scenario for DRM front/back,
e.g. buffers allocated by DomU in the DRM frontend used as is
by Dom0. The requirement I have is that the buffer is contiguous.

So, what I currently have is:
1. DomU is 1:1 mapped and is able to allocate physically contiguous
buffer, e.g. PFNs and MFNs are sequential.
2. On Dom0 side I allocate ballooned pages (because I need reservation)
and am able to map those (gnttab_map_refs with grefs from DomU):
pfn 52a35 dev_bus_addr 6c100000
pfn 52a34 dev_bus_addr 6c101000
...
pfn 52a2c dev_bus_addr 6c109000

3. Then I have to create an SG table from these, so I can pass the buffer
to real HW DRM driver: as you can see from the above PFNs they are not
sequential as those were allocated from the balloon. Thus, sg_table
is also has number of segments != 1 which is not ok for my use-case.

Still, if I do __pfn_to_mfn(page_to_pfn(balloon_page[i])) I get the

correct MFN, which is expected because p2m translation happens.

4. If I try to do a hack:
dev_bus_pages[i] = pfn_to_page(MFN[i]) and then create an SG table from
these pages then of course the sg table is configuous and
I can share the SG with HW DRM driver.
What is naturally happens next is:
"Unable to handle kernel paging request at virtual address ffff80002c100000"
[   86.263197] PC is at __clean_dcache_area_poc+0x20/0x40
[   86.268377] LR is at __swiotlb_map_page+0x80/0x98
for the very first maddr == 6c100000, because my dev_bus_addr[i] and
dev_bus_pages[i] have no translation set up.

So, the question: is there any way I can make those ballooned pages
to convert into contiguous scatter-gather? So, sg table consists
of a single entry?

I was thinking of:
1. Is it possible to update translation manually so dev_bus_addr[i]
has corresponding continuous pages, e.g. so I can do
dev_bus_pages[i] = pfn_to_page(MFN[i]) and CPU can access those?
2. I can of course allocate contiguous buffer in Dom0 and
manually (unfortunately there is no API to do that) increase/decrease
reservation for the pages as balloon driver does and use those for mapping.
This way MFN will follow PFN (we are in Dom0 which is 1:1).
But this is going to be clumsy, as I'll have to copy-paste part
of the balloon driver into mine (decrease_reservation/increase_reservation)
3. Any other neat solution?

Thank you,
Oleksandr


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.