[RFC v2 3/4] vmapple: apple-gfx: make it work on the latest macOS release

Mohamed Mediouni posted 4 patches 1 month, 1 week ago
Maintainers: Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Alexander Graf <agraf@csgraf.de>, Peter Maydell <peter.maydell@linaro.org>
[RFC v2 3/4] vmapple: apple-gfx: make it work on the latest macOS release
Posted by Mohamed Mediouni 1 month, 1 week ago
Follow changes in memory management, and 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 | 56 +++++++++++++++++++++++++++----------
 hw/display/apple-gfx.h      | 13 +++++++++
 hw/display/apple-gfx.m      | 42 +++++++++++++++++++++++++++-
 3 files changed, 95 insertions(+), 16 deletions(-)

diff --git a/hw/display/apple-gfx-mmio.m b/hw/display/apple-gfx-mmio.m
index b0b6e2993e..728441d490 100644
--- a/hw/display/apple-gfx-mmio.m
+++ b/hw/display/apple-gfx-mmio.m
@@ -19,6 +19,7 @@
 #include "hw/irq.h"
 #include "apple-gfx.h"
 #include "trace.h"
+#include "system/address-spaces.h"
 
 #import <ParavirtualizedGraphics/ParavirtualizedGraphics.h>
 
@@ -34,14 +35,25 @@
 typedef bool(^IOSFCMapMemory)(uint64_t phys, uint64_t len, bool ro, void **va,
                               void *, void *);
 
+@interface PGMemoryMapDescriptor : NSObject
+-(void)addRange:(struct PGGuestPhysicalRange_s) range;
+@end
+
 @interface PGDeviceDescriptor (IOSurfaceMapper)
 @property (readwrite, nonatomic) bool usingIOSurfaceMapper;
+@property (readwrite, nonatomic) bool enableArgumentBuffers;
+@property (readwrite, nonatomic) bool enableProcessIsolation;
+@property (readwrite, nonatomic) bool enableProtectedContent;
+
+@property (readwrite, nonatomic, copy, nullable) PGMemoryMapDescriptor* memoryMapDescriptor;
 @end
 
 @interface PGIOSurfaceHostDeviceDescriptor : NSObject
 -(PGIOSurfaceHostDeviceDescriptor *)init;
 @property (readwrite, nonatomic, copy, nullable) IOSFCMapMemory mapMemory;
 @property (readwrite, nonatomic, copy, nullable) IOSFCUnmapMemory unmapMemory;
+@property (readwrite, nonatomic, copy, nullable) PGMemoryMapDescriptor* memoryMapDescriptor;
+@property (readwrite, nonatomic) unsigned long long mmioLength;
 @property (readwrite, nonatomic, copy, nullable) IOSFCRaiseInterrupt raiseInterrupt;
 @end
 
@@ -182,20 +194,28 @@ static bool apple_gfx_mmio_unmap_surface_memory(void *ptr)
     PGIOSurfaceHostDeviceDescriptor *iosfc_desc =
         [PGIOSurfaceHostDeviceDescriptor new];
     PGIOSurfaceHostDevice *iosfc_host_dev;
-
-    iosfc_desc.mapMemory =
-        ^bool(uint64_t phys, uint64_t len, bool ro, void **va, void *e, void *f) {
-            *va = apple_gfx_mmio_map_surface_memory(phys, len, ro);
-
-            trace_apple_gfx_iosfc_map_memory(phys, len, ro, va, e, f, *va);
-
-            return *va != NULL;
-        };
-
-    iosfc_desc.unmapMemory =
-        ^bool(void *va, void *b, void *c, void *d, void *e, void *f) {
-            return apple_gfx_mmio_unmap_surface_memory(va);
-        };
+    PGMemoryMapDescriptor* memoryMapDescriptor = [PGMemoryMapDescriptor new];
+
+    if (@available(macOS 15.4, *)) {
+        FlatView* fv = address_space_to_flatview(&address_space_memory);
+        flatview_for_each_range(fv, apple_gfx_register_memory_cb, memoryMapDescriptor);
+        iosfc_desc.mmioLength = 0x10000;
+        iosfc_desc.memoryMapDescriptor = memoryMapDescriptor;
+    } else {
+        iosfc_desc.mapMemory =
+            ^bool(uint64_t phys, uint64_t len, bool ro, void **va, void *e, void *f) {
+                *va = apple_gfx_mmio_map_surface_memory(phys, len, ro);
+
+                trace_apple_gfx_iosfc_map_memory(phys, len, ro, va, e, f, *va);
+
+                return *va != NULL;
+            };
+
+        iosfc_desc.unmapMemory =
+            ^bool(void *va, void *b, void *c, void *d, void *e, void *f) {
+                return apple_gfx_mmio_unmap_surface_memory(va);
+            };
+    }
 
     iosfc_desc.raiseInterrupt = ^bool(uint32_t vector) {
         trace_apple_gfx_iosfc_raise_irq(vector);
@@ -223,13 +243,19 @@ static void apple_gfx_mmio_realize(DeviceState *dev, Error **errp)
         };
 
         desc.usingIOSurfaceMapper = true;
-        s->pgiosfc = apple_gfx_prepare_iosurface_host_device(s);
+        desc.enableArgumentBuffers = true;
+        // Process isolation needs to go through PGMemoryMapDescriptor
+        if (@available(macOS 15.4, *)) {
+            desc.enableProcessIsolation = true;
+        }
 
         if (!apple_gfx_common_realize(&s->common, dev, desc, errp)) {
             [s->pgiosfc release];
             s->pgiosfc = nil;
         }
 
+        s->pgiosfc = apple_gfx_prepare_iosurface_host_device(s);
+
         [desc release];
         desc = nil;
     }
diff --git a/hw/display/apple-gfx.h b/hw/display/apple-gfx.h
index a8b1d1efc0..0f7cf33adf 100644
--- a/hw/display/apple-gfx.h
+++ b/hw/display/apple-gfx.h
@@ -23,6 +23,13 @@
 @protocol MTLTexture;
 @protocol MTLCommandQueue;
 
+typedef struct PGGuestPhysicalRange_s
+{
+    uint64_t physicalAddress;
+    uint64_t physicalLength;
+    void *hostAddress;
+} PGGuestPhysicalRange_t;
+
 typedef QTAILQ_HEAD(, PGTask_s) PGTaskList;
 
 typedef struct AppleGFXDisplayMode {
@@ -68,6 +75,12 @@ void *apple_gfx_host_ptr_for_gpa_range(uint64_t guest_physical,
                                        uint64_t length, bool read_only,
                                        MemoryRegion **mapping_in_region);
 
+bool apple_gfx_register_memory_cb(Int128 start,
+                            Int128 len,
+                            const MemoryRegion *mr,
+                            hwaddr offset_in_region,
+                            void *opaque);
+
 extern const PropertyInfo qdev_prop_apple_gfx_display_mode;
 
 #endif
diff --git a/hw/display/apple-gfx.m b/hw/display/apple-gfx.m
index 174d56ae05..238b227176 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,41 @@ void apple_gfx_common_init(Object *obj, AppleGFXState *s, const char* obj_name)
     /* TODO: PVG framework supports serialising device state: integrate it! */
 }
 
