fix: Correct debugger and SIP init logic

Initialize debugger and SIP kernel explicitly once during root-device init.

Signed-off-by: Filip Hazubski <filip.hazubski@intel.com>

shared/source/device/device.cpp

@@ -208,10 +208,29 @@ void Device::setAsEngineInstanced() {
 }
 
 bool Device::createDeviceImpl() {
+    // init sub devices first
     if (!createSubDevices()) {
         return false;
     }
 
+    // create engines
+    if (!initDeviceWithEngines()) {
+        return false;
+    }
+
+    // go back to root-device init
+    if (isSubDevice()) {
+        return true;
+    }
+
+    // initialize common resources once
+    initializeCommonResources();
+
+    // continue proper init for all devices
+    return initDeviceFully();
+}
+
+bool Device::initDeviceWithEngines() {
     setAsEngineInstanced();
 
     auto &hwInfo = getHardwareInfo();
@@ -226,10 +245,10 @@ bool Device::createDeviceImpl() {
 
     initializeCaps();
 
-    if (!createEngines()) {
-        return false;
-    }
+    return createEngines();
+}
 
+void Device::initializeCommonResources() {
     if (getExecutionEnvironment()->isDebuggingEnabled()) {
         const auto rootDeviceIndex = getRootDeviceIndex();
         auto rootDeviceEnvironment = getExecutionEnvironment()->rootDeviceEnvironments[rootDeviceIndex].get();
@@ -240,6 +259,7 @@ bool Device::createDeviceImpl() {
         }
     }
 
+    auto &hwInfo = getHardwareInfo();
     auto &gfxCoreHelper = getGfxCoreHelper();
     auto debugSurfaceSize = gfxCoreHelper.getSipKernelMaxDbgSurfaceSize(hwInfo);
     if (this->isStateSipRequired()) {
@@ -248,7 +268,7 @@ bool Device::createDeviceImpl() {
         debugSurfaceSize = NEO::SipKernel::getSipKernel(*this, nullptr).getStateSaveAreaSize(this);
     }
 
-    const bool allocateDebugSurface = getL0Debugger() && !isSubDevice();
+    const bool allocateDebugSurface = getL0Debugger();
     if (allocateDebugSurface) {
         debugSurface = getMemoryManager()->allocateGraphicsMemoryWithProperties(
             {getRootDeviceIndex(), true,
@@ -258,6 +278,14 @@ bool Device::createDeviceImpl() {
              false,
              getDeviceBitfield()});
     }
+}
+
+bool Device::initDeviceFully() {
+    for (auto &subdevice : this->subdevices) {
+        if (subdevice && !subdevice->initDeviceFully()) {
+            return false;
+        }
+    }
 
     if (!initializeEngines()) {
         return false;
@@ -281,6 +309,7 @@ bool Device::createDeviceImpl() {
     }
     executionEnvironment->memoryManager->setDefaultEngineIndex(getRootDeviceIndex(), defaultEngineIndexWithinMemoryManager);
 
+    auto &hwInfo = getHardwareInfo();
     if (getRootDeviceEnvironment().osInterface) {
         if (hwInfo.capabilityTable.instrumentationEnabled) {
             performanceCounters = createPerformanceCountersFunc(this);
@@ -301,6 +330,8 @@ bool Device::createDeviceImpl() {
             return true;
         }
 
+        auto &gfxCoreHelper = getGfxCoreHelper();
+        auto &productHelper = getProductHelper();
         if (debugManager.flags.EnableChipsetUniqueUUID.get() != 0) {
             if (gfxCoreHelper.isChipsetUniqueUUIDSupported()) {
 

shared/source/device/device.h

@@ -218,6 +218,9 @@ class Device : public ReferenceTrackedObject<Device> {
     }
 
     MOCKABLE_VIRTUAL bool createDeviceImpl();
+    bool initDeviceWithEngines();
+    void initializeCommonResources();
+    bool initDeviceFully();
     virtual bool createEngines();
 
     void addEngineToEngineGroup(EngineControl &engine);

shared/test/common/mocks/debugger_l0_create.cpp

@@ -12,10 +12,12 @@
 
 NEO::DebugerL0CreateFn mockDebuggerL0HwFactory[IGFX_MAX_CORE];
 bool forceCreateNullptrDebugger = false;
+size_t createDebuggerCallCount = 0;
 
 namespace NEO {
 
 std::unique_ptr<Debugger> DebuggerL0::create(NEO::Device *device) {
+    createDebuggerCallCount++;
     if (forceCreateNullptrDebugger) {
         return nullptr;
     }

shared/test/unit_test/device/neo_device_tests.cpp

@@ -1541,9 +1541,9 @@ TEST_F(DeviceTests, GivenDebuggingEnabledWhenDeviceIsInitializedThenL0DebuggerIs
     EXPECT_NE(nullptr, device->getL0Debugger());
 }
 
-extern bool forceCreateNullptrDebugger;
-
 TEST_F(DeviceTests, givenDebuggerRequestedByUserAndNotAvailableWhenDeviceIsInitializedThenErrorIsPrintedButNotReturned) {
+    extern bool forceCreateNullptrDebugger;
+
     VariableBackup backupForceCreateNullptrDebugger{&forceCreateNullptrDebugger, true};
     DebugManagerStateRestore restorer;
 
@@ -1558,3 +1558,15 @@ TEST_F(DeviceTests, givenDebuggerRequestedByUserAndNotAvailableWhenDeviceIsIniti
     EXPECT_EQ(std::string("Debug mode is not enabled in the system.\n"), output);
     EXPECT_EQ(nullptr, device->getL0Debugger());
 }
+
+TEST_F(DeviceTests, givenDebuggerRequestedByUserWhenDeviceWithSubDevicesCreatedThenInitializeDebuggerOncePerRootDevice) {
+    extern size_t createDebuggerCallCount;
+
+    createDebuggerCallCount = 0;
+    auto executionEnvironment = MockDevice::prepareExecutionEnvironment(defaultHwInfo.get(), 0u);
+    executionEnvironment->setDebuggingMode(DebuggingMode::online);
+
+    UltDeviceFactory deviceFactory{1, 4, *executionEnvironment};
+    EXPECT_EQ(1u, createDebuggerCallCount);
+    EXPECT_NE(nullptr, deviceFactory.rootDevices[0]->getL0Debugger());
+}