Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1732645 > unrolled thread

[BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

Back to article view | Back to linux.kernel

Contents

  [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback Stefano Stabellini <sstabellini@kernel.org> - 2017-09-15 01:30 +0200
    Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback Greg KH <gregkh@linuxfoundation.org> - 2017-09-16 03:30 +0200
      Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap  callback Stefano Stabellini <sstabellini@kernel.org> - 2017-09-18 20:10 +0200
        Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-09-18 21:00 +0200
          Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap  callback Stefano Stabellini <sstabellini@kernel.org> - 2017-09-20 00:00 +0200
        Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap  callback Stefano Stabellini <sstabellini@kernel.org> - 2017-09-21 02:40 +0200
        Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback Juergen Gross <jgross@suse.com> - 2017-09-22 14:20 +0200

#1732645 — [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

Hi all,

We are getting reports from Xen on ARM users about DMA issues. The
problem is that the commit below
(7e91c7df29b5e196de3dc6f086c8937973bd0b88) is necessary to support mmap
on Xen on ARM. It is self-contained and doesn't affect anything outside
of Xen on ARM, so I think is a good candidate for backporting. It went
upstream in 4.11.


Could you please backport the following commit:

  commit 7e91c7df29b5e196de3dc6f086c8937973bd0b88
  Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
  Date:   Tue Feb 7 19:58:02 2017 +0200
  
      swiotlb-xen: implement xen_swiotlb_dma_mmap callback
      
      This function creates userspace mapping for the DMA-coherent memory.
    
to the stable trees up until 3.14?


Because of 00085f1efa387a8ce100e3734920f7639c80caa3 "dma-mapping: use
unsigned long for dma_attrs", the appended patch (to be applied on top)
is required for trees older than 4.8. 


Thank you!

- Stefano


diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index a0a819c..c6d47e5 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -693,7 +693,7 @@ EXPORT_SYMBOL_GPL(xen_swiotlb_set_dma_mask);
 int
 xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 		     void *cpu_addr, dma_addr_t dma_addr, size_t size,
-		     unsigned long attrs)
+		     struct dma_attrs *attrs)
 {
 #if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
 	if (__generic_dma_ops(dev)->mmap)
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index fab4fb9..4d7fdbf 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -62,5 +62,5 @@ xen_swiotlb_set_dma_mask(struct device *dev, u64 dma_mask);
 extern int
 xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 		     void *cpu_addr, dma_addr_t dma_addr, size_t size,
-		     unsigned long attrs);
+		     struct dma_attrs *attrs);
 #endif /* __LINUX_SWIOTLB_XEN_H */

[toc] | [next] | [standalone]

#1733187

On Thu, Sep 14, 2017 at 04:23:05PM -0700, Stefano Stabellini wrote:
> Hi all,
> 
> We are getting reports from Xen on ARM users about DMA issues. The
> problem is that the commit below
> (7e91c7df29b5e196de3dc6f086c8937973bd0b88) is necessary to support mmap
> on Xen on ARM. It is self-contained and doesn't affect anything outside
> of Xen on ARM, so I think is a good candidate for backporting. It went
> upstream in 4.11.

But it's a new feature, right?  How does that fit the stable kernel
rules?

> 
> 
> Could you please backport the following commit:
> 
>   commit 7e91c7df29b5e196de3dc6f086c8937973bd0b88
>   Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>   Date:   Tue Feb 7 19:58:02 2017 +0200
>   
>       swiotlb-xen: implement xen_swiotlb_dma_mmap callback
>       
>       This function creates userspace mapping for the DMA-coherent memory.
>     
> to the stable trees up until 3.14?
> 
> 
> Because of 00085f1efa387a8ce100e3734920f7639c80caa3 "dma-mapping: use
> unsigned long for dma_attrs", the appended patch (to be applied on top)
> is required for trees older than 4.8. 

What does the kvm maintainers think about this?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1734350 — Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

On Fri, 15 Sep 2017, Greg KH wrote:
> On Thu, Sep 14, 2017 at 04:23:05PM -0700, Stefano Stabellini wrote:
> > Hi all,
> > 
> > We are getting reports from Xen on ARM users about DMA issues. The
> > problem is that the commit below
> > (7e91c7df29b5e196de3dc6f086c8937973bd0b88) is necessary to support mmap
> > on Xen on ARM. It is self-contained and doesn't affect anything outside
> > of Xen on ARM, so I think is a good candidate for backporting. It went
> > upstream in 4.11.
> 
> But it's a new feature, right?  How does that fit the stable kernel
> rules?

It implements a previously unimplemented function (mmap), although it
calls the generic functions to do it. Yes, I agree with you that it
can be classified as a new feature. If that is against the stable kernel
rules, then please discard this request.

FYI the reason why it didn't raise a flag in my mind is that users
reported something like "unhandled alignment fault (11) at
0xffffa6048080, esr 0x92000061", which really looks more like a bug.


