Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Subject: Re: [PATCH 0/2] riscv: implement Zicbom-based CMO instructions + the
 t-head variant
To:     Corentin Labbe <clabbe.montjoie@gmail.com>,
        Heiko Stuebner <heiko@sntech.de>
Cc:     palmer@dabbelt.com, paul.walmsley@sifive.com,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        wefu@redhat.com, guoren@kernel.org, atishp@atishpatra.org,
        anup@brainfault.org, mick@ics.forth.gr, cmuellner@linux.com,
        philipp.tomsich@vrull.eu
References: <20220307224620.1933061-1-heiko@sntech.de> <YllWTN+15CoskNBt@Red>
From:   Samuel Holland <samuel@sholland.org>
Message-ID: <70da24dd-2d03-fc49-151d-daabb315a5f6@sholland.org>
Date:   Fri, 15 Apr 2022 21:19:23 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <YllWTN+15CoskNBt@Red>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 4/15/22 6:26 AM, Corentin Labbe wrote:
> Le Mon, Mar 07, 2022 at 11:46:18PM +0100, Heiko Stuebner a écrit :
>> This series is based on the alternatives changes done in my svpbmt series
>> and thus also depends on Atish's isa-extension parsing series.
>>
>> It implements using the cache-management instructions from the  Zicbom-
>> extension to handle cache flush, etc actions on platforms needing them.
>>
>> SoCs using cpu cores from T-Head like the Allwinne D1 implement a
>> different set of cache instructions. But while they are different,
>> instructions they provide the same functionality, so a variant can
>> easly hook into the existing alternatives mechanism on those.
>>
>>
> 
> Hello
> 
> I am testing https://github.com/smaeul/linux.git branch:origin/riscv/d1-wip which contain this serie.
> 
> I am hitting a buffer corruption problem with DMA.
> The sun8i-ce crypto driver fail self tests due to "device overran destination buffer".
> In fact the buffer is not overran by device but by dma_map_single() operation.
> 
> The following small code show the problem:
> 
> dma_addr_t dma;
> u8 *buf;
> #define BSIZE 2048
> #define DMASIZE 16
> 
> buf = kmalloc(BSIZE, GFP_KERNEL | GFP_DMA);
> for (i = 0; i < BSIZE; i++)
>     buf[i] = 0xFE;
> print_hex_dump(KERN_INFO, "DMATEST1:", DUMP_PREFIX_NONE, 16, 4, buf, 256, false);
> dma = dma_map_single(ce->dev, buf, DMASIZE, DMA_FROM_DEVICE);

This function (through dma_direct_map_page()) ends up calling
arch_sync_dma_for_device(..., ..., DMA_FROM_DEVICE), which invalidates the CPU's
cache. This is the same thing other architectures do (at least arm, arm64,
openrisc, and powerpc). So this appears to be working as intended.

Regards,
Samuel

> dma_unmap_single(ce->dev, dma, DMASIZE, DMA_FROM_DEVICE);
> print_hex_dump(KERN_INFO, "DMATEST3:", DUMP_PREFIX_NONE, 16, 4, buf, 256, false);
> 
> Will lead to:
> [    2.960040] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    2.965354] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    2.970709] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    2.976069] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    2.981440] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    2.986814] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    2.992188] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    2.997560] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.002934] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.008307] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.013680] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.019054] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.024427] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.029802] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.035175] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.040546] DMATEST1:fefefefe fefefefe fefefefe fefefefe
> [    3.401647] DMATEST3:a9c3a9c3 a9c3a9c3 a9c3a9c3 a9c3a9c3
> [    3.406982] DMATEST3:a9c3a9c3 a9c3a9c3 a9c3a9c3 a9c3a9c3
> [    3.412350] DMATEST3:a9c3a9c3 a9c3a9c3 a9c3a9c3 a9c3a9c3
> [    3.417720] DMATEST3:a9c3a9c3 a9c3a9c3 a9c3a9c3 a9c3a9c3
> [    3.423094] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.428468] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.433841] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.439213] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.444588] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.449962] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.455334] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.460707] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.466081] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.471454] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.476828] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> [    3.482200] DMATEST3:fefefefe fefefefe fefefefe fefefefe
> 
> Even with no DMA action, the buffer is corrupted.
> 
> Regards
>