Re: [RFC 0/2] Fix Coverity and other errors in ppc440_uc DMA

[Cédric Le Goater]

On 7/26/22 20:23, Peter Maydell wrote:
> This patchset is mainly trying to fix a problem that Coverity spotted
> in the dcr_write_dma() function in hw/ppc/ppc440_uc.c, where the code
> is not correctly using the cpu_physical_memory_map() function.
> While I was fixing that I noticed a second problem in this code,
> where it doesn't have a fallback for when cpu_physical_memory_map()
> says "I couldn't map that for you".
>
> I've marked these patches as RFC, partly because I don't have any
> guest that would exercise the code changes[*], 

I build these :

  
https://github.com/legoater/qemu-ppc-boot/tree/main/buildroot/qemu_ppc_sam460ex-2022.02-4-geae5011c83-20220309

but none of the DCR DMA registers are used.

There are images for the sam460ex images here :

  http://www.aros.org/nightly1.php

But AFAICT, it does not go beyond the bootloader.

> and partly because
> I don't have any documentation of the hardware to tell me how it
> should behave, so patch 2 in particular has some FIXMEs. I also
> notice that the code doesn't update any of the registers like the
> count or source/base addresses when the DMA transfer happens, which
> seems odd, but perhaps the real hardware does work like that.
>
> I think we should probably take patch 1 (which is a fairly minimal
> fix of the use-of-uninitialized-data problem),

LGTM,

Thanks,

C.




> but patch 2 is a bit more unfinished.
>
> [*] The commit 3c409c1927efde2fc that added this code says it's used
> by AmigaOS.)
>
> thanks
> -- PMM
>
> Peter Maydell (2):
>    hw/ppc/ppc440_uc: Initialize length passed to
>      cpu_physical_memory_map()
>    hw/ppc/ppc440_uc: Handle mapping failure in DMA engine
>
>   hw/ppc/ppc440_uc.c | 34 +++++++++++++++++++++++++++++++++-
>   1 file changed, 33 insertions(+), 1 deletion(-)