Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API

[Rob Herring]

On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko
<Oleksandr_Andrushchenko@xxxxxxxx> wrote:
>
> On 9/26/19 1:46 PM, Robin Murphy wrote:
> > On 2019-09-26 11:17 am, Oleksandr Andrushchenko wrote:
> >>
> >> On 9/26/19 12:49 PM, Julien Grall wrote:
> >>> Hi Rob,
> >>>
> >>>
> >>> On 9/25/19 10:50 PM, Rob Herring wrote:
> >>>> As the comment says, this isn't a DT based device. of_dma_configure()
> >>>> is going to stop allowing a NULL DT node, so this needs to be fixed.
> >>>
> >>> And this can't work on arch not selecting CONFIG_OF and can select
> >>> CONFIG_XEN_GRANT_DMA_ALLOC.
> >>>
> >>> We are lucky enough on x86 because, AFAICT, arch_setup_dma_ops is just
> >>> a nop.
> >>>
> >> No luck is needed as [1] does nothing for those platforms not using
> >> CONFIG_OF
> >>>>
> >>>> Not sure exactly what setup besides arch_setup_dma_ops is needed...
> >>>
> >>> We probably want to update dma_mask, coherent_dma_mask and
> >>> dma_pfn_offset.
> >>>
> >>> Also, while look at of_configure_dma, I noticed that we consider the
> >>> DMA will not be coherent for the grant-table. Oleksandr, do you know
> >>> why they can't be coherent?
> >> The main and the only reason to use of_configure_dma is that if we don't
> >> then we
> >> are about to stay with dma_dummy_ops [2]. It effectively means that
> >> operations on dma-bufs
> >> will end up returning errors, like [3], [4], thus not making it possible
> >> for Xen PV DRM and DMA
> >> part of gntdev driver to do what we need (dma-bufs in our use-cases
> >> allow zero-copying
> >> while using graphics buffers and many more).
> >>
> >> I didn't find any better way of achieving that, but of_configure_dma...
> >> If there is any better solution which will not break the existing
> >> functionality then
> >> I will definitely change the drivers so we do not abuse DT )
> >> Before that, please keep in mind that merging this RFC will break Xen PV
> >> DRM +
> >> DMA buf support in gntdev...
> >> Hope we can work out some acceptable solution, so everyone is happy
> >
> > As I mentioned elsewhere, the recent dma-direct rework means that
> > dma_dummy_ops are now only explicitly installed for the ACPI error
> > case, so - much as I may dislike it - you should get regular
> > (direct/SWIOTLB) ops by default again.
> Ah, my bad, I missed that change. So, if no dummy dma ops are to be used
> then
> I believe we can apply both changes, e.g. remove of_dma_configure from
> both of the drivers.

What about the dma masks? I think there's a default setup, but it is
considered a driver bug to not set its mask. xen_drm_front sets the
coherent_dma_mask (why only 32-bits though?), but not the dma_mask.
gntdev is doing neither. I could copy out what of_dma_configure does
but better for the Xen folks to decide what is needed or not and test
the change. I'm not setup to test any of this.

Rob