Skip to content

Commit 3e762b2

Browse files
LukaszJobczykLukaszJobczyk
committed
Split buffer object validation and early pinning
Change-Id: If1b136807dc8593179ce743c8e0187ee80c3e95f Signed-off-by: Lukasz Jobczyk <[email protected]>
1 parent 6b0e12e commit 3e762b2

File tree

5 files changed

+112
-19
lines changed

5 files changed

+112
-19
lines changed

‎opencl/test/unit_test/os_interface/linux/drm_buffer_object_tests.cpp‎

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ TEST_F(DrmBufferObjectTest, givenAddressThatWhenSizeIsAddedWithin32BitBoundaryWh
133133
EXPECT_TRUE(execObject.flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS);
134134
}
135135

136-
TEST_F(DrmBufferObjectTest, onPinIoctlFailed) {
136+
TEST_F(DrmBufferObjectTest, whenExecFailsThenPinFails) {
137137
std::unique_ptr<uint32_t[]> buff(new uint32_t[1024]);
138138

139139
mock->ioctl_expected.total = 1;
@@ -145,7 +145,23 @@ TEST_F(DrmBufferObjectTest, onPinIoctlFailed) {
145145

146146
bo->setAddress(reinterpret_cast<uint64_t>(buff.get()));
147147
BufferObject *boArray[1] = {boToPin.get()};
148-
auto ret = bo->pin(boArray, 1, osContext.get(), 0, 1, true);
148+
auto ret = bo->pin(boArray, 1, osContext.get(), 0, 1);
149+
EXPECT_EQ(EINVAL, ret);
150+
}
151+
152+
TEST_F(DrmBufferObjectTest, whenExecFailsThenValidateHostPtrFails) {
153+
std::unique_ptr<uint32_t[]> buff(new uint32_t[1024]);
154+
155+
mock->ioctl_expected.total = 1;
156+
mock->ioctl_res = -1;
157+
this->mock->errnoValue = EINVAL;
158+
159+
std::unique_ptr<BufferObject> boToPin(new TestedBufferObject(this->mock.get()));
160+
ASSERT_NE(nullptr, boToPin.get());
161+
162+
bo->setAddress(reinterpret_cast<uint64_t>(buff.get()));
163+
BufferObject *boArray[1] = {boToPin.get()};
164+
auto ret = bo->validateHostPtr(boArray, 1, osContext.get(), 0, 1);
149165
EXPECT_EQ(EINVAL, ret);
150166
}
151167

@@ -161,7 +177,7 @@ TEST_F(DrmBufferObjectTest, givenResidentBOWhenPrintExecutionBufferIsSetToTrueTh
161177
BufferObject *boArray[1] = {bo.get()};
162178

163179
testing::internal::CaptureStdout();
164-
auto ret = bo->pin(boArray, 1, osContext.get(), 0, 1, true);
180+
auto ret = bo->pin(boArray, 1, osContext.get(), 0, 1);
165181
EXPECT_EQ(0, ret);
166182

167183
std::string output = testing::internal::GetCapturedStdout();
@@ -207,6 +223,29 @@ TEST_F(DrmBufferObjectTest, whenPrintExecutionBufferIsSetToTrueThenMessageFoundI
207223
EXPECT_EQ(expectedValue, idx);
208224
}
209225

226+
TEST(DrmBufferObjectSimpleTest, givenInvalidBoWhenValidateHostptrIsCalledThenErrorIsReturned) {
227+
std::unique_ptr<uint32_t[]> buff(new uint32_t[256]);
228+
std::unique_ptr<DrmMockCustom> mock(new DrmMockCustom);
229+
OsContextLinux osContext(*mock, 0u, 1, aub_stream::ENGINE_RCS, PreemptionMode::Disabled, false, false, false);
230+
ASSERT_NE(nullptr, mock.get());
231+
std::unique_ptr<TestedBufferObject> bo(new TestedBufferObject(mock.get()));
232+
ASSERT_NE(nullptr, bo.get());
233+
234+
// fail DRM_IOCTL_I915_GEM_EXECBUFFER2 in pin
235+
mock->ioctl_res = -1;
236+
237+
std::unique_ptr<BufferObject> boToPin(new TestedBufferObject(mock.get()));
238+
ASSERT_NE(nullptr, boToPin.get());
239+
240+
bo->setAddress(reinterpret_cast<uint64_t>(buff.get()));
241+
mock->errnoValue = EFAULT;
242+
243+
BufferObject *boArray[1] = {boToPin.get()};
244+
auto ret = bo->pin(boArray, 1, &osContext, 0, 1);
245+
EXPECT_EQ(EFAULT, ret);
246+
mock->ioctl_res = 0;
247+
}
248+
210249
TEST(DrmBufferObjectSimpleTest, givenInvalidBoWhenPinIsCalledThenErrorIsReturned) {
211250
std::unique_ptr<uint32_t[]> buff(new uint32_t[256]);
212251
std::unique_ptr<DrmMockCustom> mock(new DrmMockCustom);
@@ -225,7 +264,7 @@ TEST(DrmBufferObjectSimpleTest, givenInvalidBoWhenPinIsCalledThenErrorIsReturned
225264
mock->errnoValue = EFAULT;
226265

227266
BufferObject *boArray[1] = {boToPin.get()};
228-
auto ret = bo->pin(boArray, 1, &osContext, 0, 1, true);
267+
auto ret = bo->validateHostPtr(boArray, 1, &osContext, 0, 1);
229268
EXPECT_EQ(EFAULT, ret);
230269
mock->ioctl_res = 0;
231270
}
@@ -258,7 +297,40 @@ TEST(DrmBufferObjectSimpleTest, givenArrayOfBosWhenPinnedThenAllBosArePinned) {
258297
BufferObject *array[3] = {boToPin.get(), boToPin2.get(), boToPin3.get()};
259298

260299
bo->setAddress(reinterpret_cast<uint64_t>(buff.get()));
261-
auto ret = bo->pin(array, 3, &osContext, 0, 1, true);
300+
auto ret = bo->pin(array, 3, &osContext, 0, 1);
301+
EXPECT_EQ(mock->ioctl_res, ret);
302+
303+
EXPECT_LT(0u, mock->execBuffer.batch_len);
304+
EXPECT_EQ(4u, mock->execBuffer.buffer_count); // 3 bos to pin plus 1 exec bo
305+
EXPECT_EQ(reinterpret_cast<uintptr_t>(boToPin->execObjectPointerFilled), mock->execBuffer.buffers_ptr);
306+
EXPECT_NE(nullptr, boToPin2->execObjectPointerFilled);
307+
EXPECT_NE(nullptr, boToPin3->execObjectPointerFilled);
308+
309+
bo->setAddress(0llu);
310+
}
311+
312+
TEST(DrmBufferObjectSimpleTest, givenArrayOfBosWhenValidatedThenAllBosArePinned) {
313+
std::unique_ptr<uint32_t[]> buff(new uint32_t[256]);
314+
std::unique_ptr<DrmMockCustom> mock(new DrmMockCustom);
315+
ASSERT_NE(nullptr, mock.get());
316+
OsContextLinux osContext(*mock, 0u, 1, aub_stream::ENGINE_RCS, PreemptionMode::Disabled, false, false, false);
317+
318+
std::unique_ptr<TestedBufferObject> bo(new TestedBufferObject(mock.get()));
319+
ASSERT_NE(nullptr, bo.get());
320+
mock->ioctl_res = 0;
321+
322+
std::unique_ptr<TestedBufferObject> boToPin(new TestedBufferObject(mock.get()));
323+
std::unique_ptr<TestedBufferObject> boToPin2(new TestedBufferObject(mock.get()));
324+
std::unique_ptr<TestedBufferObject> boToPin3(new TestedBufferObject(mock.get()));
325+
326+
ASSERT_NE(nullptr, boToPin.get());
327+
ASSERT_NE(nullptr, boToPin2.get());
328+
ASSERT_NE(nullptr, boToPin3.get());
329+
330+
BufferObject *array[3] = {boToPin.get(), boToPin2.get(), boToPin3.get()};
331+
332+
bo->setAddress(reinterpret_cast<uint64_t>(buff.get()));
333+
auto ret = bo->validateHostPtr(array, 3, &osContext, 0, 1);
262334
EXPECT_EQ(mock->ioctl_res, ret);
263335

264336
EXPECT_LT(0u, mock->execBuffer.batch_len);

‎opencl/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3118,7 +3118,7 @@ TEST_F(DrmMemoryManagerWithExplicitExpectationsTest, givenDisabledForcePinAndEna
31183118
PinBufferObject(Drm *drm) : BufferObject(drm, 1, 0, 1) {
31193119
}
31203120

3121-
int pin(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId, bool validateHostptr) override {
3121+
int validateHostPtr(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId) override {
31223122
for (size_t i = 0; i < numberOfBos; i++) {
31233123
pinnedBoArray[i] = boToPin[i];
31243124
}

‎shared/source/os_interface/linux/drm_buffer_object.cpp‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -205,18 +205,38 @@ void BufferObject::printExecutionBuffer(drm_i915_gem_execbuffer2 &execbuf, const
205205
std::cout << logger.str() << std::endl;
206206
}
207207

208-
int BufferObject::pin(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId, bool validateHostPtr) {
208+
int bindBOsWithinContext(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId) {
209209
auto retVal = 0;
210210

211-
if ((validateHostPtr && osContext->isDirectSubmissionActive()) ||
212-
(!validateHostPtr && this->drm->isBindAvailable())) {
213-
for (auto drmIterator = 0u; drmIterator < osContext->getDeviceBitfield().size(); drmIterator++) {
214-
if (osContext->getDeviceBitfield().test(drmIterator)) {
215-
for (size_t i = 0; i < numberOfBos; i++) {
216-
retVal |= boToPin[i]->bind(osContext, drmIterator);
217-
}
211+
for (auto drmIterator = 0u; drmIterator < osContext->getDeviceBitfield().size(); drmIterator++) {
212+
if (osContext->getDeviceBitfield().test(drmIterator)) {
213+
for (size_t i = 0; i < numberOfBos; i++) {
214+
retVal |= boToPin[i]->bind(osContext, drmIterator);
218215
}
219216
}
217+
}
218+
219+
return retVal;
220+
}
221+
222+
int BufferObject::pin(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId) {
223+
auto retVal = 0;
224+
225+
if (this->drm->isBindAvailable()) {
226+
retVal = bindBOsWithinContext(boToPin, numberOfBos, osContext, vmHandleId);
227+
} else {
228+
StackVec<drm_i915_gem_exec_object2, maxFragmentsCount + 1> execObject(numberOfBos + 1);
229+
retVal = this->exec(4u, 0u, 0u, false, osContext, vmHandleId, drmContextId, boToPin, numberOfBos, &execObject[0]);
230+
}
231+
232+
return retVal;
233+
}
234+
235+
int BufferObject::validateHostPtr(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId) {
236+
auto retVal = 0;
237+
238+
if (osContext->isDirectSubmissionActive()) {
239+
retVal = bindBOsWithinContext(boToPin, numberOfBos, osContext, vmHandleId);
220240
} else {
221241
StackVec<drm_i915_gem_exec_object2, maxFragmentsCount + 1> execObject(numberOfBos + 1);
222242
retVal = this->exec(4u, 0u, 0u, false, osContext, vmHandleId, drmContextId, boToPin, numberOfBos, &execObject[0]);

‎shared/source/os_interface/linux/drm_buffer_object.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ class BufferObject {
4343

4444
bool setTiling(uint32_t mode, uint32_t stride);
4545

46-
MOCKABLE_VIRTUAL int pin(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId, bool validateHostptr);
46+
int pin(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId);
47+
MOCKABLE_VIRTUAL int validateHostPtr(BufferObject *const boToPin[], size_t numberOfBos, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId);
4748

4849
int exec(uint32_t used, size_t startOffset, unsigned int flags, bool requiresCoherency, OsContext *osContext, uint32_t vmHandleId, uint32_t drmContextId, BufferObject *const residency[], size_t residencyCount, drm_i915_gem_exec_object2 *execObjectsStorage);
4950

‎shared/source/os_interface/linux/drm_memory_manager.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ NEO::BufferObject *DrmMemoryManager::allocUserptr(uintptr_t address, size_t size
199199

200200
void DrmMemoryManager::emitPinningRequest(BufferObject *bo, const AllocationData &allocationData) const {
201201
if (forcePinEnabled && pinBBs.at(allocationData.rootDeviceIndex) != nullptr && allocationData.flags.forcePin && allocationData.size >= this->pinThreshold) {
202-
pinBBs.at(allocationData.rootDeviceIndex)->pin(&bo, 1, registeredEngines[defaultEngineIndex].osContext, 0, getDefaultDrmContextId(), false);
202+
pinBBs.at(allocationData.rootDeviceIndex)->pin(&bo, 1, registeredEngines[defaultEngineIndex].osContext, 0, getDefaultDrmContextId());
203203
}
204204
}
205205

@@ -344,7 +344,7 @@ GraphicsAllocation *DrmMemoryManager::allocateGraphicsMemoryWithGpuVa(const Allo
344344

345345
BufferObject *boPtr = bo.get();
346346
if (forcePinEnabled && pinBBs.at(allocationData.rootDeviceIndex) != nullptr && alignedSize >= this->pinThreshold) {
347-
pinBBs.at(allocationData.rootDeviceIndex)->pin(&boPtr, 1, osContextLinux, 0, osContextLinux->getDrmContextIds()[0], false);
347+
pinBBs.at(allocationData.rootDeviceIndex)->pin(&boPtr, 1, osContextLinux, 0, osContextLinux->getDrmContextIds()[0]);
348348
}
349349

350350
auto allocation = new DrmAllocation(allocationData.rootDeviceIndex, allocationData.type, bo.get(), res, bo->gpuAddress, alignedSize, MemoryPool::System4KBPages);
@@ -378,7 +378,7 @@ DrmAllocation *DrmMemoryManager::allocateGraphicsMemoryForNonSvmHostPtr(const Al
378378

379379
if (validateHostPtrMemory) {
380380
auto boPtr = bo.get();
381-
int result = pinBBs.at(allocationData.rootDeviceIndex)->pin(&boPtr, 1, registeredEngines[defaultEngineIndex].osContext, 0, getDefaultDrmContextId(), true);
381+
int result = pinBBs.at(allocationData.rootDeviceIndex)->validateHostPtr(&boPtr, 1, registeredEngines[defaultEngineIndex].osContext, 0, getDefaultDrmContextId());
382382
if (result != SUCCESS) {
383383
unreference(bo.release(), true);
384384
releaseGpuRange(reinterpret_cast<void *>(gpuVirtualAddress), alignedSize, allocationData.rootDeviceIndex);
@@ -745,7 +745,7 @@ MemoryManager::AllocationStatus DrmMemoryManager::populateOsHandles(OsHandleStor
745745
}
746746

747747
if (validateHostPtrMemory) {
748-
int result = pinBBs.at(rootDeviceIndex)->pin(allocatedBos, numberOfBosAllocated, registeredEngines[defaultEngineIndex].osContext, 0, getDefaultDrmContextId(), true);
748+
int result = pinBBs.at(rootDeviceIndex)->validateHostPtr(allocatedBos, numberOfBosAllocated, registeredEngines[defaultEngineIndex].osContext, 0, getDefaultDrmContextId());
749749

750750
if (result == EFAULT) {
751751
for (uint32_t i = 0; i < numberOfBosAllocated; i++) {

0 commit comments

Comments
 (0)