On Mon, Sep 16, 2024 at 2:28 PM Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Mon, 16 Sept 2024 at 13:14, Mark Cave-Ayland > <mark.cave-ayl...@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-ayl...@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.
I concur that it should be the responsibility of the code performing the DMA write to make sure any invalidation side effects take place rather than relying on ad-hoc calls taking place later. Regardless, in the interest of reaching a conclusion here: Mark, can you provide instructions on how to verify MacOS 9 or alternatively kindly do a quick test? Thanks, Mattias
