linux-kernel - Re: [PATCH v2 3/3] arch/*/: remove CONFIG_VIRT_TO

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, 29 Jun 2022 09:38:00 +1200
From:   Michael Schmitz <schmitzmic@...il.com>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        scsi <linux-scsi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Jakub Kicinski <kuba@...nel.org>,
        Christoph Hellwig <hch@...radead.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Robin Murphy <robin.murphy@....com>,
        Linux IOMMU <iommu@...ts.linux-foundation.org>,
        Khalid Aziz <khalid@...ehiking.org>,
        "Maciej W . Rozycki" <macro@...am.me.uk>,
        Matt Wang <wwentao@...are.com>,
        Miquel van Smoorenburg <mikevs@...all.net>,
        Mark Salyzyn <salyzyn@...roid.com>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Linux-Arch <linux-arch@...r.kernel.org>,
        alpha <linux-alpha@...r.kernel.org>,
        linux-m68k <linux-m68k@...ts.linux-m68k.org>,
        Parisc List <linux-parisc@...r.kernel.org>,
        Denis Efremov <efremov@...ux.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>
Subject: Re: [PATCH v2 3/3] arch/*/: remove CONFIG_VIRT_TO_BUS

Hi Arnd,

On 28/06/22 19:08, Arnd Bergmann wrote:
> On Tue, Jun 28, 2022 at 5:25 AM Michael Schmitz <schmitzmic@...il.com> wrote:
>> Am 28.06.2022 um 09:12 schrieb Michael Schmitz:
>>
>> Leaving the bounce buffer handling in place, and taking a few other
>> liberties - this is what converting the easiest case (a3000 SCSI) might
>> look like. Any obvious mistakes? The mvme147 driver would be very
>> similar to handle (after conversion to a platform device).
>>
>> The driver allocates bounce buffers using kmalloc if it hits an
>> unaligned data buffer - can such buffers still even happen these days?
>> If I understand dma_map_single() correctly, the resulting dma handle
>> would be equally misaligned?
>>
>> To allocate a bounce buffer, would it be OK to use dma_alloc_coherent()
>> even though AFAIU memory used for DMA buffers generally isn't consistent
>> on m68k?
> I think it makes sense to skip the bounce buffering as you do here:
> the only standardized way we have for integrating that part is to
> use the swiotlb infrastructure, but as you mentioned earlier that
> part is probably too resource-heavy here for Amiga.
OK, leaving the old custom logic in place allows to convert the 24 bit 
DMA drivers more easily.
>
> I see two other problems with your patch though:
>
> a) you still duplicate the cache handling: the cache_clear()/cache_push()
> is supposed to already be done by dma_map_single() when the device
> is not cache-coherent.

That's one of the 'liberties' I alluded to. The reason I left these in 
is that I'm none too certain what device feature the DMA API uses to 
decide a device isn't cache-coherent. If it's dev->coherent_dma_mask, 
the way I set up the device in the a3000 driver should leave the 
coherent mask unchanged. For the Zorro drivers, devices are set up to 
use the same storage to store normal and coherent masks - something we 
most likely want to change. I need to think about the ramifications of 
that.

Note that zorro_esp.c uses dma_sync_single_for_device() and uses a 32 
bit coherent DMA mask which does work OK. I might  ask Adrian to test a 
change to only set dev->dma_mask, and drop the 
dma_sync_single_for_device() calls if there's any doubt on this aspect.

> b) The bounce buffer is never mapped here, instead you have the
> virt_to_phys() here, which is not the same. I think you need to map
> the pointer that actually gets passed down to the device after deciding
> to use a bouce buffer or not.

I hadn't realized that I can map the bounce buffer just as it's done for 
the SCp data buffer. Should have been obvious, but I'm still learning 
about the DMA API.

I've updated the patch now, will re-send as part of a complete series 
once done.

Cheers,

     Michael


>
>       Arnd