Skip to content

host: make system blob memory work on macOS - #182

Open
utzcoz wants to merge 4 commits into
google:mainfrom
utzcoz:macos-host-memory
Open

host: make system blob memory work on macOS#182
utzcoz wants to merge 4 commits into
google:mainfrom
utzcoz:macos-host-memory

Conversation

@utzcoz

@utzcoz utzcoz commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Two host fixes for running the guest Vulkan driver through kumquat on macOS.
Both only take effect with SystemBlob enabled.

  • vulkan: add a guest only device local memory type when the host reports a
    single host visible unified type. Metal cannot bind a tiled image to an
    imported host pointer; this gives images ordinary device memory. Unit test
    added.

  • base: put the pid in the shared memory name and unlink it on creation, so a
    killed server does not leave objects that make the next ftruncate fail.

Tested with bazel test //host/vulkan:gfxstream_emulatedphysicalmemory_tests,
and vulkaninfo, vkcube and a dEQP-VK smoke run on kosmickrisp and MoltenVK.

@utzcoz

utzcoz commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@utzcoz

utzcoz commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Friendly ping @gurchetansingh @jmacnak for reviewing.

Comment thread host/vulkan/vk_emulated_physical_device_memory.cpp Outdated
@gurchetansingh

Copy link
Copy Markdown
Collaborator

@jmacnak knows this code well, would be the right person to review

Comment thread host/vulkan/vk_emulated_physical_device_memory.cpp Outdated
@utzcoz
utzcoz force-pushed the macos-host-memory branch 3 times, most recently from 293c959 to 7129900 Compare September 10, 2026 16:01

@jmacnak jmacnak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thread about readability but overall logic LGTM

// so that the host can control its memory properties. This ensures that the guest
// only sees `VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT` and will not try to map the
// memory.
if (features.VulkanUseDedicatedAhbMemoryType.enabled()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we are inserting 2 emulated guest memory types and the indicies and offsets are getting a little more complicated to read and reason about, I wonder if we could replace the index mappings with something a bit more readable.

struct EmulatedGuestMemoryType {
   uint32_t hostMemoryTypeIndex = kInvalidMemoryTypeIndex;

   // This memory type is reserved for controlling the
   // memory properties of AHBs with the 
   // VulkanUseDedicatedAhbMemoryType feature.
   bool isReservedForAhbAllocations = false;
   
   // This memory type is reserved for optimally tiled
   // images on Apple when used with system blobs where
   // the host must not try to 
   bool isReservedForAppleSystemBlobAllocations = false;

   // The memory properties reported to the guest.
   VkMemoryType memoryType;
};
std::vector<EmulatedGuestMemoryType> mGuestMemoryTypes;

struct HostMemoryType {
   // The corresponding guest memory index.
   uint32_t guestMemoryTypeIndex = kInvalidMemoryTypeIndex;

   // The unaltered host memory properties.
   VkMemoryType memoryType;
}
std::vector<HostMemoryType> mHostMemoryTypes;

Then, inside EmulatedPhysicalDeviceMemoryProperties's constructor, we have something like

mHostMemoryProperties = hostMemoryProperties;

// Start with the original host properties
mGuestMemoryTypes.reserve(hostMemoryProperties. memoryTypeCount);
for ( <each host memory type> ) {
    mGuestMemoryTypes.push_back(EmulatedGuestMemoryType{
   .hostMemoryTypeIndex = index,
   .memoryType = hostMemoryProperties.memoryType[index]
};
}

<apply transformations on mGuestMemoryTypes>

<insert a EmulatedGuestMemoryType for apple if needed>

<insert a EmulatedGuestMemoryType for AHBs if needed>

where we are just working with objects and we can use

mGuestMemoryTypes.insert(mGuestMemoryTypes.begin() + <desired index>);

with a helper function that is self documenting:

uint32_t FindIndexForNewMemoryType(const std::vector<EmulatedGuestMemoryType>& memoryTypes) {
  // Per https://docs.vulkan.org/refpages/latest/refpages/source/VkPhysicalDeviceMemoryProperties.html, 
  //  memory types must be ordered so that earlier memory
  //  types have a strict subset of flags of later types.

 ...
}

The very end of the constructor can finally generate the flattened mGuestMemoryProperties.

WDYT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Guest memory types are now a vector of EmulatedGuestMemoryType. The Apple type is inserted with findIndexForNewMemoryType() and the vector is flattened into mGuestMemoryProperties afterwards.

@utzcoz
utzcoz force-pushed the macos-host-memory branch 4 times, most recently from 3fb83c3 to 72e8d04 Compare September 12, 2026 16:48
vkGetDeviceImageMemoryRequirements answers with the host's memory type bits
unless the format is one gfxstream decompresses. The query that names a
created image translates them first.

Nothing notices while a guest memory type index is also a host one, which holds
until a type is emulated. With one, a guest is told an image fits memory types
it does not have, and asking the same question the other way answers something
else.

Translate on the way out on every path.
A system blob's shared memory is handed to the driver as a host pointer, and a
driver states the alignment it takes in minImportedHostPointerAlignment. The
size is rounded to 4096 instead, which is the page size the guest is assumed to
have rather than anything the importing driver asked for.

On Apple silicon a page is 16KB and kosmickrisp asks for that, so an allocation
whose size is a multiple of 4096 but not of 16KB is imported against the
driver's stated requirement.

Round up to whichever is larger. Where the two agree, which is everywhere a
page is 4096, nothing changes.
System blob memory is named shared-memory-vk-N with a per process counter and
opened without O_EXCL. A server that is killed leaves its objects behind, the
next one reuses the names, and on macOS an object that already has a size
cannot be resized: ftruncate fails with EINVAL and vkAllocateMemory fails.

Put the pid in the name, and on Apple unlink the object as soon as it exists;
the descriptor keeps it alive and is what the guest is handed.
With system blobs, host visible memory is shared memory imported as a host
pointer, and Metal cannot bind a tiled image to that. A device that reports one
unified memory type, as kosmickrisp does, then has nowhere to put an image, and
kk_image_plane_bind asserts on the first one.

Add a guest only type in that case: device local, allocating from the same host
type without host visible emulation. A tiled image is offered that type alone,
since the host visible one cannot hold it; a linear image may still use either.
MoltenVK reports a device local only type of its own, so nothing changes there.

Guest memory types are now kept as a list of structs, each naming its host
type and what it is reserved for, instead of index maps in both directions.
The new type is inserted where the spec's ordering puts it: a type whose flags
are a strict subset of another's sits at a lower index, which on a unified
memory device is the front. The AHB type is still appended, as before.

Metal is what makes this the common case, so it is behind __APPLE__.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants