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

dEQP-VK.memory.external_memory_host: Tests have Invalid Usage #229

Closed
linyaa-kiwi opened this issue Oct 1, 2020 · 6 comments
Closed

dEQP-VK.memory.external_memory_host: Tests have Invalid Usage #229

linyaa-kiwi opened this issue Oct 1, 2020 · 6 comments

Comments

@linyaa-kiwi
Copy link
Contributor

The below invalid usage pattern is found in many of these tests:

  1. Call vkCreateImage with some parameters P, where the pNext chain in P contains VkExternalMemoryImageCreateInfo::handleTypes = VK_EXTERNAL_MEMORY_HANDLE_TYPE_HOST_ALLOCATION_BIT_EXT.
  2. After creating the image, confirm that parameters P are valid for image creation (vkGetPhysicalDeviceImageProperties2).
  3. Fail the test if vkGetPhysicalDeviceImageFormatPropertes2 fails.

At a high-level, this is invalid usage because the test creates the image before confirming that the parameters P are valid.

Specifically, this violates VUID-VkImageCreateInfo-imageCreateMaxMipLevels-02251, which says

Each of the following values (as described in Image Creation Limits) must not be undefined: [list of limits related to fields in VkImageCreateInfo]".

The section Image Creation Limits defines the limits in terms of successfull calls to vkGetPhysicalDeviceImageFormatProperties2. According to that sections, if the app calls vkCreateImage with a VkExternalMemoryHandleType, then the limits are well-defined only if the app first makes a successful query to vkGetPhysicalDeviceImageFormatProperties2 with the same VkExternalMemoryHandleType. If the app does not first call vkGetPhysicalDeviceImageFormatProperties2, or if the call fails, then the app must not call vkCreateImage.

The invalid usage occurs in the code below (as of 02da53e).

  1. Line 313: Create image.
  2. Line 371: Confirm that image creation parameters were valid.

tcu::TestStatus ExternalMemoryHostRenderImageTestInstance::iterate ()
{
VkClearColorValue clearColorBlue = { { 0.0f, 0.0f, 1.0f, 1.0f } };
const deUint32 queueFamilyIndex = m_context.getUniversalQueueFamilyIndex();
deUint32 hostPointerMemoryTypeBits;
deUint32 memoryTypeIndexToTest;
VkMemoryRequirements imageMemoryRequirements;
const VkImageTiling tiling = VK_IMAGE_TILING_OPTIMAL;
const VkImageUsageFlags usageFlags = (VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT);
m_image = createImage(m_testParams.m_format, tiling, usageFlags);
//check memory requirements and reallocate memory if needed
imageMemoryRequirements = getImageMemoryRequirements(m_vkd, m_device, *m_image);
if (m_testParams.m_useOffset == false)
{
VkDeviceSize requiredSize = imageMemoryRequirements.size;
if (requiredSize > m_allocationSize)
{
//calculate new size, this must me a multiple of minImportedHostPointerAlignment
VkDeviceSize newHostAllocationSize = VkDeviceSize(deCeilFloatToInt32((float(requiredSize) / float(m_minImportedHostPointerAlignment))) * m_minImportedHostPointerAlignment);
m_log << tcu::TestLog::Message << "Realloc needed (required size: " << requiredSize << "). " << "New host allocation size: " << newHostAllocationSize << ")."
<< tcu::TestLog::EndMessage;
//realocate
m_hostMemoryAlloc = deAlignedRealloc(m_hostMemoryAlloc, (size_t)newHostAllocationSize, (size_t)m_minImportedHostPointerAlignment);
m_allocationSize = newHostAllocationSize;
}
}
if (m_testParams.m_useOffset == true)
{
VkDeviceSize requiredSize = imageMemoryRequirements.size + imageMemoryRequirements.alignment;
if (requiredSize > m_allocationSize)
{
VkDeviceSize newHostAllocationSize = VkDeviceSize(deCeilFloatToInt32((float(requiredSize) / float(m_minImportedHostPointerAlignment))) * m_minImportedHostPointerAlignment);
m_log << tcu::TestLog::Message << "Realloc needed (required size: " << requiredSize << "). " << "New host allocation size: " << newHostAllocationSize << ")."
<< tcu::TestLog::EndMessage;
m_hostMemoryAlloc = deAlignedRealloc(m_hostMemoryAlloc, (size_t)newHostAllocationSize, (size_t)m_minImportedHostPointerAlignment);
m_allocationSize = newHostAllocationSize;
}
}
//check if reallocation is successfull
if (!m_hostMemoryAlloc)
TCU_FAIL("Failed to reallocate memory block.");
DE_ASSERT(deIsAlignedPtr(m_hostMemoryAlloc, (deUintptr)m_minImportedHostPointerAlignment));
//find the usable memory type index
hostPointerMemoryTypeBits = getHostPointerMemoryTypeBits(m_hostMemoryAlloc);
if (findCompatibleMemoryTypeIndexToTest(imageMemoryRequirements.memoryTypeBits, hostPointerMemoryTypeBits, &memoryTypeIndexToTest))
m_deviceMemoryAllocatedFromHostPointer = allocateMemoryFromHostPointer(memoryTypeIndexToTest);
else
TCU_THROW(NotSupportedError, "Compatible memory type not found");
// Verify image format properties before proceeding.
verifyFormatProperties(m_testParams.m_format, tiling, usageFlags);

@linyaa-kiwi
Copy link
Contributor Author

Adding people.

@djdeath
@jekstrand
@rg3igalia
@BNieuwenhuizen

jpark37 pushed a commit to jpark37/VK-GL-CTS that referenced this issue Oct 3, 2020
Add the existing waiver on PowerVR cores related to snorm filtering to
the new waiver list.

This adds both of the existing test groups to the waiver along with the
newly introduced tests which hit this bug in the affected GPUs.

Components: Vulkan
VK-GL-CTS issue: 229
VK-GL-CTS issue: 2418
VK-GL-CTS issue: 2189

Change-Id: I15b5d5121fc0a4026944bc73d772c4d9cddc9031
@rg3igalia
Copy link
Contributor

Hi, @chadversary

I've created the following pull request, which should solve the problem: #232

Please let me know if that works for you. If so, I'll close that public PR and submit the same changes for IHV review under Khronos.

@rg3igalia
Copy link
Contributor

Public PR closed and submitted for IHV review.

@linyaa-kiwi
Copy link
Contributor Author

Yes, #232 fixes the problem for me. Thanks.

@linyaa-kiwi
Copy link
Contributor Author

Waiting for Change-Id: Ied982d4a8b61a7ad97bac8a096f1a2827a0c0f49 to land in the public branch before closing this issue.

@alegal-arm
Copy link
Contributor

fixed by 578f0f6

alegal-arm pushed a commit that referenced this issue Jul 9, 2021
vulkan-cts-1.2.7.0 introduced new tests to validate that an
implementation clamps the most negative snorm values to -1.0 when
using linear filtering.

There is an existing waiver that covers some Imagination GPUs which do
not correctly round negative snorm values.

Update this waiver to apply to the newly added tests.

Affects
dEQP-VK.texture.conversion.snorm_clamp_linear.*

VK-GL-CTS issue: 229
VK-GL-CTS issue: 2989

Change-Id: Ic23f0a042f7489bcfd9fb838260e920f7d19f2c6
mnetsch pushed a commit that referenced this issue Oct 14, 2022
Update waiver #229 after test names were changed to add tests for
VK_EXT_graphics_pipeline_libraries

Components: vulkan

VK-GL-CTS issue: 229

Change-Id: I33bd7a44631c8ddb95eff3a95db04033e2773983
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants