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

[Tvrtko Ursulin]

On 30/03/2023 01:10, Teres Alexis, Alan Previn wrote:
> 
> On Wed, 2023-03-29 at 08:43 +0100, Tvrtko Ursulin wrote:
>> On 28/03/2023 18:52, Rodrigo Vivi wrote:
>>> On Tue, Mar 28, 2023 at 05:01:36PM +0000, Teres Alexis, Alan Previn wrote:
>>>> On Mon, 2023-03-27 at 17:15 +0100, Tvrtko Ursulin wrote:
>>>>
> alan:snip
>> How will the context create path look like on those platforms:
>>
>> 1. Block, then potentially error out if the full initialization failed.
>> 2. Error out "in progress" while initializing, error out "something
>> else" if initialization failed.
>>
> alan:i was thinking of taking a page from huc-authentication's get-param where we could return different values based on startup progress - in all cases we never block:

If context create never blocks then the only added value of new getparam 
is just granularity of reported statuses, versus potential overload of 
errnos from context create?

>   1. we dont support it in hw/kernel (i.e. not pxp in device-info or not enough CONFIG_foo - reusing intel_pxp_is_supported?)
>   2. we support it in kernel but internal dependencies are still in progress (i.e. we have not yet completed huc-loading/huc-authen/proxy-init - UAPI spec should include how many
> max seconds delay per platform)
>   3. we support it in kernel but internal dependencies failed (i.e. we know huc-load/authent. failed ... or we know proxy-init failed).
>   4. we support it in kernel but platform has no support (at this stage we actually attempt to create a PXP context or create the arb-session from inside i915-get-param but we ended
> up a PXP fw error indicating select list of failures such as fusing / BIOS-config / wrong-version.
>   0. we support it completely i.e. step 4's attempt to create active PXP session succeeded
> 
> I want to differentiate 3 and 4 (as opposed to return x-means-ENODEV) because i have am sure it will save debug time when facing customer issues.
> Ofc we will have to optimize the checking sequences above but at #4, we would be creating a session which might take up to ~200 milisecs for the round trip response from fw.

Not sure I get this. If getparam is trying to set this up, which is 
possibly questionable in itself, where it needs to block for 200ms 
(max?), and nothing else blocks, then where is the missing max delay 
mentioned as a starting point for the discussion? Is it expected to 
elapse while userspace is repeatedly calling getparam and it is getting 
some "in progress" value?

> We could store a flag in i915-pxp-internal-struct to indicate if we ever did succeed a pxp creation after a fresh boot ... intel_pxp_is_ready_for_active()?
> ... true only if we ever did allocate a slot successfully at least once since boot.
> This also ensure mesa init will return almost immediately except at the first time when hitting #4.

Even 200ms is possibly not good enough since boot time targets (to UI 
AFAIR) are pretty tight. Don't know... Maybe I'd need a timeline diagram 
showing the involved components to understand this properly.

But intuitively I thought that what Mesa wants is a no-cost getparam 
which would somewhat reliably tell it if the feature is supposed to be 
there and context create at a later stage, with the protected flag set, 
is supposed to work. AFAIU it can still fail at that point or probably 
block until the required setup is done.

Regards,

Tvrtko