multiple issues with VK_LAYER_AMD_switchable_graphics

[DadSchoorse]

Hi, I noticed multiple technical issues with the new layer:

• it doesn't handle multiple VkInstances correctly: g_nextLinkFuncs should store function pointers per instance because they aren't guaranteed to be identical across them.
• the 2 jsons (for i386 and amd64) have the same name. The Vulkan loader will discard one because of that. (See Multilib and layer names on Linux KhronosGroup/Vulkan-Loader#155 and loader: Do not remove duplicated layers KhronosGroup/Vulkan-Loader#525)
• a smaller issue (because many layers misbehave similar): vk_icdGetInstanceProcAddrSG(NULL, "vkGetInstanceProcAddr") returns the next function in the chain instead of itself.

Also, I would like to ask why VK_LAYER_AMD_switchable_graphics exists to begin with. It's already possible to choose a driver with VK_ICD_FILENAMES. If the intended effect is to have distro install the layer alongside amdvlk it will cause a lot of confusion why radv doesn't work anymore.

[soararing]

The reason to introduce VK_LAYER_AMD_switchable_graphics in Linux: Ubuntu 20.04 installs RADV by default, after install AMDVLK, there are two Vulkan ICDes installed, for each Vulkan app, vkEnumeratePhysicalDevices will return two physical devices, Vulkan apps don't know which physical device should be used, then we need a mechanism to identify which physical device should be used by Vulkan apps. With the AMD switchable graphics layer support, when both RADV and AMDVLK are installed, AMDVLK will be used by default, only AMDVLK physical device is returned to Vulkan apps, but at this time, if Vulkan apps wants to use RADV, end users can specify environment variable AMD_VULKAN_ICD = RADV to switch from AMDVLK to RADV, and use AMD_VULKAN_ICD = AMDVLK to switch back to AMDVLK, basically the layer has the same functionality with VK_ICD_FILENAMES, but AMD_VULKAN_ICD is much easier to use than VK_ICD_FILENAMES, especially in the case of Intel iGPU + AMD dGPU, without the layer, we may need to specify both Intel and AMD JSON file path to VK_ICD_FILENAMES.

[soararing]

it doesn't handle multiple VkInstances correctly: g_nextLinkFuncs should store function pointers per instance because they aren't guaranteed to be identical across them.

I don't see it is a problem, g_nextLinkFuncs is a global variable in the layer, there are only several Vulkan ICD function pointers stored in it, for different VkInstances in the same app process, I think these ICD function pointers have the same value for different VkInstances.

[soararing]

a smaller issue (because many layers misbehave similar): vk_icdGetInstanceProcAddrSG(NULL, "vkGetInstanceProcAddr") returns the next function in the chain instead of itself.

I will check the side effect of this behavior, we may need to fail the call if a NULL VkInstance is input.

[DadSchoorse]

for different VkInstances in the same app process, I think these ICD function pointers have the same value for different VkInstances.

The scenario I see is: The application creates 2 instances, with different layers.
instance 1 has this chain:

• Vulkan loader
• VK_LAYER_AMD_switchable_graphics
• other layer which intercepts functions used by VK_LAYER_AMD_switchable_graphics
• loader terminators and ICDs

instance 2:

• Vulkan loader
• VK_LAYER_AMD_switchable_graphics
• loader terminators and ICDs

with the creation of instance 2 the global function pointers in VK_LAYER_AMD_switchable_graphics will be overwritten. Now the application calls one of the functions that are implemented by VK_LAYER_AMD_switchable_graphics with instance 1. Instead of calling the function in the other layer VK_LAYER_AMD_switchable_graphics will directly call the terminators which is not what should happen.

[reactormonk]

Vulkan straight up broke here because of it: https://bbs.archlinux.org/viewtopic.php?pid=1949414

[soararing]

Thanks @DadSchoorse & @reactormonk, I will try to verify the issues.

[misyltoad]

This layer breaks vulkaninfo... :l

ERROR_INITIALIZATION_FAILED

[misyltoad]

Please disable this layer until you at-least fix it. This is completely unusable and it breaks multiple apps (including vulkaninfo) right now.

[Flakebi]

See also #196 for ERROR_INITIALIZATION_FAILED.

@Joshua-Ashton: Removing /usr/share/vulkan/implicit_layer.d/amd_icd64.json disables the layer.

[misyltoad]

I know how to work around it. it, I just don't understand how you can release something so completely broken and not immediately at least disable what is causing the problem for users.

[DiabeticCrab]

I would really like to see VK_LAYER_AMD_switchable_graphics getting removed and a more standard approach to be taken - i.e. like working with Khronos to make additions to vulkan-loader.

[jinjianrong]

The 2021.Q1.1 package is updated with the AMD_switchable_graphics layer disabled. The issue will be fixed in next release. Thanks All.

[jinjianrong]

The issue is fixed in 2021.Q2.1 release.