diff mbox

[4/5] drm/i915/icl/huc: Correctly authenticate the HuC for Icelake

Message ID 1525287804-2071-5-git-send-email-oscar.mateo@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

oscar.mateo@intel.com May 2, 2018, 7:03 p.m. UTC
The register to check for correct HuC authentication by the GuC
has changed in Icelake. Look into the right register & bit.

v2: rebased.
v3: rebased.
v4: Fix I915_PARAM_HUC_STATUS as well (Tony)
v5: Fix duplicate Cc

BSpec: 19686

Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Cc: Tony Ye <tony.ye@intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Spotswood <john.a.spotswood@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_reg.h |  3 +++
 drivers/gpu/drm/i915/intel_huc.c     | 23 +++++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

Comments

John Spotswood May 4, 2018, 10:26 p.m. UTC | #1
On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> The register to check for correct HuC authentication by the GuC
> has changed in Icelake. Look into the right register & bit.
> 
> v2: rebased.
> v3: rebased.
> v4: Fix I915_PARAM_HUC_STATUS as well (Tony)
> v5: Fix duplicate Cc
> 
> BSpec: 19686
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Tony Ye <tony.ye@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Spotswood <john.a.spotswood@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_reg.h |  3 +++
>  drivers/gpu/drm/i915/intel_huc.c     | 23 +++++++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index d860847..9f14f9f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -76,6 +76,9 @@
>  #define HUC_STATUS2             _MMIO(0xD3B0)
>  #define   HUC_FW_VERIFIED       (1<<7)
>  
> +#define HUC_KERNEL_LOAD_INFO	_MMIO(0xC1DC)
> +#define   HUC_LOAD_SUCCESSFUL	(1 << 0)
> +
>  #define GUC_WOPCM_SIZE			_MMIO(0xc050)
>  #define   GUC_WOPCM_SIZE_LOCKED		  (1<<0)
>  #define   GUC_WOPCM_SIZE_SHIFT		12
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 2912852..b509756 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -48,9 +48,19 @@ int intel_huc_auth(struct intel_huc *huc)
>  	struct drm_i915_private *i915 = huc_to_i915(huc);
>  	struct intel_guc *guc = &i915->guc;
>  	struct i915_vma *vma;
> +	i915_reg_t status_reg;
>  	u32 status;
> +	u32 status_ok;
>  	int ret;
>  
> +	if (INTEL_GEN(i915) >= 11) {
> +		status_reg = HUC_KERNEL_LOAD_INFO;
> +		status_ok = HUC_LOAD_SUCCESSFUL;
> +	} else {
> +		status_reg = HUC_STATUS2;
> +		status_ok = HUC_FW_VERIFIED;
> +	}
> +
>  	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>  		return -ENOEXEC;
>  
> @@ -72,9 +82,9 @@ int intel_huc_auth(struct intel_huc *huc)
>  
>  	/* Check authentication status, it should be done by now */
>  	ret = __intel_wait_for_register(i915,
> -					HUC_STATUS2,
> -					HUC_FW_VERIFIED,
> -					HUC_FW_VERIFIED,
> +					status_reg,
> +					status_ok,
> +					status_ok,
>  					2, 50, &status);

Minor question:  You are checking different registers depending on gen.
 Will the fast and slow timeout values apply equally to both situations
?

