From ef7dbc99f1abdb559fab1b82016df7eb8c711022 Mon Sep 17 00:00:00 2001 From: Compute-Runtime-Validation Date: Thu, 14 Mar 2024 14:04:40 +0100 Subject: [PATCH] Revert "fix: don't use fake userptr flag in ioctl helper xe" This reverts commit 98824fdaf6d86537348dc3fa36d0acce9ac4f83e. Signed-off-by: Compute-Runtime-Validation --- .../os_interface/linux/drm_buffer_object.cpp | 1 - .../os_interface/linux/drm_buffer_object.h | 3 --- .../os_interface/linux/drm_memory_manager.cpp | 3 +-- .../source/os_interface/linux/drm_wrappers.h | 1 - .../os_interface/linux/drm_wrappers_checks.cpp | 2 +- .../os_interface/linux/xe/ioctl_helper_xe.cpp | 18 ++++++++---------- .../os_interface/linux/xe/ioctl_helper_xe.h | 1 + .../linux/drm_memory_manager_tests.cpp | 2 +- .../linux/xe/ioctl_helper_xe_tests.cpp | 8 ++------ 9 files changed, 14 insertions(+), 25 deletions(-) diff --git a/shared/source/os_interface/linux/drm_buffer_object.cpp b/shared/source/os_interface/linux/drm_buffer_object.cpp index 2b921dca2d..4aa51c631e 100644 --- a/shared/source/os_interface/linux/drm_buffer_object.cpp +++ b/shared/source/os_interface/linux/drm_buffer_object.cpp @@ -120,7 +120,6 @@ bool BufferObject::close() { GemClose close{}; close.handle = this->handle.getBoHandle(); - close.userptr = this->userptr; PRINT_DEBUG_STRING(debugManager.flags.PrintBOCreateDestroyResult.get(), stdout, "Calling gem close on handle: BO-%d\n", this->handle.getBoHandle()); diff --git a/shared/source/os_interface/linux/drm_buffer_object.h b/shared/source/os_interface/linux/drm_buffer_object.h index 63dfcdbce5..c69ce84dc1 100644 --- a/shared/source/os_interface/linux/drm_buffer_object.h +++ b/shared/source/os_interface/linux/drm_buffer_object.h @@ -214,8 +214,6 @@ class BufferObject { BOType peekBOType() const { return boType; } void setBOType(BOType newBoType) { this->boType = newBoType; } - void setUserptr(uint64_t ptr) { this->userptr = ptr; }; - static constexpr int gpuHangDetected{-7171}; uint32_t getOsContextId(OsContext *osContext); @@ -247,7 +245,6 @@ class BufferObject { uint64_t size; uint64_t unmapSize = 0; uint64_t patIndex = CommonConstants::unsupportedPatIndex; - uint64_t userptr = 0u; CacheRegion cacheRegion = CacheRegion::defaultRegion; CachePolicy cachePolicy = CachePolicy::writeBack; StackVec bindExtHandles; diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index 066845cea2..f7c15908f4 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.cpp +++ b/shared/source/os_interface/linux/drm_memory_manager.cpp @@ -321,9 +321,8 @@ NEO::BufferObject *DrmMemoryManager::allocUserptr(uintptr_t address, size_t size DEBUG_BREAK_IF(true); return nullptr; } - res->setAddress(address); - res->setUserptr(address); + return res; } diff --git a/shared/source/os_interface/linux/drm_wrappers.h b/shared/source/os_interface/linux/drm_wrappers.h index d56df2d8a6..ea354d0d52 100644 --- a/shared/source/os_interface/linux/drm_wrappers.h +++ b/shared/source/os_interface/linux/drm_wrappers.h @@ -179,7 +179,6 @@ struct Query { struct GemClose { uint32_t handle; uint32_t reserved; - uint64_t userptr; }; struct PrimeHandle { diff --git a/shared/source/os_interface/linux/drm_wrappers_checks.cpp b/shared/source/os_interface/linux/drm_wrappers_checks.cpp index 15910166fd..92bf35fcde 100644 --- a/shared/source/os_interface/linux/drm_wrappers_checks.cpp +++ b/shared/source/os_interface/linux/drm_wrappers_checks.cpp @@ -137,7 +137,7 @@ static_assert(offsetof(Query, numItems) == offsetof(struct drm_i915_query, num_i static_assert(offsetof(Query, flags) == offsetof(struct drm_i915_query, flags)); static_assert(offsetof(Query, itemsPtr) == offsetof(struct drm_i915_query, items_ptr)); -static_assert(sizeof(GemClose) >= sizeof(drm_gem_close)); +static_assert(sizeof(GemClose) == sizeof(drm_gem_close)); static_assert(offsetof(GemClose, handle) == offsetof(drm_gem_close, handle)); static_assert(offsetof(GemClose, reserved) == offsetof(drm_gem_close, pad)); diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp index 82223ea36c..7429f45568 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp @@ -37,6 +37,9 @@ #define STRINGIFY_ME(X) return #X #define RETURN_ME(X) return X +#define XE_USERPTR_FAKE_FLAG 0x800000 +#define XE_USERPTR_FAKE_MASK 0x7FFFFF + namespace NEO { const char *IoctlHelperXe::xeGetClassName(int className) { @@ -972,7 +975,8 @@ int IoctlHelperXe::ioctl(DrmIoctl request, void *arg) { } break; case DrmIoctl::gemUserptr: { GemUserPtr *d = static_cast(arg); - updateBindInfo(0, d->userPtr, d->userSize); + d->handle = userPtrHandle++ | XE_USERPTR_FAKE_FLAG; + updateBindInfo(d->handle, d->userPtr, d->userSize); ret = 0; xeLog(" -> IoctlHelperXe::ioctl GemUserptr p=0x%llx s=0x%llx f=0x%x h=0x%x r=%d\n", d->userPtr, d->userSize, d->flags, d->handle, ret); @@ -1050,17 +1054,11 @@ int IoctlHelperXe::ioctl(DrmIoctl request, void *arg) { struct GemClose *d = static_cast(arg); int found = -1; xeShowBindTable(); - bool isUserptr = false; for (unsigned int i = 0; i < bindInfo.size(); i++) { if (d->handle == bindInfo[i].handle) { found = i; break; } - if (d->userptr == bindInfo[i].userptr) { - found = i; - isUserptr = true; - break; - } } if (found != -1) { xeLog(" removing %d: 0x%x 0x%lx 0x%lx\n", @@ -1072,7 +1070,7 @@ int IoctlHelperXe::ioctl(DrmIoctl request, void *arg) { std::unique_lock lock(xeLock); bindInfo.erase(bindInfo.begin() + found); } - if (isUserptr) { + if (d->handle & XE_USERPTR_FAKE_FLAG) { // nothing to do under XE ret = 0; } else { @@ -1310,7 +1308,7 @@ int IoctlHelperXe::xeVmBind(const VmBindParams &vmBindParams, bool isBind) { if (isBind) { bind.bind.op = DRM_XE_VM_BIND_OP_MAP; bind.bind.obj = vmBindParams.handle; - if (bindInfo[index].userptr) { + if (bindInfo[index].handle & XE_USERPTR_FAKE_FLAG) { bind.bind.op = DRM_XE_VM_BIND_OP_MAP_USERPTR; bind.bind.obj = 0; bind.bind.obj_offset = bindInfo[index].userptr; @@ -1318,7 +1316,7 @@ int IoctlHelperXe::xeVmBind(const VmBindParams &vmBindParams, bool isBind) { } else { bind.bind.op = DRM_XE_VM_BIND_OP_UNMAP; bind.bind.obj = 0; - if (bindInfo[index].userptr) { + if (bindInfo[index].handle & XE_USERPTR_FAKE_FLAG) { bind.bind.obj_offset = bindInfo[index].userptr; } } diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h index dcdcf3dab6..828f69cd1b 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h @@ -173,6 +173,7 @@ class IoctlHelperXe : public IoctlHelper { int defaultAlignment = 0; int hasVram = 0; uint32_t xeVmId = 0; + uint32_t userPtrHandle = 0; int xeFileHandle = 0; std::mutex xeLock; std::vector bindInfo; diff --git a/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp index bacad8a046..5789942f05 100644 --- a/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp @@ -1048,7 +1048,7 @@ TEST_F(DrmMemoryManagerTest, WhenAskedButNotAllowedHostPtrThenDoNotPinAfterAlloc TEST_F(DrmMemoryManagerTest, WhenUnreferenceIsCalledThenCallSucceeds) { mock->ioctlExpected.gemUserptr = 1; mock->ioctlExpected.gemClose = 1; - BufferObject *bo = memoryManager->allocUserptr(1234, (size_t)1024, rootDeviceIndex); + BufferObject *bo = memoryManager->allocUserptr(0, (size_t)1024, rootDeviceIndex); ASSERT_NE(nullptr, bo); memoryManager->unreference(bo, false); } diff --git a/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp b/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp index 7b5cc8dd4f..3b2e3295a7 100644 --- a/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp @@ -533,14 +533,11 @@ TEST(IoctlHelperXeTest, whenCallingIoctlThenProperValueIsReturned) { drm.testMode(0); { GemUserPtr test = {}; - test.userPtr = 2; + test.handle = 1; ret = mockXeIoctlHelper->ioctl(DrmIoctl::gemUserptr, &test); EXPECT_EQ(0, ret); - - EXPECT_EQ(test.userPtr, mockXeIoctlHelper->bindInfo[0].userptr); - EXPECT_EQ(0u, mockXeIoctlHelper->bindInfo[0].handle); GemClose cl = {}; - cl.userptr = test.userPtr; + cl.handle = test.handle; ret = mockXeIoctlHelper->ioctl(DrmIoctl::gemClose, &cl); EXPECT_EQ(0, ret); } @@ -645,7 +642,6 @@ TEST(IoctlHelperXeTest, whenCallingIoctlThenProperValueIsReturned) { } { GemClose test = {}; - test.handle = 1; ret = mockXeIoctlHelper->ioctl(DrmIoctl::gemClose, &test); EXPECT_EQ(0, ret); }