| Message ID | 20210814134147.14056-1-samuel@sholland.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | lib: sbi_trap: Restore redirect for access faults | expand |
On Sat, Aug 14, 2021 at 6:42 AM Samuel Holland <samuel@sholland.org> wrote: > > commit 764a17d852a8 ("lib: sbi: Implement firmware counters") added > switch cases for CAUSE_LOAD_ACCESS and CAUSE_STORE_ACCESS. This caused > them to stop being redirected to U or S mode, as that is handled in the > default switch case. As a result, an error in userspace could cause the > system to hang. Fix this by allowing the acces fault case to fall > through to the default case. Thanks for catching this. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > lib/sbi/sbi_trap.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c > index 5781fea..8d20e04 100644 > --- a/lib/sbi/sbi_trap.c > +++ b/lib/sbi/sbi_trap.c > @@ -259,11 +259,10 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs) > msg = "ecall handler failed"; > break; > case CAUSE_LOAD_ACCESS: > - sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_LOAD); > - break; > case CAUSE_STORE_ACCESS: > - sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_STORE); > - break; > + sbi_pmu_ctr_incr_fw(mcause == CAUSE_LOAD_ACCESS ? > + SBI_PMU_FW_ACCESS_LOAD : SBI_PMU_FW_ACCESS_STORE); > + /* fallthrough */ > default: > /* If the trap came from S or U mode, redirect it there */ > trap.epc = regs->mepc; > -- > 2.31.1 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi Reviewed-by: Atish Patra <atish.patra@wdc.com>
On Sat, Aug 14, 2021 at 9:42 PM Samuel Holland <samuel@sholland.org> wrote: > > commit 764a17d852a8 ("lib: sbi: Implement firmware counters") added > switch cases for CAUSE_LOAD_ACCESS and CAUSE_STORE_ACCESS. This caused > them to stop being redirected to U or S mode, as that is handled in the > default switch case. As a result, an error in userspace could cause the > system to hang. Fix this by allowing the acces fault case to fall > through to the default case. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > lib/sbi/sbi_trap.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > Should have a: Fixes 764a17d852a8 ("lib: sbi: Implement firmware counters") Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
On Tue, Aug 17, 2021 at 11:48 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Sat, Aug 14, 2021 at 9:42 PM Samuel Holland <samuel@sholland.org> wrote: > > > > commit 764a17d852a8 ("lib: sbi: Implement firmware counters") added > > switch cases for CAUSE_LOAD_ACCESS and CAUSE_STORE_ACCESS. This caused > > them to stop being redirected to U or S mode, as that is handled in the > > default switch case. As a result, an error in userspace could cause the > > system to hang. Fix this by allowing the acces fault case to fall > > through to the default case. > > > > Signed-off-by: Samuel Holland <samuel@sholland.org> > > --- > > lib/sbi/sbi_trap.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > Should have a: > > Fixes 764a17d852a8 ("lib: sbi: Implement firmware counters") > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> I have included the Fixes tag at time of merging this patch. Applied this patch to the riscv/opensbi repo. Thanks, Anup > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c index 5781fea..8d20e04 100644 --- a/lib/sbi/sbi_trap.c +++ b/lib/sbi/sbi_trap.c @@ -259,11 +259,10 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs) msg = "ecall handler failed"; break; case CAUSE_LOAD_ACCESS: - sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_LOAD); - break; case CAUSE_STORE_ACCESS: - sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_STORE); - break; + sbi_pmu_ctr_incr_fw(mcause == CAUSE_LOAD_ACCESS ? + SBI_PMU_FW_ACCESS_LOAD : SBI_PMU_FW_ACCESS_STORE); + /* fallthrough */ default: /* If the trap came from S or U mode, redirect it there */ trap.epc = regs->mepc;
commit 764a17d852a8 ("lib: sbi: Implement firmware counters") added switch cases for CAUSE_LOAD_ACCESS and CAUSE_STORE_ACCESS. This caused them to stop being redirected to U or S mode, as that is handled in the default switch case. As a result, an error in userspace could cause the system to hang. Fix this by allowing the acces fault case to fall through to the default case. Signed-off-by: Samuel Holland <samuel@sholland.org> --- lib/sbi/sbi_trap.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)