Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190925215006.12056-1-robh@kernel.org> <e898c025-32a7-1d2c-3501-c99556f7cdd4@arm.com>
 <1ae7f42e-bf93-b335-b543-653fae5cf49f@epam.com> <28440326-ed76-b014-c1b8-02125c3214b9@arm.com>
 <f63f55eb-969e-6364-5781-a227d0c04e4c@epam.com>
In-Reply-To: <f63f55eb-969e-6364-5781-a227d0c04e4c@epam.com>
From:   Rob Herring <robh@kernel.org>
Date:   Thu, 26 Sep 2019 14:27:56 -0500
Message-ID: <CAL_JsqKJP3itMOueZD7fGH2b6VNFrTuozW5tWyKN3uBg4gYMzA@mail.gmail.com>
Subject: Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API
To:     Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
Cc:     Robin Murphy <robin.murphy@arm.com>,
        Julien Grall <julien.grall@arm.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
        Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
        Oleksandr Andrushchenko <andr2000@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Sep 26, 2019 at 6:16 AM Oleksandr Andrushchenko
<Oleksandr_Andrushchenko@epam.com> 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