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@xxxxxxxxx>