Re: [PATCH v5 1/2] vmapple: apple-gfx: make it work on the latest macOS release

[Peter Xu <peterx@redhat.com>]

On Tue, Mar 10, 2026 at 03:55:41PM +0900, Akihiko Odaki wrote:
> On 2026/03/10 7:21, Philippe Mathieu-Daudé wrote:
> > +Peter/Paolo
> > 
> > On 7/3/26 04:08, Akihiko Odaki wrote:
> > > On 2026/03/06 23:52, Mohamed Mediouni wrote:
> > > > Follow changes in memory management introduced on macOS 15.4.
> > > > 
> > > > The legacy memory management API has been removed for the
> > > > IOSurface mapper on that macOS version.
> > > > 
> > > > Also enable process isolation for a sandboxed GPU process when
> > > > on a new OS.
> > > > 
> > > > Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
> > > > ---
> > > >   hw/display/apple-gfx-mmio.m | 62 ++++++++++++++++++++++++++++---------
> > > >   hw/display/apple-gfx.h      | 18 +++++++++++
> > > >   hw/display/apple-gfx.m      | 43 ++++++++++++++++++++++++-
> > > >   3 files changed, 107 insertions(+), 16 deletions(-)
> > 
> > 
> > > > diff --git a/hw/display/apple-gfx.m b/hw/display/apple-gfx.m
> > > > index e0a765fcb1..92404d20bf 100644
> > > > --- a/hw/display/apple-gfx.m
> > > > +++ b/hw/display/apple-gfx.m
> > > > @@ -21,6 +21,7 @@
> > > >   #include "system/address-spaces.h"
> > > >   #include "system/dma.h"
> > > >   #include "migration/blocker.h"
> > > > +#include "system/memory.h"
> > > >   #include "ui/console.h"
> > > >   #include "apple-gfx.h"
> > > >   #include "trace.h"
> > > > @@ -596,6 +597,37 @@ void apple_gfx_common_init(Object *obj,
> > > > AppleGFXState *s, const char* obj_name)
> > > >       /* TODO: PVG framework supports serialising device state:
> > > > integrate it! */
> > > >   }
> > > > +@interface PGDeviceDescriptor (IOSurfaceMapper)
> > > > +@property (readwrite, nonatomic, copy, nullable)
> > > > PGMemoryMapDescriptor* memoryMapDescriptor;
> > > > +@end
> > > > +
> > > > +bool apple_gfx_register_memory_cb(Int128 start, Int128 len,
> > > > +                                  const MemoryRegion *mr_const,
> > > > +                                  hwaddr offset_in_region, void
> > > > *opaque) {
> > > > +    MemoryRegion *mr = (MemoryRegion *)mr_const;
> > > 
> > > flatview_for_each_range() should be changed to pass non-const
> > > pointers instead.
> > 
> > flatview_for_each_range() was added in commit fb5ef4eeecd ("memory: Add
> > FlatView foreach function") with memory fuzzing in mind, so const MR was
> > the correct choice then IMO.
> > 
> > FlatView is more of an internal memory API, I'm not sure it should be
> > used by hw/ layer.
> > 
> > $ git grep -w flatview_for_each_range
> > hw/core/loader.c:1656:    flatview_for_each_range(fv, find_rom_cb,
> > &cbdata);
> > semihosting/arm-compat-semi.c:158:    flatview_for_each_range(fv,
> > find_ram_cb, &info);
> > tests/qtest/fuzz/generic_fuzz.c:310: flatview_for_each_range(view,
> > get_io_address_cb , &cb_info);
> > 
> > Semihosting is special, as it the ROM loader helper, and fuzzing code.
> > 
> > 
> > We provide MemoryRegionSection, MemoryListener and RamDiscardListener
> > APIs. Maybe one of them is more appropriate here? See:
> > 
> >   * A #RamDiscardManager coordinates which parts of specific RAM
> >   * #MemoryRegion regions are currently populated to be used/accessed
> >   * by the VM, notifying after parts were discarded (freeing up memory)
> >   * and before parts will be populated (consuming memory), to be used/
> >   * accessed by the VM.
> > 
> > Could this be what this code needs to do?
> 
> The problem here is that the reverse-engineered interface is insufficient to
> keep the memory map synchronized so it can only populate the memory map
> during initialization. flatview_for_each_range() fits for this purpose.
> 
> Ideally the interface may be reverse-engineered more thoroughly to
> synchronize the memory map at runtime but that needs more work.

IIUC we could still use memory_listener_register(), and I agree with Phil
it might be good to avoid exporting flatview API further to hw/ if ever
possible.

memory_listener_register() will properly replay all the memory regions
similarly when adding the new listener, via region_add().

Thanks,

-- 
Peter Xu