Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 10 Jun 2021 17:51:36 -0700 (PDT) | Subject | Re: [PATCH] riscv: xip: support runtime trap patching | From | Palmer Dabbelt <> |
| |
On Fri, 04 Jun 2021 02:52:40 PDT (-0700), vitaly.wool@konsulko.com wrote: > Hi Alex, > > On Fri, Jun 4, 2021 at 11:35 AM Alex Ghiti <alex@ghiti.fr> wrote: >> >> Le 31/05/2021 à 17:17, Alex Ghiti a écrit : >> > Hi Vitaly, >> > >> > Le 31/05/2021 à 10:53, Vitaly Wool a écrit : >> >> RISCV_ERRATA_ALTERNATIVE patches text at runtime which is currently >> >> not possible when the kernel is executed from the flash in XIP mode. >> >> Since runtime patching concerns only traps at the moment, let's just >> >> have all the traps reside in RAM anyway if RISCV_ERRATA_ALTERNATIVE >> >> is set. Thus, these functions will be patch-able even when the .text >> >> section is in flash. >> >> >> > >> > This sounds like a good fix for sifive platforms to work with XIP kernel >> > in 5.13: did you test that it actually works on HW? >> > >> >> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com> >> >> --- >> >> arch/riscv/kernel/traps.c | 13 +++++++++---- >> >> arch/riscv/kernel/vmlinux-xip.lds.S | 15 ++++++++++++++- >> >> 2 files changed, 23 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c >> >> index 0721b9798595..7bc88d8aab97 100644 >> >> --- a/arch/riscv/kernel/traps.c >> >> +++ b/arch/riscv/kernel/traps.c >> >> @@ -86,8 +86,13 @@ static void do_trap_error(struct pt_regs *regs, int >> >> signo, int code, >> >> } >> >> } >> >> +#if defined (CONFIG_XIP_KERNEL) && defined >> >> (CONFIG_RISCV_ERRATA_ALTERNATIVE) >> >> +#define __trap_section __section(".xip.traps") >> >> +#else >> >> +#define __trap_section >> >> +#endif >> > >> > Maybe we could do something more generic. At the moment, only traps are >> > subject to alternatives but that will likely expand: what about rather >> > defining a section called __alternative_section? >> >> Any thoughts about that? > > Oh sorry, I have overlooked this. I do agree this is the way to go > long term, I just wanted to do a reasonably good quick fix since the > default SiFive configuration won't even build with CONFIG_XIP enabled. > So I would probably just suggest to take the existing solution and > then think over the details (e. g. should we just use one "alternative > section" for both the functions and the table or have separate ones?) > for the generic solution.
I've taken this on fixes. It's fixing a bug and should be pretty safe, since it only has any effect on XIP kernels.
| ![\](/images/icornerr.gif) |