fix: waitUserFence on bind and unbind in Xe

ifdu · Compute-Runtime-Automation · commit ae7cff3c3e70 · 2023-05-18T15:27:47.000+02:00
This fixes illegal memory accesses by the job submitted to the GuC.
Also some unit tests are added to harness the vmBind operation.

Related-To: NEO-7996
Signed-off-by: Francois Dugast &lt;francois.dugast@intel.com&gt;
Signed-off-by: Mateusz Jablonski &lt;mateusz.jablonski@intel.com&gt;
diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp
@@ -37,17 +37,9 @@
 #define XE_USERPTR_FAKE_MASK 0x7FFFFF
 
 #define USER_FENCE_VALUE 0xc0ffee0000000000ull
-#define XE_ONE_SEC 1000
 
 namespace NEO {
 
-#define VMBIND_FENCE_TAG 0x123987
-struct XeFakeExtUserFence {
-    uint32_t tag;
-    uint64_t addr;
-    uint64_t value;
-};
-
 static_assert(DRM_XE_ENGINE_CLASS_RENDER == I915_ENGINE_CLASS_RENDER);
 static_assert(DRM_XE_ENGINE_CLASS_COPY == I915_ENGINE_CLASS_COPY);
 static_assert(DRM_XE_ENGINE_CLASS_VIDEO_DECODE == I915_ENGINE_CLASS_VIDEO);
@@ -633,9 +625,9 @@ void IoctlHelperXe::fillVmBindExtSetPat(VmBindExtSetPatT &vmBindExtSetPat, uint6
 
 void IoctlHelperXe::fillVmBindExtUserFence(VmBindExtUserFenceT &vmBindExtUserFence, uint64_t fenceAddress, uint64_t fenceValue, uint64_t nextExtension) {
     xeLog(" -> IoctlHelperXe::%s 0x%lx 0x%lx\n", __FUNCTION__, fenceAddress, fenceValue);
-    auto xeBindExtUserFence = reinterpret_cast<XeFakeExtUserFence *>(vmBindExtUserFence);
+    auto xeBindExtUserFence = reinterpret_cast<UserFenceExtension *>(vmBindExtUserFence);
     UNRECOVERABLE_IF(!xeBindExtUserFence);
-    xeBindExtUserFence->tag = VMBIND_FENCE_TAG;
+    xeBindExtUserFence->tag = UserFenceExtension::tagValue;
     xeBindExtUserFence->addr = fenceAddress;
     xeBindExtUserFence->value = fenceValue;
 }
@@ -1194,9 +1186,9 @@ int IoctlHelperXe::xeVmBind(const VmBindParams &vmBindParams, bool bindOp) {
         struct drm_xe_sync sync[1] = {};
         sync[0].flags = DRM_XE_SYNC_USER_FENCE | DRM_XE_SYNC_SIGNAL;
         extraBindFlag = XE_VM_BIND_FLAG_ASYNC;
-        auto xeBindExtUserFence = reinterpret_cast<XeFakeExtUserFence *>(vmBindParams.extensions);
+        auto xeBindExtUserFence = reinterpret_cast<UserFenceExtension *>(vmBindParams.extensions);
         UNRECOVERABLE_IF(!xeBindExtUserFence);
-        UNRECOVERABLE_IF(xeBindExtUserFence->tag != VMBIND_FENCE_TAG);
+        UNRECOVERABLE_IF(xeBindExtUserFence->tag != UserFenceExtension::tagValue);
         sync[0].addr = xeBindExtUserFence->addr;
         sync[0].timeline_value = xeBindExtUserFence->value;
 
@@ -1234,14 +1226,14 @@ int IoctlHelperXe::xeVmBind(const VmBindParams &vmBindParams, bool bindOp) {
               bind.bind.op,
               xeGetBindOpName(bind.bind.op),
               bind.num_syncs);
-
         ret = IoctlHelper::ioctl(DrmIoctl::GemVmBind, &bind);
-
-        if (!bindOp) {
-            return xeWaitUserFence(DRM_XE_UFENCE_WAIT_U64, DRM_XE_UFENCE_WAIT_EQ,
-                                   sync[0].addr,
-                                   sync[0].timeline_value, NULL, XE_ONE_SEC);
+        if (ret != 0) {
+            return ret;
         }
+
+        return xeWaitUserFence(DRM_XE_UFENCE_WAIT_U64, DRM_XE_UFENCE_WAIT_EQ,
+                               sync[0].addr,
+                               sync[0].timeline_value, NULL, XE_ONE_SEC);
     }
 
     xeLog(" -> IoctlHelperXe::%s %s found=%d vmid=0x%x h=0x%x s=0x%llx o=0x%llx l=0x%llx f=0x%llx r=%d\n",
diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h
@@ -23,6 +23,8 @@ struct drm_xe_engine_class_instance;
 
 #define PRELIM_I915_UFENCE_WAIT_SOFT (1 << 15)
 
+#define XE_ONE_SEC 1000
+
 namespace NEO {
 
 struct BindInfo {
@@ -102,8 +104,6 @@ class IoctlHelperXe : public IoctlHelper {
   private:
     template <typename... XeLogArgs>
     void xeLog(XeLogArgs &&...args) const;
-    int xeWaitUserFence(uint64_t mask, uint16_t op, uint64_t addr, uint64_t value, struct drm_xe_engine_class_instance *eci, int64_t timeout);
-    int xeVmBind(const VmBindParams &vmBindParams, bool bindOp);
     uint32_t xeSyncObjCreate(uint32_t flags);
     bool xeSyncObjWait(uint32_t *handles, uint32_t count, uint64_t absTimeoutNsec, uint32_t flags, uint32_t *firstSignaled);
     void xeSyncObjDestroy(uint32_t handle);
@@ -117,6 +117,15 @@ class IoctlHelperXe : public IoctlHelper {
     const char *xeGetBindOpName(int bindOp);
     const char *xeGetengineClassName(uint32_t engineClass);
     std::vector<uint8_t> queryData(uint32_t queryId);
+    int xeWaitUserFence(uint64_t mask, uint16_t op, uint64_t addr, uint64_t value, struct drm_xe_engine_class_instance *eci, int64_t timeout);
+    int xeVmBind(const VmBindParams &vmBindParams, bool bindOp);
+
+    struct UserFenceExtension {
+        static constexpr uint32_t tagValue = 0x123987;
+        uint32_t tag;
+        uint64_t addr;
+        uint64_t value;
+    };
 
   protected:
     int chipsetId = 0;
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
@@ -25,6 +25,7 @@ using NEO::PrelimI915::drm_syncobj_destroy;
 using NEO::PrelimI915::drm_syncobj_wait;
 
 struct MockIoctlHelperXe : IoctlHelperXe {
+    using IoctlHelperXe::bindInfo;
     using IoctlHelperXe::IoctlHelperXe;
     using IoctlHelperXe::xeDecanonize;
     using IoctlHelperXe::xeGetBindOpName;
@@ -593,15 +594,34 @@ class DrmMockXe : public DrmMockCustom {
             };
             ret = 0;
         } break;
+        case DrmIoctl::GemVmBind: {
+            ret = gemVmBindReturn;
+            auto vmBindInput = static_cast<drm_xe_vm_bind *>(arg);
+            vmBindInputs.push_back(*vmBindInput);
+
+            EXPECT_EQ(1u, vmBindInput->num_syncs);
+
+            auto &syncInput = reinterpret_cast<drm_xe_sync *>(vmBindInput->syncs)[0];
+            syncInputs.push_back(syncInput);
+        } break;
+
+        case DrmIoctl::GemWaitUserFence: {
+            ret = waitUserFenceReturn;
+            auto waitUserFenceInput = static_cast<drm_xe_wait_user_fence *>(arg);
+            waitUserFenceInputs.push_back(*waitUserFenceInput);
+        } break;
+
         case DrmIoctl::GemContextSetparam:
         case DrmIoctl::GemContextGetparam:
+
         default:
             break;
         }
         return ret;
     }
     int forceIoctlAnswer = 0;
     int setIoctlAnswer = 0;
+    int gemVmBindReturn = 0;
     const drm_xe_engine_class_instance queryEngines[9] = {
         {DRM_XE_ENGINE_CLASS_RENDER, 0, 0},
         {DRM_XE_ENGINE_CLASS_COPY, 1, 0},
@@ -615,6 +635,11 @@ class DrmMockXe : public DrmMockCustom {
 
     uint64_t queryMemUsage[37]{}; // 1 qword for num regions and 12 qwords per region
     uint64_t queryGts[27]{};      // 1 qword for num gts and 13 qwords per gt
+
+    StackVec<drm_xe_wait_user_fence, 1> waitUserFenceInputs;
+    StackVec<drm_xe_vm_bind, 1> vmBindInputs;
+    StackVec<drm_xe_sync, 1> syncInputs;
+    int waitUserFenceReturn = 0;
 };
 
 TEST(IoctlHelperXeTest, whenCallingIoctlThenProperValueIsReturned) {
@@ -977,3 +1002,136 @@ TEST(IoctlHelperXeTest, givenDisabledFtrMultiTileArchWhenCreatingEngineInfoThenM
         EXPECT_EQ(0u, hwInfo->gtSystemInfo.MultiTileArchInfo.TileMask);
     }
 }
+
+TEST(IoctlHelperXeTest, whenCallingVmBindThenWaitUserFenceIsCalled) {
+    DebugManagerStateRestore restorer;
+    auto executionEnvironment = std::make_unique<MockExecutionEnvironment>();
+    DrmMockXe drm{*executionEnvironment->rootDeviceEnvironments[0]};
+    auto xeIoctlHelper = std::make_unique<MockIoctlHelperXe>(drm);
+
+    uint64_t fenceAddress = 0x4321;
+    uint64_t fenceValue = 0x789;
+
+    BindInfo mockBindInfo{};
+    mockBindInfo.handle = 0x1234;
+    xeIoctlHelper->bindInfo.push_back(mockBindInfo);
+
+    VmBindExtUserFenceT vmBindExtUserFence{};
+
+    xeIoctlHelper->fillVmBindExtUserFence(vmBindExtUserFence, fenceAddress, fenceValue, 0u);
+
+    VmBindParams vmBindParams{};
+    vmBindParams.handle = mockBindInfo.handle;
+    vmBindParams.extensions = castToUint64(&vmBindExtUserFence);
+
+    drm.vmBindInputs.clear();
+    drm.syncInputs.clear();
+    drm.waitUserFenceInputs.clear();
+    EXPECT_EQ(0, xeIoctlHelper->vmBind(vmBindParams));
+    EXPECT_EQ(1u, drm.vmBindInputs.size());
+    EXPECT_EQ(1u, drm.syncInputs.size());
+    EXPECT_EQ(1u, drm.waitUserFenceInputs.size());
+    {
+        auto &sync = drm.syncInputs[0];
+
+        EXPECT_EQ(fenceAddress, sync.addr);
+        EXPECT_EQ(fenceValue, sync.timeline_value);
+
+        auto &waitUserFence = drm.waitUserFenceInputs[0];
+
+        EXPECT_EQ(fenceAddress, waitUserFence.addr);
+        EXPECT_EQ(static_cast<uint16_t>(DRM_XE_UFENCE_WAIT_EQ), waitUserFence.op);
+        EXPECT_EQ(static_cast<uint16_t>(DRM_XE_UFENCE_WAIT_SOFT_OP), waitUserFence.flags);
+        EXPECT_EQ(fenceValue, waitUserFence.value);
+        EXPECT_EQ(static_cast<uint64_t>(DRM_XE_UFENCE_WAIT_U64), waitUserFence.mask);
+        EXPECT_EQ(static_cast<uint16_t>(XE_ONE_SEC), waitUserFence.timeout);
+        EXPECT_EQ(0u, waitUserFence.num_engines);
+        EXPECT_EQ(0u, waitUserFence.instances);
+    }
+    drm.vmBindInputs.clear();
+    drm.syncInputs.clear();
+    drm.waitUserFenceInputs.clear();
+    EXPECT_EQ(0, xeIoctlHelper->vmUnbind(vmBindParams));
+    EXPECT_EQ(1u, drm.vmBindInputs.size());
+    EXPECT_EQ(1u, drm.syncInputs.size());
+    EXPECT_EQ(1u, drm.waitUserFenceInputs.size());
+    {
+        auto &sync = drm.syncInputs[0];
+
+        EXPECT_EQ(fenceAddress, sync.addr);
+        EXPECT_EQ(fenceValue, sync.timeline_value);
+
+        auto &waitUserFence = drm.waitUserFenceInputs[0];
+
+        EXPECT_EQ(fenceAddress, waitUserFence.addr);
+        EXPECT_EQ(static_cast<uint16_t>(DRM_XE_UFENCE_WAIT_EQ), waitUserFence.op);
+        EXPECT_EQ(static_cast<uint16_t>(DRM_XE_UFENCE_WAIT_SOFT_OP), waitUserFence.flags);
+        EXPECT_EQ(fenceValue, waitUserFence.value);
+        EXPECT_EQ(static_cast<uint64_t>(DRM_XE_UFENCE_WAIT_U64), waitUserFence.mask);
+        EXPECT_EQ(static_cast<uint16_t>(XE_ONE_SEC), waitUserFence.timeout);
+        EXPECT_EQ(0u, waitUserFence.num_engines);
+        EXPECT_EQ(0u, waitUserFence.instances);
+    }
+}
+
+TEST(IoctlHelperXeTest, whenGemVmBindFailsThenErrorIsPropagated) {
+    DebugManagerStateRestore restorer;
+    auto executionEnvironment = std::make_unique<MockExecutionEnvironment>();
+    DrmMockXe drm{*executionEnvironment->rootDeviceEnvironments[0]};
+    auto xeIoctlHelper = std::make_unique<MockIoctlHelperXe>(drm);
+
+    uint64_t fenceAddress = 0x4321;
+    uint64_t fenceValue = 0x789;
+
+    BindInfo mockBindInfo{};
+    mockBindInfo.handle = 0x1234;
+    xeIoctlHelper->bindInfo.push_back(mockBindInfo);
+
+    VmBindExtUserFenceT vmBindExtUserFence{};
+
+    xeIoctlHelper->fillVmBindExtUserFence(vmBindExtUserFence, fenceAddress, fenceValue, 0u);
+
+    VmBindParams vmBindParams{};
+    vmBindParams.handle = mockBindInfo.handle;
+    vmBindParams.extensions = castToUint64(&vmBindExtUserFence);
+
+    drm.waitUserFenceInputs.clear();
+
+    int errorValue = -1;
+    drm.gemVmBindReturn = errorValue;
+    EXPECT_EQ(errorValue, xeIoctlHelper->vmBind(vmBindParams));
+    EXPECT_EQ(0u, drm.waitUserFenceInputs.size());
+
+    EXPECT_EQ(errorValue, xeIoctlHelper->vmUnbind(vmBindParams));
+    EXPECT_EQ(0u, drm.waitUserFenceInputs.size());
+}
+
+TEST(IoctlHelperXeTest, whenUserFenceFailsThenErrorIsPropagated) {
+    DebugManagerStateRestore restorer;
+    auto executionEnvironment = std::make_unique<MockExecutionEnvironment>();
+    DrmMockXe drm{*executionEnvironment->rootDeviceEnvironments[0]};
+    auto xeIoctlHelper = std::make_unique<MockIoctlHelperXe>(drm);
+
+    uint64_t fenceAddress = 0x4321;
+    uint64_t fenceValue = 0x789;
+
+    BindInfo mockBindInfo{};
+    mockBindInfo.handle = 0x1234;
+    xeIoctlHelper->bindInfo.push_back(mockBindInfo);
+
+    VmBindExtUserFenceT vmBindExtUserFence{};
+
+    xeIoctlHelper->fillVmBindExtUserFence(vmBindExtUserFence, fenceAddress, fenceValue, 0u);
+
+    VmBindParams vmBindParams{};
+    vmBindParams.handle = mockBindInfo.handle;
+    vmBindParams.extensions = castToUint64(&vmBindExtUserFence);
+
+    drm.waitUserFenceInputs.clear();
+
+    int errorValue = -1;
+    drm.waitUserFenceReturn = errorValue;
+
+    EXPECT_EQ(errorValue, xeIoctlHelper->vmBind(vmBindParams));
+    EXPECT_EQ(errorValue, xeIoctlHelper->vmUnbind(vmBindParams));
+}
