[PATCH v3] HID: intel-thc-hid: intel-thc: restore PRD table capacity

[PATCH v3] HID: intel-thc-hid: intel-thc: restore PRD table capacity

[Emfox Zhou]

The controller writes the frame length into the PRD entry it filled and
marks the end of the frame there; only the read path puts the capacity
back. thc_reset_dma_settings() rewinds the read pointers, so the ring
reads as empty while the entries keep those lengths, and the next frame
that does not fit runs past the end of its table: the controller reports
a buffer overrun, which is a transfer error and leads back into the same
reset, with the tables untouched.

Rebuild the receive tables in thc_dma_configure(), after the pointers are
rewound and before the engines are started again, which covers probe, the
RxDMA reset and the resume path. All four engines are stopped at that
point, so nothing reads the tables while they are written, and
thc_copy_sgls_to_prd() returns early for a channel whose tables are gone.

Fixes: a688404b2e20 ("HID: intel-thc-hid: intel-thc: Add THC DMA interfaces")
Signed-off-by: Emfox Zhou <emfox@debian.org>
---
Changes in v3:
  - Restore the tables from thc_dma_configure() rather than from
    thc_reset_dma_settings(): the reset also runs on the SWDMA path, where
    nothing stops a reader.
  - Do not touch a table that has already been released.
  - v1 was posted under an older subject.

Link: https://lore.kernel.org/r/20261115001100.313270-1-emfox@debian.org
---
 drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
index c80ef8c20..64fd890e5 100644
--- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
+++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
@@ -211,6 +211,9 @@ static void thc_copy_sgls_to_prd(struct thc_device *dev,
 {
 	unsigned int i;
 
+	if (!config->is_enabled || !config->prd_tbls)
+		return;
+
 	memset(config->prd_tbls, 0, array_size(PRD_TABLE_SIZE, config->prd_tbl_num));
 
 	for (i = 0; i < config->prd_tbl_num; i++)
@@ -491,6 +494,10 @@ int thc_dma_configure(struct thc_device *dev)
 		return -EINVAL;
 	}
 
+	/* Pointers rewound, entries still short: restore before restart. */
+	thc_copy_sgls_to_prd(dev, &dma_ctx->dma_config[THC_RXDMA1]);
+	thc_copy_sgls_to_prd(dev, &dma_ctx->dma_config[THC_RXDMA2]);
+
 	for (chan = 0; chan < MAX_THC_DMA_CHANNEL; chan++) {
 		dma_set_prd_base_addr(dev,
 				      dma_ctx->dma_config[chan].prd_tbls_dma_handle,

base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb

Re: [PATCH v3] HID: intel-thc-hid: intel-thc: restore PRD table capacity

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] HID: intel-thc-hid: missing THC_SWDMA PRD restore in
  thc_dma_configure()
- [Medium] HID: intel-thc-hid: un-restored RxDMA2 PRD capacity in
  thc_swdma_read()
--

commit dd12d53039d8c9ed77913f6942ca6c87e513e4db
Author: Emfox Zhou <emfox@debian.org>

HID: intel-thc-hid: intel-thc: restore PRD table capacity

This commit restores the PRD tables for the THC_RXDMA1 and THC_RXDMA2
channels in thc_dma_configure() after rewinding the DMA pointers and
guards thc_copy_sgls_to_prd() against disabled or unallocated channels.

Link: https://lore.kernel.org/r/20261115001100.313270-1-emfox@debian.org

> diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> index c80ef8c205742..64fd890e5a583 100644
> --- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> +++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c

[ ... ]

> @@ -491,6 +494,10 @@ int thc_dma_configure(struct thc_device *dev)
>  		return -EINVAL;
>  	}
>  
> +	/* Pointers rewound, entries still short: restore before restart. */
> +	thc_copy_sgls_to_prd(dev, &dma_ctx->dma_config[THC_RXDMA1]);
> +	thc_copy_sgls_to_prd(dev, &dma_ctx->dma_config[THC_RXDMA2]);
> +
>  	for (chan = 0; chan < MAX_THC_DMA_CHANNEL; chan++) {
>  		dma_set_prd_base_addr(dev,
>  				      dma_ctx->dma_config[chan].prd_tbls_dma_handle,

[Severity: Medium]
Should this also restore dma_ctx->dma_config[THC_SWDMA]?

thc_reset_dma_settings() rewinds the read pointer for THC_SWDMA as well.
If thc_swdma_read() is interrupted or times out in
wait_event_interruptible_timeout() after the controller writes a shorter
frame into THC_SWDMA's PRD table, thc_dma_read() is never reached to
restore the entry capacity via thc_copy_one_sgl_to_prd().  Subsequent
larger SWDMA reads will then fail with a hardware DMA buffer overrun even
after thc_dma_configure() runs.

[Severity: Medium]
Does the thc_reset_dma_settings() path in thc_swdma_read() also leave
THC_RXDMA2 PRD entries shortened?

thc_swdma_read_start() pauses THC_RXDMA1 and THC_RXDMA2 and calls
thc_reset_dma_settings(), which rewinds their TPCRP pointers to 0, and
thc_swdma_read_completion() resets them again before restarting
THC_RXDMA2 with an empty ring (TPCWP set to 0x80).  If the controller
wrote a short frame into a THC_RXDMA2 PRD slot just before
thc_swdma_read_start() paused the engine and before the IRQ thread
consumed and restored it via thc_rxdma_read(), rewinding the pointers
makes the ring appear empty without restoring that PRD table, so the next
larger input report on that slot will trigger a buffer overrun and forced
reset.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115040756.602092-1-emfox@debian.org?part=1