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
>
> ___
