linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] riscv: xip: support runtime trap patching
@ 2021-05-31  8:53 Vitaly Wool
  2021-05-31 15:17 ` Alex Ghiti
  0 siblings, 1 reply; 6+ messages in thread
From: Vitaly Wool @ 2021-05-31  8:53 UTC (permalink / raw)
  To: linux-riscv
  Cc: linux-kernel, Palmer Dabbelt, Nicolas Pitre, Alex Ghiti, Vitaly Wool

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.

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
 #define DO_ERROR_INFO(name, signo, code, str)				\
-asmlinkage __visible void name(struct pt_regs *regs)			\
+asmlinkage __visible __trap_section void name(struct pt_regs *regs)	\
 {									\
 	do_trap_error(regs, signo, code, regs->epc, "Oops - " str);	\
 }
@@ -111,7 +116,7 @@ DO_ERROR_INFO(do_trap_store_misaligned,
 int handle_misaligned_load(struct pt_regs *regs);
 int handle_misaligned_store(struct pt_regs *regs);
 
-asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
+asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs *regs)
 {
 	if (!handle_misaligned_load(regs))
 		return;
@@ -119,7 +124,7 @@ asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
 		      "Oops - load address misaligned");
 }
 
-asmlinkage void do_trap_store_misaligned(struct pt_regs *regs)
+asmlinkage void __trap_section do_trap_store_misaligned(struct pt_regs *regs)
 {
 	if (!handle_misaligned_store(regs))
 		return;
@@ -146,7 +151,7 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
 	return GET_INSN_LENGTH(insn);
 }
 
-asmlinkage __visible void do_trap_break(struct pt_regs *regs)
+asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
 {
 #ifdef CONFIG_KPROBES
 	if (kprobe_single_step_handler(regs))
diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S b/arch/riscv/kernel/vmlinux-xip.lds.S
index 4b29b9917f99..a3ff09c4c3f9 100644
--- a/arch/riscv/kernel/vmlinux-xip.lds.S
+++ b/arch/riscv/kernel/vmlinux-xip.lds.S
@@ -99,9 +99,22 @@ SECTIONS
 	}
 	PERCPU_SECTION(L1_CACHE_BYTES)
 
-	. = ALIGN(PAGE_SIZE);
+	. = ALIGN(8);
+	.alternative : {
+		__alt_start = .;
+		*(.alternative)
+		__alt_end = .;
+	}
 	__init_end = .;
 
+	. = ALIGN(16);
+	.xip.traps : {
+		__xip_traps_start = .;
+		*(.xip.traps)
+		__xip_traps_end = .;
+	}
+
+	. = ALIGN(PAGE_SIZE);
 	.sdata : {
 		__global_pointer$ = . + 0x800;
 		*(.sdata*)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: xip: support runtime trap patching
  2021-05-31  8:53 [PATCH] riscv: xip: support runtime trap patching Vitaly Wool
@ 2021-05-31 15:17 ` Alex Ghiti
  2021-06-01 10:48   ` Vitaly Wool
  2021-06-04  9:35   ` Alex Ghiti
  0 siblings, 2 replies; 6+ messages in thread
From: Alex Ghiti @ 2021-05-31 15:17 UTC (permalink / raw)
  To: Vitaly Wool, linux-riscv; +Cc: linux-kernel, Palmer Dabbelt, Nicolas Pitre

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?

>   #define DO_ERROR_INFO(name, signo, code, str)				\
> -asmlinkage __visible void name(struct pt_regs *regs)			\
> +asmlinkage __visible __trap_section void name(struct pt_regs *regs)	\
>   {									\
>   	do_trap_error(regs, signo, code, regs->epc, "Oops - " str);	\
>   }
> @@ -111,7 +116,7 @@ DO_ERROR_INFO(do_trap_store_misaligned,
>   int handle_misaligned_load(struct pt_regs *regs);
>   int handle_misaligned_store(struct pt_regs *regs);
>   
> -asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
> +asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs *regs)
>   {
>   	if (!handle_misaligned_load(regs))
>   		return;
> @@ -119,7 +124,7 @@ asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
>   		      "Oops - load address misaligned");
>   }
>   
> -asmlinkage void do_trap_store_misaligned(struct pt_regs *regs)
> +asmlinkage void __trap_section do_trap_store_misaligned(struct pt_regs *regs)
>   {
>   	if (!handle_misaligned_store(regs))
>   		return;
> @@ -146,7 +151,7 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
>   	return GET_INSN_LENGTH(insn);
>   }
>   
> -asmlinkage __visible void do_trap_break(struct pt_regs *regs)
> +asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
>   {
>   #ifdef CONFIG_KPROBES
>   	if (kprobe_single_step_handler(regs))
> diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S b/arch/riscv/kernel/vmlinux-xip.lds.S
> index 4b29b9917f99..a3ff09c4c3f9 100644
> --- a/arch/riscv/kernel/vmlinux-xip.lds.S
> +++ b/arch/riscv/kernel/vmlinux-xip.lds.S
> @@ -99,9 +99,22 @@ SECTIONS
>   	}
>   	PERCPU_SECTION(L1_CACHE_BYTES)
>   
> -	. = ALIGN(PAGE_SIZE);
> +	. = ALIGN(8);
> +	.alternative : {
> +		__alt_start = .;
> +		*(.alternative)
> +		__alt_end = .;
> +	}
>   	__init_end = .;
>   
> +	. = ALIGN(16);

Why 16 here?

> +	.xip.traps : {
> +		__xip_traps_start = .;
> +		*(.xip.traps)
> +		__xip_traps_end = .;
> +	}
> +
> +	. = ALIGN(PAGE_SIZE);
>   	.sdata : {
>   		__global_pointer$ = . + 0x800;
>   		*(.sdata*)
> 

Thanks,

Alex

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: xip: support runtime trap patching
  2021-05-31 15:17 ` Alex Ghiti
@ 2021-06-01 10:48   ` Vitaly Wool
  2021-06-04  9:35   ` Alex Ghiti
  1 sibling, 0 replies; 6+ messages in thread
From: Vitaly Wool @ 2021-06-01 10:48 UTC (permalink / raw)
  To: Alex Ghiti; +Cc: linux-riscv, LKML, Palmer Dabbelt, Nicolas Pitre

On Mon, May 31, 2021 at 5:17 PM Alex Ghiti <alex@ghiti.fr> wrote:
>
> 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?

I don't have any SiFive HW so strictly speaking, not quite. However, I
created a test configuration for Polarfire with a basically no-op
runtime patching and it apparently worked. Traps end up in RAM and are
changeable.

> > 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?
>
> >   #define DO_ERROR_INFO(name, signo, code, str)                               \
> > -asmlinkage __visible void name(struct pt_regs *regs)                 \
> > +asmlinkage __visible __trap_section void name(struct pt_regs *regs)  \
> >   {                                                                   \
> >       do_trap_error(regs, signo, code, regs->epc, "Oops - " str);     \
> >   }
> > @@ -111,7 +116,7 @@ DO_ERROR_INFO(do_trap_store_misaligned,
> >   int handle_misaligned_load(struct pt_regs *regs);
> >   int handle_misaligned_store(struct pt_regs *regs);
> >
> > -asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
> > +asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs *regs)
> >   {
> >       if (!handle_misaligned_load(regs))
> >               return;
> > @@ -119,7 +124,7 @@ asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
> >                     "Oops - load address misaligned");
> >   }
> >
> > -asmlinkage void do_trap_store_misaligned(struct pt_regs *regs)
> > +asmlinkage void __trap_section do_trap_store_misaligned(struct pt_regs *regs)
> >   {
> >       if (!handle_misaligned_store(regs))
> >               return;
> > @@ -146,7 +151,7 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
> >       return GET_INSN_LENGTH(insn);
> >   }
> >
> > -asmlinkage __visible void do_trap_break(struct pt_regs *regs)
> > +asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
> >   {
> >   #ifdef CONFIG_KPROBES
> >       if (kprobe_single_step_handler(regs))
> > diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S b/arch/riscv/kernel/vmlinux-xip.lds.S
> > index 4b29b9917f99..a3ff09c4c3f9 100644
> > --- a/arch/riscv/kernel/vmlinux-xip.lds.S
> > +++ b/arch/riscv/kernel/vmlinux-xip.lds.S
> > @@ -99,9 +99,22 @@ SECTIONS
> >       }
> >       PERCPU_SECTION(L1_CACHE_BYTES)
> >
> > -     . = ALIGN(PAGE_SIZE);
> > +     . = ALIGN(8);
> > +     .alternative : {
> > +             __alt_start = .;
> > +             *(.alternative)
> > +             __alt_end = .;
> > +     }
> >       __init_end = .;
> >
> > +     . = ALIGN(16);
>
> Why 16 here?

Honestly, I don't remember. I believe it should have been 8 but didn't
bother to check, thought it was not a big deal anyway :)

Best regards,
   Vitaly

> > +     .xip.traps : {
> > +             __xip_traps_start = .;
> > +             *(.xip.traps)
> > +             __xip_traps_end = .;
> > +     }
> > +
> > +     . = ALIGN(PAGE_SIZE);
> >       .sdata : {
> >               __global_pointer$ = . + 0x800;
> >               *(.sdata*)
> >
>
> Thanks,
>
> Alex

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: xip: support runtime trap patching
  2021-05-31 15:17 ` Alex Ghiti
  2021-06-01 10:48   ` Vitaly Wool
@ 2021-06-04  9:35   ` Alex Ghiti
  2021-06-04  9:52     ` Vitaly Wool
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Ghiti @ 2021-06-04  9:35 UTC (permalink / raw)
  To: Vitaly Wool, linux-riscv; +Cc: linux-kernel, Palmer Dabbelt, Nicolas Pitre

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?

Thanks,

Alex

> 
>>   #define DO_ERROR_INFO(name, signo, code, str)                \
>> -asmlinkage __visible void name(struct pt_regs *regs)            \
>> +asmlinkage __visible __trap_section void name(struct pt_regs *regs)    \
>>   {                                    \
>>       do_trap_error(regs, signo, code, regs->epc, "Oops - " str);    \
>>   }
>> @@ -111,7 +116,7 @@ DO_ERROR_INFO(do_trap_store_misaligned,
>>   int handle_misaligned_load(struct pt_regs *regs);
>>   int handle_misaligned_store(struct pt_regs *regs);
>> -asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
>> +asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs 
>> *regs)
>>   {
>>       if (!handle_misaligned_load(regs))
>>           return;
>> @@ -119,7 +124,7 @@ asmlinkage void do_trap_load_misaligned(struct 
>> pt_regs *regs)
>>                 "Oops - load address misaligned");
>>   }
>> -asmlinkage void do_trap_store_misaligned(struct pt_regs *regs)
>> +asmlinkage void __trap_section do_trap_store_misaligned(struct 
>> pt_regs *regs)
>>   {
>>       if (!handle_misaligned_store(regs))
>>           return;
>> @@ -146,7 +151,7 @@ static inline unsigned long 
>> get_break_insn_length(unsigned long pc)
>>       return GET_INSN_LENGTH(insn);
>>   }
>> -asmlinkage __visible void do_trap_break(struct pt_regs *regs)
>> +asmlinkage __visible __trap_section void do_trap_break(struct pt_regs 
>> *regs)
>>   {
>>   #ifdef CONFIG_KPROBES
>>       if (kprobe_single_step_handler(regs))
>> diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S 
>> b/arch/riscv/kernel/vmlinux-xip.lds.S
>> index 4b29b9917f99..a3ff09c4c3f9 100644
>> --- a/arch/riscv/kernel/vmlinux-xip.lds.S
>> +++ b/arch/riscv/kernel/vmlinux-xip.lds.S
>> @@ -99,9 +99,22 @@ SECTIONS
>>       }
>>       PERCPU_SECTION(L1_CACHE_BYTES)
>> -    . = ALIGN(PAGE_SIZE);
>> +    . = ALIGN(8);
>> +    .alternative : {
>> +        __alt_start = .;
>> +        *(.alternative)
>> +        __alt_end = .;
>> +    }
>>       __init_end = .;
>> +    . = ALIGN(16);
> 
> Why 16 here?
> 
>> +    .xip.traps : {
>> +        __xip_traps_start = .;
>> +        *(.xip.traps)
>> +        __xip_traps_end = .;
>> +    }
>> +
>> +    . = ALIGN(PAGE_SIZE);
>>       .sdata : {
>>           __global_pointer$ = . + 0x800;
>>           *(.sdata*)
>>
> 
> Thanks,
> 
> Alex
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: xip: support runtime trap patching
  2021-06-04  9:35   ` Alex Ghiti
@ 2021-06-04  9:52     ` Vitaly Wool
  2021-06-11  0:51       ` Palmer Dabbelt
  0 siblings, 1 reply; 6+ messages in thread
From: Vitaly Wool @ 2021-06-04  9:52 UTC (permalink / raw)
  To: Alex Ghiti; +Cc: linux-riscv, LKML, Palmer Dabbelt, Nicolas Pitre

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.

Best regards,
   Vitaly

> Thanks,
>
> Alex
>
> >
> >>   #define DO_ERROR_INFO(name, signo, code, str)                \
> >> -asmlinkage __visible void name(struct pt_regs *regs)            \
> >> +asmlinkage __visible __trap_section void name(struct pt_regs *regs)    \
> >>   {                                    \
> >>       do_trap_error(regs, signo, code, regs->epc, "Oops - " str);    \
> >>   }
> >> @@ -111,7 +116,7 @@ DO_ERROR_INFO(do_trap_store_misaligned,
> >>   int handle_misaligned_load(struct pt_regs *regs);
> >>   int handle_misaligned_store(struct pt_regs *regs);
> >> -asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
> >> +asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs
> >> *regs)
> >>   {
> >>       if (!handle_misaligned_load(regs))
> >>           return;
> >> @@ -119,7 +124,7 @@ asmlinkage void do_trap_load_misaligned(struct
> >> pt_regs *regs)
> >>                 "Oops - load address misaligned");
> >>   }
> >> -asmlinkage void do_trap_store_misaligned(struct pt_regs *regs)
> >> +asmlinkage void __trap_section do_trap_store_misaligned(struct
> >> pt_regs *regs)
> >>   {
> >>       if (!handle_misaligned_store(regs))
> >>           return;
> >> @@ -146,7 +151,7 @@ static inline unsigned long
> >> get_break_insn_length(unsigned long pc)
> >>       return GET_INSN_LENGTH(insn);
> >>   }
> >> -asmlinkage __visible void do_trap_break(struct pt_regs *regs)
> >> +asmlinkage __visible __trap_section void do_trap_break(struct pt_regs
> >> *regs)
> >>   {
> >>   #ifdef CONFIG_KPROBES
> >>       if (kprobe_single_step_handler(regs))
> >> diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S
> >> b/arch/riscv/kernel/vmlinux-xip.lds.S
> >> index 4b29b9917f99..a3ff09c4c3f9 100644
> >> --- a/arch/riscv/kernel/vmlinux-xip.lds.S
> >> +++ b/arch/riscv/kernel/vmlinux-xip.lds.S
> >> @@ -99,9 +99,22 @@ SECTIONS
> >>       }
> >>       PERCPU_SECTION(L1_CACHE_BYTES)
> >> -    . = ALIGN(PAGE_SIZE);
> >> +    . = ALIGN(8);
> >> +    .alternative : {
> >> +        __alt_start = .;
> >> +        *(.alternative)
> >> +        __alt_end = .;
> >> +    }
> >>       __init_end = .;
> >> +    . = ALIGN(16);
> >
> > Why 16 here?
> >
> >> +    .xip.traps : {
> >> +        __xip_traps_start = .;
> >> +        *(.xip.traps)
> >> +        __xip_traps_end = .;
> >> +    }
> >> +
> >> +    . = ALIGN(PAGE_SIZE);
> >>       .sdata : {
> >>           __global_pointer$ = . + 0x800;
> >>           *(.sdata*)
> >>
> >
> > Thanks,
> >
> > Alex
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: xip: support runtime trap patching
  2021-06-04  9:52     ` Vitaly Wool
@ 2021-06-11  0:51       ` Palmer Dabbelt
  0 siblings, 0 replies; 6+ messages in thread
From: Palmer Dabbelt @ 2021-06-11  0:51 UTC (permalink / raw)
  To: vitaly.wool; +Cc: alex, linux-riscv, linux-kernel, nico

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-11  0:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31  8:53 [PATCH] riscv: xip: support runtime trap patching Vitaly Wool
2021-05-31 15:17 ` Alex Ghiti
2021-06-01 10:48   ` Vitaly Wool
2021-06-04  9:35   ` Alex Ghiti
2021-06-04  9:52     ` Vitaly Wool
2021-06-11  0:51       ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).