Here we introduce a new fault injection for SSR trigger.
To trigger the SSR:
echo 100 > /sys/kernel/debug/venus/fail_ssr/probability
echo 1 > /sys/kernel/debug/venus/fail_ssr/times
signed-off-by: Dikshita Agarwal <[email protected]>
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/qcom/venus/core.c | 15 ++++++++++++++-
drivers/media/platform/qcom/venus/dbgfs.c | 21 +++++++++++++++++++++
drivers/media/platform/qcom/venus/dbgfs.h | 1 +
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index 877eca1..abfa5d6 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -265,6 +265,19 @@ static void venus_assign_register_offsets(struct venus_core *core)
}
}
+static irqreturn_t venus_isr_thread(int irq, void *dev_id)
+{
+ struct venus_core *core = dev_id;
+ irqreturn_t ret;
+
+ ret = hfi_isr_thread(irq, dev_id);
+
+ if (ret == IRQ_HANDLED && venus_fault_inject_ssr())
+ hfi_core_trigger_ssr(core, HFI_TEST_SSR_SW_ERR_FATAL);
+
+ return ret;
+}
+
static int venus_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -320,7 +333,7 @@ static int venus_probe(struct platform_device *pdev)
INIT_DELAYED_WORK(&core->work, venus_sys_error_handler);
init_waitqueue_head(&core->sys_err_done);
- ret = devm_request_threaded_irq(dev, core->irq, hfi_isr, hfi_isr_thread,
+ ret = devm_request_threaded_irq(dev, core->irq, hfi_isr, venus_isr_thread,
IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
"venus", core);
if (ret)
diff --git a/drivers/media/platform/qcom/venus/dbgfs.c b/drivers/media/platform/qcom/venus/dbgfs.c
index 52de47f..ec15078 100644
--- a/drivers/media/platform/qcom/venus/dbgfs.c
+++ b/drivers/media/platform/qcom/venus/dbgfs.c
@@ -4,13 +4,34 @@
*/
#include <linux/debugfs.h>
+#include <linux/fault-inject.h>
#include "core.h"
+#ifdef CONFIG_FAULT_INJECTION
+static DECLARE_FAULT_ATTR(venus_ssr_attr);
+#endif
+
+#ifdef CONFIG_FAULT_INJECTION
+bool venus_fault_inject_ssr(void)
+{
+ return should_fail(&venus_ssr_attr, 1);
+}
+#else
+bool venus_fault_inject_ssr(void)
+{
+ return false;
+}
+#endif
+
void venus_dbgfs_init(struct venus_core *core)
{
core->root = debugfs_create_dir("venus", NULL);
debugfs_create_x32("fw_level", 0644, core->root, &venus_fw_debug);
+
+#ifdef CONFIG_FAULT_INJECTION
+ fault_create_debugfs_attr("fail_ssr", core->root, &venus_ssr_attr);
+#endif
}
void venus_dbgfs_deinit(struct venus_core *core)
diff --git a/drivers/media/platform/qcom/venus/dbgfs.h b/drivers/media/platform/qcom/venus/dbgfs.h
index b7b621a..b0d0686 100644
--- a/drivers/media/platform/qcom/venus/dbgfs.h
+++ b/drivers/media/platform/qcom/venus/dbgfs.h
@@ -8,5 +8,6 @@ struct venus_core;
void venus_dbgfs_init(struct venus_core *core);
void venus_dbgfs_deinit(struct venus_core *core);
+bool venus_fault_inject_ssr(void);
#endif
--
2.7.4
Quoting Dikshita Agarwal (2022-06-15 02:30:09)
> Here we introduce a new fault injection for SSR trigger.
>
> To trigger the SSR:
> echo 100 > /sys/kernel/debug/venus/fail_ssr/probability
> echo 1 > /sys/kernel/debug/venus/fail_ssr/times
>
> signed-off-by: Dikshita Agarwal <[email protected]>
> Signed-off-by: Stanimir Varbanov <[email protected]>
Why is Stan's SoB here?
> diff --git a/drivers/media/platform/qcom/venus/dbgfs.c b/drivers/media/platform/qcom/venus/dbgfs.c
> index 52de47f..ec15078 100644
> --- a/drivers/media/platform/qcom/venus/dbgfs.c
> +++ b/drivers/media/platform/qcom/venus/dbgfs.c
> @@ -4,13 +4,34 @@
> */
>
> #include <linux/debugfs.h>
> +#include <linux/fault-inject.h>
>
> #include "core.h"
>
> +#ifdef CONFIG_FAULT_INJECTION
> +static DECLARE_FAULT_ATTR(venus_ssr_attr);
> +#endif
This endif isn't needed.
> +
> +#ifdef CONFIG_FAULT_INJECTION
and this ifdef isn't either. The two can be combined.
> +bool venus_fault_inject_ssr(void)
> +{
> + return should_fail(&venus_ssr_attr, 1);
> +}
> +#else
> +bool venus_fault_inject_ssr(void)
> +{
> + return false;
> +}
Put this part in the header file and make it static inline. Then the
compiler is going to inline the false to the if condition and optimize
the entire branch away unless the config is enabled. It would also be
nice to extern the venus_ssr_attr so that the should_fail() can be
directly inlined into the interrupt handler.
> +#endif
> +
> void venus_dbgfs_init(struct venus_core *core)
> {
> core->root = debugfs_create_dir("venus", NULL);
> debugfs_create_x32("fw_level", 0644, core->root, &venus_fw_debug);
> +
> +#ifdef CONFIG_FAULT_INJECTION
> + fault_create_debugfs_attr("fail_ssr", core->root, &venus_ssr_attr);
> +#endif
> }
>
> void venus_dbgfs_deinit(struct venus_core *core)
> diff --git a/drivers/media/platform/qcom/venus/dbgfs.h b/drivers/media/platform/qcom/venus/dbgfs.h
> index b7b621a..b0d0686 100644
> --- a/drivers/media/platform/qcom/venus/dbgfs.h
> +++ b/drivers/media/platform/qcom/venus/dbgfs.h
> @@ -8,5 +8,6 @@ struct venus_core;
+#include <linux/fault-inject.h>
>
> void venus_dbgfs_init(struct venus_core *core);
> void venus_dbgfs_deinit(struct venus_core *core);
#ifdef CONFIG_FAULT_INJECTION
extern struct fault_attr venus_ssr_attr;
static inline venus_fault_inject_ssr(void)
{
return should_fail(&venus_ssr_attr, 1);
}
#else
static inline bool venus_fault_inject_ssr(void) { return false; }
#endif
Thanks Stephen for your review.
Addressed all comments in v3.
Thanks,
Dikshita Agarwal
-----Original Message-----
From: Stephen Boyd <[email protected]>
Sent: Thursday, June 16, 2022 7:19 AM
To: Dikshita Agarwal (QUIC) <[email protected]>; [email protected]; [email protected]
Cc: [email protected]; [email protected]; Vikash Garodia (QUIC) <[email protected]>
Subject: Re: [PATCH v2] venus: Add support for SSR trigger using fault injection
Quoting Dikshita Agarwal (2022-06-15 02:30:09)
> Here we introduce a new fault injection for SSR trigger.
>
> To trigger the SSR:
> echo 100 > /sys/kernel/debug/venus/fail_ssr/probability
> echo 1 > /sys/kernel/debug/venus/fail_ssr/times
>
> signed-off-by: Dikshita Agarwal <[email protected]>
> Signed-off-by: Stanimir Varbanov <[email protected]>
Why is Stan's SoB here?
He is co-developer hence added the same.
Added co-developed-by as well in latest patch.
> diff --git a/drivers/media/platform/qcom/venus/dbgfs.c
> b/drivers/media/platform/qcom/venus/dbgfs.c
> index 52de47f..ec15078 100644
> --- a/drivers/media/platform/qcom/venus/dbgfs.c
> +++ b/drivers/media/platform/qcom/venus/dbgfs.c
> @@ -4,13 +4,34 @@
> */
>
> #include <linux/debugfs.h>
> +#include <linux/fault-inject.h>
>
> #include "core.h"
>
> +#ifdef CONFIG_FAULT_INJECTION
> +static DECLARE_FAULT_ATTR(venus_ssr_attr);
> +#endif
This endif isn't needed.
> +
> +#ifdef CONFIG_FAULT_INJECTION
and this ifdef isn't either. The two can be combined.
> +bool venus_fault_inject_ssr(void)
> +{
> + return should_fail(&venus_ssr_attr, 1); } #else bool
> +venus_fault_inject_ssr(void) {
> + return false;
> +}
Put this part in the header file and make it static inline. Then the compiler is going to inline the false to the if condition and optimize the entire branch away unless the config is enabled. It would also be nice to extern the venus_ssr_attr so that the should_fail() can be directly inlined into the interrupt handler.
> +#endif
> +
> void venus_dbgfs_init(struct venus_core *core) {
> core->root = debugfs_create_dir("venus", NULL);
> debugfs_create_x32("fw_level", 0644, core->root,
> &venus_fw_debug);
> +
> +#ifdef CONFIG_FAULT_INJECTION
> + fault_create_debugfs_attr("fail_ssr", core->root,
> +&venus_ssr_attr); #endif
> }
>
> void venus_dbgfs_deinit(struct venus_core *core) diff --git
> a/drivers/media/platform/qcom/venus/dbgfs.h
> b/drivers/media/platform/qcom/venus/dbgfs.h
> index b7b621a..b0d0686 100644
> --- a/drivers/media/platform/qcom/venus/dbgfs.h
> +++ b/drivers/media/platform/qcom/venus/dbgfs.h
> @@ -8,5 +8,6 @@ struct venus_core;
+#include <linux/fault-inject.h>
>
> void venus_dbgfs_init(struct venus_core *core); void
> venus_dbgfs_deinit(struct venus_core *core);
#ifdef CONFIG_FAULT_INJECTION
extern struct fault_attr venus_ssr_attr; static inline venus_fault_inject_ssr(void) {
return should_fail(&venus_ssr_attr, 1); } #else static inline bool venus_fault_inject_ssr(void) { return false; } #endif