+@interface PGMemoryMapDescriptor : NSObject
+-(void)addRange:(struct PGGuestPhysicalRange_s) range;
+@end
+
+@interface PGDeviceDescriptor (IOSurfaceMapper)
+@property (readwrite, nonatomic, copy, nullable) PGMemoryMapDescriptor* memoryMapDescriptor;
+@end
+
+bool apple_gfx_register_memory_cb(Int128 start,
+                            Int128 len,
+                            const MemoryRegion *mr,
+                            hwaddr offset_in_region,
+                            void *opaque) {
+    PGGuestPhysicalRange_t range;
+    PGMemoryMapDescriptor* memoryMapDescriptor = opaque;
+    if (mr->ram) {
+        range.physicalAddress = start;
+        range.physicalLength = len;
+        range.hostAddress = memory_region_get_ram_ptr(mr);
+        [memoryMapDescriptor addRange:range];
+    }
+    return false;
+}
+
+static void apple_gfx_register_memory(AppleGFXState *s,
+                                                     PGDeviceDescriptor *desc)
+{
+    PGMemoryMapDescriptor* memoryMapDescriptor = [PGMemoryMapDescriptor new];
+
+    FlatView* fv = address_space_to_flatview(&address_space_memory);
+    flatview_for_each_range(fv, apple_gfx_register_memory_cb, memoryMapDescriptor);
+
+    desc.memoryMapDescriptor = memoryMapDescriptor;
+}
+
 static void apple_gfx_register_task_mapping_handlers(AppleGFXState *s,
                                                      PGDeviceDescriptor *desc)
 {
@@ -763,7 +799,11 @@ bool apple_gfx_common_realize(AppleGFXState *s, DeviceState *dev,
 
     desc.device = s->mtl;
 
-    apple_gfx_register_task_mapping_handlers(s, desc);
+    if (@available(macOS 15.4, *)) {
+        apple_gfx_register_memory(s, desc);
+    } else {
+        apple_gfx_register_task_mapping_handlers(s, desc);
+    }
 
     s->cursor_show = true;
 
-- 
2.50.1 (Apple Git-155)
Re: [RFC v2 3/4] vmapple: apple-gfx: make it work on the latest macOS release
Posted by Alexander Graf 1 month, 1 week ago
On 07.10.25 22:31, Mohamed Mediouni wrote:
> Follow changes in memory management, and enable process isolation for a sandboxed GPU process when on a new OS.


Please detail here why this is necessary.


>
> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
> ---
>   hw/display/apple-gfx-mmio.m | 56 +++++++++++++++++++++++++++----------
>   hw/display/apple-gfx.h      | 13 +++++++++
>   hw/display/apple-gfx.m      | 42 +++++++++++++++++++++++++++-
>   3 files changed, 95 insertions(+), 16 deletions(-)
>
> diff --git a/hw/display/apple-gfx-mmio.m b/hw/display/apple-gfx-mmio.m
> index b0b6e2993e..728441d490 100644
> --- a/hw/display/apple-gfx-mmio.m
> +++ b/hw/display/apple-gfx-mmio.m
> @@ -19,6 +19,7 @@
>   #include "hw/irq.h"
>   #include "apple-gfx.h"
>   #include "trace.h"
> +#include "system/address-spaces.h"
>   
>   #import <ParavirtualizedGraphics/ParavirtualizedGraphics.h>
>   
> @@ -34,14 +35,25 @@
>   typedef bool(^IOSFCMapMemory)(uint64_t phys, uint64_t len, bool ro, void **va,
>                                 void *, void *);
>   
> +@interface PGMemoryMapDescriptor : NSObject
> +-(void)addRange:(struct PGGuestPhysicalRange_s) range;
> +@end
> +
>   @interface PGDeviceDescriptor (IOSurfaceMapper)
>   @property (readwrite, nonatomic) bool usingIOSurfaceMapper;
> +@property (readwrite, nonatomic) bool enableArgumentBuffers;
> +@property (readwrite, nonatomic) bool enableProcessIsolation;
> +@property (readwrite, nonatomic) bool enableProtectedContent;
> +
> +@property (readwrite, nonatomic, copy, nullable) PGMemoryMapDescriptor* memoryMapDescriptor;
>   @end
>   
>   @interface PGIOSurfaceHostDeviceDescriptor : NSObject
>   -(PGIOSurfaceHostDeviceDescriptor *)init;
>   @property (readwrite, nonatomic, copy, nullable) IOSFCMapMemory mapMemory;
>   @property (readwrite, nonatomic, copy, nullable) IOSFCUnmapMemory unmapMemory;
> +@property (readwrite, nonatomic, copy, nullable) PGMemoryMapDescriptor* memoryMapDescriptor;
> +@property (readwrite, nonatomic) unsigned long long mmioLength;
>   @property (readwrite, nonatomic, copy, nullable) IOSFCRaiseInterrupt raiseInterrupt;
>   @end
>   
> @@ -182,20 +194,28 @@ static bool apple_gfx_mmio_unmap_surface_memory(void *ptr)
>       PGIOSurfaceHostDeviceDescriptor *iosfc_desc =
>           [PGIOSurfaceHostDeviceDescriptor new];
>       PGIOSurfaceHostDevice *iosfc_host_dev;
> -
> -    iosfc_desc.mapMemory =
> -        ^bool(uint64_t phys, uint64_t len, bool ro, void **va, void *e, void *f) {
> -            *va = apple_gfx_mmio_map_surface_memory(phys, len, ro);
> -
> -            trace_apple_gfx_iosfc_map_memory(phys, len, ro, va, e, f, *va);
> -
> -            return *va != NULL;
> -        };
> -
> -    iosfc_desc.unmapMemory =
> -        ^bool(void *va, void *b, void *c, void *d, void *e, void *f) {
> -            return apple_gfx_mmio_unmap_surface_memory(va);
> -        };
> +    PGMemoryMapDescriptor* memoryMapDescriptor = [PGMemoryMapDescriptor new];
> +
> +    if (@available(macOS 15.4, *)) {


This needs a big fat comment explaining what this condition is checking 
for, why it's doing that and what the effect of it is. I would also 
recommend to wrap this in something like a static inline bool function 
that gives it a descriptive name.


> +        FlatView* fv = address_space_to_flatview(&address_space_memory);
> +        flatview_for_each_range(fv, apple_gfx_register_memory_cb, memoryMapDescriptor);
> +        iosfc_desc.mmioLength = 0x10000;


Why 0x10000?


> +        iosfc_desc.memoryMapDescriptor = memoryMapDescriptor;
> +    } else {
> +        iosfc_desc.mapMemory =
> +            ^bool(uint64_t phys, uint64_t len, bool ro, void **va, void *e, void *f) {
> +                *va = apple_gfx_mmio_map_surface_memory(phys, len, ro);
> +
> +                trace_apple_gfx_iosfc_map_memory(phys, len, ro, va, e, f, *va);
> +
> +                return *va != NULL;
> +            };
> +
> +        iosfc_desc.unmapMemory =
> +            ^bool(void *va, void *b, void *c, void *d, void *e, void *f) {
> +                return apple_gfx_mmio_unmap_surface_memory(va);
> +            };
> +    }
>   
>       iosfc_desc.raiseInterrupt = ^bool(uint32_t vector) {
>           trace_apple_gfx_iosfc_raise_irq(vector);
> @@ -223,13 +243,19 @@ static void apple_gfx_mmio_realize(DeviceState *dev, Error **errp)
>           };
>   
>           desc.usingIOSurfaceMapper = true;
> -        s->pgiosfc = apple_gfx_prepare_iosurface_host_device(s);
> +        desc.enableArgumentBuffers = true;
> +        // Process isolation needs to go through PGMemoryMapDescriptor
> +        if (@available(macOS 15.4, *)) {
> +            desc.enableProcessIsolation = true;
> +        }
>   
>           if (!apple_gfx_common_realize(&s->common, dev, desc, errp)) {
>               [s->pgiosfc release];
>               s->pgiosfc = nil;
>           }
>   
> +        s->pgiosfc = apple_gfx_prepare_iosurface_host_device(s);
> +
>           [desc release];
>           desc = nil;
>       }
> diff --git a/hw/display/apple-gfx.h b/hw/display/apple-gfx.h
> index a8b1d1efc0..0f7cf33adf 100644
> --- a/hw/display/apple-gfx.h
> +++ b/hw/display/apple-gfx.h
> @@ -23,6 +23,13 @@
>   @protocol MTLTexture;
>   @protocol MTLCommandQueue;
>   
> +typedef struct PGGuestPhysicalRange_s
> +{
> +    uint64_t physicalAddress;
> +    uint64_t physicalLength;
> +    void *hostAddress;
> +} PGGuestPhysicalRange_t;
> +
>   typedef QTAILQ_HEAD(, PGTask_s) PGTaskList;
>   
>   typedef struct AppleGFXDisplayMode {
> @@ -68,6 +75,12 @@ void *apple_gfx_host_ptr_for_gpa_range(uint64_t guest_physical,
>                                          uint64_t length, bool read_only,
>                                          MemoryRegion **mapping_in_region);
>   
> +bool apple_gfx_register_memory_cb(Int128 start,
> +                            Int128 len,
> +                            const MemoryRegion *mr,
> +                            hwaddr offset_in_region,
> +                            void *opaque);
> +
>   extern const PropertyInfo qdev_prop_apple_gfx_display_mode;
>   
>   #endif
> diff --git a/hw/display/apple-gfx.m b/hw/display/apple-gfx.m
> index 174d56ae05..238b227176 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,41 @@ void apple_gfx_common_init(Object *obj, AppleGFXState *s, const char* obj_name)
>       /* TODO: PVG framework supports serialising device state: integrate it! */
>   }
>   
> +@interface PGMemoryMapDescriptor : NSObject
> +-(void)addRange:(struct PGGuestPhysicalRange_s) range;
> +@end
> +
> +@interface PGDeviceDescriptor (IOSurfaceMapper)
> +@property (readwrite, nonatomic, copy, nullable) PGMemoryMapDescriptor* memoryMapDescriptor;
> +@end
> +
> +bool apple_gfx_register_memory_cb(Int128 start,
> +                            Int128 len,
> +                            const MemoryRegion *mr,
> +                            hwaddr offset_in_region,
> +                            void *opaque) {
> +    PGGuestPhysicalRange_t range;
> +    PGMemoryMapDescriptor* memoryMapDescriptor = opaque;
> +    if (mr->ram) {
> +        range.physicalAddress = start;
> +        range.physicalLength = len;
> +        range.hostAddress = memory_region_get_ram_ptr(mr);
> +        [memoryMapDescriptor addRange:range];
> +    }
> +    return false;
> +}


By creating a full external function for the new isolated case, but 
keeping the inline mapMemory function for the non-isolated case this 
creates an awkward asymmetry. Either move both into functions or keep 
both inline.


Alex

> +
> +static void apple_gfx_register_memory(AppleGFXState *s,
> +                                                     PGDeviceDescriptor *desc)
> +{
> +    PGMemoryMapDescriptor* memoryMapDescriptor = [PGMemoryMapDescriptor new];
> +
> +    FlatView* fv = address_space_to_flatview(&address_space_memory);
> +    flatview_for_each_range(fv, apple_gfx_register_memory_cb, memoryMapDescriptor);
> +
> +    desc.memoryMapDescriptor = memoryMapDescriptor;
> +}
> +
>   static void apple_gfx_register_task_mapping_handlers(AppleGFXState *s,
>                                                        PGDeviceDescriptor *desc)
>   {
> @@ -763,7 +799,11 @@ bool apple_gfx_common_realize(AppleGFXState *s, DeviceState *dev,
>   
>       desc.device = s->mtl;
>   
> -    apple_gfx_register_task_mapping_handlers(s, desc);
> +    if (@available(macOS 15.4, *)) {
> +        apple_gfx_register_memory(s, desc);
> +    } else {
> +        apple_gfx_register_task_mapping_handlers(s, desc);
> +    }
>   
>       s->cursor_show = true;
>
Re: [RFC v2 3/4] vmapple: apple-gfx: make it work on the latest macOS release
Posted by Philippe Mathieu-Daudé 1 month, 1 week ago
On 7/10/25 23:19, Alexander Graf wrote:
> 
> On 07.10.25 22:31, Mohamed Mediouni wrote:
>> Follow changes in memory management, and enable process isolation for 
>> a sandboxed GPU process when on a new OS.
> 
> 
> Please detail here why this is necessary.
> 
> 
>>
>> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>> ---
>>   hw/display/apple-gfx-mmio.m | 56 +++++++++++++++++++++++++++----------
>>   hw/display/apple-gfx.h      | 13 +++++++++
>>   hw/display/apple-gfx.m      | 42 +++++++++++++++++++++++++++-
>>   3 files changed, 95 insertions(+), 16 deletions(-)


>> +        FlatView* fv = address_space_to_flatview(&address_space_memory);
>> +        flatview_for_each_range(fv, apple_gfx_register_memory_cb, 
>> memoryMapDescriptor);
>> +        iosfc_desc.mmioLength = 0x10000;
> 
> 
> Why 0x10000?

Maybe we want qemu_real_host_page_size() here?