Re: [PATCH v6 1/4] memory: prevent dma-reentracy issues

[Peter Maydell]

On Fri, 10 Mar 2023 at 13:29, Alexander Bulekov <alxndr@bu.edu> wrote:
>
> On 230310 0802, Alexander Bulekov wrote:
> > On 230310 1245, Peter Maydell wrote:
> > > On Fri, 10 Mar 2023 at 12:32, Alexander Bulekov <alxndr@bu.edu> wrote:
> > > > This MR seems to be "lsi-ram".
> > > >
> > > > From hw/scsi/lsi53c895a.c:
> > > >
> > > > memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
> > > >         "lsi-ram", 0x2000);
> > > >
> > > > So the LSI device is reading an LSI "Script" from its own IO region.. In
> > > > this particular case, I think there was no reason to use
> > > > memory_region_init_io rather than memory_region_init_ram, but this makes
> > > > me worried that there are other devices that use something like this.
> > >
> > > This particular device predates the entire MemoryRegion set of
> > > abstractions, so it might have seemed easier at the time.
> > > The endianness handling of the current code is also a bit
> > > confusing and might make it tricky to convert to a RAM MR.
> >
> > With my trivial mr_io - > mr_ram conversion, I no longer hit the
> > re-entrancy tracepoint, and my livecd boots, but it's probably not
> > exhaustively using the script functionality..
> >
> > Does the endianness actually cause a problem? As long as all accesses
> > (CPU -> LSI_RAM and LSI -> LSI_RAM) occur through the address_space API,
> > are we safe?

I'm not sure -- I looked at the code and I wasn't confident
on exactly what the combination of the DEVICE_LITTLE_ENDIAN
MemoryRegion and the use of stn_le_p/ldn_le_p would be.
I think it ought to be possible to use a RAM MR, but we'd
need to check the handling on both BE and LE hosts. Migration
compatibility is the other thing we would need to check, to
avoid accidentally switching from "script_ram[] contents are
in order X" to "they are in order Y"...

> Or maybe a rom_device with memory_region_rom_device_set_romd(romd_mode =
> false) is better here?

I don't think that helps -- if we can let the guest do direct
reads from the region then it's equally OK to let it do
direct writes, so we could just use a RAM MR.

-- PMM