Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Fri, Jan 13, 2017 at 06:19:53PM +, Srivatsa, Anusha wrote: > >> + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > >> + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); This is not required on drm-tip. > >> + /* Specify auth action and where public signature is. */ > >> + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > >> + data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; > >> + > >> + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); > >> + if (ret) { > >> + DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret); > >> + goto out; > >> + } > >> + > >> + /* Check authentication status, it should be done by now */ > >> + ret = intel_wait_for_register(dev_priv, > >> + HUC_STATUS2, > >> + HUC_FW_VERIFIED, > >> + HUC_FW_VERIFIED, > >> + 50); > >> + > >> + if (ret) { > >> + DRM_ERROR("HuC: Authentication failed %d\n", ret); > >> + goto out; > >> + } > >> + > >> + DRM_INFO("HuC Authentication Successful!\n"); You still seem surprised. Is this a useful user message? What does it mean for the user? Avoid using jargon when talking to the user. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
>-Original Message- >From: Wajdeczko, Michal >Sent: Friday, January 13, 2017 10:18 AM >To: Srivatsa, Anusha >Cc: intel-gfx@lists.freedesktop.org; Chris Wilson ; >Hiler, Arkadiusz ; Alex Dai ; >Peter >Antoine >Subject: Re: [PATCH 7/8] drm/i915/huc: Support HuC authentication > >On Fri, Jan 13, 2017 at 10:08:42AM -0800, Anusha Srivatsa wrote: >> The HuC authentication is done by host2guc call. The HuC RSA keys are >> sent to GuC for authentication. >> >> v2: rebased on top of drm-tip. Changed name format and upped version >> 1.7. >> v3: changed wait_for_atomic to wait_for >> v4: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() and place >> the prototype in intel_guc.h,correct the comments. >> v5: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c to >> intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). >> Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ >> AUTHENTICATE_HUC >> v6: rebased. Add newline on DRM_ERRORs that already dont have one. >> v7: rebased. Replace wait_for with intel_wait_for_register() since the >> latter employs sleep optimisations for quick responses- as pointed out >> by Chris Wilson. >> v8: rebased. Cleanup the intel_guc_auth_huc() by removing checks >> already performed in earlier functions. Make comments more descriptive. >> v9: rebased. Changed the bias for pinning the HuC object. Move >> intel_guc_auth_huc() to intel_huc.c. Change DRM_DEBUGs to DRM_ERRORs >> in intel_guc_auth_huc(). Add return status to DRM_ERRORs. >> v10: Replace DRM_ERROR with DRM_INFO for cases that are non- >> erroneous. >> >> Cc: Chris Wilson >> Cc: Arkadiusz Hiler >> Cc: Michal Wajdeczko >> Tested-by: Xiang Haihao >> Signed-off-by: Anusha Srivatsa >> Signed-off-by: Alex Dai >> Signed-off-by: Peter Antoine >> --- >> drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + >> drivers/gpu/drm/i915/intel_guc_loader.c | 2 ++ >> drivers/gpu/drm/i915/intel_huc.c| 53 >+ >> drivers/gpu/drm/i915/intel_uc.h | 1 + >> 4 files changed, 57 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h >> b/drivers/gpu/drm/i915/intel_guc_fwif.h >> index ed1ab40..25691f0 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h >> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h >> @@ -505,6 +505,7 @@ enum intel_guc_action { >> INTEL_GUC_ACTION_ENTER_S_STATE = 0x501, >> INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, >> INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, >> +INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, >> INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, >> INTEL_GUC_ACTION_LIMIT >> }; >> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c >> b/drivers/gpu/drm/i915/intel_guc_loader.c >> index 527558f..bb127a4 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_loader.c >> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c >> @@ -530,6 +530,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) >> intel_uc_fw_status_repr(guc_fw->fetch_status), >> intel_uc_fw_status_repr(guc_fw->load_status)); >> >> +intel_guc_auth_huc(dev_priv); >> + >> if (i915.enable_guc_submission) { >> if (i915.guc_log_level >= 0) >> gen9_enable_guc_interrupts(dev_priv); >> diff --git a/drivers/gpu/drm/i915/intel_huc.c >> b/drivers/gpu/drm/i915/intel_huc.c >> index 8b84ba8..4ae34b5 100644 >> --- a/drivers/gpu/drm/i915/intel_huc.c >> +++ b/drivers/gpu/drm/i915/intel_huc.c >> @@ -284,3 +284,56 @@ void intel_huc_fini(struct drm_i915_private *dev_priv) >> huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; } >> >> +/** >> + * intel_guc_auth_huc() - authenticate ucode >> + * @dev_priv: the drm_i915_device >> + * >> + * Triggers a HuC fw authentication request to the GuC via >> +intel_guc_action_ >> + * authenticate_huc interface. >> + */ >> +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) { >> +struct intel_guc *guc = &dev_priv->guc; >> +struct intel_huc *huc = &dev_priv->huc; >> +struct i915_vma *vma; >> +int ret; >> +u32 data[2]; >> + >> +vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0, >> +PIN_OFFSET_BIAS | GUC_WOPCM_TOP); >> +if (IS_ERR(vma)) { >> +DRM_ERROR("failed to pin huc fw object %d\n", > >Maybe this message should start with "HuC:" to match other error messages used >below ? Anyway, > >Reviewed-by: Michal Wajdeczko Thanks a lot Michal! >Thanks, >Michal > > >> +(int)PTR_ERR(vma)); >> +return; >> +} >> + >> +/* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ >> +I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); >> + >> +/* Specify auth action and where public signature is. */ >> +data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; >> +data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; >> + >> +ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); >> +if (ret) { >> +DRM_ERROR("HuC: GuC di
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Fri, Jan 13, 2017 at 10:08:42AM -0800, Anusha Srivatsa wrote: > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-tip. Changed name format and upped > version 1.7. > v3: changed wait_for_atomic to wait_for > v4: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v5: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > v6: rebased. Add newline on DRM_ERRORs that already dont have one. > v7: rebased. Replace wait_for with intel_wait_for_register() since > the latter employs sleep optimisations for quick responses- as pointed > out by Chris Wilson. > v8: rebased. Cleanup the intel_guc_auth_huc() by removing checks > already performed in earlier functions. Make comments more descriptive. > v9: rebased. Changed the bias for pinning the HuC object. Move > intel_guc_auth_huc() to intel_huc.c. Change DRM_DEBUGs to DRM_ERRORs > in intel_guc_auth_huc(). Add return status to DRM_ERRORs. > v10: Replace DRM_ERROR with DRM_INFO for cases that are non- > erroneous. > > Cc: Chris Wilson > Cc: Arkadiusz Hiler > Cc: Michal Wajdeczko > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c | 2 ++ > drivers/gpu/drm/i915/intel_huc.c| 53 > + > drivers/gpu/drm/i915/intel_uc.h | 1 + > 4 files changed, 57 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index ed1ab40..25691f0 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -505,6 +505,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_ENTER_S_STATE = 0x501, > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > + INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > INTEL_GUC_ACTION_LIMIT > }; > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 527558f..bb127a4 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -530,6 +530,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev_priv); > + > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_huc.c > b/drivers/gpu/drm/i915/intel_huc.c > index 8b84ba8..4ae34b5 100644 > --- a/drivers/gpu/drm/i915/intel_huc.c > +++ b/drivers/gpu/drm/i915/intel_huc.c > @@ -284,3 +284,56 @@ void intel_huc_fini(struct drm_i915_private *dev_priv) > huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; > } > > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev_priv: the drm_i915_device > + * > + * Triggers a HuC fw authentication request to the GuC via intel_guc_action_ > + * authenticate_huc interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0, > + PIN_OFFSET_BIAS | GUC_WOPCM_TOP); > + if (IS_ERR(vma)) { > + DRM_ERROR("failed to pin huc fw object %d\n", Maybe this message should start with "HuC:" to match other error messages used below ? Anyway, Reviewed-by: Michal Wajdeczko Thanks, Michal > + (int)PTR_ERR(vma)); > + return; > + } > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + > + /* Specify auth action and where public signature is. */ > + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > + data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; > + > + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret); > + goto out; > + } > + > + /* Check authentication status, it should be done by now */ > + ret = intel_wait_for_register(dev_priv, > + HUC_STATUS2, > + HUC_FW_VERIFIED, > + HUC_FW_VERIFIED, > + 50); > + > +
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
>-Original Message- >From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] >Sent: Friday, January 13, 2017 9:25 AM >To: Srivatsa, Anusha >Cc: intel-gfx@lists.freedesktop.org; Hiler, Arkadiusz >; >Wajdeczko, Michal ; Alex Dai >; Peter Antoine >Subject: Re: [PATCH 7/8] drm/i915/huc: Support HuC authentication > >On Fri, Jan 13, 2017 at 09:07:08AM -0800, Anusha Srivatsa wrote: >> +/** >> + * intel_guc_auth_huc() - authenticate ucode >> + * @dev_priv: the drm_i915_device >> + * >> + * Triggers a HuC fw authentication request to the GuC via >> +intel_guc_action_ >> + * authenticate_huc interface. >> + */ >> +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) { >> +struct intel_guc *guc = &dev_priv->guc; >> +struct intel_huc *huc = &dev_priv->huc; >> +struct i915_vma *vma; >> +int ret; >> +u32 data[2]; >> + >> +vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0, >> +PIN_OFFSET_BIAS | GUC_WOPCM_TOP); >> +if (IS_ERR(vma)) { >> +DRM_ERROR("failed to pin huc fw object %d\n", >> +(int)PTR_ERR(vma)); >> +return; >> +} >> + >> +/* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ >> +I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); >> + >> +/* Specify auth action and where public signature is. */ >> +data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; >> +data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; >> + >> +ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); >> +if (ret) { >> +DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret); >> +goto out; >> +} >> + >> +/* Check authentication status, it should be done by now */ >> +ret = intel_wait_for_register(dev_priv, >> +HUC_STATUS2, >> +HUC_FW_VERIFIED, >> +HUC_FW_VERIFIED, >> +50); >> + >> +if (ret) { >> +DRM_ERROR("HuC: Authentication failed %d\n", ret); >> +goto out; >> +} >> + >> +DRM_ERROR("HuC Authentication Successful!\n"); > >Probably don't want to proclaim using the HuC as an error ;-) -Chris Oh... I was actually thinking it is good if it proclaimed Wont it be useful message to know? Anusha >-- >Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Fri, Jan 13, 2017 at 09:07:08AM -0800, Anusha Srivatsa wrote: > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev_priv: the drm_i915_device > + * > + * Triggers a HuC fw authentication request to the GuC via intel_guc_action_ > + * authenticate_huc interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0, > + PIN_OFFSET_BIAS | GUC_WOPCM_TOP); > + if (IS_ERR(vma)) { > + DRM_ERROR("failed to pin huc fw object %d\n", > + (int)PTR_ERR(vma)); > + return; > + } > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + > + /* Specify auth action and where public signature is. */ > + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > + data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; > + > + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret); > + goto out; > + } > + > + /* Check authentication status, it should be done by now */ > + ret = intel_wait_for_register(dev_priv, > + HUC_STATUS2, > + HUC_FW_VERIFIED, > + HUC_FW_VERIFIED, > + 50); > + > + if (ret) { > + DRM_ERROR("HuC: Authentication failed %d\n", ret); > + goto out; > + } > + > + DRM_ERROR("HuC Authentication Successful!\n"); Probably don't want to proclaim using the HuC as an error ;-) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Wed, Jan 04, 2017 at 06:55:54AM -0800, Anusha Srivatsa wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > v14: rebased. > v15: rebased. Add newline on DRM_ERRORs that already dont have one. > v16: rebased. Replace wait_for with intel_wait_for_register() since > the latter employs sleep optimisations for quick responses- as pointed > out by Chris Wilson. > > Cc: Chris Wilson > Cc: Arkadiusz Hiler > Cc: Michal Wajdeczko Typo in my email ;( > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c | 2 + > drivers/gpu/drm/i915/intel_uc.c | 70 > - > drivers/gpu/drm/i915/intel_uc.h | 1 + > 4 files changed, 72 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index ed1ab40..ce4e05e 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -506,6 +506,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > + INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, Can we keep actions in order of their code values? > INTEL_GUC_ACTION_LIMIT > }; > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index ed57ab3..0508054 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev_priv); > + > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index c6be352..d1a4d79 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -46,7 +46,7 @@ static bool intel_guc_recv(struct intel_guc *guc, u32 > *status) > int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len) > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > - u32 status; > + u32 status = 0; > int i; > int ret; > > @@ -71,7 +71,11 @@ int intel_guc_send(struct intel_guc *guc, const u32 > *action, u32 len) >* up to that length of time, then switch to a slower sleep-wait loop. >* No inte_guc_send command should ever take longer than 10ms. >*/ > - ret = wait_for_us(intel_guc_recv(guc, &status), 10); > + ret = intel_wait_for_register(dev_priv, > + HUC_STATUS2, > + HUC_FW_VERIFIED, > + HUC_FW_VERIFIED, > + 50); > if (ret) > ret = wait_for(intel_guc_recv(guc, &status), 10); > if (status != INTEL_GUC_STATUS_SUCCESS) { > @@ -140,3 +144,65 @@ int intel_guc_log_control(struct intel_guc *guc, u32 > control_val) > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev_priv: the drm_i915_device > + * > + * Triggers a HuC fw authentication request to the GuC via intel_guc_action_ > + * authenticate_huc interface. > + * interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_NONE || > + huc->fw.load_status == INTEL_UC_FIRMWARE_NONE) To catch the case when there is no Huc fw, maybe only first check is needed as in intel_huc_load()? > + return; > + > + if (guc->fw.load_status != INTEL_UC_FIRMWARE
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Wed, Jan 04, 2017 at 06:55:54AM -0800, Anusha Srivatsa wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > v14: rebased. > v15: rebased. Add newline on DRM_ERRORs that already dont have one. > v16: rebased. Replace wait_for with intel_wait_for_register() since > the latter employs sleep optimisations for quick responses- as pointed > out by Chris Wilson. > > Cc: Chris Wilson > Cc: Arkadiusz Hiler > Cc: Michal Wajdeczko Typo in Michal's address. > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Wed, Jan 04, 2017 at 06:55:54AM -0800, Anusha Srivatsa wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > v14: rebased. > v15: rebased. Add newline on DRM_ERRORs that already dont have one. > v16: rebased. Replace wait_for with intel_wait_for_register() since > the latter employs sleep optimisations for quick responses- as pointed > out by Chris Wilson. > > Cc: Chris Wilson > Cc: Arkadiusz Hiler > Cc: Michal Wajdeczko > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c | 2 + > drivers/gpu/drm/i915/intel_uc.c | 70 > - > drivers/gpu/drm/i915/intel_uc.h | 1 + > 4 files changed, 72 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index ed1ab40..ce4e05e 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -506,6 +506,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > + INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, > INTEL_GUC_ACTION_LIMIT > }; > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index ed57ab3..0508054 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev_priv); > + > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index c6be352..d1a4d79 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -46,7 +46,7 @@ static bool intel_guc_recv(struct intel_guc *guc, u32 > *status) > int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len) > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > - u32 status; > + u32 status = 0; > int i; > int ret; > > @@ -71,7 +71,11 @@ int intel_guc_send(struct intel_guc *guc, const u32 > *action, u32 len) >* up to that length of time, then switch to a slower sleep-wait loop. >* No inte_guc_send command should ever take longer than 10ms. >*/ > - ret = wait_for_us(intel_guc_recv(guc, &status), 10); > + ret = intel_wait_for_register(dev_priv, > + HUC_STATUS2, > + HUC_FW_VERIFIED, > + HUC_FW_VERIFIED, > + 50); Why do all suddenly intel_guc_send() starts caring about HUC? I think you've misplaced the check, missed that you were in the wrong place and "fixed" status not being set properly by initializing it with the 0. > if (ret) > ret = wait_for(intel_guc_recv(guc, &status), 10); > if (status != INTEL_GUC_STATUS_SUCCESS) { > @@ -140,3 +144,65 @@ int intel_guc_log_control(struct intel_guc *guc, u32 > control_val) > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev_priv: the drm_i915_device > + * > + * Triggers a HuC fw authentication request to the GuC via intel_guc_action_ > + * authenticate_huc interface. > + * interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_NONE || > + huc->fw.load_status == INTEL_UC_FIRMWARE_NONE) > + return; > + > + if (guc->
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
>-Original Message- >From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] >Sent: Thursday, December 22, 2016 3:30 PM >To: Srivatsa, Anusha >Cc: intel-gfx@lists.freedesktop.org; Alex Dai ; Peter Antoine > >Subject: Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication > >On Thu, Dec 22, 2016 at 03:12:23PM -0800, Anusha Srivatsa wrote: >> +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) { >> +struct intel_guc *guc = &dev_priv->guc; >> +struct intel_huc *huc = &dev_priv->huc; >> +struct i915_vma *vma; >> +int ret; >> +u32 data[2]; >> + >> +/* Bypass the case where there is no HuC firmware */ >> +if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_NONE || >> +huc->fw.load_status == INTEL_UC_FIRMWARE_NONE) >> +return; >> + >> +if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { >> +DRM_ERROR("HuC: GuC fw wasn't loaded. Can't >authenticate\n"); >> +return; >> +} >> + >> +if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { >> +DRM_ERROR("HuC: fw wasn't loaded. Nothing to >authenticate\n"); >> +return; >> +} >> + >> +vma = i915_gem_object_ggtt_pin(huc->fw.uc_fw_obj, NULL, 0, 0, 0); >> +if (IS_ERR(vma)) { >> +DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma)); >> +return; >> +} >> + >> + >> +/* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ >> +I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > >Still working on stopping this from frequently popping up in code outside of >the >GTT routines. So, basically beautify the code such that the GTT routines do not pop out in non GTT parts of code. Correct? > >> +/* Specify auth action and where public signature is. */ >> +data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; >> +data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; >> + >> +ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); >> +if (ret) { >> +DRM_ERROR("HuC: GuC did not ack Auth request\n"); >> +goto out; >> +} >> + >> +/* Check authentication status, it should be done by now */ >> +ret = wait_for((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, >50); > >ret = intel_wait_for_register(dev_priv, > HUC_STATUS2, > HUC_FW_VERIFIED, > HUC_FW_VERIFIED, > 50); > >wait_for() is a rather large macro, and intel_wait_for_register() employs the >spin >then sleep optimisation for quick responses. Thankyou for bringing this to my notice. Anusha >-Chris > >-- >Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Thu, Dec 22, 2016 at 03:12:23PM -0800, Anusha Srivatsa wrote: > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_NONE || > + huc->fw.load_status == INTEL_UC_FIRMWARE_NONE) > + return; > + > + if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate\n"); > + return; > + } > + > + if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: fw wasn't loaded. Nothing to authenticate\n"); > + return; > + } > + > + vma = i915_gem_object_ggtt_pin(huc->fw.uc_fw_obj, NULL, 0, 0, 0); > + if (IS_ERR(vma)) { > + DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma)); > + return; > + } > + > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); Still working on stopping this from frequently popping up in code outside of the GTT routines. > + /* Specify auth action and where public signature is. */ > + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > + data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; > + > + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request\n"); > + goto out; > + } > + > + /* Check authentication status, it should be done by now */ > + ret = wait_for((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, 50); ret = intel_wait_for_register(dev_priv, HUC_STATUS2, HUC_FW_VERIFIED, HUC_FW_VERIFIED, 50); wait_for() is a rather large macro, and intel_wait_for_register() employs the spin then sleep optimisation for quick responses. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Thu, Dec 15, 2016 at 02:29:49PM -0800, anushasr wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > v14: rebased. > > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c | 2 ++ > drivers/gpu/drm/i915/intel_uc.c | 62 > + > drivers/gpu/drm/i915/intel_uc.h | 1 + > 4 files changed, 66 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index ed1ab40..ce4e05e 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -506,6 +506,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > + INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, > INTEL_GUC_ACTION_LIMIT > }; > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 2257495..7605f36 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev_priv); > + > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index 8ae6795..b90ac57 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -138,3 +138,65 @@ int intel_guc_log_control(struct intel_guc *guc, u32 > control_val) > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev_priv: the drm_i915_device > + * > + * Triggers a HuC fw authentication request to the GuC via intel_guc_action_ > + * authenticate_huc interface. > + * interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_NONE || > + huc->fw.load_status == INTEL_UC_FIRMWARE_NONE) > + return; > + > + if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate"); Why this DRM_ERROR does not have tailing "\n"? Same goes for couple more in here. > + return; > + } > + > + if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: fw wasn't loaded. Nothing to authenticate"); > + return; > + } > + > + vma = i915_gem_object_ggtt_pin(huc->fw.uc_fw_obj, NULL, 0, 0, 0); > + if (IS_ERR(vma)) { > + DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma)); > + return; > + } > + > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + > + /* Specify auth action and where public signature is. */ > + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > + data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset; > + > + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request\n"); > + goto out; > + } > + > + /* Check authentication status, it should be done by now */ > + ret = wait_for((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, 50); > + if (ret) { > + DRM_ERROR("HuC: Authentication failed\n"); > + goto out; > + } > + > +out: > + i915_vma_unpin(vma); > +} > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > index 57aef56..e69d47c 10064
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
>-Original Message- >From: Michal Wajdeczko [mailto:michal.wajdec...@linux.intel.com] >Sent: Friday, December 9, 2016 4:37 AM >To: Srivatsa, Anusha >Cc: intel-gfx@lists.freedesktop.org; Alex Dai ; Peter Antoine > >Subject: Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication > >On Thu, Dec 08, 2016 at 03:02:18PM -0800, anushasr wrote: >> From: Peter Antoine >> >> The HuC authentication is done by host2guc call. The HuC RSA keys are >> sent to GuC for authentication. >> >> v2: rebased on top of drm-intel-nightly. >> changed name format and upped version 1.7. >> v3: rebased on top of drm-intel-nightly. >> v4: changed wait_for_automic to wait_for >> v5: rebased. >> v7: rebased. >> v8: rebased. >> v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() and place >> the prototype in intel_guc.h,correct the comments. >> v10: rebased. >> v11: rebased. >> v12: rebased on top of drm-tip >> v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c to >> intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). >> Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ >> AUTHENTICATE_HUC >> >> Tested-by: Xiang Haihao >> Signed-off-by: Anusha Srivatsa >> Signed-off-by: Alex Dai >> Signed-off-by: Peter Antoine >> --- >> drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + >> drivers/gpu/drm/i915/intel_guc_loader.c | 2 ++ >> drivers/gpu/drm/i915/intel_uc.c | 61 >+ >> drivers/gpu/drm/i915/intel_uc.h | 1 + >> 4 files changed, 65 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h >> b/drivers/gpu/drm/i915/intel_guc_fwif.h >> index c1e7faf..94a974d 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h >> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h >> @@ -504,6 +504,7 @@ enum intel_guc_action { >> INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, >> INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, >> INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, >> +INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, >> INTEL_GUC_ACTION_LIMIT >> }; >> >> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c >> b/drivers/gpu/drm/i915/intel_guc_loader.c >> index b971351..89d092b 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_loader.c >> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c >> @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) >> intel_uc_fw_status_repr(guc_fw->fetch_status), >> intel_uc_fw_status_repr(guc_fw->load_status)); >> >> +intel_guc_auth_huc(dev_priv); >> + >> if (i915.enable_guc_submission) { >> if (i915.guc_log_level >= 0) >> gen9_enable_guc_interrupts(dev_priv); >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c index 8ae6795..445b9ad 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -138,3 +138,64 @@ int intel_guc_log_control(struct intel_guc *guc, >> u32 control_val) >> >> return intel_guc_send(guc, action, ARRAY_SIZE(action)); } >> + >> +/** >> + * intel_guc_auth_huc() - authenticate ucode >> + * @dev: the drm device > >Mismatched param name. > > >> + * >> + * Triggers a HuC fw authentication request to the GuC via host-2-guc >> + * interface. >> + */ >> +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > >Can we use *guc as the first param to match other intel_guc functions? > In function intel_guc_init() and intel_guc_load() we are using dev_priv as the first parameter. >> +{ >> +struct intel_guc *guc = &dev_priv->guc; >> +struct intel_huc *huc = &dev_priv->huc; >> +struct i915_vma *vma; >> +int ret; >> +u32 data[2]; >> + >> +/* Bypass the case where there is no HuC firmware */ >> +if (huc->huc_fw.fetch_status == UC_FIRMWARE_NONE || >> +huc->huc_fw.load_status == UC_FIRMWARE_NONE) >> +return; >> + >> +if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS) { >> +DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate"); > >Hmm, this looks like late handling of earlier error. >Note that other functions in this file assume that Guc is working fine. Michal, after intel_uc_init_early() which initializes a mutex lock, intel_guc_auth_huc() is the first function that the control goes to and hence all
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Thu, Dec 08, 2016 at 03:02:18PM -0800, anushasr wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c | 2 ++ > drivers/gpu/drm/i915/intel_uc.c | 61 > + > drivers/gpu/drm/i915/intel_uc.h | 1 + > 4 files changed, 65 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index c1e7faf..94a974d 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -504,6 +504,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > + INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, > INTEL_GUC_ACTION_LIMIT > }; > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index b971351..89d092b 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev_priv); > + > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index 8ae6795..445b9ad 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -138,3 +138,64 @@ int intel_guc_log_control(struct intel_guc *guc, u32 > control_val) > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev: the drm device Mismatched param name. > + * > + * Triggers a HuC fw authentication request to the GuC via host-2-guc > + * interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) Can we use *guc as the first param to match other intel_guc functions? > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->huc_fw.fetch_status == UC_FIRMWARE_NONE || > + huc->huc_fw.load_status == UC_FIRMWARE_NONE) > + return; > + > + if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate"); Hmm, this looks like late handling of earlier error. Note that other functions in this file assume that Guc is working fine. > + return; > + } > + > + if (huc->huc_fw.load_status != UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: fw wasn't loaded. Nothing to authenticate"); > + return; > + } > + > + vma = i915_gem_object_ggtt_pin(huc->huc_fw.uc_fw_obj, NULL, 0, 0, 0); > + if (IS_ERR(vma)) { > + DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma)); > + return; > + } > + > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + > + /* Specify auth action and where public signature is. */ > + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > + data[1] = i915_ggtt_offset(vma) + huc->huc_fw.rsa_offset; > + > + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); Hmm, maybe this function shall be split into two parts: intel_huc_auth() in intel_huc_loader.c that contains most of the logic from current function, but calls intel_guc_auth_huc() from this file that just sends action to the guc (similar to other simple functions in this file. > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request\n"); > + goto out; > + } > + > + /* Check authentication status, it should be d
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Thu, Dec 08, 2016 at 03:02:18PM -0800, anushasr wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c > to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc(). > Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_ > AUTHENTICATE_HUC > > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine Reviewed-by: Arkadiusz Hiler > --- > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c | 2 ++ > drivers/gpu/drm/i915/intel_uc.c | 61 > + > drivers/gpu/drm/i915/intel_uc.h | 1 + > 4 files changed, 65 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index c1e7faf..94a974d 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -504,6 +504,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > + INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, > INTEL_GUC_ACTION_LIMIT > }; > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index b971351..89d092b 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev_priv); > + > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index 8ae6795..445b9ad 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -138,3 +138,64 @@ int intel_guc_log_control(struct intel_guc *guc, u32 > control_val) > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev: the drm device > + * > + * Triggers a HuC fw authentication request to the GuC via host-2-guc > + * interface. > + */ > +void intel_guc_auth_huc(struct drm_i915_private *dev_priv) > +{ > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->huc_fw.fetch_status == UC_FIRMWARE_NONE || > + huc->huc_fw.load_status == UC_FIRMWARE_NONE) > + return; > + > + if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate"); > + return; > + } > + > + if (huc->huc_fw.load_status != UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: fw wasn't loaded. Nothing to authenticate"); > + return; > + } > + > + vma = i915_gem_object_ggtt_pin(huc->huc_fw.uc_fw_obj, NULL, 0, 0, 0); > + if (IS_ERR(vma)) { > + DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma)); > + return; > + } > + > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + > + /* Specify auth action and where public signature is. */ > + data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC; > + data[1] = i915_ggtt_offset(vma) + huc->huc_fw.rsa_offset; > + > + ret = intel_guc_send(guc, data, ARRAY_SIZE(data)); > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request\n"); > + goto out; > + } > + > + /* Check authentication status, it should be done by now */ > + ret = wait_for((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, 50); > + if (ret) { > + DRM_ERROR("HuC: Authentication failed\n"); > + goto out; > + } > + > +out: > + i915_vma_unpin(vma); > +} > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > index ac92946..1db8bc2 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -196,6 +196,7 @@ int intel_guc_sam
Re: [Intel-gfx] [PATCH 7/8] drm/i915/huc: Support HuC authentication
On Wed, Nov 30, 2016 at 03:31:33PM -0800, Anusha Srivatsa wrote: > From: Peter Antoine > > The HuC authentication is done by host2guc call. The HuC RSA keys > are sent to GuC for authentication. > > v2: rebased on top of drm-intel-nightly. > changed name format and upped version 1.7. > v3: rebased on top of drm-intel-nightly. > v4: changed wait_for_automic to wait_for > v5: rebased. > v7: rebased. > v8: rebased. > v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc() > and place the prototype in intel_guc.h,correct the comments. > v10: rebased. > v11: rebased. > v12: rebased on top of drm-tip > > Tested-by: Xiang Haihao > Signed-off-by: Anusha Srivatsa > Signed-off-by: Alex Dai > Signed-off-by: Peter Antoine > Reviewed-by: Dave Gordon > Reviewed-by: Jeff McGee > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 63 > ++ > drivers/gpu/drm/i915/intel_guc_fwif.h | 1 + > drivers/gpu/drm/i915/intel_guc_loader.c| 2 + > 3 files changed, 66 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > b/drivers/gpu/drm/i915/i915_guc_submission.c > index 4bae8e4..d5c205d 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -26,6 +26,7 @@ > #include > #include "i915_drv.h" > #include "intel_uc.h" > +#include "intel_huc.h" > > /** > * DOC: GuC-based command submission > @@ -1638,3 +1639,65 @@ int i915_guc_log_control(struct drm_i915_private > *dev_priv, u64 control_val) > > return ret; > } > + > +/** > + * intel_guc_auth_huc() - authenticate ucode > + * @dev: the drm device > + * > + * Triggers a HuC fw authentication request to the GuC via host-2-guc > + * interface. > + */ > +void intel_guc_auth_huc(struct drm_device *dev) This should belong to intel_uc.c > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; > + struct i915_vma *vma; > + int ret; > + u32 data[2]; > + > + /* Bypass the case where there is no HuC firmware */ > + if (huc->huc_fw.fetch_status == UC_FIRMWARE_NONE || > + huc->huc_fw.load_status == UC_FIRMWARE_NONE) > + return; > + > + if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: GuC fw wasn't loaded. Can't authenticate"); > + return; > + } > + > + if (huc->huc_fw.load_status != UC_FIRMWARE_SUCCESS) { > + DRM_ERROR("HuC: fw wasn't loaded. Nothing to authenticate"); > + return; > + } > + > + vma = i915_gem_object_ggtt_pin(huc->huc_fw.uc_fw_obj, NULL, 0, 0, 0); > + if (IS_ERR(vma)) { > + DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma)); > + return; > + } > + > + > + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */ > + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + > + /* Specify auth action and where public signature is. */ > + data[0] = HOST2GUC_ACTION_AUTHENTICATE_HUC; s/HOST2GUC/INTEL_GUC/ > + data[1] = i915_ggtt_offset(vma) + huc->huc_fw.rsa_offset; > + > + ret = host2guc_action(guc, data, ARRAY_SIZE(data)); s/host2guc_action/intel_guc_send/ > + if (ret) { > + DRM_ERROR("HuC: GuC did not ack Auth request\n"); > + goto out; > + } > + > + /* Check authentication status, it should be done by now */ > + ret = wait_for((I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED) > 0, 50); > + if (ret) { > + DRM_ERROR("HuC: Authentication failed\n"); > + goto out; > + } > + > +out: > + i915_vma_unpin(vma); > +} > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h > b/drivers/gpu/drm/i915/intel_guc_fwif.h > index c07d9da..e51e063 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h > @@ -513,6 +513,7 @@ enum intel_guc_action { > INTEL_GUC_ACTION_EXIT_S_STATE = 0x502, > INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003, > INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000, > + HOST2GUC_ACTION_AUTHENTICATE_HUC = 0x4000, s/HOST2GUC/INTEL_GUC/ > INTEL_GUC_ACTION_LIMIT > }; > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 7ca5556..31d09f8 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -529,6 +529,8 @@ int intel_guc_setup(struct drm_device *dev) > intel_uc_fw_status_repr(guc_fw->fetch_status), > intel_uc_fw_status_repr(guc_fw->load_status)); > > + intel_guc_auth_huc(dev); > + You do not have this symbol declared in any header, it's not visible in this compilation unit. > if (i915.enable_guc_submission) { > if (i915.guc_log_level >= 0) > gen9_enable_guc_interrupts(dev_priv); > -- > 2.7.4 > > ___