| 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
| ||
|
Message-ID: <7bc5091b-fb60-454d-8fd1-805d63b6d818@gmail.com>
Date: Sun, 6 Oct 2024 20:50:54 +0300
From: Péter Ujfalusi <peter.ujfalusi@...il.com>
To: Jai Luthra <jai.luthra@...ux.dev>, Vinod Koul <vkoul@...nel.org>
Cc: Vignesh Raghavendra <vigneshr@...com>,
Francesco Dolcini <francesco.dolcini@...adex.com>,
Devarsh Thakkar <devarsht@...com>, Rishikesh Donadkar <r-donadkar@...com>,
dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] dmaengine: ti: k3-udma: Set EOP for all TRs in cyclic
BCDMA transfer
On 9/30/24 8:02 PM, Jai Luthra wrote:
> From: Jai Luthra <j-luthra@...com>
>
> When receiving data in cyclic mode from PDMA peripherals, where reload
> count is set to infinite, any TR in the set can potentially be the last
> one of the overall transfer. In such cases, the EOP flag needs to be set
> in each TR and PDMA's Static TR "Z" parameter should be set, matching
> the size of the TR.
>
> This is required for the teardown to function properly and cleanup the
> internal state memory. This only affects platforms using BCDMA and not
> those using UDMA-P, which could set EOP flag in the teardown TR
> automatically.
>
> Similarly when transmitting data in cyclic mode to PDMA peripherals, the
> EOP flag needs to be set to get the teardown completion signal
> correctly.
Acked-by: Peter Ujfalusi <peter.ujfalusi@...il.com>
> Fixes: 017794739702 ("dmaengine: ti: k3-udma: Initial support for K3 BCDMA")
> Tested-by: Francesco Dolcini <francesco.dolcini@...adex.com> # Toradex Verdin AM62
> Signed-off-by: Jai Luthra <j-luthra@...com>
> Signed-off-by: Jai Luthra <jai.luthra@...ux.dev>
> ---
> Changes in v2:
> - Fix commit message and comments to make it clear that this change is
> only needed for BCDMA
> - Drop the redundant pkt_mode check
> - Link to v1: https://lore.kernel.org/r/20240918-z_cnt-v1-1-2c58fbfb07d6@linux.dev
> ---
> drivers/dma/ti/k3-udma.c | 62 ++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index 406ee199c2ac1cffc29edb475df574cf9f0cf222..b3f27b3f92098a65afe9656ca31f9b8027294c16 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -3185,27 +3185,40 @@ static int udma_configure_statictr(struct udma_chan *uc, struct udma_desc *d,
>
> d->static_tr.elcnt = elcnt;
>
> - /*
> - * PDMA must to close the packet when the channel is in packet mode.
> - * For TR mode when the channel is not cyclic we also need PDMA to close
> - * the packet otherwise the transfer will stall because PDMA holds on
> - * the data it has received from the peripheral.
> - */
> if (uc->config.pkt_mode || !uc->cyclic) {
> + /*
> + * PDMA must close the packet when the channel is in packet mode.
> + * For TR mode when the channel is not cyclic we also need PDMA
> + * to close the packet otherwise the transfer will stall because
> + * PDMA holds on the data it has received from the peripheral.
> + */
> unsigned int div = dev_width * elcnt;
>
> if (uc->cyclic)
> d->static_tr.bstcnt = d->residue / d->sglen / div;
> else
> d->static_tr.bstcnt = d->residue / div;
> + } else if (uc->ud->match_data->type == DMA_TYPE_BCDMA &&
> + uc->config.dir == DMA_DEV_TO_MEM &&
> + uc->cyclic) {
> + /*
> + * For cyclic mode with BCDMA we have to set EOP in each TR to
> + * prevent short packet errors seen on channel teardown. So the
> + * PDMA must close the packet after every TR transfer by setting
> + * burst count equal to the number of bytes transferred.
> + */
> + struct cppi5_tr_type1_t *tr_req = d->hwdesc[0].tr_req_base;
>
> - if (uc->config.dir == DMA_DEV_TO_MEM &&
> - d->static_tr.bstcnt > uc->ud->match_data->statictr_z_mask)
> - return -EINVAL;
> + d->static_tr.bstcnt =
> + (tr_req->icnt0 * tr_req->icnt1) / dev_width;
> } else {
> d->static_tr.bstcnt = 0;
> }
>
> + if (uc->config.dir == DMA_DEV_TO_MEM &&
> + d->static_tr.bstcnt > uc->ud->match_data->statictr_z_mask)
> + return -EINVAL;
> +
> return 0;
> }
>
> @@ -3450,8 +3463,9 @@ udma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> /* static TR for remote PDMA */
> if (udma_configure_statictr(uc, d, dev_width, burst)) {
> dev_err(uc->ud->dev,
> - "%s: StaticTR Z is limited to maximum 4095 (%u)\n",
> - __func__, d->static_tr.bstcnt);
> + "%s: StaticTR Z is limited to maximum %u (%u)\n",
> + __func__, uc->ud->match_data->statictr_z_mask,
> + d->static_tr.bstcnt);
>
> udma_free_hwdesc(uc, d);
> kfree(d);
> @@ -3476,6 +3490,7 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr,
> u16 tr0_cnt0, tr0_cnt1, tr1_cnt0;
> unsigned int i;
> int num_tr;
> + u32 period_csf = 0;
>
> num_tr = udma_get_tr_counters(period_len, __ffs(buf_addr), &tr0_cnt0,
> &tr0_cnt1, &tr1_cnt0);
> @@ -3498,6 +3513,20 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr,
> period_addr = buf_addr |
> ((u64)uc->config.asel << K3_ADDRESS_ASEL_SHIFT);
>
> + /*
> + * For BCDMA <-> PDMA transfers, the EOP flag needs to be set on the
> + * last TR of a descriptor, to mark the packet as complete.
> + * This is required for getting the teardown completion message in case
> + * of TX, and to avoid short-packet error in case of RX.
> + *
> + * As we are in cyclic mode, we do not know which period might be the
> + * last one, so set the flag for each period.
> + */
> + if (uc->config.ep_type == PSIL_EP_PDMA_XY &&
> + uc->ud->match_data->type == DMA_TYPE_BCDMA) {
> + period_csf = CPPI5_TR_CSF_EOP;
> + }
> +
> for (i = 0; i < periods; i++) {
> int tr_idx = i * num_tr;
>
> @@ -3525,8 +3554,10 @@ udma_prep_dma_cyclic_tr(struct udma_chan *uc, dma_addr_t buf_addr,
> }
>
> if (!(flags & DMA_PREP_INTERRUPT))
> - cppi5_tr_csf_set(&tr_req[tr_idx].flags,
> - CPPI5_TR_CSF_SUPR_EVT);
> + period_csf |= CPPI5_TR_CSF_SUPR_EVT;
> +
> + if (period_csf)
> + cppi5_tr_csf_set(&tr_req[tr_idx].flags, period_csf);
>
> period_addr += period_len;
> }
> @@ -3655,8 +3686,9 @@ udma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> /* static TR for remote PDMA */
> if (udma_configure_statictr(uc, d, dev_width, burst)) {
> dev_err(uc->ud->dev,
> - "%s: StaticTR Z is limited to maximum 4095 (%u)\n",
> - __func__, d->static_tr.bstcnt);
> + "%s: StaticTR Z is limited to maximum %u (%u)\n",
> + __func__, uc->ud->match_data->statictr_z_mask,
> + d->static_tr.bstcnt);
>
> udma_free_hwdesc(uc, d);
> kfree(d);
>
> ---
> base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
> change-id: 20240918-z_cnt-3ca5daec76bf
>
> Best regards,
--
Péter
Powered by blists - more mailing lists