* gnttab_map_refs and contiguous buffer @ 2017-03-23 15:40 Oleksandr Andrushchenko 2017-03-27 10:16 ` arm64: dma_to_phys/phys_to_dma need to be properly implemented Oleksandr Andrushchenko 0 siblings, 1 reply; 12+ messages in thread From: Oleksandr Andrushchenko @ 2017-03-23 15:40 UTC (permalink / raw) To: xen-devel 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-23 15:40 gnttab_map_refs and contiguous buffer Oleksandr Andrushchenko @ 2017-03-27 10:16 ` Oleksandr Andrushchenko 2017-03-27 20:23 ` Stefano Stabellini 0 siblings, 1 reply; 12+ messages in thread From: Oleksandr Andrushchenko @ 2017-03-27 10:16 UTC (permalink / raw) To: xen-devel; +Cc: Julien Grall, Stefano Stabellini 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-27 10:16 ` arm64: dma_to_phys/phys_to_dma need to be properly implemented Oleksandr Andrushchenko @ 2017-03-27 20:23 ` Stefano Stabellini 2017-03-28 6:42 ` Oleksandr Andrushchenko 0 siblings, 1 reply; 12+ messages in thread From: Stefano Stabellini @ 2017-03-27 20:23 UTC (permalink / raw) To: Oleksandr Andrushchenko; +Cc: xen-devel, Julien Grall, 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-27 20:23 ` Stefano Stabellini @ 2017-03-28 6:42 ` Oleksandr Andrushchenko 2017-03-28 12:40 ` Andrii Anisov 2017-03-28 19:20 ` Stefano Stabellini 0 siblings, 2 replies; 12+ messages in thread From: Oleksandr Andrushchenko @ 2017-03-28 6:42 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Julien Grall 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-28 6:42 ` Oleksandr Andrushchenko @ 2017-03-28 12:40 ` Andrii Anisov 2017-03-28 13:04 ` Oleksandr Andrushchenko 2017-03-28 19:20 ` Stefano Stabellini 1 sibling, 1 reply; 12+ messages in thread From: Andrii Anisov @ 2017-03-28 12:40 UTC (permalink / raw) To: Oleksandr Andrushchenko, Stefano Stabellini; +Cc: xen-devel, Julien Grall 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-28 12:40 ` Andrii Anisov @ 2017-03-28 13:04 ` Oleksandr Andrushchenko 0 siblings, 0 replies; 12+ messages in thread From: Oleksandr Andrushchenko @ 2017-03-28 13:04 UTC (permalink / raw) To: Andrii Anisov, Stefano Stabellini; +Cc: xen-devel, Julien Grall 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-28 6:42 ` Oleksandr Andrushchenko 2017-03-28 12:40 ` Andrii Anisov @ 2017-03-28 19:20 ` Stefano Stabellini 2017-03-29 6:17 ` Oleksandr Andrushchenko 1 sibling, 1 reply; 12+ messages in thread From: Stefano Stabellini @ 2017-03-28 19:20 UTC (permalink / raw) To: Oleksandr Andrushchenko; +Cc: xen-devel, Julien Grall, 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-28 19:20 ` Stefano Stabellini @ 2017-03-29 6:17 ` Oleksandr Andrushchenko 2017-03-29 22:36 ` Stefano Stabellini 0 siblings, 1 reply; 12+ messages in thread From: Oleksandr Andrushchenko @ 2017-03-29 6:17 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Julien Grall 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-29 6:17 ` Oleksandr Andrushchenko @ 2017-03-29 22:36 ` Stefano Stabellini 2017-03-29 22:48 ` Julien Grall 2017-03-31 8:15 ` Oleksandr Andrushchenko 0 siblings, 2 replies; 12+ messages in thread From: Stefano Stabellini @ 2017-03-29 22:36 UTC (permalink / raw) To: Oleksandr Andrushchenko; +Cc: xen-devel, Julien Grall, 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-29 22:36 ` Stefano Stabellini @ 2017-03-29 22:48 ` Julien Grall 2017-03-31 8:15 ` Oleksandr Andrushchenko 1 sibling, 0 replies; 12+ messages in thread From: Julien Grall @ 2017-03-29 22:48 UTC (permalink / raw) To: Stefano Stabellini, Oleksandr Andrushchenko; +Cc: xen-devel, nd 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-29 22:36 ` Stefano Stabellini 2017-03-29 22:48 ` Julien Grall @ 2017-03-31 8:15 ` Oleksandr Andrushchenko 2017-03-31 17:56 ` Stefano Stabellini 1 sibling, 1 reply; 12+ messages in thread From: Oleksandr Andrushchenko @ 2017-03-31 8:15 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Julien Grall 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: arm64: dma_to_phys/phys_to_dma need to be properly implemented 2017-03-31 8:15 ` Oleksandr Andrushchenko @ 2017-03-31 17:56 ` Stefano Stabellini 0 siblings, 0 replies; 12+ messages in thread From: Stefano Stabellini @ 2017-03-31 17:56 UTC (permalink / raw) To: Oleksandr Andrushchenko; +Cc: xen-devel, Julien Grall, 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-31 17:56 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-23 15:40 gnttab_map_refs and contiguous buffer Oleksandr Andrushchenko 2017-03-27 10:16 ` arm64: dma_to_phys/phys_to_dma need to be properly implemented Oleksandr Andrushchenko 2017-03-27 20:23 ` Stefano Stabellini 2017-03-28 6:42 ` Oleksandr Andrushchenko 2017-03-28 12:40 ` Andrii Anisov 2017-03-28 13:04 ` Oleksandr Andrushchenko 2017-03-28 19:20 ` Stefano Stabellini 2017-03-29 6:17 ` Oleksandr Andrushchenko 2017-03-29 22:36 ` Stefano Stabellini 2017-03-29 22:48 ` Julien Grall 2017-03-31 8:15 ` Oleksandr Andrushchenko 2017-03-31 17:56 ` Stefano Stabellini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.