Re: WARNING: drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:364 vchiq_prepare_bulk_data

[Stefan Wahren <wahrenst@gmx.net>]

Hi,

Am 10.06.24 um 12:25 schrieb Robin Murphy:
> On 2024-06-10 10:24 am, Phil Elwell wrote:
>> On Mon, 10 Jun 2024 at 10:20, Arnd Bergmann <arnd@arndb.de> wrote:
>>>
>>> On Mon, Jun 10, 2024, at 11:15, Laurent Pinchart wrote:
>>>> On Mon, Jun 10, 2024 at 11:00:12AM +0200, Arnd Bergmann wrote:
>>>>> On Mon, Jun 10, 2024, at 10:26, Phil Elwell wrote:
>>>>>> On Mon, 10 Jun 2024 at 07:00, Arnd Bergmann <arnd@arndb.de> wrote:
>>>>>>
>>>>>> Why is swiotlb involved at all? The DMA controller on BCM2837 can
>>>>>> access all RAM that is visible to the ARM cores.
>>>>>
>>>>> When a device is not cache-coherent and the buffer is not
>>>>> cache aligned, we now use swiotlb to avoid clobbering data
>>>>> in the same cache line during DMA synchronization.
>>>>>
>>>>> We used to rely on kmalloc() returning buffers that are
>>>>> cacheline aligned, but that was very expensive.
>>>>
>>>> Could we reject buffers provided by userspace that are not
>>>> cache-aligned ?
>>>
>>> My guess is that this will likely break existing applications,
>>> in which case we cannot.
>>>
>>> It's probably a good idea to take a look at what buffers
>>> are actually passed by userspace today. That would also
>>> help decide how we allocate bounce buffers if we have to.
>>> E.g. it's a big difference if the buffers are always
>>> within a few bytes, kilobytes or megabytes.
>>>
>>>       Arnd
>>
>> vchiq sends partial cache lines at the start and of reads (as seen
>> from the ARM host) out of band, so the only misaligned DMA transfers
>> should be from ARM to VPU. This should not require a bounce buffer.
>
> Hmm, indeed the dma_kmalloc_safe() takes into account that unaligned
> DMA_TO_DEVICE does not need bouncing, so it would suggest that
> something's off in what vchiq is asking for.
I'm available to debug this further, but i need more guidance here.

At least i extend the output for the error case:

-               WARN_ON(len == 0);
-               WARN_ON(i && (i != (dma_buffers - 1)) && (len &
~PAGE_MASK));
-               WARN_ON(i && (addr & ~PAGE_MASK));
+               if (len == 0)
+                       pr_warn_once("%s: sg_dma_len() == 0\n", __func__);
+               else if (i && (i != (dma_buffers - 1)) && (len &
~PAGE_MASK))
+                       pr_warn_once("%s: following block not page
aligned\n", __func__);
+               else if (i && (addr & ~PAGE_MASK)) {
+                       pr_warn_once("%s: block %u, DMA address %pad
doesn't align with PAGE_MASK 0x%lx\n", __func__, i, &addr, PAGE_MASK);
+                       pr_warn_once("sg_dma_is_swiotlb: %d, dma_flags:
%x\n", sg_dma_is_swiotlb(sg), sg->dma_flags);
+               }

Example result:

[   84.180527] create_pagelist: block 1, DMA address 0x00000000f5f74800
doesn't align with PAGE_MASK 0xfffffffffffff000
[   84.180553] sg_dma_is_swiotlb: 0, dma_flags: 0

Is this helpful?

>
> Thanks,
> Robin.