Re: [Xen-devel] Xen ARM community call - meeting minutes and date for the next one

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 19 Jan 2017, Pooya Keshavarzi wrote:
> Hi Stefano,
> 
> On 01/13/2017 07:39 PM, Stefano Stabellini wrote:
> > On Fri, 13 Jan 2017, Pooya.Keshavarzi wrote:
> >> On 01/12/2017 07:50 PM, Stefano Stabellini wrote:
> >>> On Thu, 12 Jan 2017, Pooya.Keshavarzi wrote:
> >>>>
> >>>> Firstly sorry for the late reply on this.
> >>>>
> >>>> Regarding the problem with swiotlb-xen here are some more details:
> >>>>
> >>>> If we limit Dom0's memory such that only low-memory (up to 32-bit 
> >>>> addressable memory) is available to Dom0, then swiotlb-xen does not have 
> >>>> to use bounce buffers and the devices (e.g. USB, ethernet) would work.
> >>>>
> >>>> But when there is some high memory also available to Dom0, the 
> >>>> followings happen:
> >>>>  - If the the device address happens to be in the device's DMA window 
> >>>> (see xen_swiotlb_map_page()), then the device would work.
> >>>>  - Otherwise if it has to allocate and map a bounce buffer, then the 
> >>>> device would not work.
> >>>
> >>> From what you wrote it looks like the xen_swiotlb_map_page path: 
> >>>
> >>>   if (dma_capable(dev, dev_addr, size) &&
> >>>       !range_straddles_page_boundary(phys, size) &&
> >>>           !xen_arch_need_swiotlb(dev, phys, dev_addr) &&
> >>>           !swiotlb_force) {
> >>>           /* we are not interested in the dma_addr returned by
> >>>            * xen_dma_map_page, only in the potential cache flushes 
> >>> executed
> >>>            * by the function. */
> >>>           xen_dma_map_page(dev, page, dev_addr, offset, size, dir, attrs);
> >>>           return dev_addr;
> >>>   }
> >>>
> >>> works, but the other does not. Does it match your understanding? Have
> >>> you done any digging to find the reason why the bounce buffer code path
> >>> is broken on your platform?
> >>
> >> Yes, The above path works but the other one doesn't.
> >> I did some digging but failed to find out what's the problem. The returned 
> >> address from swiotlb_tbl_map_single() is within the memory range allocated 
> >> earlier for Xen software IO TLB and is dma capable, so it seem to be OK.
> >>
> >> What's your suggestion for further digging?
> > 
> > Is the device DMA coherent?
> No.
> 
> > I take that it fails even without running any guests, correct?
> Yes, fails only with Dom0.
> 
> > A few things come to mind:
> > 
> > - In xen_dma_map_page, does it take the local path or the foreign path
> >   (if(local)...) when it fails?
> When it fails, it takes the foreign path.
> 
> > - Check that xen_swiotlb_init initializes the swiotlb memory region
> >   appriopriately and that it falls in a memory range supported by the
> >   device.
> The only thing is this warning:
> xen:swiotlb_xen: Warning: only able to allocate 4 MB for software IO TLB
> 
> The allocation in the first memory bank can be solved by this, and then 
> swiotlb-xen allocates in the proper memory range.
> https://patchwork.kernel.org/patch/9347329/
> We had a similar issue.
> 
> 
> > - Check that xen_phys_to_bus(map) returns the right dev_addr. As a
> >   reference, I know that on some arm32 platforms it wouldn't return the
> >   right value.
> It returns the same address as map.
> 
>  
> > - Does the following patch fixes the issue for you?
> Yes :) it seems that it solves the issue for Ethernet.
> And now it takes the local path in xen_dma_map_page.
> 
> But why?

This is a genuine bug in swiotlb-xen. xen_swiotlb_map_page swaps the
original page for one from the swiotlb pool. The call to
swiotlb_tbl_map_single returns the new page. However, we are calling
xen_dma_map_page passing the dev_addr of the original page, rather than
the new one. xen_dma_map_page wrongly assumes that the page is foreign
because phys_addr and dma_addr don't match.

Thank you for reporting the issue and for testing. I'll submit a patch
shortly. I'll add your signed-off-by to it, if that's OK.


  
> > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> > index 87e6035..17c65fa 100644
> > --- a/drivers/xen/swiotlb-xen.c
> > +++ b/drivers/xen/swiotlb-xen.c
> > @@ -409,9 +409,9 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, 
> > struct page *page,
> >     if (map == SWIOTLB_MAP_ERROR)
> >             return DMA_ERROR_CODE;
> >  
> > +   dev_addr = xen_phys_to_bus(map);
> >     xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
> >                                     dev_addr, map & ~PAGE_MASK, size, dir, 
> > attrs);
> > -   dev_addr = xen_phys_to_bus(map);
> >  
> >     /*
> >      * Ensure that the address returned is DMA'ble 
> 
> 
> And following patch solves the issue for USB, MMC, SDC.
> 
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 7399782..69b5c62 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -567,6 +567,7 @@ xen_swiotlb_map_sg_attrs(struct device *hwdev, struct 
> scatterlist *sgl,
>                               sg_dma_len(sgl) = 0;
>                               return 0;
>                       }
> +                     dev_addr = xen_phys_to_bus(map);
>                       xen_dma_map_page(hwdev, pfn_to_page(map >> PAGE_SHIFT),
>                                               dev_addr,
>                                               map & ~PAGE_MASK,
> 
> 
> BR,
> Pooya
> 

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