Skip to content
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.

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

VK_EXT_external_memory_metal validation #8498

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
16 changes: 15 additions & 1 deletion layers/core_checks/cc_device_memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,14 @@ bool CoreChecks::IgnoreAllocationSize(const VkMemoryAllocateInfo &allocate_info)
if (import_memory_win32 && (import_memory_win32->handleType & ignored_allocation) != 0) {
return true;
}
#endif
#elif VK_USE_PLATFORM_METAL_EXT
const VkExternalMemoryHandleTypeFlags ignored_allocation = VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT |
VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT;
const auto import_memory_metal = vku::FindStructInPNextChain<VkImportMemoryMetalHandleInfoEXT>(allocate_info.pNext);
if (import_memory_metal && (import_memory_metal->handleType & ignored_allocation) != 0) {
return true;
}
#endif // VK_USE_PLATFORM_METAL_EXT
// Handles 01874 cases
const auto export_info = vku::FindStructInPNextChain<VkExportMemoryAllocateInfo>(allocate_info.pNext);
if (export_info && (export_info->handleTypes & VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID)) {
Expand Down Expand Up @@ -756,6 +763,13 @@ bool CoreChecks::PreCallValidateAllocateMemory(VkDevice device, const VkMemoryAl
}
}
#endif

#ifdef VK_USE_PLATFORM_METAL_EXT
if (IsExtEnabled(device_extensions.vk_ext_external_memory_metal)) {
skip |= ValidateAllocateMemoryMetal(*pAllocateInfo, dedicated_allocate_info, allocate_info_loc);
}
#endif // VK_USE_PLATFORM_METAL_EXT

return skip;
}

Expand Down
130 changes: 130 additions & 0 deletions layers/core_checks/cc_external_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -549,3 +549,133 @@ bool CoreChecks::PreCallValidateExportMetalObjectsEXT(VkDevice device, VkExportM
return skip;
}
#endif // VK_USE_PLATFORM_METAL_EXT

