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 <[email protected]>
---
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;
