Re: [PATCH] usb: dwc2: extend treatment for incomplete transfer

[Boris ARZUR <boris@konbu.org>]

Hi Antti,

>we should be aligning the data[0] pointer inside the struct instead.
I believe you are correct. Now, I checked to see at runtime if temp->data was
aligned and it was. I cannot tell you why :) That code is copy-paste from
the tegra-ehci driver.

>with the alignment hacks altogether and hit the issue with a heavier
I feel bad about the alignment hacks as well, and would like the original
allocation from the URB thing to be aligned... no additional kmalloc,
no memcpy.

Is there a reason why we shouldn't try to fix that?

>pointer are separate and no corruptions should occur.
The corruptions themselves are bad, and should be cured.

Thanks, Boris.

Antti Seppälä wrote:
>Hi Guenter,
>
>On Wed, 19 Feb 2020 at 23:11, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> Yes, those patches didn't address the core problem. Can you test with the
>> attached two patches ?
>>
>> Thanks,
>> Guenter
>
>I took a look at your patch (usb: dwc2: Simplify DMA alignment code)
>and I don't believe it is correct.
>
>The patch re-introduces the dma_aligned_buffer struct and takes some
>care to align the beginning of the struct to dma cache lines. However
>we should be aligning the data[0] pointer inside the struct instead.
>With the code in the patch data[0] gets pushed to be at an offset from
>the alignment by kmalloc_ptr and old_xfer_buffer pointers. In other
>words data[0] is now not aligned to dma cache boundaries.
>
>Reviewing the code got me thinking that what if we stopped playing
>with the alignment hacks altogether and hit the issue with a heavier
>hammer instead? Attached you can find a new patch that introduces a
>list to keep track of the allocations. The code then looks up the
>entry from the list when it is time to restore the original pointer.
>This way the allocations for the aligned dma area and the original
>pointer are separate and no corruptions should occur.
>
>Thoughts, comments? I should note that the patch has received only
>light testing and not very thorough thinking. I can prepare a proper
>patch to be sent inline if the idea seems worth exploring further.
>
>-- 
>Antti