Message-ID: <20220820125717.588722-1-tudor.ambarus@microchip.com>
Date:   Sat, 20 Aug 2022 15:56:44 +0300
From:   Tudor Ambarus <tudor.ambarus@...rochip.com>
To:     <vkoul@...nel.org>, <peda@...ntia.se>, <du@...ntia.se>,
        <regressions@...mhuis.info>
CC:     <ludovic.desroches@...rochip.com>, <maciej.sosnowski@...el.com>,
        <tudor.ambarus@...rochip.com>, <dan.j.williams@...el.com>,
        <nicolas.ferre@...rochip.com>, <mripard@...nel.org>,
        <torfl6749@...il.com>, <linux-kernel@...r.kernel.org>,
        <dmaengine@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: [PATCH 00/33] dmaengine: at_hdmac: Fix concurrency bugs and then convert to virt-dma

Peter Rosin reported a memory corryption bug on Atmel SAMA5D31 that
happened under high CPU load and low memory resources. See:
https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/

The problem was two fold:
1/ The atmel nand driver uses dma_map_single() but failed to call its
dma_unmap_single() counterpart. As the DMA address space is a shared
resource, one could render the machine unusable by consuming all DMA
addresses. Also, failing to call dma_unmap_single translated to failing
to invalidate the data cache withing that specific region. Since SAMA5D3
uses VIPT (Virtually Indexed Physically Tagged) cache model, cache aliases
occured. The issue occurs when a physical memory page is mapped twice (or
more) in the virtual memory space. With VIPT models, the choice of the
cache line depends on the virtual address so even if two different virtual
addresses are mapped to point to the very same physical address, they may
use two different cache lines. Then dma_map_single() or dma_map_sg()
functions, only cleans / invalidates the data cache lines associated to
their virtual address argument. The cache lines used by the second virtual
memory address won't be cleaned / invalidated hence cache coherency issues
may occur. This was fixed at:
https://lore.kernel.org/linux-mtd/20220728074014.145406-1-tudor.ambarus@microchip.com/

2/ at_hdmac driver had poor list handling and concurrency bugs. We
experienced calling of the completion call twice for the same descriptor.

Fix all the concurrency bugs in at_hdmac driver and then convert the driver
to use virt-dma.

The series was tested with NAND (prep_dma_memcpy), MMC (prep_dma_slave_sg),
usart (cyclic mode), dmatest (memcpy, memset). All went fine. With the
conversion to virt-dma I replaced the election of a new transfer in the
tasklet with the election of the new transfer in the interrupt handler. We
should have a shorter idle window as we remove the scheduling latency of
the tasklet. Using mtd_speedtest showed similar performances when using
NAND with DMA.

There are other some cosmetic patches that could facelift this 13 years old
driver, but I'll let those for another series.

Tudor Ambarus (33):
  dmaengine: at_hdmac: Keep register definitions and structures private
    to at_hdmac.c
  dmaengine: at_hdmac: Use bitfield access macros
  dmaengine: at_hdmac: Rename "dma_common" to "dma_device"
  dmaengine: at_hdmac: Rename "chan_common" to "dma_chan"
  dmaengine: at_hdmac: Remove unused member of at_dma_chan
  dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when
    txstate is NULL
  dmaengine: at_hdmac: Fix at_lli struct definition
  dmaengine: at_hdmac: Don't start transactions at tx_submit level
  dmaengine: at_hdmac: Start transfer for cyclic channels in
    issue_pending
  dmaengine: at_hdmac: Fix premature completion of desc in issue_pending
  dmaengine: at_hdmac: Do not call the complete callback on
    device_terminate_all
  dmaengine: at_hdmac: Protect atchan->status with the channel lock
  dmaengine: at_hdmac: Fix concurrency problems by removing
    atc_complete_all()
  dmaengine: at_hdmac: Fix concurrency over descriptor
  dmaengine: at_hdmac: Free the memset buf without holding the chan lock
  dmaengine: at_hdmac: Fix concurrency over the active list
  dmaengine: at_hdmac: Fix descriptor handling when issuing it to
    hardware
  dmaengine: at_hdmac: Fix completion of unissued descriptor in case of
    errors
  dmaengine: at_hdmac: Don't allow CPU to reorder channel enable
  dmaengine: at_hdmac: Do not print messages on console while holding
    the lock
  dmaengine: at_hdmac: Fix impossible condition
  dmaengine: at_hdmac: Pass residue by address to avoid unneccessary
    implicit casts
  dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue
  dmaengine: at_hdmac: Introduce atc_get_llis_residue()
  dmaengine: at_hdmac: Remove superfluous cast
  dmaengine: at_hdmac: Use devm_kzalloc() and struct_size()
  dmaengine: at_hdmac: Use devm_platform_ioremap_resource
  dmaengine: at_hdmac: Use devm_request_irq()
  dmaengine: at_hdmac: Use devm_clk_get()
  dmaengine: at_hdmac: Check return code of dma_async_device_register
  dmaengine: at_hdmac: Use pm_ptr()
  dmaengine: at_hdmac: Set include entries in alphabetic order
  dmaengine: at_hdmac: Convert driver to use virt-dma

 MAINTAINERS                 |    1 -
 drivers/dma/Kconfig         |    1 +
 drivers/dma/at_hdmac.c      | 1901 ++++++++++++++++++-----------------
 drivers/dma/at_hdmac_regs.h |  478 ---------
 4 files changed, 985 insertions(+), 1396 deletions(-)
 delete mode 100644 drivers/dma/at_hdmac_regs.h

-- 
2.25.1

Powered by blists - more mailing lists