bool CoreChecks::ValidateAllocateMemoryMetal(const VkMemoryAllocateInfo &allocate_info,
const VkMemoryDedicatedAllocateInfo *dedicated_allocation_info,
const Location &allocate_info_loc) const {
bool skip = false;

#if VK_USE_PLATFORM_METAL_EXT
// When dealing with Metal external memory, we can have the following 3 scenarios:
// 1. Allocation will be exported. Contains VkExportMemoryAllocateInfo
// 2. Allocation is being imported. Contains VkImportMemoryMetalHandleInfoEXT
// 3. Previous 2 combined
// Whenever the memory will be used for an image, VK_EXT_external_memory_metal requires that the allocation
// is dedicated to that single resource.

// Case 1 and partially 3
if (const auto export_memory_info = vku::FindStructInPNextChain<VkExportMemoryAllocateInfo>(allocate_info.pNext)) {
if ((export_memory_info->handleTypes == VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT) && dedicated_allocation_info == nullptr) {
skip |= LogError("VUID-UNASSIGNED-VK_EXT_external_memory-no-VkMemoryDedicatedAllocateInfo-export", device,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spencer-lunarg thoughts on where this VUID should go? Part of VkExportMemoryAllocateInfo or VkMemoryDedicatedAllocateInfo? I'm inclined towards the former since it also requires VkExportMemoryAllocateInfo::handleTypes to be VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT, but I'm not sure how cases where a struct is required by another struct are handled in the spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say VkExportMemoryAllocateInfo

Also not there are things such as VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT, there are some extra VUs for some handleTypes that do requires VkMemoryDedicatedAllocateInfo, but its not for all to my knowledge

allocate_info_loc.dot(Field::pNext),
"VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT requires textures to be imported as a dedicated"
"allocation.");
}
}

// Case 2 and remaining 3
const auto import_memory_metal_info = vku::FindStructInPNextChain<VkImportMemoryMetalHandleInfoEXT>(allocate_info.pNext);
if (import_memory_metal_info == nullptr) {
return skip;
}

// Ensure user is importing correct type before checking any other VUID since we will assume correct type
if ((import_memory_metal_info->handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT) &&
(import_memory_metal_info->handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT)) {
skip |= LogError("VUID-VK_EXT_external_memory-incorrect-handle-type", device,
allocate_info_loc.dot(Struct::VkImportMemoryMetalHandleInfoEXT, Field::handleType),
"current value is %s", string_VkExternalMemoryHandleTypeFlagBits(import_memory_metal_info->handleType));
return skip;
}

// Only images require to be created as a dedicated allocation. This allows us to check if the format
// used by the image allows for importing such images from external memory. We cannot do that with
// buffers since we lack the usage, so it's not possible to know if the device allows import
// operations on buffers.
if(import_memory_metal_info->handleType == VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT) {
if (dedicated_allocation_info == nullptr) {
skip |= LogError("VUID-UNASSIGNED-VK_EXT_external_memory-no-VkMemoryDedicatedAllocateInfo-import", device,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spencer-lunarg similar situation as the previous one

allocate_info_loc.dot(Field::pNext),
"VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT requires textures to be imported as a dedicated"
"allocation.");
// Early out since the image comes from VkMemoryDedicatedAllocateInfoKHR and there's none.
return skip;
}

// Unsure if there should be a VUID that enforces image to not be NULL when importing a MTLTEXTURE type
if (dedicated_allocation_info->image == VK_NULL_HANDLE) {
skip |= LogError("VUID-UNASSIGNED-VK_EXT_external_memory-dedicated-null-image-import", device,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above 2

allocate_info_loc.dot(Struct::VkMemoryDedicatedAllocateInfo, Field::image),
"must be a valid image handle.");
// Early out since there's no image in VkMemoryDedicatedAllocateInfoKHR.
return skip;
}

auto image_state_ptr = Get<vvl::Image>(dedicated_allocation_info->image);
VkFormat image_format = image_state_ptr->safe_create_info.format;
VkExternalImageFormatProperties external_image_format_properties;
external_image_format_properties.sType = VK_STRUCTURE_TYPE_EXTERNAL_IMAGE_FORMAT_PROPERTIES;
external_image_format_properties.pNext = nullptr;
VkFormatProperties2 format_properties;
format_properties.sType = VK_STRUCTURE_TYPE_FORMAT_PROPERTIES_2;
format_properties.pNext = &external_image_format_properties;
DispatchGetPhysicalDeviceFormatProperties2(physical_device, image_format, &format_properties);

if ((external_image_format_properties.externalMemoryProperties.externalMemoryFeatures & VK_EXTERNAL_MEMORY_FEATURE_IMPORTABLE_BIT) == 0u) {
skip |= LogError("VUID-UNASSIGNED-VK_EXT_external_memory-unsupported-format-import", device,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here

allocate_info_loc.dot(Struct::VkImportMemoryMetalHandleInfoEXT, Field::handleType),
"does not support importing image with format %s", string_VkFormat(image_format));
}
}
#endif // VK_USE_PLATFORM_METAL_EXT

return skip;
}

