On 11/26/2025 1:31 AM, Jani Nikula wrote:
On Tue, 25 Nov 2025, Daniele Ceraolo Spurio <[email protected]> 
wrote:
On 11/25/2025 2:28 AM, Jani Nikula wrote:
On Fri, 14 Nov 2025, Daniele Ceraolo Spurio <[email protected]> 
wrote:
The PXP flow requires us to communicate with CSME, which we do via a
mei component. Since the mei component binding is async and can take
a bit to complete, we don't wait for it during i915 load. If userspace
queries the state before the async binding is complete, we return an
"init in progress" state, with the expectation that it will eventually
transition to "init complete" if the CSME device is functional.

Mesa CI is flashing a custom coreboot on their Chromebooks that hides
the CSME device, which means that we never transition to the "init
complete" state. While from an interface POV it is not incorrect to not
end up in "init complete" if the CSME is missing, we can mitigate the
impact of this by simply checking if the CSME device is available at
all before attempting to initialize PXP.

v2: rebase on updated mei patch.
v3: rebase on exported pci id list.

Reported-by: Valentine Burley <[email protected]>
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14516
Signed-off-by: Daniele Ceraolo Spurio <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: Alexander Usyskin <[email protected]>
Cc: Alan Previn <[email protected]>
---
   drivers/gpu/drm/i915/pxp/intel_pxp.c | 25 +++++++++++++++++++++++++
   1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index 2860474ad2d0..26e7c5c26bac 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -3,6 +3,7 @@
    * Copyright(c) 2020 Intel Corporation.
    */
+#include <linux/mei_me.h>
   #include <linux/workqueue.h>
#include <drm/drm_print.h>
@@ -197,6 +198,15 @@ static struct intel_gt 
*find_gt_for_required_protected_content(struct drm_i915_p
        return NULL;
   }
+static bool mei_device_available(void)
+{
+#if IS_ENABLED(CONFIG_INTEL_MEI_ME)
+       return pci_dev_present(mei_me_pci_tbl);
+#else
+       return false;
+#endif
+}
+
I think it's just ugly to have this in i915. IMO the function belongs in
mei.
That is what I had in v1 [1], but there were concerns from Greg about
the state changing after we check it and the relevant required locking
to avoid that. In i915 we don't care if the state changes after we check
it and therefore we don't need locking, so I moved the actual check to
i915 with an explanation (see comment in the code below).
I'm not sure how it makes a difference where the check is.

I just think the whole mei_device_available() function above is a bunch
of mei implementation details that i915 should not have to know. i915
shouldn't have to do any CONFIG_INTEL_MEI_ME stuff, i915 shouldn't have
to know mei_me_pci_tbl or what it means.

I'm ok with moving this back to mei if Greg is ok with having it there without any locking (since I wouldn't even know what locking to add on the mei side in this scenario).
Greg, any feedback here?

Thanks,
Daniele


BR,
Jani.

[1]
https://patchwork.freedesktop.org/patch/664208/?series=151677&rev=1
Daniele

BR,
Jani.

   int intel_pxp_init(struct drm_i915_private *i915)
   {
        struct intel_gt *gt;
@@ -205,6 +215,21 @@ int intel_pxp_init(struct drm_i915_private *i915)
        if (intel_gt_is_wedged(to_gt(i915)))
                return -ENOTCONN;
+ /*
+        * iGPUs require CSME to be available to use PXP. Note that the
+        * availability of CSME might change after we check, but we only support
+        * PXP in the case where the CSME device is available from boot and
+        * stays that way. If CSME was not initially available and appears later
+        * we'll just continue without PXP, while if it was available and is
+        * then removed we'll catch it via the component unbind callback and
+        * handle it gracefully. Therefore, we don't require any locking around
+        * the state checking.
+        */
+       if (!IS_DGFX(i915) && !mei_device_available()) {
+               drm_dbg(&i915->drm, "skipping PXP init due to missing ME 
device\n");
+               return -ENODEV;
+       }
+
        /*
         * NOTE: Get the ctrl_gt before checking intel_pxp_is_supported since
         * we still need it if PXP's backend tee transport is needed.

Reply via email to