| Message ID | 20240616170553.2832-1-jszhang@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | riscv: convert bottom half of exception handling to C | expand |
On Mon, Jun 17, 2024 at 3:21 AM Jisheng Zhang <jszhang@kernel.org> wrote: > > For readability, maintainability and future scalability, convert the > bottom half of the exception handling to C. > > During the conversion, I found Anton fixed a performance issue > and my patches will touch the same exception asm code, so I include > Anton's patch for completeness. I also cooked a similar patch to avoid > corrupting the RAS in ret_from_fork() per the inspiration. > > Mostly the assembly code is converted to C in a relatively > straightforward manner. > > However, there are two modifications I need to mention: > > 1. the CSR_CAUSE reg reading and saving is moved to the C code > because we need the cause to dispatch the exception handling, > if we keep the cause reading and saving, we either pass it to > do_traps() via. 2nd param or get it from pt_regs which an extra > memory load is needed, I don't like any of the two solutions becase > the exception handling sits in hot code path, every instruction > matters. > > 2.To cope with SIFIVE_CIP_453 errata, it looks like we don't need > alternative mechanism any more after the asm->c convertion. Just > replace the excp_vect_table two entries. > > > > Anton Blanchard (1): > riscv: Improve exception and system call latency > I've retested this patch with the rest of the series applied. I can confirm that the performance improvement is still there. Definitely thumbs up on my end. > Jisheng Zhang (5): > riscv: avoid corrupting the RAS > riscv: convert bottom half of exception handling to C > riscv: errata: remove ALT_INSN_FAULT and ALT_PAGE_FAULT > riscv: errata: sifive: remove NOMMU handling > riscv: remove asmlinkage from updated functions > > arch/riscv/errata/sifive/errata.c | 25 +++++++--- > arch/riscv/errata/sifive/errata_cip_453.S | 4 -- > arch/riscv/include/asm/asm-prototypes.h | 7 +-- > arch/riscv/include/asm/errata_list.h | 21 ++------ > arch/riscv/kernel/entry.S | 61 ++--------------------- > arch/riscv/kernel/stacktrace.c | 4 +- > arch/riscv/kernel/traps.c | 57 ++++++++++++++++++--- > 7 files changed, 81 insertions(+), 98 deletions(-) > > -- > 2.43.0 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Jun 19, 2024 at 08:16:58AM +1000, Cyril Bur wrote: > On Mon, Jun 17, 2024 at 3:21 AM Jisheng Zhang <jszhang@kernel.org> wrote: > > > > For readability, maintainability and future scalability, convert the > > bottom half of the exception handling to C. > > > > During the conversion, I found Anton fixed a performance issue > > and my patches will touch the same exception asm code, so I include > > Anton's patch for completeness. I also cooked a similar patch to avoid > > corrupting the RAS in ret_from_fork() per the inspiration. > > > > Mostly the assembly code is converted to C in a relatively > > straightforward manner. > > > > However, there are two modifications I need to mention: > > > > 1. the CSR_CAUSE reg reading and saving is moved to the C code > > because we need the cause to dispatch the exception handling, > > if we keep the cause reading and saving, we either pass it to > > do_traps() via. 2nd param or get it from pt_regs which an extra > > memory load is needed, I don't like any of the two solutions becase > > the exception handling sits in hot code path, every instruction > > matters. > > > > 2.To cope with SIFIVE_CIP_453 errata, it looks like we don't need > > alternative mechanism any more after the asm->c convertion. Just > > replace the excp_vect_table two entries. > > > > > > > > Anton Blanchard (1): > > riscv: Improve exception and system call latency > > > I've retested this patch with the rest of the series applied. I can confirm > that the performance improvement is still there. Definitely thumbs up on my end. Thanks! The 2nd patch is inspired by Anton's patch. The remainings are just to convert the asm to c in straight forward style. If possible, could you please add your Tested-by tag? Or even better review the series ;) Thanks > > > Jisheng Zhang (5): > > riscv: avoid corrupting the RAS > > riscv: convert bottom half of exception handling to C > > riscv: errata: remove ALT_INSN_FAULT and ALT_PAGE_FAULT > > riscv: errata: sifive: remove NOMMU handling > > riscv: remove asmlinkage from updated functions > > > > arch/riscv/errata/sifive/errata.c | 25 +++++++--- > > arch/riscv/errata/sifive/errata_cip_453.S | 4 -- > > arch/riscv/include/asm/asm-prototypes.h | 7 +-- > > arch/riscv/include/asm/errata_list.h | 21 ++------ > > arch/riscv/kernel/entry.S | 61 ++--------------------- > > arch/riscv/kernel/stacktrace.c | 4 +- > > arch/riscv/kernel/traps.c | 57 ++++++++++++++++++--- > > 7 files changed, 81 insertions(+), 98 deletions(-) > > > > -- > > 2.43.0 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Mon, Jun 17, 2024 at 01:05:47AM +0800, Jisheng Zhang wrote: >For readability, maintainability and future scalability, convert the >bottom half of the exception handling to C. > >During the conversion, I found Anton fixed a performance issue >and my patches will touch the same exception asm code, so I include >Anton's patch for completeness. I also cooked a similar patch to avoid >corrupting the RAS in ret_from_fork() per the inspiration. nit: Probably corruption is wrong word here giving the notion that software got some capability to corrupt uarch structures. It's simply mismatched # of call and # of rets. Imbalance (instead of calling it corruption) in return address stack (RAS) leading to incorret predictions on return. > >Mostly the assembly code is converted to C in a relatively >straightforward manner. > >However, there are two modifications I need to mention: > >1. the CSR_CAUSE reg reading and saving is moved to the C code >because we need the cause to dispatch the exception handling, >if we keep the cause reading and saving, we either pass it to >do_traps() via. 2nd param or get it from pt_regs which an extra >memory load is needed, I don't like any of the two solutions becase >the exception handling sits in hot code path, every instruction >matters. > >2.To cope with SIFIVE_CIP_453 errata, it looks like we don't need >alternative mechanism any more after the asm->c convertion. Just >replace the excp_vect_table two entries. > > > >Anton Blanchard (1): > riscv: Improve exception and system call latency > >Jisheng Zhang (5): > riscv: avoid corrupting the RAS > riscv: convert bottom half of exception handling to C > riscv: errata: remove ALT_INSN_FAULT and ALT_PAGE_FAULT > riscv: errata: sifive: remove NOMMU handling > riscv: remove asmlinkage from updated functions > > arch/riscv/errata/sifive/errata.c | 25 +++++++--- > arch/riscv/errata/sifive/errata_cip_453.S | 4 -- > arch/riscv/include/asm/asm-prototypes.h | 7 +-- > arch/riscv/include/asm/errata_list.h | 21 ++------ > arch/riscv/kernel/entry.S | 61 ++--------------------- > arch/riscv/kernel/stacktrace.c | 4 +- > arch/riscv/kernel/traps.c | 57 ++++++++++++++++++--- > 7 files changed, 81 insertions(+), 98 deletions(-) > >-- >2.43.0 > >
On Thu, Jun 20, 2024 at 1:25 AM Jisheng Zhang <jszhang@kernel.org> wrote: > > On Wed, Jun 19, 2024 at 08:16:58AM +1000, Cyril Bur wrote: > > On Mon, Jun 17, 2024 at 3:21 AM Jisheng Zhang <jszhang@kernel.org> wrote: > > > > > > For readability, maintainability and future scalability, convert the > > > bottom half of the exception handling to C. > > > > > > During the conversion, I found Anton fixed a performance issue > > > and my patches will touch the same exception asm code, so I include > > > Anton's patch for completeness. I also cooked a similar patch to avoid > > > corrupting the RAS in ret_from_fork() per the inspiration. > > > > > > Mostly the assembly code is converted to C in a relatively > > > straightforward manner. > > > > > > However, there are two modifications I need to mention: > > > > > > 1. the CSR_CAUSE reg reading and saving is moved to the C code > > > because we need the cause to dispatch the exception handling, > > > if we keep the cause reading and saving, we either pass it to > > > do_traps() via. 2nd param or get it from pt_regs which an extra > > > memory load is needed, I don't like any of the two solutions becase > > > the exception handling sits in hot code path, every instruction > > > matters. > > > > > > 2.To cope with SIFIVE_CIP_453 errata, it looks like we don't need > > > alternative mechanism any more after the asm->c convertion. Just > > > replace the excp_vect_table two entries. > > > > > > > > > > > > Anton Blanchard (1): > > > riscv: Improve exception and system call latency > > > > > I've retested this patch with the rest of the series applied. I can confirm > > that the performance improvement is still there. Definitely thumbs up on my end. > > Thanks! The 2nd patch is inspired by Anton's patch. The remainings > are just to convert the asm to c in straight forward style. > > If possible, could you please add your Tested-by tag? Or even better > review the series ;) > I only saw this response after having a look at 3/6. I'm a little nervous about a full review as I don't know all the riscv catches yet. I'll say that the testing of the series wasn't massive. I just booted it enough to exercise that codepath and get some numbers. Having said that I'm fine putting a Tested-by for the series, it did boot to userspace after all :). > Thanks > > > > > Jisheng Zhang (5): > > > riscv: avoid corrupting the RAS > > > riscv: convert bottom half of exception handling to C > > > riscv: errata: remove ALT_INSN_FAULT and ALT_PAGE_FAULT > > > riscv: errata: sifive: remove NOMMU handling > > > riscv: remove asmlinkage from updated functions > > > > > > arch/riscv/errata/sifive/errata.c | 25 +++++++--- > > > arch/riscv/errata/sifive/errata_cip_453.S | 4 -- > > > arch/riscv/include/asm/asm-prototypes.h | 7 +-- > > > arch/riscv/include/asm/errata_list.h | 21 ++------ > > > arch/riscv/kernel/entry.S | 61 ++--------------------- > > > arch/riscv/kernel/stacktrace.c | 4 +- > > > arch/riscv/kernel/traps.c | 57 ++++++++++++++++++--- > > > 7 files changed, 81 insertions(+), 98 deletions(-) > > > > > > -- > > > 2.43.0 > > > > > > > > > _______________________________________________ > > > linux-riscv mailing list > > > linux-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv