From a651625473d96c8c328adb444ee7455631e0502f Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Tue, 24 Mar 2020 15:05:12 +0100 Subject: [PATCH] Correct validation during creating command queue check if device is associated with context Related-To: NEO-3691 Change-Id: I7dfe12376bb2bb2c764b471315072a29068a0cb7 Signed-off-by: Mateusz Jablonski --- opencl/source/api/api.cpp | 10 ++++++++++ opencl/source/context/context.cpp | 9 +++++++++ opencl/source/context/context.h | 2 ++ .../unit_test/api/cl_create_command_queue_tests.inl | 10 ++++++++-- ...cl_create_command_queue_with_properties_tests.cpp | 8 ++++++++ opencl/test/unit_test/context/context_tests.cpp | 12 ++++++++++++ .../unit_test/context/driver_diagnostics_tests.cpp | 8 ++++---- .../unit_test/context/driver_diagnostics_tests.h | 7 +------ 8 files changed, 54 insertions(+), 12 deletions(-) diff --git a/opencl/source/api/api.cpp b/opencl/source/api/api.cpp index da9cb07ea6..9787172b06 100644 --- a/opencl/source/api/api.cpp +++ b/opencl/source/api/api.cpp @@ -527,6 +527,10 @@ cl_command_queue CL_API_CALL clCreateCommandQueue(cl_context context, if (retVal != CL_SUCCESS) { break; } + if (!pContext->isDeviceAssociated(*pDevice)) { + retVal = CL_INVALID_DEVICE; + break; + } cl_queue_properties props[] = { CL_QUEUE_PROPERTIES, properties, @@ -4719,6 +4723,12 @@ cl_command_queue CL_API_CALL clCreateCommandQueueWithProperties(cl_context conte return commandQueue; } + if (!pContext->isDeviceAssociated(*pDevice)) { + err.set(CL_INVALID_DEVICE); + TRACING_EXIT(clCreateCommandQueueWithProperties, &commandQueue); + return commandQueue; + } + auto minimumCreateDeviceQueueFlags = static_cast(CL_QUEUE_ON_DEVICE | CL_QUEUE_OUT_OF_ORDER_EXEC_MODE_ENABLE); auto tokenValue = properties ? *properties : 0; diff --git a/opencl/source/context/context.cpp b/opencl/source/context/context.cpp index 3f0e64d967..2b97ec9ea6 100644 --- a/opencl/source/context/context.cpp +++ b/opencl/source/context/context.cpp @@ -376,4 +376,13 @@ SchedulerKernel &Context::getSchedulerKernel() { return *static_cast(schedulerBuiltIn->pKernel); } +bool Context::isDeviceAssociated(const ClDevice &clDevice) const { + for (const auto &device : devices) { + if (device == &clDevice) { + return true; + } + } + return false; +} + } // namespace NEO diff --git a/opencl/source/context/context.h b/opencl/source/context/context.h index ef87af3d37..e7263b6a09 100644 --- a/opencl/source/context/context.h +++ b/opencl/source/context/context.h @@ -141,6 +141,8 @@ class Context : public BaseObject<_cl_context> { SchedulerKernel &getSchedulerKernel(); + bool isDeviceAssociated(const ClDevice &clDevice) const; + protected: Context(void(CL_CALLBACK *pfnNotify)(const char *, const void *, size_t, void *) = nullptr, void *userData = nullptr); diff --git a/opencl/test/unit_test/api/cl_create_command_queue_tests.inl b/opencl/test/unit_test/api/cl_create_command_queue_tests.inl index 3cc83652eb..ad094e5368 100644 --- a/opencl/test/unit_test/api/cl_create_command_queue_tests.inl +++ b/opencl/test/unit_test/api/cl_create_command_queue_tests.inl @@ -36,12 +36,18 @@ TEST_F(clCreateCommandQueueTest, GivenCorrectParametersWhenCreatingCommandQueueT TEST_F(clCreateCommandQueueTest, GivenNullContextWhenCreatingCommandQueueThenInvalidContextErrorIsReturned) { clCreateCommandQueue(nullptr, devices[testedRootDeviceIndex], 0, &retVal); - ASSERT_EQ(CL_INVALID_CONTEXT, retVal); + EXPECT_EQ(CL_INVALID_CONTEXT, retVal); } TEST_F(clCreateCommandQueueTest, GivenNullDeviceWhenCreatingCommandQueueThenInvalidDeviceErrorIsReturned) { clCreateCommandQueue(pContext, nullptr, 0, &retVal); - ASSERT_EQ(CL_INVALID_DEVICE, retVal); + EXPECT_EQ(CL_INVALID_DEVICE, retVal); +} + +TEST_F(clCreateCommandQueueTest, GivenDeviceNotAssociatedWithContextWhenCreatingCommandQueueThenInvalidDeviceErrorIsReturned) { + EXPECT_NE(devices[0], devices[testedRootDeviceIndex]); + clCreateCommandQueue(pContext, devices[0], 0, &retVal); + EXPECT_EQ(CL_INVALID_DEVICE, retVal); } TEST_F(clCreateCommandQueueTest, GivenInvalidPropertiesWhenCreatingCommandQueueThenInvalidValueErrorIsReturned) { diff --git a/opencl/test/unit_test/api/cl_create_command_queue_with_properties_tests.cpp b/opencl/test/unit_test/api/cl_create_command_queue_with_properties_tests.cpp index 2f163365a5..4c3da1677e 100644 --- a/opencl/test/unit_test/api/cl_create_command_queue_with_properties_tests.cpp +++ b/opencl/test/unit_test/api/cl_create_command_queue_with_properties_tests.cpp @@ -239,6 +239,14 @@ TEST_F(clCreateCommandQueueWithPropertiesApi, GivenNullDeviceWhenCreatingCommand EXPECT_EQ(retVal, CL_INVALID_DEVICE); } +TEST_F(clCreateCommandQueueWithPropertiesApi, GivenDeviceNotAssociatedWithContextWhenCreatingCommandQueueWithPropertiesThenInvalidDeviceErrorIsReturned) { + cl_int retVal = CL_OUT_OF_HOST_MEMORY; + EXPECT_NE(devices[0], devices[testedRootDeviceIndex]); + auto cmdq = clCreateCommandQueueWithProperties(pContext, devices[0], nullptr, &retVal); + EXPECT_EQ(nullptr, cmdq); + EXPECT_EQ(retVal, CL_INVALID_DEVICE); +} + TEST_F(clCreateCommandQueueWithPropertiesApi, GivenSizeWhichExceedsMaxDeviceQueueSizeWhenCreatingCommandQueueWithPropertiesThenInvalidQueuePropertiesErrorIsReturned) { cl_int retVal = CL_SUCCESS; cl_queue_properties ooq[] = {CL_QUEUE_PROPERTIES, CL_QUEUE_OUT_OF_ORDER_EXEC_MODE_ENABLE | CL_QUEUE_ON_DEVICE | CL_QUEUE_ON_DEVICE_DEFAULT, CL_QUEUE_SIZE, (cl_uint)0xffffffff, 0, 0}; diff --git a/opencl/test/unit_test/context/context_tests.cpp b/opencl/test/unit_test/context/context_tests.cpp index e91cfb0620..e9423ae772 100644 --- a/opencl/test/unit_test/context/context_tests.cpp +++ b/opencl/test/unit_test/context/context_tests.cpp @@ -411,3 +411,15 @@ TEST(Context, givenContextWhenCheckIfAllocationsAreMultiStorageThenReturnProperV context.contextType = ContextType::CONTEXT_TYPE_UNRESTRICTIVE; EXPECT_TRUE(context.areMultiStorageAllocationsPreferred()); } + +TEST(Context, givenContextWhenIsDeviceAssociatedIsCalledWithItsDeviceThenTrueIsReturned) { + MockContext context; + EXPECT_TRUE(context.isDeviceAssociated(*context.getDevice(0))); +} + +TEST(Context, givenContextWhenIsDeviceAssociatedIsCalledWithNotAssociatedDeviceThenFalseIsReturned) { + MockContext context0; + MockContext context1; + EXPECT_FALSE(context0.isDeviceAssociated(*context1.getDevice(0))); + EXPECT_FALSE(context1.isDeviceAssociated(*context0.getDevice(0))); +} \ No newline at end of file diff --git a/opencl/test/unit_test/context/driver_diagnostics_tests.cpp b/opencl/test/unit_test/context/driver_diagnostics_tests.cpp index 95b3c2afa2..630ae68b1a 100644 --- a/opencl/test/unit_test/context/driver_diagnostics_tests.cpp +++ b/opencl/test/unit_test/context/driver_diagnostics_tests.cpp @@ -111,7 +111,7 @@ TEST_P(PerformanceHintCommandQueueTest, GivenProfilingFlagAndPreemptionFlagWhenC if (profilingEnabled) { properties = CL_QUEUE_PROFILING_ENABLE; } - cmdQ = clCreateCommandQueue(context, clDevice, properties, &retVal); + cmdQ = clCreateCommandQueue(context, context->getDevice(0), properties, &retVal); ASSERT_NE(nullptr, cmdQ); ASSERT_EQ(CL_SUCCESS, retVal); @@ -123,7 +123,7 @@ TEST_P(PerformanceHintCommandQueueTest, GivenProfilingFlagAndPreemptionFlagWhenC EXPECT_EQ(profilingEnabled, containsHint(expectedHint, userData)); snprintf(expectedHint, DriverDiagnostics::maxHintStringSize, DriverDiagnostics::hintFormat[PROFILING_ENABLED_WITH_DISABLED_PREEMPTION], 0); - if (device->getHardwareInfo().platform.eProductFamily < IGFX_SKYLAKE && preemptionSupported && profilingEnabled) { + if (context->getDevice(0)->getHardwareInfo().platform.eProductFamily < IGFX_SKYLAKE && preemptionSupported && profilingEnabled) { EXPECT_TRUE(containsHint(expectedHint, userData)); } else { EXPECT_FALSE(containsHint(expectedHint, userData)); @@ -136,7 +136,7 @@ TEST_P(PerformanceHintCommandQueueTest, GivenEnabledProfilingFlagAndSupportedPre properties[0] = CL_QUEUE_PROPERTIES; properties[1] = CL_QUEUE_PROFILING_ENABLE; } - cmdQ = clCreateCommandQueueWithProperties(context, clDevice, properties, &retVal); + cmdQ = clCreateCommandQueueWithProperties(context, context->getDevice(0), properties, &retVal); ASSERT_NE(nullptr, cmdQ); ASSERT_EQ(CL_SUCCESS, retVal); @@ -148,7 +148,7 @@ TEST_P(PerformanceHintCommandQueueTest, GivenEnabledProfilingFlagAndSupportedPre EXPECT_EQ(profilingEnabled, containsHint(expectedHint, userData)); snprintf(expectedHint, DriverDiagnostics::maxHintStringSize, DriverDiagnostics::hintFormat[PROFILING_ENABLED_WITH_DISABLED_PREEMPTION], 0); - if (device->getHardwareInfo().platform.eProductFamily < IGFX_SKYLAKE && preemptionSupported && profilingEnabled) { + if (context->getDevice(0)->getHardwareInfo().platform.eProductFamily < IGFX_SKYLAKE && preemptionSupported && profilingEnabled) { EXPECT_TRUE(containsHint(expectedHint, userData)); } else { EXPECT_FALSE(containsHint(expectedHint, userData)); diff --git a/opencl/test/unit_test/context/driver_diagnostics_tests.h b/opencl/test/unit_test/context/driver_diagnostics_tests.h index 9326ffcc2f..d77c5e8adb 100644 --- a/opencl/test/unit_test/context/driver_diagnostics_tests.h +++ b/opencl/test/unit_test/context/driver_diagnostics_tests.h @@ -110,18 +110,13 @@ struct PerformanceHintCommandQueueTest : public PerformanceHintTest, void SetUp() override { PerformanceHintTest::SetUp(); std::tie(profilingEnabled, preemptionSupported) = GetParam(); - device = new MockDevice; - clDevice = new MockClDevice{device}; - clDevice->deviceInfo.preemptionSupported = preemptionSupported; + static_cast(context->getDevice(0))->deviceInfo.preemptionSupported = preemptionSupported; } void TearDown() override { clReleaseCommandQueue(cmdQ); - delete clDevice; PerformanceHintTest::TearDown(); } - MockDevice *device = nullptr; - MockClDevice *clDevice = nullptr; cl_command_queue cmdQ = nullptr; bool profilingEnabled = false; bool preemptionSupported = false;