linux-kernel - Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqKJP3itMOueZD7fGH2b6VNFrTuozW5tWyKN3uBg4gYMzA@mail.gmail.com>
Date:   Thu, 26 Sep 2019 14:27:56 -0500
From:   Rob Herring <robh@...nel.org>
To:     Oleksandr Andrushchenko <Oleksandr_Andrushchenko@...m.com>
Cc:     Robin Murphy <robin.murphy@....com>,
        Julien Grall <julien.grall@....com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Juergen Gross <jgross@...e.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
        Oleksandr Andrushchenko <andr2000@...il.com>
Subject: Re: [RFC PATCH] xen/gntdev: Stop abusing DT of_dma_configure API

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