On 26/03/2023 14:18, Rodrigo Vivi wrote:
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.
I like option 2 better because we have to report support as fast as we
can when enumerating devices on the system for example.
If I understand correctly, with the get param, most apps won't ever be
blocking on any PXP stuff if they don't use it.
Only the ones that require protected support might block.
-Lionel
