Call zeInitDrivers in L0 provider #1101

igchor · 2025-02-14T19:53:07Z

This PR is based on: https://github.com/oneapi-src/unified-memory-framework/pull/1086/files

And fixes problems in oneapi-src/unified-runtime#2667

igchor · 2025-02-14T20:42:30Z

The tests are failing on CI most likely because we have an older L0 loader version installed and zeInitDrivers might not work. I'm not sure if we need to support older version so perhas we can just update the runners?

If we do need to support older version, we will probably need to implement a fallback to zeInit (which is supported by older loaders but deprecated now).

vinser52 · 2025-02-17T11:11:11Z

src/libumf.c

+        fini_ze_global_state();
+        fini_cu_global_state();
+        LOG_DEBUG("UMF library finalized");
+


Why do we need it earlier?

Reverted, I need that to be able to free memory in fini_ze_global_state, but we don't actually need to keep the driver handles so it's not necessary.

vinser52 · 2025-02-17T11:31:11Z

src/provider/provider_level_zero.c

@@ -267,6 +297,15 @@ static void init_ze_global_state(void) {
        return;
    }
    ze_lib_handle = lib_handle;
+
+    umf_result_t result = ze_init_drivers();


Hmmm, I am thinking about who is responsible for calling zeInitDrivers.
I suppose that the client who uses UMF and Level Zero is responsible for that. At the moment, the client calls UMF to create Level Zero it should already create the context and get the device, so the driver should be initialized.

I agree with @vinser52

As discussed, UMF needs to call init function, even if the client called it already.

vinser52 · 2025-02-17T11:42:09Z

The tests are failing on CI most likely because we have an older L0 loader version installed and zeInitDrivers might not work. I'm not sure if we need to support older version so perhas we can just update the runners?

If we do need to support older version, we will probably need to implement a fallback to zeInit (which is supported by older loaders but deprecated now).

So, I suppose it should be the client's responsibility to call zeInit/zeInitDrivers, as we have today in the tests.

nrspruit · 2025-02-18T23:04:31Z

src/provider/provider_level_zero.c

+    ze_init_driver_type_desc_t desc = {
+        .stype = ZE_STRUCTURE_TYPE_INIT_DRIVER_TYPE_DESC,
+        .pNext = NULL,
+        .flags = UINT32_MAX};


If you want only GPU Drivers, you need to set this to _GPU otherwise you will get back GPUs and NPUs.

According to the L0 spec, zeInitDrivers must be called (by every library) before calling any other APIs. Not calling zeInitDrivers causes crash when using statically linked L0 loader in UR.

vinser52 · 2025-02-19T09:46:46Z

.github/workflows/reusable_basic.yml

-        --proxy
-        --umf-version ${{env.UMF_VERSION}}
-        --shared-library
+  # macos-build:


Why is MacOS CI disabled?

It should not be disabled here.

ldorau · 2025-02-19T10:07:34Z

.github/workflows/reusable_basic.yml

-        --proxy
-        --umf-version ${{env.UMF_VERSION}}
-        --shared-library
+  # macos-build:


It should not be disabled here.

src/provider/provider_level_zero.c

vinser52 · 2025-02-19T10:46:39Z

src/provider/provider_level_zero.c

+    ze_init_driver_type_desc_t desc = {
+        .stype = ZE_STRUCTURE_TYPE_INIT_DRIVER_TYPE_DESC,
+        .pNext = NULL,
+        .flags = ZE_INIT_DRIVER_TYPE_FLAG_GPU};


Why only GPUs?
That is why I was against calling zeInit or zeInitDrivers from UMF's code. It is the client's responsibility to initialize L0 properly.

I'm also against initializing Level Zero in UMF. Could we avoid that?

If we don't call zeInit/zeInitDrivers, the entire provider won't work. As for what specifically we are initializing - GPU is currently the only thing that makes sense from the UMF perspective. If there will be some other device types in the future that we want to support we can extend params to allow user to specify that.

I do not know how user can create level zero provider without zeInit on their side. They need to provoide context/device handle, which need level zero inited?

+1 for what @lplewa wrote.

@igchor are there any issues when adapter statically linked with libze_loader? If so, can we have a simple test/reproducer?

Yes, without zeInitDrivers/zeInit calling any L0 function results in a crash. I can prepare some simple reproducer.

igchor requested a review from a team as a code owner February 14, 2025 19:53

igchor force-pushed the call_ze_init branch from 2a11659 to 05d5f1a Compare February 14, 2025 20:02

vinser52 reviewed Feb 17, 2025

View reviewed changes

igchor force-pushed the call_ze_init branch 2 times, most recently from 1d741ab to b302607 Compare February 18, 2025 22:31

nrspruit reviewed Feb 18, 2025

View reviewed changes

Call zeInitDrivers in L0 provider

c00f22d

According to the L0 spec, zeInitDrivers must be called (by every library) before calling any other APIs. Not calling zeInitDrivers causes crash when using statically linked L0 loader in UR.

igchor force-pushed the call_ze_init branch from b302607 to c04099c Compare February 18, 2025 23:11

nrspruit approved these changes Feb 18, 2025

View reviewed changes

kswiecicki approved these changes Feb 19, 2025

View reviewed changes

vinser52 reviewed Feb 19, 2025

View reviewed changes

ldorau requested changes Feb 19, 2025

View reviewed changes

vinser52 reviewed Feb 19, 2025

View reviewed changes

src/provider/provider_level_zero.c Show resolved Hide resolved

vinser52 reviewed Feb 19, 2025

View reviewed changes

igchor force-pushed the call_ze_init branch from c04099c to c00f22d Compare February 19, 2025 15:45

igchor closed this Feb 25, 2025

Call zeInitDrivers in L0 provider #1101

Call zeInitDrivers in L0 provider #1101

Uh oh!

Conversation

igchor commented Feb 14, 2025

Uh oh!

igchor commented Feb 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vinser52 commented Feb 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vinser52 Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vinser52 Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vinser52 Feb 19, 2025 •

edited

Loading

vinser52 Feb 19, 2025 •

edited

Loading