On 2/26/2020 6:02 AM, Michal Wajdeczko wrote:
On 25.02.2020 08:49, Ye, Tony wrote:
On 2/21/2020 11:32 PM, Michal Wajdeczko wrote:
From commit 84b1ca2f0e68 ("drm/i915/uc: prefer intel_gt over i915
in GuC/HuC paths") we stopped using HUC_STATUS error -ENODEV only
to indicate lack of HuC hardware and we started to use this error
also for all other cases when HuC was not in use or supported.
Fix that by relying again on HAS_GT_UC macro, since currently
used function intel_huc_is_supported() is based on HuC firmware
support which could be unsupported also due to force disabled
GuC firmware.
v2: use 0 only for disabled, add more error codes for other failures
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Tony Ye <tony.ye@xxxxxxxxx>
Cc: Robert M. Fosha <robert.m.fosha@xxxxxxxxx>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> #v1
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 29 +++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index a74b65694512..301bb5d5e59a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -200,9 +200,13 @@ int intel_huc_auth(struct intel_huc *huc)
* This function reads status register to verify if HuC
* firmware was successfully loaded.
*
- * Returns: 1 if HuC firmware is loaded and verified,
- * 0 if HuC firmware is not loaded and -ENODEV if HuC
- * is not present on this platform.
+ * Returns:
+ * * 1 if HuC firmware is loaded and verified,
+ * * 0 if HuC firmware was disabled,
+ * * -ENODEV if HuC is not present on this platform,
+ * * -ENOPKG if HuC firmware was not installed,
+ * * -ENOEXEC if HuC firmware is invalid,
+ * * -EACCES if HuC firmware was not authenticated.
*/
int intel_huc_check_status(struct intel_huc *huc)
{
@@ -210,11 +214,26 @@ int intel_huc_check_status(struct intel_huc *huc)
intel_wakeref_t wakeref;
u32 status = 0;
- if (!intel_huc_is_supported(huc))
+ if (!HAS_GT_UC(gt->i915))
return -ENODEV;
+ switch (__intel_uc_fw_status(&huc->fw)) {
+ case INTEL_UC_FIRMWARE_NOT_SUPPORTED:
+ case INTEL_UC_FIRMWARE_DISABLED:
+ return 0;
+ case INTEL_UC_FIRMWARE_MISSING:
+ return -ENOPKG;
+ case INTEL_UC_FIRMWARE_ERROR:
+ return -ENOEXEC;
What about INTEL_UC_FIRMWARE_FAIL?
I assumed that we don't need to handle that case here, since we are
still checking HuC status register below.
But if you want we can improve:
1) return early if FAIL, then check register anyway
2) return early if FAIL, trust fw state and return 1 without checking
register (as same register was already checked when we mark fw state as
RUNNING)
The current version looks good to me. Thanks.
Reviewed-by: Tony Ye <tony.ye@xxxxxxxxx>
Regards,
Tony
+ default:
+ break;
+ }
+
with_intel_runtime_pm(gt->uncore->rpm, wakeref)
status = intel_uncore_read(gt->uncore, huc->status.reg);
- return (status & huc->status.mask) == huc->status.value;
+ if ((status & huc->status.mask) != huc->status.value)
+ return -EACCES;
+
+ return 1;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
