linux-kernel - Re: [PATCH 0/3] More ARM DMA ops cleanup

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: <5c617d66-f04b-df26-bf7a-7f479d081ac2@arm.com>
Date:   Tue, 30 Aug 2022 16:36:19 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Yongqin Liu <yongqin.liu@...aro.org>
Cc:     Christoph Hellwig <hch@....de>, linux@...linux.org.uk,
        linux-arm-kernel@...ts.infradead.org, m.szyprowski@...sung.com,
        arnd@...nel.org, iommu@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org,
        "Bajjuri, Praneeth" <praneeth@...com>,
        Sumit Semwal <sumit.semwal@...aro.org>
Subject: Re: [PATCH 0/3] More ARM DMA ops cleanup

On 2022-08-30 16:19, Yongqin Liu wrote:
> Hi, Robin
> 
> Thanks for the kind reply!
> 
> On Tue, 30 Aug 2022 at 17:48, Robin Murphy <robin.murphy@....com> wrote:
>>
>> On 2022-08-27 13:24, Yongqin Liu wrote:
>>> Hi, Robin, Christoph
>>>
>>> With the changes landed in the mainline kernel,
>>> one problem is exposed with our out of tree pvr module.
>>> Like the source here[1], arm_dma_ops.sync_single_for_cpu is called in
>>> the format like the following:
>>>       arm_dma_ops.sync_single_for_cpu(NULL, pStart, pEnd - pStart,
>>> DMA_FROM_DEVICE);
>>>
>>> Not sure if you could give some suggestions on what I should do next
>>> to make the pvr module work again.
>>
>> Wow, that driver reinvents so many standard APIs for no apparent reason
>> it's not even funny.
>>
>> Anyway, from a brief look it seemingly already knows how to call the DMA
>> API semi-correctly, so WTF that's doing behind an #ifdef, who knows?
>> However it's still so completely wrong in general - fundamentally broken
>> AArch64 set/way cache maintenance!? - that it looks largely beyond help.
>> "Throw CONFIG_DMA_API_DEBUG at it and cry" is about the extent of
>> support I'm prepared to provide for that mess.
> 
> For the moment, I do not care about the AArch64 lines, like if we only
> say the following two lines:
>      arm_dma_ops.sync_single_for_device(NULL, pStart, pEnd - pStart,
> DMA_TO_DEVICE);
>      arm_dma_ops.sync_single_for_cpu(NULL, pStart, pEnd - pStart,
> DMA_FROM_DEVICE);
> 
> Could you please give some suggestions for that?

Remove them. Then remove the #ifdef __arch64__ too, since the code under 
there is doing a passable impression of generic DMA API usage, as long 
as one ignores the bigger picture.

arm64 already uses dma-direct. To say you don't care about the arm64 
code when asking how to deal with ARM having now been converted to use 
dma-direct as well is supremely missing the point.

Robin.
> 
> Thanks,
> Yongqin Liu
> 
> 
>>> Thanks in advance!
>>>
>>> [1]: https://android-git.linaro.org/kernel/omap-modules.git/tree/pvr/services4/srvkm/env/linux/osfunc.c?h=android-mainline#n4615
>>>
>>> Thanks,
>>> Yongqin Liu
>>>
>>> On Thu, 21 Apr 2022 at 22:35, Robin Murphy <robin.murphy@....com> wrote:
>>>>
>>>> On 2022-04-21 15:13, Christoph Hellwig wrote:
>>>>> On Thu, Apr 21, 2022 at 12:36:56PM +0100, Robin Murphy wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Thanks to Christoph's latest series, I'm reminded that, if we're going
>>>>>> to give the ARM DMA ops some cleanup this cycle, it's as good a time as
>>>>>> any to dust off these old patches and add them on top as well. I've
>>>>>> based these on the arm-dma-direct branch which I assume matches the
>>>>>> patches posted at [1].
>>>>>
>>>>> All these do look sensible to me.  But weren't you working on replacing
>>>>> the ARM iommu dma_ops with dma-іommu anyway?
>>>>
>>>> Yes, that's somewhat entangled with the IOMMU bus ops stuff, so I'll
>>>> probably get to the point of having to revisit it in a couple of months
>>>> or so. These patches are off the bottom of that stack from my first
>>>> attempt, where the aim was to make the current ops the same shape first
>>>> so that the switch is then easier to reason about (particularly in terms
>>>> of sounding out any issues with the hooking up of dev->dma_coherent,
>>>> although your series will now be taking most of the load off there).
>>>>
>>>> Cheers,
>>>> Robin.
>>>
>>>
>>>
> 
> 
>