@samitouri / QOSamiQemu / commits / 5a2fa06b09

hw/dma/pl080: Fix transfer logic in PL080

The logic in the PL080 for transferring data has multiple bugs: * The TransferSize field in the channel control register counts in units of the source width; because our loop may do multiple source loads if the destination width is greater than the source width, we need to decrement it by (xsize / swidth), not by 1, each loop * It is documented in the TRM that it is a software error to program the source and destination width such that SWidth < DWidth and TransferSize * SWidth is not a multiple of DWidth. (This would mean that there isn't enough data to do a full final destination write.) We weren't doing anything sensible with this case. The TRM doesn't document what the hardware actually does (though it drops some hints that suggest that it probably over-reads from the source). * In the loop to write to the destination, each loop adds swidth to ch->dest for each loop and also uses (ch->dest + n) as the destination address. This moves the destination address on further than we should each time round the loop, and also is incrementing ch->dest by swidth when it should be dwidth. This patch fixes these problems: * decrement TransferSize by the correct amount * log and ignore the transfer size mismatch case * correct the loop logic for the destination writes A repro case which exercises some of this is as follows. It configures swidth to 1 byte, dwidth to 4 bytes, and transfer size 4, for a transfer from 0x00000000 to 0x000010000. Examining the destination memory in the QEMU monitor should show that the source data 0x44332211 has all been copied, but before this fix it is not: ./qemu-system-arm -M versatilepb -m 128M -nographic -S \ -device loader,addr=0x00000000,data=0x44332211,data-len=4 \ -device loader,addr=0x00001000,data=0x00000000,data-len=4 \ -device loader,addr=0x10130030,data=0x00000001,data-len=4 \ -device loader,addr=0x10130100,data=0x00000000,data-len=4 \ -device loader,addr=0x10130104,data=0x00001000,data-len=4 \ -device loader,addr=0x10130108,data=0x00000000,data-len=4 \ -device loader,addr=0x1013010C,data=0x9e47f004,data-len=4 \ -device loader,addr=0x10130110,data=0x0000c001,data-len=4 Without this patch the QEMU monitor shows: (qemu) xp /1wx 0x00001000 00001000: 0x00002211 Correct result: (qemu) xp /1wx 0x00001000 00001000: 0x44332211 Cc: qemu-stable@nongnu.org Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Tao Ding <dingtao0430@163.com> [PMM: Wrote up what we are fixing in the commit message] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Tao Ding committed Mar 24, 2026 at 14:02 UTC 5a2fa06b0957adad46ba1abe923bca04aad9a4d2

1 file changed +11 -6

hw/dma/pl080.c

+11 -6

@@ -179,23 +179,28 @@ again:

179 c, extract32(ch->ctrl, 21, 3));

180 continue;

181 }

182 -

183 - for (n = 0; n < dwidth; n+= swidth) {

182 + if ((size * swidth) % dwidth) {

183 + qemu_log_mask(LOG_GUEST_ERROR,

184 + "pl080: channel %d: transfer size mismatch: size=%d swidth=%d dwidth=%d\n",

185 + c, size, swidth, dwidth);

186 + continue;

187 + }

188 + xsize = MAX(swidth, dwidth);

189 + for (n = 0; n < xsize; n += swidth) {

190 address_space_read(&s->downstream_as, ch->src,

191 MEMTXATTRS_UNSPECIFIED, buff + n, swidth);

192 if (ch->ctrl & PL080_CCTRL_SI)

193 ch->src += swidth;

194 }

189 - xsize = (dwidth < swidth) ? swidth : dwidth;

195 /* ??? This may pad the value incorrectly for dwidth < 32. */

196 for (n = 0; n < xsize; n += dwidth) {

192 - address_space_write(&s->downstream_as, ch->dest + n,

197 + address_space_write(&s->downstream_as, ch->dest,

198 MEMTXATTRS_UNSPECIFIED, buff + n, dwidth);

199 if (ch->ctrl & PL080_CCTRL_DI)

195 - ch->dest += swidth;

200 + ch->dest += dwidth;

201 }

202

198 - size--;

203 + size -= xsize / swidth;

204 ch->ctrl = (ch->ctrl & 0xfffff000) | size;

205 if (size == 0) {

206 /* Transfer complete. */