Re: [PATCH v3 2/2] drm/i915/pxp: Do not support PXP if CSME is not available

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 14 Nov 2025, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> 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 <valentine.burley@collabora.com>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14516
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Alexander Usyskin <alexander.usyskin@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>  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.

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.

-- 
Jani Nikula, Intel