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

[BALATON Zoltan]

On Tue, 26 Jul 2022, 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[*], 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), but patch 2 is a bit
> more unfinished.
>
> [*] The commit 3c409c1927efde2fc that added this code says it's used
> by AmigaOS.)

AmigaOS still boots with these patches and I see no difference so

Tested-by: BALATON Zoltan <balaton@eik.bme.hu>

(I did not check what parameters AmigaOS uses (could not find a simple 
trace option for that, may need to add some debug printfs to test that) so 
not sure if the added code was actually run or it still only uses the code 
path as before. Fixing the map length should show some effect but I don't 
see any.)

Regards,
BALATON Zoltan

> 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(-)