Re: [Intel-gfx] [PATCH v5 5/7] drm/i915/mtl/huc: auth HuC via GSC

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

On Wed, 2023-05-31 at 16:54 -0700, Ceraolo Spurio, Daniele wrote:
> The full authentication via the GSC requires an heci packet submission
> to the GSC FW via the GSC CS. The GSC has new PXP command for this
> (literally called NEW_HUC_AUTH).
> The intel_huc_auth function is also updated to handle both authentication
> types.
> 
> 
alan:snip

> @@ -399,6 +416,9 @@ void intel_huc_fini(struct intel_huc *huc)
>  	 */
>  	delayed_huc_load_fini(huc);
>  
> +	if (huc->heci_pkt)
> +		i915_vma_unpin_and_release(&huc->heci_pkt, 0);
alan: nit: i just realized that for consistency (init mirror-ing fini), we should be doing this heci releasing after the intel_uc_fw_fini below.
But since the below object isnt referencing the heci packet, this doens't matter, so consider a nit.

alan:snip
> @@ -470,31 +491,41 @@ int intel_huc_auth(struct intel_huc *huc)
>  	if (!intel_uc_fw_is_loaded(&huc->fw))
>  		return -ENOEXEC;
>  
> -	/* GSC will do the auth */
> +	/* GSC will do the auth with the load */
>  	if (intel_huc_is_loaded_by_gsc(huc))
>  		return -ENODEV;
alan: nit: sorry for another late comment but merely a nit - wondering if we should add a warn in here (to catch
if we could end up here) but its okay - this in theory shouldnt happen anyway.

alan:snip


> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
alan:snip



Only a couple of late-comer-nits that you can ignore, else LGTM:
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>