[Xen-devel] Upstream Dom0 DRM problems regarding swiotlb

[Konrad Rzeszutek Wilk]

On Fri, Feb 15, 2019 at 11:07:22AM -0500, Michael Labriola wrote:
> On Fri, Feb 15, 2019 at 12:57 AM Juergen Gross <jgross at suse.com> wrote:
> >
> > On 14/02/2019 18:57, Christoph Hellwig wrote:
> > > On Thu, Feb 14, 2019 at 07:03:38AM +0100, Juergen Gross wrote:
> > >>> The thing which is different between Xen PV guests and most others (all
> > >>> others(?), now that Lguest and UML have been dropped) is that what Linux
> > >>> thinks of as PFN $N isn't necessarily adjacent to PFN $N+1 in system
> > >>> physical address space.
> > >>>
> > >>> Therefore, code which has a buffer spanning a page boundary can't just
> > >>> convert a pointer to the buffer into a physical address, and hand that
> > >>> address to a device.  You generally end up with either memory corruption
> > >>> (DMA hitting the wrong page allocated to the guest), or an IOMMU fault
> > >>> (DMA hitting a pages which isn't allocated to the guest).
> > >
> > > The Linux DMA API allows for dma_map_page / dma_map_single calls to
> > > spawn 4k boundaries.  If Xen doesn't support that it will have to bounce
> > > buffer for that case (and get horrible performance).
> > >
> > > But the latter text seems to agree with that.  So what is the actual
> > > problem that started this discussion?
> > >
> >
> > See https://lists.xen.org/archives/html/xen-devel/2019-02/threads.html#00818
> 
> I believe the actual problem is either:
> 
> 1) The radeon/amdgpu drivers are calling ttm_populate_and_map_pages()
> which *should* work on a Xen PV host, but doesn't and needs to be
> fixed.
> 
> or
> 
> 2) The radeon/amdgpu drivers are calling ttm_populate_and_map_pages()
> which *cannot* work in Xen, and they should go back to calling
> ttm_dma_populate() in that case.

The Nvidia one has this (correct):

1583 #if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)                        
1584         if (swiotlb_nr_tbl()) {                                                 
1585                 return ttm_dma_populate((void *)ttm, dev, ctx);                 
1586         }                                                                       
1587 #endif  

The Radeon has this - where now it adds 'need_swiotlb':

695 #ifdef CONFIG_SWIOTLB                                                           
 696         if (rdev->need_swiotlb && swiotlb_nr_tbl()) {                           
 697                 return ttm_dma_populate(&gtt->ttm, rdev->dev, ctx);             
 698         }                                                                       
 699 #endif   

The problem is fairly simple - the platform _requires_ to use
DMA API.

But the driver's have their own 'need_swiotlb' which ignores the platform
and sets it based on the device's DMA width:

  rdev->need_swiotlb = drm_get_max_iomem() > ((u64)1 << dma_bits);

There should be an extra check to see if the platform requires
to use DMA API.

> 
> I'm having a hard time figuring out which of those is correct.
> 
> -- 
> Michael D Labriola
> 21 Rip Van Winkle Cir
> Warwick, RI 02886
> 401-316-9844 (cell)
> 401-848-8871 (work)
> 401-234-1306 (home)