[PATCH v2] hw/dma: don't allow weird transfer lengths for bcm2835

Alex Bennée posted 1 patch 1 day, 10 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260710131500.2323848-1-alex.bennee@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@mailo.com>
hw/dma/bcm2835_dma.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
[PATCH v2] hw/dma: don't allow weird transfer lengths for bcm2835
Posted by Alex Bennée 1 day, 10 hours ago
The datasheet doesn't explicitly say that TXFR_LEN has to be word
aligned but the fact there is a DMA_D_WIDTH flag to select between 32
bit and 128 bit strongly implies that is how it works. The downstream
rpi kernel also goes to efforts to not write sub-4 byte lengths so
lets:

  - fail when mis-programmed and report GUEST_ERROR
  - catch setting D_WIDTH for 128 bit and report UNIMP
  - add comments that the DEBUG register isn't a straight write

Yodel did some digging into the specs (see discussion link):

{A} AMBA AXI Protocol Version: 2.0 Specification
    https://documentation-service.arm.com/static/64256e84314e245d086bc88f

{B} BCM2835 ARM Peripherals
    https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf

[1] {A} (p. 10-2)
[2] {B} (p. 51)
[3] {A} (p. 14-5)
[4] {A} (p. 4-3)
[5] {A} (p. 9-4)
[6] {B} (p. 53)

However was unable to come up with an unambiguous conclusion without
testing on the real hardware. So in the absence of certainty and for
the sake of addressing the DoS I suggest we merge as is for now.

Link: https://lore.kernel.org/all/20251111105429.3993300-1-alex.bennee@linaro.org/
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3201
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - remove test case
  - expand commit message and link to discussion on RFC patch
  - add TODO should anyone want to test on read HW
  - drop the debug comment
---
 hw/dma/bcm2835_dma.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
index d07459de2d0..b842aef4b74 100644
--- a/hw/dma/bcm2835_dma.c
+++ b/hw/dma/bcm2835_dma.c
@@ -86,6 +86,23 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
         }
         xlen_td = xlen;
 
+        if (ch->ti & BCM2708_DMA_D_WIDTH) {
+            qemu_log_mask(LOG_UNIMP, "%s: 128bit transfers not yet supported", __func__);
+            ch->cs |= BCM2708_DMA_ERR;
+            break;
+        }
+
+        /*
+         * Datasheet implies 32bit or 128bit transfers only
+         *
+         * TODO: test on real HW and report back.
+         */
+        if (xlen & 0x3) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: bad transfer size\n", __func__);
+            ch->cs |= BCM2708_DMA_ERR;
+            break;
+        }
+
         while (ylen != 0) {
             /* Normal transfer mode */
             while (xlen != 0) {
-- 
2.47.3