>  	if (ret) {
>  		DRM_ERROR("HuC: Firmware not verified %#x\n",
> status);
> @@ -112,7 +122,12 @@ int intel_huc_check_status(struct intel_huc
> *huc)
>  		return -ENODEV;
>  
>  	intel_runtime_pm_get(dev_priv);
> -	status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		status = I915_READ(HUC_KERNEL_LOAD_INFO) &
> HUC_LOAD_SUCCESSFUL;
> +	else
> +		status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
>  	intel_runtime_pm_put(dev_priv);
>  
>  	return status;
oscar.mateo@intel.com May 8, 2018, 9:36 p.m. UTC | #2
On 05/04/2018 03:26 PM, John Spotswood wrote:
> On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
>> The register to check for correct HuC authentication by the GuC
>> has changed in Icelake. Look into the right register & bit.
>>
>> v2: rebased.
>> v3: rebased.
>> v4: Fix I915_PARAM_HUC_STATUS as well (Tony)
>> v5: Fix duplicate Cc
>>
>> BSpec: 19686
>>
>> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Tony Ye <tony.ye@intel.com>
>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: John Spotswood <john.a.spotswood@intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc_reg.h |  3 +++
>>   drivers/gpu/drm/i915/intel_huc.c     | 23 +++++++++++++++++++----
>>   2 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
>> b/drivers/gpu/drm/i915/intel_guc_reg.h
>> index d860847..9f14f9f 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
>> @@ -76,6 +76,9 @@
>>   #define HUC_STATUS2             _MMIO(0xD3B0)
>>   #define   HUC_FW_VERIFIED       (1<<7)
>>   
>> +#define HUC_KERNEL_LOAD_INFO	_MMIO(0xC1DC)
>> +#define   HUC_LOAD_SUCCESSFUL	(1 << 0)
>> +
>>   #define GUC_WOPCM_SIZE			_MMIO(0xc050)
>>   #define   GUC_WOPCM_SIZE_LOCKED		  (1<<0)
>>   #define   GUC_WOPCM_SIZE_SHIFT		12
>> diff --git a/drivers/gpu/drm/i915/intel_huc.c
>> b/drivers/gpu/drm/i915/intel_huc.c
>> index 2912852..b509756 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/intel_huc.c
>> @@ -48,9 +48,19 @@ int intel_huc_auth(struct intel_huc *huc)
>>   	struct drm_i915_private *i915 = huc_to_i915(huc);
>>   	struct intel_guc *guc = &i915->guc;
>>   	struct i915_vma *vma;
>> +	i915_reg_t status_reg;
>>   	u32 status;
>> +	u32 status_ok;
>>   	int ret;
>>   
>> +	if (INTEL_GEN(i915) >= 11) {
>> +		status_reg = HUC_KERNEL_LOAD_INFO;
>> +		status_ok = HUC_LOAD_SUCCESSFUL;
>> +	} else {
>> +		status_reg = HUC_STATUS2;
>> +		status_ok = HUC_FW_VERIFIED;
>> +	}
>> +
>>   	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>>   		return -ENOEXEC;
>>   
>> @@ -72,9 +82,9 @@ int intel_huc_auth(struct intel_huc *huc)
>>   
>>   	/* Check authentication status, it should be done by now */
>>   	ret = __intel_wait_for_register(i915,
>> -					HUC_STATUS2,
>> -					HUC_FW_VERIFIED,
>> -					HUC_FW_VERIFIED,
>> +					status_reg,
>> +					status_ok,
>> +					status_ok,
>>   					2, 50, &status);
> Minor question:  You are checking different registers depending on gen.
>   Will the fast and slow timeout values apply equally to both situations
> ?

AFAICT, the process to authenticate the HuC has not changed, and the 
only difference is which register to use to check the result. If the 
timeout values were fine before, they should be fine now...

>
>>   	if (ret) {
>>   		DRM_ERROR("HuC: Firmware not verified %#x\n",
>> status);
>> @@ -112,7 +122,12 @@ int intel_huc_check_status(struct intel_huc
>> *huc)
>>   		return -ENODEV;
>>   
>>   	intel_runtime_pm_get(dev_priv);
>> -	status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
>> +
>> +	if (INTEL_GEN(dev_priv) >= 11)
>> +		status = I915_READ(HUC_KERNEL_LOAD_INFO) &
>> HUC_LOAD_SUCCESSFUL;
>> +	else
>> +		status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
>> +
>>   	intel_runtime_pm_put(dev_priv);
>>   
>>   	return status;
John Spotswood May 8, 2018, 11:22 p.m. UTC | #3
On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> The register to check for correct HuC authentication by the GuC
> has changed in Icelake. Look into the right register & bit.
> 
> v2: rebased.
> v3: rebased.
> v4: Fix I915_PARAM_HUC_STATUS as well (Tony)
> v5: Fix duplicate Cc
> 
> BSpec: 19686
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Tony Ye <tony.ye@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Spotswood <john.a.spotswood@intel.com>

Reviewed-by: John Spotswood <john.a.spotswood@intel.com>

> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_reg.h |  3 +++
>  drivers/gpu/drm/i915/intel_huc.c     | 23 +++++++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index d860847..9f14f9f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -76,6 +76,9 @@
>  #define HUC_STATUS2             _MMIO(0xD3B0)
>  #define   HUC_FW_VERIFIED       (1<<7)
>  
> +#define HUC_KERNEL_LOAD_INFO	_MMIO(0xC1DC)
> +#define   HUC_LOAD_SUCCESSFUL	(1 << 0)
> +
>  #define GUC_WOPCM_SIZE			_MMIO(0xc050)
>  #define   GUC_WOPCM_SIZE_LOCKED		  (1<<0)
>  #define   GUC_WOPCM_SIZE_SHIFT		12
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 2912852..b509756 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -48,9 +48,19 @@ int intel_huc_auth(struct intel_huc *huc)
>  	struct drm_i915_private *i915 = huc_to_i915(huc);
>  	struct intel_guc *guc = &i915->guc;
>  	struct i915_vma *vma;
> +	i915_reg_t status_reg;
>  	u32 status;
> +	u32 status_ok;
>  	int ret;
>  
> +	if (INTEL_GEN(i915) >= 11) {
> +		status_reg = HUC_KERNEL_LOAD_INFO;
> +		status_ok = HUC_LOAD_SUCCESSFUL;
> +	} else {
> +		status_reg = HUC_STATUS2;
> +		status_ok = HUC_FW_VERIFIED;
> +	}
> +
>  	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>  		return -ENOEXEC;
>  
> @@ -72,9 +82,9 @@ int intel_huc_auth(struct intel_huc *huc)
>  
>  	/* Check authentication status, it should be done by now */
>  	ret = __intel_wait_for_register(i915,
> -					HUC_STATUS2,
> -					HUC_FW_VERIFIED,
> -					HUC_FW_VERIFIED,
> +					status_reg,
> +					status_ok,
> +					status_ok,
>  					2, 50, &status);
>  	if (ret) {
>  		DRM_ERROR("HuC: Firmware not verified %#x\n",
> status);
> @@ -112,7 +122,12 @@ int intel_huc_check_status(struct intel_huc
> *huc)
>  		return -ENODEV;
>  
>  	intel_runtime_pm_get(dev_priv);
> -	status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		status = I915_READ(HUC_KERNEL_LOAD_INFO) &
> HUC_LOAD_SUCCESSFUL;
> +	else
> +		status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
>  	intel_runtime_pm_put(dev_priv);
>  
>  	return status;
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index d860847..9f14f9f 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -76,6 +76,9 @@ 
 #define HUC_STATUS2             _MMIO(0xD3B0)
 #define   HUC_FW_VERIFIED       (1<<7)
 
+#define HUC_KERNEL_LOAD_INFO	_MMIO(0xC1DC)
+#define   HUC_LOAD_SUCCESSFUL	(1 << 0)
+
 #define GUC_WOPCM_SIZE			_MMIO(0xc050)
 #define   GUC_WOPCM_SIZE_LOCKED		  (1<<0)
 #define   GUC_WOPCM_SIZE_SHIFT		12
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 2912852..b509756 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -48,9 +48,19 @@  int intel_huc_auth(struct intel_huc *huc)
 	struct drm_i915_private *i915 = huc_to_i915(huc);
 	struct intel_guc *guc = &i915->guc;
 	struct i915_vma *vma;
+	i915_reg_t status_reg;
 	u32 status;
+	u32 status_ok;
 	int ret;
 
+	if (INTEL_GEN(i915) >= 11) {
+		status_reg = HUC_KERNEL_LOAD_INFO;
+		status_ok = HUC_LOAD_SUCCESSFUL;
+	} else {
+		status_reg = HUC_STATUS2;
+		status_ok = HUC_FW_VERIFIED;
+	}
+
 	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return -ENOEXEC;
 
@@ -72,9 +82,9 @@  int intel_huc_auth(struct intel_huc *huc)
 
 	/* Check authentication status, it should be done by now */
 	ret = __intel_wait_for_register(i915,
-					HUC_STATUS2,
-					HUC_FW_VERIFIED,
-					HUC_FW_VERIFIED,
+					status_reg,
+					status_ok,
+					status_ok,
 					2, 50, &status);
 	if (ret) {
 		DRM_ERROR("HuC: Firmware not verified %#x\n", status);
@@ -112,7 +122,12 @@  int intel_huc_check_status(struct intel_huc *huc)
 		return -ENODEV;
 
 	intel_runtime_pm_get(dev_priv);
-	status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		status = I915_READ(HUC_KERNEL_LOAD_INFO) & HUC_LOAD_SUCCESSFUL;
+	else
+		status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+
 	intel_runtime_pm_put(dev_priv);
 
 	return status;