On 11/25/2022 5:54 AM, Ville Syrjälä wrote:
On Thu, Nov 10, 2022 at 04:56:51PM -0800, Daniele Ceraolo Spurio wrote:
The fence is always initialized in huc_init_early, but the cleanup in
huc_fini is only being run if HuC is enabled. This causes a leaking of
the debug object when HuC is disabled/not supported, which can in turn
trigger a warning if we try to register a new debug offset at the same
address on driver reload.
To fix the issue, make sure to always run the cleanup code.
This oopsing in ci now. Somehow the patchwork run did not
hit that oops.
Can you point me to the oops log? I opened a few recent runs at random
but I wasn't able to find it.
Note that I did spot a potential issue that hits platforms that don't
have VCS engines (introduced due to a MTL change to support HuC only on
the media GT) and I already have a fix for that on the ML:
https://patchwork.freedesktop.org/series/111288/
But without looking at the oops logs or knowing on which platform it was
on I don't know if it's the same issue or not.
Daniele
Reported-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Reported-by: Brian Norris <briannorris@xxxxxxxxxxxx>
Fixes: 27536e03271d ("drm/i915/huc: track delayed HuC load with a fence")
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Cc: Brian Norris <briannorris@xxxxxxxxxxxx>
Cc: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
Cc: John Harrison <John.C.Harrison@xxxxxxxxx>
---
Note: I didn't manage to repro the reported warning, but I did confirm
that we weren't correctly calling i915_sw_fence_fini and that this patch
fixes that.
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 12 +++++++-----
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 1 +
2 files changed, 8 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 fbc8bae14f76..83735a1528fe 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -300,13 +300,15 @@ int intel_huc_init(struct intel_huc *huc)
void intel_huc_fini(struct intel_huc *huc)
{
- if (!intel_uc_fw_is_loadable(&huc->fw))
- return;
-
+ /*
+ * the fence is initialized in init_early, so we need to clean it up
+ * even if HuC loading is off.
+ */
delayed_huc_load_complete(huc);
-
i915_sw_fence_fini(&huc->delayed_load.fence);
- intel_uc_fw_fini(&huc->fw);
+
+ if (intel_uc_fw_is_loadable(&huc->fw))
+ intel_uc_fw_fini(&huc->fw);
}
void intel_huc_suspend(struct intel_huc *huc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index dbd048b77e19..41f08b55790e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -718,6 +718,7 @@ int intel_uc_runtime_resume(struct intel_uc *uc)
static const struct intel_uc_ops uc_ops_off = {
.init_hw = __uc_check_hw,
+ .fini = __uc_fini, /* to clean-up the init_early initialization */
};
static const struct intel_uc_ops uc_ops_on = {
--
2.37.3
