Date: Thu, 20 Jun 2024 09:59:31 +1000
From: Cyril Bur <cyrilbur@...storrent.com>
To: Jisheng Zhang <jszhang@...nel.org>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Samuel Holland <samuel.holland@...ive.com>, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [CAUTION - External Sender] Re: [PATCH 0/6] riscv: convert bottom
 half of exception handling to C

On Thu, Jun 20, 2024 at 1:25 AM Jisheng Zhang <jszhang@...nel.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@...nel.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@...ts.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists