gnttab_map_refs and contiguous buffer

gnttab_map_refs and contiguous buffer

[Oleksandr Andrushchenko]

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@lists.xen.org
https://lists.xen.org/xen-devel

arm64: dma_to_phys/phys_to_dma need to be properly implemented

[Oleksandr Andrushchenko]

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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Stefano Stabellini]

Hello Oleksandr,

Just to clarify, you are talking about dma_to_phys/phys_to_dma in Linux
(not in Xen), right?

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. 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


So, why are you calling dma_to_phys and phys_to_dma instead of the
dma_map_ops functions?

Cheers,

Stefano

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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Oleksandr Andrushchenko]

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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Andrii Anisov]

Hello,

I guess Oleksandr stepped into the issue described here 
https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg03454.html .
He uses an old version of the patch but effectively calls 
__generic_dma_ops(dev)->get_sgtable() with foreign pages so faced troubles.

Sincerely,
Andrii Anisov

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

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

[Oleksandr Andrushchenko]

indeed, this seems to be the case

So, in that mail thread we have the following statement:

"However, if that's not the case in any of the drivers, it
would lead to memory corruptions." with respect to get_sgtable

and pfn != mfn.

So, do you guys think I'll have to patch the offending driver?
Any other solution/workaround here?

Thank you,
Oleksandr

On 03/28/2017 03:40 PM, Andrii Anisov wrote:
> Hello,
>
> I guess Oleksandr stepped into the issue described here 
> https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg03454.html 
> .
> He uses an old version of the patch but effectively calls 
> __generic_dma_ops(dev)->get_sgtable() with foreign pages so faced 
> troubles.
>
> Sincerely,
> Andrii Anisov


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

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

[Stefano Stabellini]

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

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

[Oleksandr Andrushchenko]

Hi, Stefano!

Ok, probably I need to put more details into the use-case
so it is clear. What I am doing is a DRM driver which
utilizes PRIME buffer sharing [1] to implement zero-copy
of display buffers between DomU and Dom0. PRIME is based on
DMA Buffer Sharing API [2], so this is the reason I am
dealing with sg_table here.

On 03/28/2017 10:20 PM, Stefano Stabellini wrote:
> 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?
What happening here is:
------------- my driver ------------
1. I create an sg table from pages with pfn != mfn (PFN_0/MFN_u)
using drm_prime_pages_to_sg [3] which effectively is
sg_alloc_table_from_pages
------------- DRM framework ------------
2. I pass the sgt via PRIME to the real display driver
and it does drm_gem_map_dma_buf [4]
3. So, at this point everyting is just fine, because sgt is
correct (sgl->page_link points to my PFN_0 and p2m translation
succeeds)
------------- real HW DRM driver ------------
4. When real HW display driver accesses sgt it calls dma_get_sgtable
[5] and then dma_map_sg [6]. And all this is happening on the sgt
which my driver has provided, but PFN_0 is not honored anymore
because dma_get_sgtable is expected to be able to figure out
pfn from the corresponding DMA address.

So, strictly speaking real HW DRM driver has no issues,
the API it uses is perfectly valid.
>
>> 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.
This is the case
> Is it legitimate that one of those pages is foreign or is it a mistake?
This is the goal - I want pages from DomU to be directly
accessed by the HW in Dom0 (I have DomU 1:1 mapped,
so even contiguous buffers can come from DomU, if not
1:1 then IPMMU will be in place)
> If it is a mistake, you could fix it.
 From the above - this is the intention
>   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.
As seen from the above the problematic part is not in my
driver, it is either DRM framework or HW display driver
>   Don't use the sg_ dma api, use the
> regular dma api instead.
I use what DRM provides and dma_xxx if something is missed
>
>
> 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?
This is the case, see [5]
>
> 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).
Yes, I saw this code and it helped me to figure out
where the use-case fails
> 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;
> 	}
Agree, the crucial part here phys_to_page(phys): we need mfn->pfn
>
> Now the question is, how do we get the pseudo-physical address of the
> page?
yes, this is the root of the problem
> 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.
Me neither, this is why I hope community will help here...
Otherwise I'll need to patch kernel drivers if it's still possible.

Thank you for helping,
Oleksandr

[1] 
https://01.org/linuxgraphics/gfx-docs/drm/drm-memory-management.html#drm-prime-support
[2] http://lxr.free-electrons.com/source/Documentation/dma-buf-sharing.txt
[3] 
http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_prime.c?v=4.9#L766
[4] 
http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_prime.c?v=4.9#L242
[5] https://patchwork.kernel.org/patch/9468093/
[6] https://patchwork.kernel.org/patch/9289859/


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

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

[Stefano Stabellini]

On Wed, 29 Mar 2017, Oleksandr Andrushchenko wrote:
> Hi, Stefano!
> 
> Ok, probably I need to put more details into the use-case
> so it is clear. What I am doing is a DRM driver which
> utilizes PRIME buffer sharing [1] to implement zero-copy
> of display buffers between DomU and Dom0. PRIME is based on
> DMA Buffer Sharing API [2], so this is the reason I am
> dealing with sg_table here.
> 
> On 03/28/2017 10:20 PM, Stefano Stabellini wrote:
> > 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?
> What happening here is:
> ------------- my driver ------------
> 1. I create an sg table from pages with pfn != mfn (PFN_0/MFN_u)
> using drm_prime_pages_to_sg [3] which effectively is
> sg_alloc_table_from_pages
> ------------- DRM framework ------------
> 2. I pass the sgt via PRIME to the real display driver
> and it does drm_gem_map_dma_buf [4]
> 3. So, at this point everyting is just fine, because sgt is
> correct (sgl->page_link points to my PFN_0 and p2m translation
> succeeds)
> ------------- real HW DRM driver ------------
> 4. When real HW display driver accesses sgt it calls dma_get_sgtable
> [5] and then dma_map_sg [6]. And all this is happening on the sgt
> which my driver has provided, but PFN_0 is not honored anymore
> because dma_get_sgtable is expected to be able to figure out
> pfn from the corresponding DMA address.
> 
> So, strictly speaking real HW DRM driver has no issues,
> the API it uses is perfectly valid.
> > 
> > > 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.
> This is the case
> > Is it legitimate that one of those pages is foreign or is it a mistake?
> This is the goal - I want pages from DomU to be directly
> accessed by the HW in Dom0 (I have DomU 1:1 mapped,
> so even contiguous buffers can come from DomU, if not
> 1:1 then IPMMU will be in place)
> > If it is a mistake, you could fix it.
> From the above - this is the intention
> >   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.
> As seen from the above the problematic part is not in my
> driver, it is either DRM framework or HW display driver
> >   Don't use the sg_ dma api, use the
> > regular dma api instead.
> I use what DRM provides and dma_xxx if something is missed
> > 
> > 
> > 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?
> This is the case, see [5]
> > 
> > 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).
> Yes, I saw this code and it helped me to figure out
> where the use-case fails
> > 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;
> > 	}
> Agree, the crucial part here phys_to_page(phys): we need mfn->pfn
> > 
> > Now the question is, how do we get the pseudo-physical address of the
> > page?
> yes, this is the root of the problem
> > 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.
> Me neither, this is why I hope community will help here...
> Otherwise I'll need to patch kernel drivers if it's still possible.

If you can come up with a patch that only affects
xen_swiotlb_get_sgtable, and translates successfully void *cpu_addr into
a physical address using "at", I think I would take that patch. I would
recommend to test the patch on ARM32 too, where virt_to_phys reliably
fails.

I don't think we can ask the community to drop dma_get_sgtable on the
basis that the DMA api is broken.

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

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

[Julien Grall]

Hi,

On 29/03/2017 23:36, Stefano Stabellini wrote:
> On Wed, 29 Mar 2017, Oleksandr Andrushchenko wrote:
> If you can come up with a patch that only affects
> xen_swiotlb_get_sgtable, and translates successfully void *cpu_addr into
> a physical address using "at", I think I would take that patch. I would
> recommend to test the patch on ARM32 too, where virt_to_phys reliably
> fails.

I haven't fully read the thread. I would recommend to ask the 
ARM64/ARM32 kernel maintainers there advice here.

Cheers,

-- 
Julien Grall

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

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

[Oleksandr Andrushchenko]

Hi, Stefano

Unfortunately I had to switch to other tasks,

but I'll get back to this issue asap

Thank you


On 03/30/2017 01:36 AM, Stefano Stabellini wrote:
> On Wed, 29 Mar 2017, Oleksandr Andrushchenko wrote:
>> Hi, Stefano!
>>
>> Ok, probably I need to put more details into the use-case
>> so it is clear. What I am doing is a DRM driver which
>> utilizes PRIME buffer sharing [1] to implement zero-copy
>> of display buffers between DomU and Dom0. PRIME is based on
>> DMA Buffer Sharing API [2], so this is the reason I am
>> dealing with sg_table here.
>>
>> On 03/28/2017 10:20 PM, Stefano Stabellini wrote:
>>> 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?
>> What happening here is:
>> ------------- my driver ------------
>> 1. I create an sg table from pages with pfn != mfn (PFN_0/MFN_u)
>> using drm_prime_pages_to_sg [3] which effectively is
>> sg_alloc_table_from_pages
>> ------------- DRM framework ------------
>> 2. I pass the sgt via PRIME to the real display driver
>> and it does drm_gem_map_dma_buf [4]
>> 3. So, at this point everyting is just fine, because sgt is
>> correct (sgl->page_link points to my PFN_0 and p2m translation
>> succeeds)
>> ------------- real HW DRM driver ------------
>> 4. When real HW display driver accesses sgt it calls dma_get_sgtable
>> [5] and then dma_map_sg [6]. And all this is happening on the sgt
>> which my driver has provided, but PFN_0 is not honored anymore
>> because dma_get_sgtable is expected to be able to figure out
>> pfn from the corresponding DMA address.
>>
>> So, strictly speaking real HW DRM driver has no issues,
>> the API it uses is perfectly valid.
>>>> 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.
>> This is the case
>>> Is it legitimate that one of those pages is foreign or is it a mistake?
>> This is the goal - I want pages from DomU to be directly
>> accessed by the HW in Dom0 (I have DomU 1:1 mapped,
>> so even contiguous buffers can come from DomU, if not
>> 1:1 then IPMMU will be in place)
>>> If it is a mistake, you could fix it.
>>  From the above - this is the intention
>>>    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.
>> As seen from the above the problematic part is not in my
>> driver, it is either DRM framework or HW display driver
>>>    Don't use the sg_ dma api, use the
>>> regular dma api instead.
>> I use what DRM provides and dma_xxx if something is missed
>>>
>>> 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?
>> This is the case, see [5]
>>> 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).
>> Yes, I saw this code and it helped me to figure out
>> where the use-case fails
>>> 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;
>>> 	}
>> Agree, the crucial part here phys_to_page(phys): we need mfn->pfn
>>> Now the question is, how do we get the pseudo-physical address of the
>>> page?
>> yes, this is the root of the problem
>>> 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.
>> Me neither, this is why I hope community will help here...
>> Otherwise I'll need to patch kernel drivers if it's still possible.
> If you can come up with a patch that only affects
> xen_swiotlb_get_sgtable, and translates successfully void *cpu_addr into
> a physical address using "at", I think I would take that patch. I would
> recommend to test the patch on ARM32 too, where virt_to_phys reliably
> fails.
>
> I don't think we can ask the community to drop dma_get_sgtable on the
> basis that the DMA api is broken.


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

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

[Stefano Stabellini]

Ok, no worries

On Fri, 31 Mar 2017, Oleksandr Andrushchenko wrote:
> Hi, Stefano
> 
> Unfortunately I had to switch to other tasks,
> 
> but I'll get back to this issue asap
> 
> Thank you
> 
> 
> On 03/30/2017 01:36 AM, Stefano Stabellini wrote:
> > On Wed, 29 Mar 2017, Oleksandr Andrushchenko wrote:
> > > Hi, Stefano!
> > > 
> > > Ok, probably I need to put more details into the use-case
> > > so it is clear. What I am doing is a DRM driver which
> > > utilizes PRIME buffer sharing [1] to implement zero-copy
> > > of display buffers between DomU and Dom0. PRIME is based on
> > > DMA Buffer Sharing API [2], so this is the reason I am
> > > dealing with sg_table here.
> > > 
> > > On 03/28/2017 10:20 PM, Stefano Stabellini wrote:
> > > > 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?
> > > What happening here is:
> > > ------------- my driver ------------
> > > 1. I create an sg table from pages with pfn != mfn (PFN_0/MFN_u)
> > > using drm_prime_pages_to_sg [3] which effectively is
> > > sg_alloc_table_from_pages
> > > ------------- DRM framework ------------
> > > 2. I pass the sgt via PRIME to the real display driver
> > > and it does drm_gem_map_dma_buf [4]
> > > 3. So, at this point everyting is just fine, because sgt is
> > > correct (sgl->page_link points to my PFN_0 and p2m translation
> > > succeeds)
> > > ------------- real HW DRM driver ------------
> > > 4. When real HW display driver accesses sgt it calls dma_get_sgtable
> > > [5] and then dma_map_sg [6]. And all this is happening on the sgt
> > > which my driver has provided, but PFN_0 is not honored anymore
> > > because dma_get_sgtable is expected to be able to figure out
> > > pfn from the corresponding DMA address.
> > > 
> > > So, strictly speaking real HW DRM driver has no issues,
> > > the API it uses is perfectly valid.
> > > > > 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.
> > > This is the case
> > > > Is it legitimate that one of those pages is foreign or is it a mistake?
> > > This is the goal - I want pages from DomU to be directly
> > > accessed by the HW in Dom0 (I have DomU 1:1 mapped,
> > > so even contiguous buffers can come from DomU, if not
> > > 1:1 then IPMMU will be in place)
> > > > If it is a mistake, you could fix it.
> > >  From the above - this is the intention
> > > >    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.
> > > As seen from the above the problematic part is not in my
> > > driver, it is either DRM framework or HW display driver
> > > >    Don't use the sg_ dma api, use the
> > > > regular dma api instead.
> > > I use what DRM provides and dma_xxx if something is missed
> > > > 
> > > > 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?
> > > This is the case, see [5]
> > > > 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).
> > > Yes, I saw this code and it helped me to figure out
> > > where the use-case fails
> > > > 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;
> > > > 	}
> > > Agree, the crucial part here phys_to_page(phys): we need mfn->pfn
> > > > Now the question is, how do we get the pseudo-physical address of the
> > > > page?
> > > yes, this is the root of the problem
> > > > 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.
> > > Me neither, this is why I hope community will help here...
> > > Otherwise I'll need to patch kernel drivers if it's still possible.
> > If you can come up with a patch that only affects
> > xen_swiotlb_get_sgtable, and translates successfully void *cpu_addr into
> > a physical address using "at", I think I would take that patch. I would
> > recommend to test the patch on ARM32 too, where virt_to_phys reliably
> > fails.
> > 
> > I don't think we can ask the community to drop dma_get_sgtable on the
> > basis that the DMA api is broken.
> 

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