Re: [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
From: sashiko-bot@kernel.org
Date: 2026-07-13 03:17:50
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] `thc_rxdma_reset` improperly clears descriptor addresses for all DMA channels without pausing them first, potentially causing active TX/SW DMA to access physical address 0. - [High] Error paths in `thc_rxdma_reset` fail to restore the interrupt quiesce state, permanently disabling device interrupts. --
--- Patch [1]: [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA ---commit 64b22c0d882751a67b709351d2c2e3daf211bb52 Author: Even Xu [off-list ref] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA This commit adds a new helper function thc_rxdma_reset() to perform a read DMA reset. It is intended to be called for RxDMA recovery when fatal DMA errors occur.
quoted hunk ↗ jump to hunk
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 6ee675e0a7384..7ceb8aeeccd37 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
[ ... ]
+int thc_rxdma_reset(struct thc_device *dev)
+{
+ int ret;
+
+ if (mutex_lock_interruptible(&dev->thc_bus_lock))
+ return -EINTR;
+
+ ret = thc_interrupt_quiesce(dev, true);
+ if (ret) {
+ dev_err(dev->dev, "Quiesce interrupt failed during RxDMA reset\n");
+ goto end;
+ }
+
+ ret = thc_wait_for_dma_pause(dev, THC_RXDMA1);
+ if (ret) {
+ dev_err(dev->dev, "Wait for RxDMA1 pause failed during RxDMA reset\n");
+ goto end;[Severity: High] Does this skip unquiescing the interrupts? If thc_wait_for_dma_pause() fails, we jump to the end label and return without calling thc_interrupt_quiesce(dev, false). Will this leave device interrupts permanently disabled?
+ }
+
+ ret = thc_wait_for_dma_pause(dev, THC_RXDMA2);
+ if (ret) {
+ dev_err(dev->dev, "Wait for RxDMA2 pause failed during RxDMA reset\n");
+ goto end;
+ }
+
+ thc_dma_unconfigure(dev);
[Severity: Critical]
Can this cause active TX or SW DMA engines to access physical address 0?
thc_rxdma_reset() pauses THC_RXDMA1 and THC_RXDMA2, but thc_dma_unconfigure()
unconditionally iterates over MAX_THC_DMA_CHANNEL and sets the PRD base
address to 0 for all channels, including THC_TXDMA and THC_SWDMA:
void thc_dma_unconfigure(struct thc_device *dev)
{
int chan;
for (chan = 0; chan < MAX_THC_DMA_CHANNEL; chan++) {
dma_set_prd_base_addr(dev, 0, &dev->dma_ctx->dma_config[chan]);
dma_clear_prd_control(dev, &dev->dma_ctx->dma_config[chan]);
}
...
}
If a previous thc_dma_write() or thc_swdma_read() timed out, it unlocks
thc_bus_lock but leaves the TX/SW DMA engine running in hardware. Overwriting
the PRD base address to 0 while the engine is active could result in memory
corruption or an IOMMU fault.
+
+ ret = thc_dma_configure(dev);
+ if (ret) {
+ dev_err(dev->dev, "Re-config DMA failed during RxDMA reset\n");
+ goto end;
+ }
+
+ thc_interrupt_quiesce(dev, false);
+
+end:
+ mutex_unlock(&dev->thc_bus_lock);
+ return ret;
+}-- Sashiko AI review · https://sashiko.dev/#/patchset/20260713030412.2531933-1-even.xu@intel.com?part=1