Re: [Intel-gfx] [PATCH v6 5/8] drm/i915/pxp: Add ARB session creation and cleanup

["Teres Alexis, Alan Previn" <alan.previn.teres.alexis@intel.com>]

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?