#ifdef VK_USE_PLATFORM_METAL_EXT
bool CoreChecks::PreCallValidateGetMemoryMetalHandleEXT(VkDevice device,
const VkMemoryGetMetalHandleInfoEXT* pGetMetalHandleInfo, MTLResource_id* pHandle,
const ErrorObject& error_obj) const {
bool skip = false;
const Location get_metal_handle_info = error_obj.location.dot(Field::pGetMetalHandleInfo);
auto memory = Get<vvl::DeviceMemory>(pGetMetalHandleInfo->memory);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto memory = Get<vvl::DeviceMemory>(pGetMetalHandleInfo->memory);
auto memory = Get<vvl::DeviceMemory>(pGetMetalHandleInfo->memory);
ASSERT_AND_RETURN_SKIP(memory)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

ASSERT_AND_RETURN_SKIP(memory);
auto export_memory_allocate_info = vku::FindStructInPNextChain<VkExportMemoryAllocateInfo>(memory->safe_allocate_info.pNext);
if (export_memory_allocate_info == nullptr) {
skip |= LogError("VUID-UNASSIGNED-VK_EXT_external_memory-memory-not-created-with-VkExportMemoryAllocateInfo", device,
get_metal_handle_info.dot(Field::memory).dot(Field::pNext),
"device memory missing VkExportMemoryAllocateInfo at creation");
}
else if ((export_memory_allocate_info->handleTypes & pGetMetalHandleInfo->handleType) == 0u) {
skip |= LogError("VUID-VK_EXT_external_memory-memory-allocation-missing-handle-type", device,
get_metal_handle_info.dot(Field::handleType),
"device memory was created with (%s) handle types. Missing %s type",
string_VkExternalMemoryHandleTypeFlags(export_memory_allocate_info->handleTypes).c_str(),
string_VkExternalMemoryHandleTypeFlagBits(pGetMetalHandleInfo->handleType));
}

if ((pGetMetalHandleInfo->handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT) &&
(pGetMetalHandleInfo->handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT)) {
skip |= LogError("VUID-VK_EXT_external_memory-get-handle-incorrect-handle-type", device,
get_metal_handle_info.dot(Field::handleType),
"current value is %s", string_VkExternalMemoryHandleTypeFlagBits(pGetMetalHandleInfo->handleType));
}
return skip;
}

bool CoreChecks::PreCallValidateGetMemoryMetalHandlePropertiesEXT(VkDevice device,
VkExternalMemoryHandleTypeFlagBits handleType, MTLResource_id handle,
VkMemoryMetalHandlePropertiesEXT* pMemoryMetalHandleProperties,
const ErrorObject& error_obj) const {
bool skip = false;

if ((handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT) &&
(handleType != VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT)) {
skip |= LogError("VUID-VK_EXT_external_memory-get-handle-incorrect-handle-type", device,
error_obj.location.dot(Field::handleType),
"current value is %s", string_VkExternalMemoryHandleTypeFlagBits(handleType));
}

return skip;
}
#endif // VK_USE_PLATFORM_METAL_EXT
12 changes: 12 additions & 0 deletions layers/core_checks/core_validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,9 @@ class CoreChecks : public ValidationStateTracker {
bool ValidatePhysicalDeviceQueueFamilies(uint32_t queue_family_count, const uint32_t* queue_families, const Location& loc,
const char* vuid) const;
bool ValidateAllocateMemoryANDROID(const VkMemoryAllocateInfo& allocate_info, const Location& allocate_info_loc) const;
bool ValidateAllocateMemoryMetal(const VkMemoryAllocateInfo& allocate_info,
const VkMemoryDedicatedAllocateInfo *dedicated_allocation_info,
const Location& allocate_info_loc) const;
bool ValidateGetImageMemoryRequirementsANDROID(const VkImage image, const Location& loc) const;
bool ValidateGetPhysicalDeviceImageFormatProperties2ANDROID(const VkPhysicalDeviceImageFormatInfo2* pImageFormatInfo,
const VkImageFormatProperties2* pImageFormatProperties,
Expand Down Expand Up @@ -1878,6 +1881,15 @@ class CoreChecks : public ValidationStateTracker {
const VkSemaphoreGetZirconHandleInfoFUCHSIA* pGetZirconHandleInfo,
zx_handle_t* pZirconHandle, const RecordObject& record_obj) override;
#endif
#ifdef VK_USE_PLATFORM_METAL_EXT
bool PreCallValidateGetMemoryMetalHandleEXT(VkDevice device,
const VkMemoryGetMetalHandleInfoEXT* pGetMetalHandleInfo, MTLResource_id* pHandle,
const ErrorObject& error_obj) const override;
bool PreCallValidateGetMemoryMetalHandlePropertiesEXT(VkDevice device,
VkExternalMemoryHandleTypeFlagBits handleType, MTLResource_id handle,
VkMemoryMetalHandlePropertiesEXT* pMemoryMetalHandleProperties,
const ErrorObject& error_obj) const override;
#endif // VK_USE_PLATFORM_METAL_EXT
#ifdef VK_USE_PLATFORM_WIN32_KHR
bool PreCallValidateGetMemoryWin32HandleKHR(VkDevice device, const VkMemoryGetWin32HandleInfoKHR* pGetWin32HandleInfo,
HANDLE* pHandle, const ErrorObject& error_obj) const override;
Expand Down
6 changes: 6 additions & 0 deletions layers/stateless/sl_external_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,12 @@ ExternalOperationsInfo GetExternalOperationsInfo(const void *pNext) {
ext.total_import_ops += (import_info_qnx && import_info_qnx->buffer);
#endif // VK_USE_PLATFORM_SCREEN_QNX

#ifdef VK_USE_PLATFORM_METAL_EXT
// VK_EXT_external_memory_metal
auto import_info_metal = vku::FindStructInPNextChain<VkImportMemoryMetalHandleInfoEXT>(pNext);
ext.total_import_ops += (import_info_metal && import_info_metal->handle);
#endif // VK_USE_PLATFORM_METAL_EXT

return ext;
}
} // namespace
Expand Down
23 changes: 23 additions & 0 deletions layers/vulkan/generated/best_practices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2994,6 +2994,29 @@ void BestPractices::PostCallRecordGetScreenBufferPropertiesQNX(VkDevice device,
}
#endif // VK_USE_PLATFORM_SCREEN_QNX

#ifdef VK_USE_PLATFORM_METAL_EXT
void BestPractices::PostCallRecordGetMemoryMetalHandleEXT(VkDevice device, const VkMemoryGetMetalHandleInfoEXT* pGetMetalHandleInfo,
MTLResource_id* pHandle, const RecordObject& record_obj) {
ValidationStateTracker::PostCallRecordGetMemoryMetalHandleEXT(device, pGetMetalHandleInfo, pHandle, record_obj);

if (record_obj.result < VK_SUCCESS) {
LogErrorCode(record_obj);
}
}

void BestPractices::PostCallRecordGetMemoryMetalHandlePropertiesEXT(VkDevice device, VkExternalMemoryHandleTypeFlagBits handleType,
MTLResource_id handle,
VkMemoryMetalHandlePropertiesEXT* pMemoryMetalHandleProperties,
const RecordObject& record_obj) {
ValidationStateTracker::PostCallRecordGetMemoryMetalHandlePropertiesEXT(device, handleType, handle,
pMemoryMetalHandleProperties, record_obj);

if (record_obj.result < VK_SUCCESS) {
LogErrorCode(record_obj);
}
}
#endif // VK_USE_PLATFORM_METAL_EXT

void BestPractices::PostCallRecordCreateAccelerationStructureKHR(VkDevice device,
const VkAccelerationStructureCreateInfoKHR* pCreateInfo,
const VkAllocationCallbacks* pAllocator,
Expand Down
10 changes: 10 additions & 0 deletions layers/vulkan/generated/best_practices.h
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,16 @@ void PostCallRecordGetScreenBufferPropertiesQNX(VkDevice device, const struct _s
VkScreenBufferPropertiesQNX* pProperties, const RecordObject& record_obj) override;

#endif // VK_USE_PLATFORM_SCREEN_QNX
#ifdef VK_USE_PLATFORM_METAL_EXT
void PostCallRecordGetMemoryMetalHandleEXT(VkDevice device, const VkMemoryGetMetalHandleInfoEXT* pGetMetalHandleInfo,
MTLResource_id* pHandle, const RecordObject& record_obj) override;

void PostCallRecordGetMemoryMetalHandlePropertiesEXT(VkDevice device, VkExternalMemoryHandleTypeFlagBits handleType,
MTLResource_id handle,
VkMemoryMetalHandlePropertiesEXT* pMemoryMetalHandleProperties,
const RecordObject& record_obj) override;

#endif // VK_USE_PLATFORM_METAL_EXT
void PostCallRecordCreateAccelerationStructureKHR(VkDevice device, const VkAccelerationStructureCreateInfoKHR* pCreateInfo,
const VkAllocationCallbacks* pAllocator,
VkAccelerationStructureKHR* pAccelerationStructure,
Expand Down
Loading