linux-kernel - Re: use of dma_direct_set_offset in (allwinner) drivers

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]

Date:   Wed, 4 Nov 2020 09:14:11 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Christoph Hellwig <hch@....de>
Cc:     Chen-Yu Tsai <wens@...e.org>, Yong Deng <yong.deng@...ewell.com>,
        Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        iommu@...ts.linux-foundation.org, linux-media@...r.kernel.org,
        devel@...verdev.osuosl.org
Subject: Re: use of dma_direct_set_offset in (allwinner) drivers

Hi Christoph,

On Tue, Nov 03, 2020 at 10:55:38AM +0100, Christoph Hellwig wrote:
> Linux 5.10-rc1 switched from having a single dma offset in struct device
> to a set of DMA ranges, and introduced a new helper to set them,
> dma_direct_set_offset.
> 
> This in fact surfaced that a bunch of drivers that violate our layering
> and set the offset from drivers, which meant we had to reluctantly
> export the symbol to set up the DMA range.
> 
> The drivers are:
> 
> drivers/gpu/drm/sun4i/sun4i_backend.c
> 
>   This just use dma_direct_set_offset as a fallback.  Is there any good
>   reason to not just kill off the fallback?
> 
> drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> 
>   Same as above.

So, the history of this is:

  - We initially introduced the support for those two controllers
    assuming that there was a direct mapping between the physical and
    DMA addresses. It turns out it didn't and the DMA accesses were
    going through a secondary, dedicated, bus that didn't have the same
    mapping of the RAM than the CPU.

    4690803b09c6 ("drm/sun4i: backend: Offset layer buffer address by DRAM starting address")

  - This dedicated bus is undocumented and barely used in the vendor
    kernel so this was overlooked, and it's fairly hard to get infos on
    it for all the SoCs we support. We added the DT support for it
    though on some SoCs we had enough infos to do so:

    c43a4469402f ("dt-bindings: interconnect: Add a dma interconnect name")
    22f88e311399 ("ARM: dts: sun5i: Add the MBUS controller")

    This explains the check on the interconnect property

  - However, due to the stable DT rule, we still need to operate without
    regressions on older DTs that wouldn't have that property (and for
    SoCs we haven't figured out). Hence the fallback.

> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> 
>   This driver unconditionally sets the offset.  Why can't we do this
>   in the device tree?
> 
> drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> 
>   Same as above.
>

We should make those two match the previous ones, but we'll have the
same issue here eventually. Most likely they were never ran on an SoC
for which we have the MBUS figured out.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)