Re: [PULL 1/9] softmmu: Support concurrent bounce buffers

[Peter Maydell]

On Mon, 16 Sept 2024 at 13:14, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 16/09/2024 12:44, Peter Maydell wrote:
>
> > On Mon, 16 Sept 2024 at 12:29, Mark Cave-Ayland
> > <mark.cave-ayland@ilande.co.uk> wrote:
> >> I'm fairly sure that this patch would break MacOS 9 which was the reason 
> >> that
> >> dma_memory_unmap() was added here in the first place: what I was finding 
> >> was that
> >> without the dma_memory_unmap() the destination RAM wasn't being 
> >> invalidated (or
> >> marked dirty), causing random crashes during boot.
> >
> > dma_memory_unmap() of something you never mapped is
> > definitely wrong. Whatever is going on here, leaving the unmap
> > call in after you removed the dma_memory_map() call is just
> > papering over the actual cause of the crashes.
> >
> >> Would the issue be solved by adding a corresponding dma_memory_map() 
> >> beforehand at
> >> the relevant places in hw/ide/macio.c? If that's required as part of the 
> >> setup for
> >> bounce buffers then I can see how not having this present could cause 
> >> problems.
> >
> > The only purpose of this API is sequences of:
> >    host_ptr = dma_memory_map(...);
> >    access the host_ptr directly;
> >    dma_memory_unmap(...);
> >
> > The bounce-buffer stuff is an internal implementation detail
> > of making this API work when the DMA is going to a device.
> >
> > We need to find whatever the actual cause of the macos failure is.
> > Mattias' suggested change looks right to me.
> >
> > I do wonder if something needs the memory barrier than
> > unmap does as part of its operation, e.g. in the
> > implementation of the dma_blk_* functions.
>
> It has been a few years now, but I'm fairly sure the issue was that 
> dma_blk_read()
> didn't mark RAM containing code as dirty/invalid, and since MacOS 9 used 
> overlays
> then it would crash randomly trying to execute stale memory. 
> dma_memory_unmap()
> checks to see if the direction was to RAM, and then marks the memory dirty 
> allowing
> the new code to get picked up after a MMU fault.

dma_blk_io() does its writes into guest memory by doing
a dma_memory_map()/write-to-host-pointer/dma_memory_unmap()
sequence, though (this is done in dma_blk_cb()).

More generally there should be *no* path for doing writes to
guest memory that does not handle the dirty-memory case:
so if there is one we need to find and fix it.

thanks
-- PMM