> > Could you please backport the following commit:
> > 
> >   commit 7e91c7df29b5e196de3dc6f086c8937973bd0b88
> >   Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >   Date:   Tue Feb 7 19:58:02 2017 +0200
> >   
> >       swiotlb-xen: implement xen_swiotlb_dma_mmap callback
> >       
> >       This function creates userspace mapping for the DMA-coherent memory.
> >     
> > to the stable trees up until 3.14?
> > 
> > 
> > Because of 00085f1efa387a8ce100e3734920f7639c80caa3 "dma-mapping: use
> > unsigned long for dma_attrs", the appended patch (to be applied on top)
> > is required for trees older than 4.8. 
> 
> What does the kvm maintainers think about this?

That would be the Xen maintainers right? In that case, Boris, Juergen,
please let us know what you think.

[toc] | [prev] | [next] | [standalone]

#1734394

On 09/18/2017 02:08 PM, Stefano Stabellini wrote:
> On Fri, 15 Sep 2017, Greg KH wrote:
>> On Thu, Sep 14, 2017 at 04:23:05PM -0700, Stefano Stabellini wrote:
>>> Hi all,
>>>
>>> We are getting reports from Xen on ARM users about DMA issues. The
>>> problem is that the commit below
>>> (7e91c7df29b5e196de3dc6f086c8937973bd0b88) is necessary to support mmap
>>> on Xen on ARM. It is self-contained and doesn't affect anything outside
>>> of Xen on ARM, so I think is a good candidate for backporting. It went
>>> upstream in 4.11.
>> But it's a new feature, right?  How does that fit the stable kernel
>> rules?
> It implements a previously unimplemented function (mmap), although it
> calls the generic functions to do it. Yes, I agree with you that it
> can be classified as a new feature. If that is against the stable kernel
> rules, then please discard this request.
>
> FYI the reason why it didn't raise a flag in my mind is that users
> reported something like "unhandled alignment fault (11) at
> 0xffffa6048080, esr 0x92000061", which really looks more like a bug.
>
>
>>> Could you please backport the following commit:
>>>
>>>   commit 7e91c7df29b5e196de3dc6f086c8937973bd0b88
>>>   Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>   Date:   Tue Feb 7 19:58:02 2017 +0200
>>>   
>>>       swiotlb-xen: implement xen_swiotlb_dma_mmap callback
>>>       
>>>       This function creates userspace mapping for the DMA-coherent memory.
>>>     
>>> to the stable trees up until 3.14?
>>>
>>>
>>> Because of 00085f1efa387a8ce100e3734920f7639c80caa3 "dma-mapping: use
>>> unsigned long for dma_attrs", the appended patch (to be applied on top)
>>> is required for trees older than 4.8. 
>> What does the kvm maintainers think about this?
> That would be the Xen maintainers right? In that case, Boris, Juergen,
> please let us know what you think.


This is a nop for x86 so it's safe from that perspective. I can't find
mmap op for ARM though (xen_get_dma_ops(dev)->mmap).

-boris

[toc] | [prev] | [next] | [standalone]

#1735288 — Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

On Mon, 18 Sep 2017, Boris Ostrovsky wrote:
> On 09/18/2017 02:08 PM, Stefano Stabellini wrote:
> > On Fri, 15 Sep 2017, Greg KH wrote:
> >> On Thu, Sep 14, 2017 at 04:23:05PM -0700, Stefano Stabellini wrote:
> >>> Hi all,
> >>>
> >>> We are getting reports from Xen on ARM users about DMA issues. The
> >>> problem is that the commit below
> >>> (7e91c7df29b5e196de3dc6f086c8937973bd0b88) is necessary to support mmap
> >>> on Xen on ARM. It is self-contained and doesn't affect anything outside
> >>> of Xen on ARM, so I think is a good candidate for backporting. It went
> >>> upstream in 4.11.
> >> But it's a new feature, right?  How does that fit the stable kernel
> >> rules?
> > It implements a previously unimplemented function (mmap), although it
> > calls the generic functions to do it. Yes, I agree with you that it
> > can be classified as a new feature. If that is against the stable kernel
> > rules, then please discard this request.
> >
> > FYI the reason why it didn't raise a flag in my mind is that users
> > reported something like "unhandled alignment fault (11) at
> > 0xffffa6048080, esr 0x92000061", which really looks more like a bug.
> >
> >
> >>> Could you please backport the following commit:
> >>>
> >>>   commit 7e91c7df29b5e196de3dc6f086c8937973bd0b88
> >>>   Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>>   Date:   Tue Feb 7 19:58:02 2017 +0200
> >>>   
> >>>       swiotlb-xen: implement xen_swiotlb_dma_mmap callback
> >>>       
> >>>       This function creates userspace mapping for the DMA-coherent memory.
> >>>     
> >>> to the stable trees up until 3.14?
> >>>
> >>>
> >>> Because of 00085f1efa387a8ce100e3734920f7639c80caa3 "dma-mapping: use
> >>> unsigned long for dma_attrs", the appended patch (to be applied on top)
> >>> is required for trees older than 4.8. 
> >> What does the kvm maintainers think about this?
> > That would be the Xen maintainers right? In that case, Boris, Juergen,
> > please let us know what you think.
> 
> 
> This is a nop for x86 so it's safe from that perspective. I can't find
> mmap op for ARM though (xen_get_dma_ops(dev)->mmap).

