With [`VK_EXT_device_address_binding_report`](https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_dev...) we can get [debug_util](https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_deb...) callbacks used to track memory bindings. Since it's the host's implementation that starts the callback we have to be sure that we have a way of converting it to the client side's variant before it's added to the handle map - i.e. we don't know the host handle at that time yet.
This is [used by vkd3d-proton](https://github.com/HansKristian-Work/vkd3d-proton/pull/1962). Requires Mesa with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28649.
before (note the missing BIND for VkDevice which actually means `VkDeviceMemory`): ``` vkd3d-proton % VKD3D_TEST_FILTER=create_placed_resource_size VKD3D_CONFIG=fault VKD3D_DEBUG=trace ~/src/wine/build/wine ./tests/d3d12.exe 2>&1 | grep vkd3d_address_binding_callback trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkImage || VA ffff800100200000 || size 000000000019a000. trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkImage || VA ffff800100200000 || size 000000000019a000. trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. :trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkDevice || VA ffff800100200000 || size 0000000001000000. 232285.553:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. 232285.553:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. 232285.553:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkDevice || VA ffff800100200000 || size 0000000001000000. ```
after: ``` % VKD3D_TEST_FILTER=create_placed_resource_size VKD3D_CONFIG=fault VKD3D_DEBUG=trace ~/src/wine/build/wine ./tests/d3d12.exe 2>&1 | grep vkd3d_address_binding_callback 232338.036:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkDevice || VA ffff800100200000 || size 0000000001000000. 232338.036:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkImage || VA ffff800100200000 || size 000000000019a000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkImage || VA ffff800100200000 || size 000000000019a000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkDevice || VA ffff800100200000 || size 0000000001000000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkDevice || VA ffff800100200000 || size 0000000001000000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: BIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkBuffer || VA ffff800100200000 || size 0000000001000000. 232338.037:0020:0024:trace:vkd3d-proton:vkd3d_address_binding_callback: UNBIND || VkDevice || VA ffff800100200000 || size 0000000001000000. ```
[The spec guarantees](https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#de...) the following:
An application can receive multiple callbacks if multiple VkDebugUtilsMessengerEXT objects are created. A callback will always be executed in the same thread as the originating Vulkan call.
As of TLS I went with a suggestion from IRC:
``` <ivyl_> There's an issue with how we handle callbacks related to VK_EXT_debug_utils and VK_EXT_debug_report. In wine_vkAllocateMemory() we call device->funcs.p_vkAllocateMemory() which may execute callback. <ivyl_> At this point we still don't have handle mapping added because we don't know what host_memory handle will be. <ivyl_> AFAIU there's a spec guarantee that callback will be executed in the same thread as the call causing it, so I was thinking about maybe keeping `memory` in a tls (if enable_wrapper_list) and using that? <ivyl_> but that's I don't see any precedent of using thread local storage on the unix side <ivyl_> tls value would be short-lived in such case - only for the duration of the call to `device->funcs.p_vkAllocateMemory()` <jacekc_> we usually use teb on unix side, see ntuser_thread_info ```
-- v4: winevulkan: Make device memory wrapper available in callbacks.
From: Arkadiusz Hiler ahiler@codeweavers.com
With VK_EXT_device_address_binding_report we can get debug_util callbacks used to track memory bindings. Since it's the host's implementation that starts the callback we have to be sure that we have a way of converting it to the client side's variant before it's added to the handle map - i.e. we don't know the host handle at that time yet. --- dlls/winevulkan/vulkan.c | 16 ++++++++++++++++ include/ntuser.h | 1 + 2 files changed, 17 insertions(+)
diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index f6e06bcc085..91a93ffab03 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -169,6 +169,17 @@ static uint64_t client_handle_from_host(struct wine_instance *instance, uint64_t return result; }
+static void set_transient_client_handle(struct wine_instance *instance, uint64_t client_handle) +{ + if (!instance->enable_wrapper_list) return; + NtUserGetThreadInfo()->vulkan_data = client_handle; +} + +static uint64_t get_transient_handle(void) +{ + return (uint64_t)NtUserGetThreadInfo()->vulkan_data; +} + static VkBool32 debug_utils_callback_conversion(VkDebugUtilsMessageSeverityFlagBitsEXT severity, VkDebugUtilsMessageTypeFlagsEXT message_types, const VkDebugUtilsMessengerCallbackDataEXT *callback_data, @@ -210,6 +221,8 @@ static VkBool32 debug_utils_callback_conversion(VkDebugUtilsMessageSeverityFlagB if (wine_vk_is_type_wrapped(callback_data->pObjects[i].objectType)) { object_name_infos[i].objectHandle = client_handle_from_host(object->instance, callback_data->pObjects[i].objectHandle); + if (!object_name_infos[i].objectHandle) + object_name_infos[i].objectHandle = get_transient_handle(); if (!object_name_infos[i].objectHandle) { WARN("handle conversion failed 0x%s\n", wine_dbgstr_longlong(callback_data->pObjects[i].objectHandle)); @@ -1911,7 +1924,10 @@ VkResult wine_vkAllocateMemory(VkDevice handle, const VkMemoryAllocateInfo *allo if (!(memory = malloc(sizeof(*memory)))) return VK_ERROR_OUT_OF_HOST_MEMORY;
+ set_transient_client_handle(device->phys_dev->instance, (uintptr_t)memory); result = device->funcs.p_vkAllocateMemory(device->host_device, &info, NULL, &memory->host_memory); + set_transient_client_handle(device->phys_dev->instance, 0); + if (result != VK_SUCCESS) { free(memory); diff --git a/include/ntuser.h b/include/ntuser.h index bd11567290f..2baaee68335 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -87,6 +87,7 @@ struct ntuser_thread_info UINT default_imc; /* default input context */ UINT64 client_imm; /* client IMM thread info */ UINT64 wmchar_data; /* client data for WM_CHAR mappings */ + UINT64 vulkan_data; /* used by winevulkan for tls */ };
static inline struct ntuser_thread_info *NtUserGetThreadInfo(void)
``` mfmediaengine:mfmediaengine start dlls/mfmediaengine/tests/mfmediaengine.c mfmediaengine.c:1343: Test marked todo: Unexpected res 0x102. mfmediaengine.c:157: created L"C:\users\gitlab\Temp\rgb32frame.bmp" mfmediaengine.c:2336: Test marked todo: Got unexpected refcount 2. mfmediaengine.c:2347: Test marked todo: Unexpected hr 0xc00d36ee. mfmediaengine.c:2616: Test failed: Unexpected time 0.133467. mfmediaengine.c:2635: Test marked todo: Got unexpected refcount 1. mfmediaengine.c:2646: Test marked todo: Unexpected hr 0xc00d36ee. 030c:mfmediaengine: 538 tests executed (5 marked as todo, 0 as flaky, 1 failure), 0 skipped. ```
is the only failure here, unrelated to the changes
This merge request was approved by Rémi Bernon.
Alexandre Julliard (@julliard) commented about include/ntuser.h:
UINT default_imc; /* default input context */ UINT64 client_imm; /* client IMM thread info */ UINT64 wmchar_data; /* client data for WM_CHAR mappings */
- UINT64 vulkan_data; /* used by winevulkan for tls */
Do we really want to allow random libraries to add their data to the ntuser TLS?
On Mon Jun 24 13:07:16 2024 +0000, Alexandre Julliard wrote:
Do we really want to allow random libraries to add their data to the ntuser TLS?
What's the alternative for TLS here?
On Mon Jun 24 13:11:05 2024 +0000, Arek Hiler wrote:
What's the alternative for TLS here?
IIRC @jacek suggested to use it but we could probably use a vulkan-specific thread data with pthread_key_create.
On Mon Jun 24 13:42:10 2024 +0000, Rémi Bernon wrote:
IIRC @jacek suggested to use it but we could probably use a vulkan-specific thread data with pthread_key_create.
No, I never suggested using it in winevulkan, I was only replying to "but that's I don't see any precedent of using thread local storage on the unix side" with an example.
-
Alexandre Julliard (＠julliard) -
Arek Hiler (＠ivyl)
-
Arkadiusz Hiler
-
Jacek Caban (＠jacek)
-
Rémi Bernon (＠rbernon)