[Intel-gfx] [PATCH v2] drm/i915/pxp: don't start pxp without mei_pxp bind

[Juston Li]

On Mon, Aug 15, 2022 at 8:36 AM Ceraolo Spurio, Daniele
<daniele.ceraolospurio at intel.com> wrote:
>
>
>
> On 8/9/2022 5:42 PM, Juston Li wrote:
> > pxp will not start correctly until after mei_pxp bind completes and
> > intel_pxp_init_hw() is called.
> >
> > This fixes a race condition during bootup where we observed a small
> > window for pxp commands to be sent before mei_pxp bind completed.
> >
> > Changes since v1:
> > - check pxp_component instead of pxp_component_added (Daniele)
> > - pxp_component needs tee_mutex (Daniele)
> > - return -EAGAIN so caller knows to retry (Daniele)
>
> Sorry for the follow up, but I have reflected on this a bit more and I
> was thinking that maybe it would be better to wait a bit for the
> component to be bound, instead of immediately failing the call. Do you
> have a measure of how long after your failure you see the component bind
> complete? The wait would make sense only if relatively short (< 1s).

No worries, that sounds better.
Media driver does end up allowing it in its retry loop but it's trying
quite a few times.

The bind happens shortly after, ~ 0.08 seconds.

 2022-08-15T16:27:12.190032Z ERR kernel: [    8.384979] i915
0000:00:02.0: [drm] 1st PXP start no mei_pxp
 2022-08-15T16:27:12.238027Z INFO kernel: [    8.433168] pci
0000:00:16.0: attach allowed to drvr mei_me [trusted device]
 2022-08-15T16:27:12.238046Z INFO kernel: [    8.433222] mei_me
0000:00:16.0: enabling device (0000 -> 0002)
 2022-08-15T16:27:12.255546Z INFO pciguard[953]: INFO pciguard:
[udev_monitor.cc(106)] UdevEvent: pci change ALLOWED mei_me /
sys/devices/pci0000:00/0000:00:16.0
 2022-08-15T16:27:12.262298Z INFO kernel: [    8.457702] mei_pxp
0000:00:16.0-fbf6fcf1-96cf-4e2e-a6a6-1bab8cbe36b1: bound
0000:00:02.0 (ops 0xffffffffb6cc6850)
 2022-08-15T16:27:12.264027Z INFO kernel: [    8.458920] mei_hdcp
0000:00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04: bound
0000:00:02.0 (ops 0xffffffffb6cbcce0)

Juston

> Daniele
>
> > Signed-off-by: Juston Li <justonli at google.com>
> > ---
> >   drivers/gpu/drm/i915/pxp/intel_pxp.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > index 15311eaed848..8b395ebc430a 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> > @@ -187,6 +187,14 @@ int intel_pxp_start(struct intel_pxp *pxp)
> >       if (!intel_pxp_is_enabled(pxp))
> >               return -ENODEV;
> >
> > +     mutex_lock(&pxp->tee_mutex);
> > +     /* check if mei_pxp is bound */
> > +     if (!pxp->pxp_component) {
> > +             mutex_unlock(&pxp->tee_mutex);
> > +             return -EAGAIN;
> > +     }
> > +     mutex_unlock(&pxp->tee_mutex);
> > +
> >       mutex_lock(&pxp->arb_mutex);
> >
> >       if (pxp->arb_is_valid)
>