[PATCH v5 3/4] winevulkan: Implement VK_KHR_external_memory_win32 for buffers.

[Derek Lesho]

On 5/20/21 3:41 PM, Georg Lehmann wrote:
>
>
> On 20.05.21 20:31, Derek Lesho wrote:
>>
>>   +static inline void 
>> wine_vk_normalize_handle_types_win(VkExternalMemoryHandleTypeFlags 
>> *types)
>> +{
>> +    *types &=
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_BIT |
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_KMT_BIT |
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_D3D11_TEXTURE_BIT |
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_D3D11_TEXTURE_KMT_BIT |
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_D3D12_HEAP_BIT |
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_D3D12_RESOURCE_BIT |
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_HOST_ALLOCATION_BIT_EXT |
>> + VK_EXTERNAL_MEMORY_HANDLE_TYPE_HOST_MAPPED_FOREIGN_MEMORY_BIT_EXT;
>> +}
>> +
>> +static inline void 
>> wine_vk_normalize_handle_types_host(VkExternalMemoryHandleTypeFlags 
>> *types)
>> +{
>> +    *types &=
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT |
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_HOST_ALLOCATION_BIT_EXT |
>> +/*      predicated on VK_KHR_external_memory_dma_buf
>> +        VK_EXTERNAL_MEMORY_HANDLE_TYPE_DMA_BUF_BIT_EXT | */
>
> Why is this here?
Because VK_KHR_external_memory_dma_buf is blacklisted, and the enum 
value is not defined.  I put the comment here to note that, if we ever 
need to back another handle type w/ the DMA BUF handle type, and thereby 
un-blacklist the extension, we should add the bit.
>
>> + VK_EXTERNAL_MEMORY_HANDLE_TYPE_HOST_MAPPED_FOREIGN_MEMORY_BIT_EXT;
>> +}
>> +
>> +static void 
>> wine_vk_get_physical_device_external_buffer_properties(VkPhysicalDevice 
>> phys_dev,
>> +        void 
>> (*p_vkGetPhysicalDeviceExternalBufferProperties)(VkPhysicalDevice, 
>> const VkPhysicalDeviceExternalBufferInfo *, 
>> VkExternalBufferProperties *),
>> +        const VkPhysicalDeviceExternalBufferInfo *buffer_info, 
>> VkExternalBufferProperties *properties)
>> +{
>> +    VkPhysicalDeviceExternalBufferInfo buffer_info_dup = *buffer_info;
>> +
>> +    if (buffer_info_dup.handleType == 
>> VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_WIN32_BIT)
>> +        buffer_info_dup.handleType = 
>> VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT;
>> + wine_vk_normalize_handle_types_host(&buffer_info_dup.handleType);
>
> This allows the application to query with FD_BIT. Before you modify 
> any bits you should check if handleType &~ (all the types that we 
> support) and set all the results to 0 if true.

True, it looks like the spec doesn't forbid applications from calling 
this function with non-native bits.  I suppose I could run a 
wine_vk_normalize_handle_types_win on the input parameter then error out 
if that cases the resulting handle type to be 0.  I'm guessing you 
didn't comment on the lack of such a validation in vkAllocateMemory's 
handling of VkExportMemoryAllocateInfo::handleTypes because there it 
would be undefined behavior.  (Since the invalid types wouldn't be 
exposed here).