Conversation
Contributor
Author
Contributor
Author
|
Friendly ping @gurchetansingh @jmacnak for reviewing. |
utzcoz
force-pushed
the
macos-host-memory
branch
from
September 9, 2026 13:54
eba0d61 to
2b5933a
Compare
Collaborator
|
@jmacnak knows this code well, would be the right person to review |
jmacnak
requested changes
Sep 9, 2026
utzcoz
force-pushed
the
macos-host-memory
branch
3 times, most recently
from
September 10, 2026 16:01
293c959 to
7129900
Compare
jmacnak
reviewed
Sep 11, 2026
jmacnak
left a comment
jmacnak
left a comment
Member
There was a problem hiding this comment.
One thread about readability but overall logic LGTM
utzcoz
force-pushed
the
macos-host-memory
branch
6 times, most recently
from
September 16, 2026 11:16
f0f5808 to
18adce1
Compare
utzcoz
force-pushed
the
macos-host-memory
branch
from
September 21, 2026 17:51
18adce1 to
9817065
Compare
gurchetansingh
approved these changes
Sep 22, 2026
Contributor
Author
|
Hi @jmacnak , could you review it again? There is an old requested change from you, and it might require your approval to remove this flag. |
jmacnak
reviewed
Sep 23, 2026
auto-merge was automatically disabled
September 24, 2026 14:39
Head branch was pushed to by a user without write access
utzcoz
force-pushed
the
macos-host-memory
branch
from
September 24, 2026 14:39
9817065 to
7ebfd60
Compare
Contributor
Author
|
Hi @jmacnak @gurchetansingh @kocdemir , I have addressed new comments. PTAL. |
utzcoz
force-pushed
the
macos-host-memory
branch
from
September 24, 2026 15:01
7ebfd60 to
6e5a182
Compare
Contributor
Author
|
I decide to keep device buffer query regeneration part of vk_decoder.py only and update it. |
Contributor
Author
|
Hi @jmacnak @gurchetansingh @kocdemir , friendly ping for reviewing. |
kocdemir
reviewed
Sep 28, 2026
kocdemir
reviewed
Sep 28, 2026
utzcoz
force-pushed
the
macos-host-memory
branch
from
September 28, 2026 15:01
6e5a182 to
8df6b85
Compare
Contributor
Author
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.
… Apple 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 create with O_EXCL, unlinking and making the object again when the name is taken. What has the name then is either left over from a dead process or belongs to a live one, which keeps it through its descriptor either way; sharing it by accident is what O_CREAT alone did.
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__.
vkGetDeviceBufferMemoryRequirements is the one requirements query the decoder still hands to the driver directly, so with an emulated memory type its answer names host types, the way the device image query's did. Add the global state handler that translates it. The decoder reaches it once the guest's generator routes the query through global state, which is a mesa change; until the decoder is regenerated from that, it is not called.
Regenerated with mesa's generate-gfxstream-vulkan.sh at caf81bd201a, keeping what mesa 4b40fa7ace8 changes: vkGetDeviceBufferMemoryRequirements and its KHR alias now go through global state, so the handler that gives the guest its own memory types is reached. The generator's other output since the decoder was last regenerated is left for a regeneration of its own.
The shared memory code has an Apple path of its own now, and nothing ran its tests there. Let the reusable Bazel workflow take a test list, and give the macOS job the base tests, which need no graphics driver. The WorkerThread case that expects a ready future before start() is not run on any platform and fails on all of them; it is filtered out until it is fixed.
utzcoz
force-pushed
the
macos-host-memory
branch
from
September 30, 2026 02:42
8df6b85 to
ce5580e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.