Re: [Intel-gfx] [RFC-v3 06/13] drm/i915/pxp: Enable PXP irq worker and callback stub

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

Quoting Huang, Sean Z (2020-12-09 09:03:00)
> Create the irq worker that serves as callback handler, those
> callback stubs should be called while the hardware key teardown
> occurs.
> 
> Signed-off-by: Huang, Sean Z <sean.z.huang@intel.com>

<SNIP>

> +static int intel_pxp_global_terminate_complete_callback(struct intel_pxp *pxp)
> +{
> +       int ret = 0;
> +       struct intel_gt *gt = container_of(pxp, typeof(*gt), pxp);
> +
> +       mutex_lock(&pxp->ctx.mutex);
> +
> +       if (pxp->ctx.global_state_attacked) {
> +               pxp->ctx.global_state_attacked = false;

This software tracking should not be needed.

> +
> +               /* Re-create the arb session after teardown handle complete */
> +               ret = intel_pxp_arb_create_session(pxp);
> +               if (ret) {
> +                       drm_err(&gt->i915->drm, "Failed to create arb session\n");
> +                       goto end;
> +               }
> +       }
> +end:
> +       mutex_unlock(&pxp->ctx.mutex);
> +       return ret;
> +}
> +
> +static void intel_pxp_irq_work(struct work_struct *work)
> +{
> +       struct intel_pxp *pxp = container_of(work, typeof(*pxp), irq_work);
> +       struct intel_gt *gt = container_of(pxp, typeof(*gt), pxp);
> +       u32 events = 0;
> +
> +       spin_lock_irq(&gt->irq_lock);
> +       events = fetch_and_zero(&pxp->current_events);

"current_events" is not a great name for variable to be handled by
irq_work function.

> +       spin_unlock_irq(&gt->irq_lock);
> +
> +       if (events & PXP_IRQ_VECTOR_DISPLAY_PXP_STATE_TERMINATED ||
> +           events & PXP_IRQ_VECTOR_DISPLAY_APP_TERM_PER_FW_REQ)
> +               intel_pxp_teardown_required_callback(pxp);

On this path we should go and invalidate all contexts and buffer objects
that are created with PROTECTED_CONTENT flag.

We should only track the lack of the single session, and hold off from
submitting any newly created contexts until the next event.

> +
> +       if (events & PXP_IRQ_VECTOR_PXP_DISP_STATE_RESET_COMPLETE)
> +               intel_pxp_global_terminate_complete_callback(pxp);

After this, we should be able to proceed with creation/usage of
PROTECTED_CONTENT flagged items as the session is available.

I only see the session being created and other software tracking I'm not
following exactly.

So the only boolean state we should track is if the arb_session is valid
or is "in play" as the terminology seems to be.

Regards, Joonas

> +
> +       spin_lock_irq(&gt->irq_lock);
> +       intel_pxp_write_irq_mask_reg(gt, 0);
> +       spin_unlock_irq(&gt->irq_lock);
> +}

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx