On Sat, Mar 25, 2023 at 02:19:21AM -0400, Teres Alexis, Alan Previn wrote:
> alan:snip
>
> >
> @@ -353,8 +367,20 @@ int intel_pxp_start(struct intel_pxp *pxp)
> alan:snip
> > > + if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) {
> > > + /*
> > > + * GSC-fw loading, GSC-proxy init (requiring an mei component driver) and
> > > + * HuC-fw loading must all occur first before we start requesting for PXP
> > > + * sessions. Checking HuC authentication (the last dependency) will suffice.
> > > + * Let's use a much larger 8 second timeout considering all the types of
> > > + * dependencies prior to that.
> > > + */
> > > + if (wait_for(intel_huc_is_authenticated(&pxp->ctrl_gt->uc.huc), 8000))
> >
> > This big timeout needs an ack from userspace drivers, as intel_pxp_start
> > is called during context creation and the current way to query if the
> > feature is supported is to create a protected context. Unfortunately, we
> > do need to wait to confirm that PXP is available (although in most cases
> > it shouldn't take even close to 8 secs), because until everything is
> > setup we're not sure if things will work as expected. I see 2 potential
> > mitigations in case the timeout doesn't work as-is:
> >
> > 1) we return -EAGAIN (or another dedicated error code) to userspace if
> > the prerequisite steps aren't done yet. This would indicate that the
> > feature is there, but that we haven't completed the setup yet. The
> > caller can then decide if they want to retry immediately or later. Pro:
> > more flexibility for userspace; Cons: new interface return code.
> >
> > 2) we add a getparam to say if PXP is supported in HW and the support is
> > compiled in i915. Userspace can query this as a way to check the feature
> > support and only create the context if they actually need it for PXP
> > operations. Pro: simpler kernel implementation; Cons: new getparam, plus
> > even if the getparam returns true the pxp_start could later fail, so
> > userspace needs to handle that case.
> >
>
> alan: I've cc'd Rodrigo, Joonas and Lionel. Folks - what are your thoughts on above issue?
> Recap: On MTL, only when creating a GEM Protected (PXP) context for the very first time after
> a driver load, it will be dependent on (1) loading the GSC firmware, (2) GuC loading the HuC
> firmware and (3) GSC authenticating the HuC fw. But step 3 also depends on additional
> GSC-proxy-init steps that depend on a new mei-gsc-proxy component driver. I'd used the
> 8 second number based on offline conversations with Daniele but that is a worse-case.
> Alternatively, should we change UAPI instead to return -EAGAIN as per Daniele's proposal?
> I believe we've had the get-param conversation offline recently and the direction was to
> stick with attempting to create the context as it is normal in 3D UMD when it comes to
> testing capabilities for other features too.
>
> Thoughts?
I like the option 1 more. This extra return handling won't break compatibility.