arch/arm/mm/dma-mapping.c:arm_dma_mmap
arch/arm/mm/dma-mapping.c:arm_coherent_dma_mmap
arch/arm64/mm/dma-mapping.c:__swiotlb_mmap

[toc] | [prev] | [next] | [standalone]

#1736291 — Re: [BACKPORT] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

[Multipart message — attachments visible in raw view] — view raw

On Wed, 20 Sep 2017, Leonard Crestez wrote:
> On Mon, 2017-09-18 at 11:08 -0700, Stefano Stabellini wrote:
> On Fri, 15 Sep 2017, Greg KH wrote:
> > On Thu, Sep 14, 2017 at 04:23:05PM -0700, Stefano Stabellini wrote:
> > > Hi all,
> > > 
> > > We are getting reports from Xen on ARM users about DMA issues. The
> > > problem is that the commit below
> > > (7e91c7df29b5e196de3dc6f086c8937973bd0b88) is necessary to support mmap
> > > on Xen on ARM. It is self-contained and doesn't affect anything outside
> > > of Xen on ARM, so I think is a good candidate for backporting. It went
> > > upstream in 4.11.
> > 
> > But it's a new feature, right?  How does that fit the stable kernel
> > rules?
> 
> 
> It implements a previously unimplemented function (mmap), although it
> calls the generic functions to do it. Yes, I agree with you that it
> can be classified as a new feature. If that is against the stable kernel
> rules, then please discard this request.
> 
> 
> FYI the reason why it didn't raise a flag in my mind is that users
> reported something like "unhandled alignment fault (11) at
> 0xffffa6048080, esr 0x92000061", which really looks more like a bug.
> 
> I am the one who reported this, on the #xenarm IRC channel.

Thank you for jumping into this thread.


> Not implementing mmap in dma_map_ops means that dma_common_mmap is
> called by dma_map_attrs as a fallback. The end result is not something
> like -ENOSYS but what seem to be corrupt mappings.
> 
> However I agree that backporting might be excessive. I ran into this by
> experimenting with using a GPU from dom0. It seems reasonable to get
> kernel crashes if you try this kind of stuff.
> 
> This patch results in calling __swiotlb_mmap instead of
> dma_common_mmap. I don't know the implementation details of the DMA api
> but the interesting difference between these paths seems to be the way
> pfn is fetched (from dma_addr instead of the kernel virt addr).

Yes, on ARM and ARM64 dma_map_ops functions can return pages for which
virt_to_page doesn't work as expected (for example on ARM alloc_coherent
returns an ioremap'ped virtual address, I don't remember the details of
the ARM64 implementation right now). This is why the dma_map_ops
functions are implemented by looking up the physical address from the
dma address.

[toc] | [prev] | [next] | [standalone]

#1737441

On 18/09/17 20:08, Stefano Stabellini wrote:
> On Fri, 15 Sep 2017, Greg KH wrote:
>> On Thu, Sep 14, 2017 at 04:23:05PM -0700, Stefano Stabellini wrote:
>>> Hi all,
>>>
>>> We are getting reports from Xen on ARM users about DMA issues. The
>>> problem is that the commit below
>>> (7e91c7df29b5e196de3dc6f086c8937973bd0b88) is necessary to support mmap
>>> on Xen on ARM. It is self-contained and doesn't affect anything outside
>>> of Xen on ARM, so I think is a good candidate for backporting. It went
>>> upstream in 4.11.
>>
>> But it's a new feature, right?  How does that fit the stable kernel
>> rules?
> 
> It implements a previously unimplemented function (mmap), although it
> calls the generic functions to do it. Yes, I agree with you that it
> can be classified as a new feature. If that is against the stable kernel
> rules, then please discard this request.
> 
> FYI the reason why it didn't raise a flag in my mind is that users
> reported something like "unhandled alignment fault (11) at
> 0xffffa6048080, esr 0x92000061", which really looks more like a bug.
> 
> 
>>> Could you please backport the following commit:
>>>
>>>   commit 7e91c7df29b5e196de3dc6f086c8937973bd0b88
>>>   Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>   Date:   Tue Feb 7 19:58:02 2017 +0200
>>>   
>>>       swiotlb-xen: implement xen_swiotlb_dma_mmap callback
>>>       
>>>       This function creates userspace mapping for the DMA-coherent memory.
>>>     
>>> to the stable trees up until 3.14?
>>>
>>>
>>> Because of 00085f1efa387a8ce100e3734920f7639c80caa3 "dma-mapping: use
>>> unsigned long for dma_attrs", the appended patch (to be applied on top)
>>> is required for trees older than 4.8. 
>>
>> What does the kvm maintainers think about this?
> 
> That would be the Xen maintainers right? In that case, Boris, Juergen,
> please let us know what you think.
> 

I have no specific preference.


Juergen

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web