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

[Peter Maydell]

On Wed, 27 Jul 2022 at 12:55, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Wed, 27 Jul 2022, Cédric Le Goater wrote:
> > 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.
>
> Likely I did not know how this function works when implementing it and may
> have used it wrong but none of the reviews spotted it either. (I may have
> used some other DMA device model as an inspiration but don't remember
> which.)
>
> >> 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".
>
> When can that happen? If only in cases when guest gives invalid parameters
> then maybe we don't have to bother with that and can let it fail but
> having a fallback does not hurt.

Mostly it happens when the thing being DMA'd to or from is not RAM.
Ordinarily I wouldn't expect that to be likely, but the DMA device
here has a "don't advance the src/destination" option which I assume
would be used for things like DMA'ing to or from a device FIFO.
Perhaps AmigaOS doesn't in practice do that.

> >> 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
>
> I remember I've found some info on this in some similar SoC that had docs
> on-line but don't remember which. Maybe 440EPx/GPx or something like that.
> It may not be the same but seems similar enough for AmigaOS to work. I
> think the two main sources were PPC440EPx/GRx Embedded Processor User's
> Manual and NXP/Freescale Application Note AN2661 Software Migration from
> the IBM (AMCC) 440GP to the MPC8540 which seem to be similar to 460EX and
> have some info on the DMA controller registers.

Thanks, I'll see if I can track those down.

-- PMM