Skip to content

pull in leegao's multi-fd AHB mapping fix #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
utkarshdalal merged 1 commit into from
Jul 23, 2026
+44 −12
Merged

pull in leegao's multi-fd AHB mapping fix #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/vulkan/wrapper/wrapper_device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ wrapper_CreateImage(VkDevice _device,
{
VK_FROM_HANDLE(wrapper_device, device, _device);
VkResult res;
VkImageCreateInfo create_info = *pCreateInfo;
VkImageCreateInfo create_info;
bool is_emulated_bgra8 = false;
bool is_wsi_image = false;

Expand All @@ -1025,6 +1025,12 @@ wrapper_CreateImage(VkDevice _device,
prev = (VkBaseInStructure *) s;
}

/* Copy after the unlink above: when the emulated-bgra8 struct is the
* head of the pNext chain, the unlink only updates pCreateInfo->pNext,
* so a copy taken earlier would still pass the unknown struct to the
* driver. */
create_info = *pCreateInfo;

@leegao leegao Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ahh I see what it's trying to say, yeah that's a legitimate type, since VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EMULATED_B8G8R8A8_CREATE_INFO_EXT is always at the head of the chain

fortunately, it'll just raise a validation warning (the WSI code already injects VK_STRUCTURE_TYPE_WSI_IMAGE_CREATE_INFO_MESA for the system driver), but good catch

An alternative structure is to leave the copy at the top, and change the iteration to

   VkBaseInStructure *prev = &create_info;
   for (const VkBaseInStructure *s = pCreateInfo->pNext; s; s = s->pNext) {
      if (s->sType == VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EMULATED_B8G8R8A8_CREATE_INFO_EXT) {
         is_emulated_bgra8 = true;
         prev->pNext = s->pNext; // unlink
         break;
      }
      prev = (VkBaseInStructure *) s;
   }

which has the benefit of not mutating (directly) the upstream pCreateInfo, though we do still change the linked pNext link in place instead of deep-copying everything out so it probably doesn't really matter (we also know who the caller is when this sType extension is sent in, because it's an internal callsite)


if (is_emulated_bcn(device->physical, pCreateInfo->format)) {
create_info.format = get_format_for_bcn(pCreateInfo->format);
if (create_info.flags & VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT)
Expand Down
48 changes: 37 additions & 11 deletions src/vulkan/wrapper/wrapper_device_memory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,12 +528,16 @@ wrapper_AllocateMemory(VkDevice _device,
wrapper_device_memory_destroy(mem);

if (dedicated_allocate_info && dedicated_allocate_info->image != VK_NULL_HANDLE) {
struct wrapper_image *img = get_wrapper_image_from_handle(device, dedicated_allocate_info->image);
/* resource_mutex is already held here and simple_mtx is not

@leegao leegao Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh yeah this is a great catch, it's also a good reason for why getters should never be locked by an exclusive mutex since only upstream controls (and should orchestrate) synchronizaton

You should also change this comment to a TODO to refactor the get-handles to avoid this specific footgun in the future

* recursive, so search the image table directly instead of
* going through get_wrapper_image_from_handle. */
struct wrapper_image *img = _mesa_hash_table_u64_search(
device->image_table, (uint64_t)dedicated_allocate_info->image);
if (img && img->is_wsi_image) {
// Fixes failure to blit on ion-heap (< GKI 5.10) Mali devices at the cost of
// not being able to mmap these.
WRAPPER_LOG(error, "EXT_map_memory_placed emulation failed for swapchain image, bypassing emulation");
simple_mtx_lock(&device->resource_mutex);
simple_mtx_unlock(&device->resource_mutex);

@leegao leegao Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh yeah, that's a legitimate copy-paste type, good catch

goto fallback;
}
}
Expand Down Expand Up @@ -609,23 +613,45 @@ wrapper_MapMemory2KHR(VkDevice _device,

if (mem->ahardware_buffer) {
const native_handle_t *handle;
int idx;

handle = AHardwareBuffer_getNativeHandle(mem->ahardware_buffer);
fd = handle->data[0];
/* The AHB native handle may carry several fds (e.g. metadata pipes
* alongside the actual memory fd); pick the first one that is
* seekable and large enough to back the allocation. */
for (idx = 0; idx < handle->numFds; idx++) {
off_t size = lseek(handle->data[idx], 0, SEEK_END);
if (size < 0) {
WRAPPER_LOG(error, "lseek failed on AHB fd (idx=%d, fd=%d): errno %d, trying next fd",
idx, handle->data[idx], errno);
continue;
}
if ((size_t)size >= mem->alloc_size)
break;
}
if (idx >= handle->numFds) {
WRAPPER_LOG(error, "No usable AHB fd with size >= alloc_size %zu", mem->alloc_size);
result = VK_ERROR_MEMORY_MAP_FAILED;
goto fail;
}
fd = handle->data[idx];
}
else {
fd = mem->fd;
}

if (pMemoryMapInfo->size == VK_WHOLE_SIZE) {
int res = lseek(fd, 0, SEEK_END);
if (res < 0) {
WRAPPER_LOG(error, "Failed lseek for file descriptor %d", fd);
result = VK_ERROR_MEMORY_MAP_FAILED;
goto fail;
if (mem->alloc_size > 0) {
mem->map_size = mem->alloc_size;
} else {
off_t res = lseek(fd, 0, SEEK_END);

@leegao leegao Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And a potential future TODO here to fortify this in the future is to just plumb the size from AllocateMemory down to here so we don't always have to guess, but that's for later

if (res < 0) {
WRAPPER_LOG(error, "Failed lseek for file descriptor %d: errno %d", fd, errno);
result = VK_ERROR_MEMORY_MAP_FAILED;
goto fail;
}
mem->map_size = res;
}
mem->map_size = mem->alloc_size > 0 ?
mem->alloc_size : res;
}
else
mem->map_size = pMemoryMapInfo->size;
Expand Down
Loading