Re: [PATCH v5 1/7] memory: Export memory_region_set_ops()

[Peter Xu <peterx@redhat.com>]

On Mon, May 25, 2026 at 10:57:39PM +0200, BALATON Zoltan wrote:
> On Mon, 25 May 2026, Peter Maydell wrote:
> > On Mon, 25 May 2026 at 16:47, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> > > 
> > > Make memory_region_set_ops() function public. In some cases such as
> > > when devices have configurable endianness or different behaviour based
> > > on settings it is necessary to change the ops callback after the
> > > memory region is created. Export memory_region_set_ops() function for
> > > this.
> > 
> > If some other CPU is in the middle of using the old MemoryRegionOps
> > when the device swaps them out under its feet, what happens?
> 
> In my limited test with PPC and ati-vga this seems to work and the switch
> would happen in device register write so nothing else is expected to run
> accessing the device at the same time at that point. I can't tell for all
> possible cases but that would then be a problem in the caller not in this
> function. Callers are expected to call it when appropriate which I could
> mention in the doc comment if needed.
> 
> > You could also remove the old MR from its container and add a different
> > one with the different behaviour when the guest changes the config,
> > or have both of them in the container and toggle which is visible
> > with memory_region_set_enabled(). That might potentially be more
> > awkward but it would avoid having to look into the memory region
> > API internals.
> 
> That could also work if I had a container but it's switching the endianness
> of a PCI BAR which is registered with pci_register_bar() so it's easiest to
> switch the ops on the region than hacking around it using two more otherwise
> unneeded memory regions as I also can't swap PCI BARs so it would need a
> container aditionally to two regions using the same callbacks just with
> different endianness. Allowing setting the ops of a single region seems
> simpler.

I think PeterM's proposal makes more sense.

Having two MRs registered under the same offset of parent and dynamically
enable/disable seems to be a common way to solve similar problems in QEMU.
I don't see why it is hacky if we can allow dynamic flip of mr->ops, which
smells more risky.

Exporting that API may invite more abuse, which I'm not sure it's good.

The other thing I want to mention is having ops flippable relies on "BQL
serializes everything", but it's already not true with exceptions like
HPET, see memory_region_enable_lockless_io().  I think this will introduce
tech debts that we don't necessarily need.

Thanks,

-- 
Peter Xu