Needed for Red Dead Redemption 2 under vkd3d-proton.
RDR2 requires us to use the native Vulkan loader, and vkd3d-proton recently enabled VK_KHR_fragment_shading_rate which exposes physical device procs unknown to the loader.
Previously, when we queried vkGetPhysicalDeviceFragmentShadingRatesKHR via. vkGetInstanceProcAddr, the native loader tried to call vk_icdGetPhysicalDeviceProcAddr from the WineVulkan ICD to resolve this, which didn't exist causing the loader to nullptr itself and crash.
This commit implements the vk_icdGetPhysicalDeviceProcAddr export, required for ICD version 4, via. forwarding to wine_vkGetInstanceProcAddr and fixes that problem.
Signed-off-by: Joshua Ashton joshua@froggi.es --- dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 3ad210e4701..f32ba9b90f1 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -2647,6 +2647,7 @@ class VkGenerator(object): def generate_vulkan_spec(self, f): self._generate_copyright(f, spec_file=True) f.write("@ stdcall -private vk_icdGetInstanceProcAddr(ptr str) wine_vk_icdGetInstanceProcAddr\n") + f.write("@ stdcall -private vk_icdGetPhysicalDeviceProcAddr(ptr str) wine_vk_icdGetPhysicalDeviceProcAddr\n") f.write("@ stdcall -private vk_icdNegotiateLoaderICDInterfaceVersion(ptr) wine_vk_icdNegotiateLoaderICDInterfaceVersion\n") f.write("@ cdecl -norelay native_vkGetInstanceProcAddrWINE(ptr str)\n")
diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 971394eb9dd..2e02714a9fd 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -37,9 +37,7 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0 DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2);
/* For now default to 4 as it felt like a reasonable version feature wise to support. - * Don't support the optional vk_icdGetPhysicalDeviceProcAddr introduced in this version - * as it is unlikely we will implement physical device extensions, which the loader is not - * aware of. Version 5 adds more extensive version checks. Something to tackle later. + * Version 5 adds more extensive version checks. Something to tackle later. */ #define WINE_VULKAN_ICD_VERSION 4
@@ -1239,6 +1237,17 @@ void * WINAPI wine_vk_icdGetInstanceProcAddr(VkInstance instance, const char *na return wine_vkGetInstanceProcAddr(instance, name); }
+void * WINAPI wine_vk_icdGetPhysicalDeviceProcAddr(VkInstance instance, const char *name) +{ + TRACE("%p, %s\n", instance, debugstr_a(name)); + + /* This is exported directly by the ICD, despite the loader documentation + * stating that this is queried via vk_icdGetInstanceProcAddr + */ + + return wine_vkGetInstanceProcAddr(instance, name); +} + VkResult WINAPI wine_vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t *supported_version) { uint32_t req_version;
Signed-off-by: Liam Middlebrook lmiddlebrook@nvidia.com
One non-review comment written inline.
On 3/22/21 6:02 PM, Joshua Ashton wrote:
Needed for Red Dead Redemption 2 under vkd3d-proton.
RDR2 requires us to use the native Vulkan loader, and vkd3d-proton recently enabled VK_KHR_fragment_shading_rate which exposes physical device procs unknown to the loader.
Previously, when we queried vkGetPhysicalDeviceFragmentShadingRatesKHR via. vkGetInstanceProcAddr, the native loader tried to call vk_icdGetPhysicalDeviceProcAddr from the WineVulkan ICD to resolve this, which didn't exist causing the loader to nullptr itself and crash.
This commit implements the vk_icdGetPhysicalDeviceProcAddr export, required for ICD version 4, via. forwarding to wine_vkGetInstanceProcAddr and fixes that problem.
Signed-off-by: Joshua Ashton joshua@froggi.es
dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 3ad210e4701..f32ba9b90f1 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -2647,6 +2647,7 @@ class VkGenerator(object): def generate_vulkan_spec(self, f): self._generate_copyright(f, spec_file=True) f.write("@ stdcall -private vk_icdGetInstanceProcAddr(ptr str) wine_vk_icdGetInstanceProcAddr\n")
f.write("@ stdcall -private vk_icdGetPhysicalDeviceProcAddr(ptr str) wine_vk_icdGetPhysicalDeviceProcAddr\n") f.write("@ stdcall -private vk_icdNegotiateLoaderICDInterfaceVersion(ptr) wine_vk_icdNegotiateLoaderICDInterfaceVersion\n") f.write("@ cdecl -norelay native_vkGetInstanceProcAddrWINE(ptr str)\n")diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 971394eb9dd..2e02714a9fd 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -37,9 +37,7 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0 DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2);
/* For now default to 4 as it felt like a reasonable version feature wise to support.
- Don't support the optional vk_icdGetPhysicalDeviceProcAddr introduced in this version
- as it is unlikely we will implement physical device extensions, which the loader is not
- aware of. Version 5 adds more extensive version checks. Something to tackle later.
*/ #define WINE_VULKAN_ICD_VERSION 4
- Version 5 adds more extensive version checks. Something to tackle later.
@@ -1239,6 +1237,17 @@ void * WINAPI wine_vk_icdGetInstanceProcAddr(VkInstance instance, const char *na return wine_vkGetInstanceProcAddr(instance, name); }
+void * WINAPI wine_vk_icdGetPhysicalDeviceProcAddr(VkInstance instance, const char *name) +{
- TRACE("%p, %s\n", instance, debugstr_a(name));
- /* This is exported directly by the ICD, despite the loader documentation
* stating that this is queried via vk_icdGetInstanceProcAddr*/
Could you file an issue / send a pull-request to the loader repository that addresses this hole in documentation?
Thanks,
Liam Middlebrook
- return wine_vkGetInstanceProcAddr(instance, name);
+}
- VkResult WINAPI wine_vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t *supported_version) { uint32_t req_version;
On 3/23/21 1:31 AM, Liam Middlebrook wrote:
Signed-off-by: Liam Middlebrook lmiddlebrook@nvidia.com
One non-review comment written inline.
On 3/22/21 6:02 PM, Joshua Ashton wrote:
Needed for Red Dead Redemption 2 under vkd3d-proton.
RDR2 requires us to use the native Vulkan loader, and vkd3d-proton recently enabled VK_KHR_fragment_shading_rate which exposes physical device procs unknown to the loader.
Previously, when we queried vkGetPhysicalDeviceFragmentShadingRatesKHR via. vkGetInstanceProcAddr, the native loader tried to call vk_icdGetPhysicalDeviceProcAddr from the WineVulkan ICD to resolve this, which didn't exist causing the loader to nullptr itself and crash.
This commit implements the vk_icdGetPhysicalDeviceProcAddr export, required for ICD version 4, via. forwarding to wine_vkGetInstanceProcAddr and fixes that problem.
Signed-off-by: Joshua Ashton joshua@froggi.es
dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 3ad210e4701..f32ba9b90f1 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -2647,6 +2647,7 @@ class VkGenerator(object): def generate_vulkan_spec(self, f): self._generate_copyright(f, spec_file=True) f.write("@ stdcall -private vk_icdGetInstanceProcAddr(ptr str) wine_vk_icdGetInstanceProcAddr\n") + f.write("@ stdcall -private vk_icdGetPhysicalDeviceProcAddr(ptr str) wine_vk_icdGetPhysicalDeviceProcAddr\n") f.write("@ stdcall -private vk_icdNegotiateLoaderICDInterfaceVersion(ptr) wine_vk_icdNegotiateLoaderICDInterfaceVersion\n") f.write("@ cdecl -norelay native_vkGetInstanceProcAddrWINE(ptr str)\n") diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 971394eb9dd..2e02714a9fd 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -37,9 +37,7 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0 DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2); /* For now default to 4 as it felt like a reasonable version feature wise to support.
- Don't support the optional vk_icdGetPhysicalDeviceProcAddr
introduced in this version
- as it is unlikely we will implement physical device extensions,
which the loader is not
- aware of. Version 5 adds more extensive version checks. Something
to tackle later.
- Version 5 adds more extensive version checks. Something to tackle
later. */ #define WINE_VULKAN_ICD_VERSION 4 @@ -1239,6 +1237,17 @@ void * WINAPI wine_vk_icdGetInstanceProcAddr(VkInstance instance, const char *na return wine_vkGetInstanceProcAddr(instance, name); } +void * WINAPI wine_vk_icdGetPhysicalDeviceProcAddr(VkInstance instance, const char *name) +{ + TRACE("%p, %s\n", instance, debugstr_a(name));
+ /* This is exported directly by the ICD, despite the loader documentation + * stating that this is queried via vk_icdGetInstanceProcAddr + */
Could you file an issue / send a pull-request to the loader repository that addresses this hole in documentation?
https://github.com/KhronosGroup/Vulkan-Loader/issues/558
Done.
- Joshie 🐸✨
Thanks,
Liam Middlebrook
+ return wine_vkGetInstanceProcAddr(instance, name); +}
VkResult WINAPI wine_vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t *supported_version) { uint32_t req_version;
On 23.03.21 02:02, Joshua Ashton wrote:
Needed for Red Dead Redemption 2 under vkd3d-proton.
RDR2 requires us to use the native Vulkan loader, and vkd3d-proton recently enabled VK_KHR_fragment_shading_rate which exposes physical device procs unknown to the loader.
Previously, when we queried vkGetPhysicalDeviceFragmentShadingRatesKHR via. vkGetInstanceProcAddr, the native loader tried to call vk_icdGetPhysicalDeviceProcAddr from the WineVulkan ICD to resolve this, which didn't exist causing the loader to nullptr itself and crash.
This commit implements the vk_icdGetPhysicalDeviceProcAddr export, required for ICD version 4, via. forwarding to wine_vkGetInstanceProcAddr and fixes that problem.
Signed-off-by: Joshua Ashton joshua@froggi.es
dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 3ad210e4701..f32ba9b90f1 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -2647,6 +2647,7 @@ class VkGenerator(object): def generate_vulkan_spec(self, f): self._generate_copyright(f, spec_file=True) f.write("@ stdcall -private vk_icdGetInstanceProcAddr(ptr str) wine_vk_icdGetInstanceProcAddr\n")
f.write("@ stdcall -private vk_icdGetPhysicalDeviceProcAddr(ptr str) wine_vk_icdGetPhysicalDeviceProcAddr\n") f.write("@ stdcall -private vk_icdNegotiateLoaderICDInterfaceVersion(ptr) wine_vk_icdNegotiateLoaderICDInterfaceVersion\n") f.write("@ cdecl -norelay native_vkGetInstanceProcAddrWINE(ptr str)\n")diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 971394eb9dd..2e02714a9fd 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -37,9 +37,7 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0 DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2);
/* For now default to 4 as it felt like a reasonable version feature wise to support.
- Don't support the optional vk_icdGetPhysicalDeviceProcAddr introduced in this version
- as it is unlikely we will implement physical device extensions, which the loader is not
- aware of. Version 5 adds more extensive version checks. Something to tackle later.
*/ #define WINE_VULKAN_ICD_VERSION 4
- Version 5 adds more extensive version checks. Something to tackle later.
@@ -1239,6 +1237,17 @@ void * WINAPI wine_vk_icdGetInstanceProcAddr(VkInstance instance, const char *na return wine_vkGetInstanceProcAddr(instance, name); }
+void * WINAPI wine_vk_icdGetPhysicalDeviceProcAddr(VkInstance instance, const char *name) +{
- TRACE("%p, %s\n", instance, debugstr_a(name));
- /* This is exported directly by the ICD, despite the loader documentation
* stating that this is queried via vk_icdGetInstanceProcAddr*/
We should still support getting it via vk_icdGetInstanceProcAddr.
- return wine_vkGetInstanceProcAddr(instance, name);
This is not correct. vk_icdGetPhysicalDeviceProcAddr should only return function pointers for physical device level functions. This returns function pointers for *all* functions, so the loader will treat every unknown function as a physical device function. [1]
The proper way to do this is by generating a new dispatch table similar to vk_instance_dispatch_table in vulkan_thunks.c with only functions that take a physical device as the first argument.
Thanks,
Georg Lehmann
[1] https://github.com/KhronosGroup/Vulkan-Loader/blob/master/loader/LoaderAndLa...
+}
- VkResult WINAPI wine_vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t *supported_version) { uint32_t req_version;
Thanks for the feedback, after discussion we found a few places where this could patch break that needs addressing.
- Joshie 🐸✨
On 3/23/21 9:31 AM, Georg Lehmann wrote:
On 23.03.21 02:02, Joshua Ashton wrote:
Needed for Red Dead Redemption 2 under vkd3d-proton.
RDR2 requires us to use the native Vulkan loader, and vkd3d-proton recently enabled VK_KHR_fragment_shading_rate which exposes physical device procs unknown to the loader.
Previously, when we queried vkGetPhysicalDeviceFragmentShadingRatesKHR via. vkGetInstanceProcAddr, the native loader tried to call vk_icdGetPhysicalDeviceProcAddr from the WineVulkan ICD to resolve this, which didn't exist causing the loader to nullptr itself and crash.
This commit implements the vk_icdGetPhysicalDeviceProcAddr export, required for ICD version 4, via. forwarding to wine_vkGetInstanceProcAddr and fixes that problem.
Signed-off-by: Joshua Ashton joshua@froggi.es
dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 3ad210e4701..f32ba9b90f1 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -2647,6 +2647,7 @@ class VkGenerator(object): def generate_vulkan_spec(self, f): self._generate_copyright(f, spec_file=True) f.write("@ stdcall -private vk_icdGetInstanceProcAddr(ptr str) wine_vk_icdGetInstanceProcAddr\n") + f.write("@ stdcall -private vk_icdGetPhysicalDeviceProcAddr(ptr str) wine_vk_icdGetPhysicalDeviceProcAddr\n") f.write("@ stdcall -private vk_icdNegotiateLoaderICDInterfaceVersion(ptr) wine_vk_icdNegotiateLoaderICDInterfaceVersion\n") f.write("@ cdecl -norelay native_vkGetInstanceProcAddrWINE(ptr str)\n") diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 971394eb9dd..2e02714a9fd 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -37,9 +37,7 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0 DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2); /* For now default to 4 as it felt like a reasonable version feature wise to support.
- Don't support the optional vk_icdGetPhysicalDeviceProcAddr
introduced in this version
- as it is unlikely we will implement physical device extensions,
which the loader is not
- aware of. Version 5 adds more extensive version checks. Something
to tackle later.
- Version 5 adds more extensive version checks. Something to tackle
later. */ #define WINE_VULKAN_ICD_VERSION 4 @@ -1239,6 +1237,17 @@ void * WINAPI wine_vk_icdGetInstanceProcAddr(VkInstance instance, const char *na return wine_vkGetInstanceProcAddr(instance, name); } +void * WINAPI wine_vk_icdGetPhysicalDeviceProcAddr(VkInstance instance, const char *name) +{ + TRACE("%p, %s\n", instance, debugstr_a(name));
+ /* This is exported directly by the ICD, despite the loader documentation + * stating that this is queried via vk_icdGetInstanceProcAddr + */
We should still support getting it via vk_icdGetInstanceProcAddr.
+ return wine_vkGetInstanceProcAddr(instance, name);
This is not correct. vk_icdGetPhysicalDeviceProcAddr should only return function pointers for physical device level functions. This returns function pointers for *all* functions, so the loader will treat every unknown function as a physical device function. [1]
The proper way to do this is by generating a new dispatch table similar to vk_instance_dispatch_table in vulkan_thunks.c with only functions that take a physical device as the first argument.
Thanks,
Georg Lehmann
[1] https://github.com/KhronosGroup/Vulkan-Loader/blob/master/loader/LoaderAndLa...
+}
VkResult WINAPI wine_vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t *supported_version) { uint32_t req_version;
-
Georg Lehmann -
Joshua Ashton -
Liam Middlebrook