| Message ID | 20260827-spacemit-k3-v3-1-5d8dbb68539d@linux.spacemit.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/5] lib: sbi: select expected trap handler per hart | expand |
Hi Troy, Have you tried stack protector? I changed them to use extern symbol directly because they break stack protector. Please check my patchset sent last year: https://lore.kernel.org/all/20250703151957.2545958-1-alvinga@andestech.com/ Thanks, Alvin On Thu, 27 Aug 2026 17:19:19 +0800, Troy Mitchell wrote: > The expected trap handler is selected once by the cold boot hart. This > breaks heterogeneous systems where the cold boot hart implements H but > another hart does not, because the H-aware handler accesses mtval2 and > mtinst. > > Select the handler from the current hart's MISA at each use so every > hart uses only the CSRs it implements. > > Fixes: 1de66d170e71 ("lib: Optimize unpriv load/store implementation") > Reported-by: Bo Gan <ganboing@gmail.com> > Link: https://lore.kernel.org/r/e702f291-dde8-4b99-a65e-182d2b847720@gmail.com > Suggested-by: Bo Gan <ganboing@gmail.com> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com> > Reviewed-by: Bo Gan <ganboing@gmail.com> > --- > include/sbi/sbi_csr_detect.h | 4 ++-- > include/sbi/sbi_hart.h | 2 +- > lib/sbi/sbi_hart.c | 9 +++++---- > lib/sbi/sbi_illegal_atomic.c | 4 ++-- > lib/sbi/sbi_unpriv.c | 6 +++--- > 5 files changed, 13 insertions(+), 12 deletions(-) > > diff --git a/include/sbi/sbi_csr_detect.h b/include/sbi/sbi_csr_detect.h > index 097c31c8..31e50db9 100644 > --- a/include/sbi/sbi_csr_detect.h > +++ b/include/sbi/sbi_csr_detect.h > @@ -16,9 +16,9 @@ > > #define csr_read_allowed(csr_num, trap) \ > ({ \ > + register ulong mtvec = sbi_hart_expected_trap_addr(); \ > register ulong tinfo asm("a3") = (ulong)trap; \ > register ulong ttmp asm("a4"); \ > - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ > register ulong ret = 0; \ > ((struct sbi_trap_info *)(trap))->cause = 0; \ > asm volatile( \ > @@ -35,9 +35,9 @@ > > #define csr_write_allowed(csr_num, trap, value) \ > ({ \ > + register ulong mtvec = sbi_hart_expected_trap_addr(); \ > register ulong tinfo asm("a3") = (ulong)trap; \ > register ulong ttmp asm("a4"); \ > - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ > ((struct sbi_trap_info *)(trap))->cause = 0; \ > asm volatile( \ > "add %[ttmp], %[tinfo], zero\n" \ > diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h > index 543393bb..6f4ee31e 100644 > --- a/include/sbi/sbi_hart.h > +++ b/include/sbi/sbi_hart.h > @@ -135,7 +135,7 @@ struct sbi_scratch; > int sbi_hart_reinit(struct sbi_scratch *scratch); > int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot); > > -extern void (*sbi_hart_expected_trap)(void); > +ulong sbi_hart_expected_trap_addr(void); > > unsigned int sbi_hart_mhpm_mask(struct sbi_scratch *scratch); > void sbi_hart_delegation_dump(struct sbi_scratch *scratch, > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c > index bee88557..14e44955 100644 > --- a/lib/sbi/sbi_hart.c > +++ b/lib/sbi/sbi_hart.c > @@ -25,7 +25,11 @@ > extern void __sbi_expected_trap(void); > extern void __sbi_expected_trap_hext(void); > > -void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap; > +ulong sbi_hart_expected_trap_addr(void) > +{ > + return misa_extension('H') ? (ulong)&__sbi_expected_trap_hext : > + (ulong)&__sbi_expected_trap; > +} > > unsigned long hart_features_offset; > > @@ -712,9 +716,6 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot) > csr_write(CSR_MIP, 0); > > if (cold_boot) { > - if (misa_extension('H')) > - sbi_hart_expected_trap = &__sbi_expected_trap_hext; > - > hart_features_offset = sbi_scratch_alloc_offset( > sizeof(struct sbi_hart_features)); > if (!hart_features_offset) > diff --git a/lib/sbi/sbi_illegal_atomic.c b/lib/sbi/sbi_illegal_atomic.c > index 977a9ad0..30f5118e 100644 > --- a/lib/sbi/sbi_illegal_atomic.c > +++ b/lib/sbi/sbi_illegal_atomic.c > @@ -30,7 +30,7 @@ int sbi_illegal_atomic(ulong insn, struct sbi_trap_regs *regs) > { \ > register ulong tinfo asm("a3"); \ > register ulong mstatus = 0; \ > - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ > + register ulong mtvec = sbi_hart_expected_trap_addr(); \ > type ret = 0; \ > trap->cause = 0; \ > asm volatile( \ > @@ -57,7 +57,7 @@ int sbi_illegal_atomic(ulong insn, struct sbi_trap_regs *regs) > { \ > register ulong tinfo asm("a3"); \ > register ulong mstatus = 0; \ > - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ > + register ulong mtvec = sbi_hart_expected_trap_addr(); \ > type ret = 0; \ > trap->cause = 0; \ > asm volatile( \ > diff --git a/lib/sbi/sbi_unpriv.c b/lib/sbi/sbi_unpriv.c > index 60becedc..1550d111 100644 > --- a/lib/sbi/sbi_unpriv.c > +++ b/lib/sbi/sbi_unpriv.c > @@ -33,9 +33,9 @@ union sbi_unpriv_data { > type sbi_load_##type(const type *addr, \ > struct sbi_trap_info *trap) \ > { \ > + register ulong mtvec = sbi_hart_expected_trap_addr(); \ > register ulong tinfo asm("a3") = (ulong)trap; \ > register ulong mstatus = 0; \ > - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ > type ret = 0; \ > trap->cause = 0; \ > asm volatile( \ > @@ -58,9 +58,9 @@ union sbi_unpriv_data { > void sbi_store_##type(type *addr, type val, \ > struct sbi_trap_info *trap) \ > { \ > + register ulong mtvec = sbi_hart_expected_trap_addr(); \ > register ulong tinfo asm("a3") = (ulong)trap; \ > register ulong mstatus = 0; \ > - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ > trap->cause = 0; \ > asm volatile( \ > "csrrw %[mtvec], " STR(CSR_MTVEC) ", %[mtvec]\n" \ > @@ -207,7 +207,7 @@ ulong sbi_get_insn(ulong mepc, struct sbi_trap_info *trap) > register ulong tinfo asm("a3"); > register ulong ttmp asm("a4"); > register ulong mstatus = 0; > - register ulong mtvec = (ulong)sbi_hart_expected_trap; > + register ulong mtvec = sbi_hart_expected_trap_addr(); > ulong insn = 0; > > trap->cause = 0; > > -- > 2.55.0 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
On Mon, Aug 31, 2026 at 10:21 AM +0800, Alvin Chang wrote: > Have you tried stack protector? > > I changed them to use extern symbol directly because they break stack > protector. > Please check my patchset sent last year: > https://lore.kernel.org/all/20250703151957.2545958-1-alvinga@andestech.com/ I had not tested this version with stack protector enabled. I have now built and booted it under QEMU with CONFIG_STACK_PROTECTOR_ALL. It boots, but the generated code confirms that the accessor itself is instrumented and introduces an out-of-line call in this low-level path. I will replace it with a per-hart handler address stored in scratch and loaded directly at each use, then test it on K3 with stack protector enabled. - Troy
diff --git a/include/sbi/sbi_csr_detect.h b/include/sbi/sbi_csr_detect.h index 097c31c8..31e50db9 100644 --- a/include/sbi/sbi_csr_detect.h +++ b/include/sbi/sbi_csr_detect.h @@ -16,9 +16,9 @@ #define csr_read_allowed(csr_num, trap) \ ({ \ + register ulong mtvec = sbi_hart_expected_trap_addr(); \ register ulong tinfo asm("a3") = (ulong)trap; \ register ulong ttmp asm("a4"); \ - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ register ulong ret = 0; \ ((struct sbi_trap_info *)(trap))->cause = 0; \ asm volatile( \ @@ -35,9 +35,9 @@ #define csr_write_allowed(csr_num, trap, value) \ ({ \ + register ulong mtvec = sbi_hart_expected_trap_addr(); \ register ulong tinfo asm("a3") = (ulong)trap; \ register ulong ttmp asm("a4"); \ - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ ((struct sbi_trap_info *)(trap))->cause = 0; \ asm volatile( \ "add %[ttmp], %[tinfo], zero\n" \ diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h index 543393bb..6f4ee31e 100644 --- a/include/sbi/sbi_hart.h +++ b/include/sbi/sbi_hart.h @@ -135,7 +135,7 @@ struct sbi_scratch; int sbi_hart_reinit(struct sbi_scratch *scratch); int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot); -extern void (*sbi_hart_expected_trap)(void); +ulong sbi_hart_expected_trap_addr(void); unsigned int sbi_hart_mhpm_mask(struct sbi_scratch *scratch); void sbi_hart_delegation_dump(struct sbi_scratch *scratch, diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c index bee88557..14e44955 100644 --- a/lib/sbi/sbi_hart.c +++ b/lib/sbi/sbi_hart.c @@ -25,7 +25,11 @@ extern void __sbi_expected_trap(void); extern void __sbi_expected_trap_hext(void); -void (*sbi_hart_expected_trap)(void) = &__sbi_expected_trap; +ulong sbi_hart_expected_trap_addr(void) +{ + return misa_extension('H') ? (ulong)&__sbi_expected_trap_hext : + (ulong)&__sbi_expected_trap; +} unsigned long hart_features_offset; @@ -712,9 +716,6 @@ int sbi_hart_init(struct sbi_scratch *scratch, bool cold_boot) csr_write(CSR_MIP, 0); if (cold_boot) { - if (misa_extension('H')) - sbi_hart_expected_trap = &__sbi_expected_trap_hext; - hart_features_offset = sbi_scratch_alloc_offset( sizeof(struct sbi_hart_features)); if (!hart_features_offset) diff --git a/lib/sbi/sbi_illegal_atomic.c b/lib/sbi/sbi_illegal_atomic.c index 977a9ad0..30f5118e 100644 --- a/lib/sbi/sbi_illegal_atomic.c +++ b/lib/sbi/sbi_illegal_atomic.c @@ -30,7 +30,7 @@ int sbi_illegal_atomic(ulong insn, struct sbi_trap_regs *regs) { \ register ulong tinfo asm("a3"); \ register ulong mstatus = 0; \ - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ + register ulong mtvec = sbi_hart_expected_trap_addr(); \ type ret = 0; \ trap->cause = 0; \ asm volatile( \ @@ -57,7 +57,7 @@ int sbi_illegal_atomic(ulong insn, struct sbi_trap_regs *regs) { \ register ulong tinfo asm("a3"); \ register ulong mstatus = 0; \ - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ + register ulong mtvec = sbi_hart_expected_trap_addr(); \ type ret = 0; \ trap->cause = 0; \ asm volatile( \ diff --git a/lib/sbi/sbi_unpriv.c b/lib/sbi/sbi_unpriv.c index 60becedc..1550d111 100644 --- a/lib/sbi/sbi_unpriv.c +++ b/lib/sbi/sbi_unpriv.c @@ -33,9 +33,9 @@ union sbi_unpriv_data { type sbi_load_##type(const type *addr, \ struct sbi_trap_info *trap) \ { \ + register ulong mtvec = sbi_hart_expected_trap_addr(); \ register ulong tinfo asm("a3") = (ulong)trap; \ register ulong mstatus = 0; \ - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ type ret = 0; \ trap->cause = 0; \ asm volatile( \ @@ -58,9 +58,9 @@ union sbi_unpriv_data { void sbi_store_##type(type *addr, type val, \ struct sbi_trap_info *trap) \ { \ + register ulong mtvec = sbi_hart_expected_trap_addr(); \ register ulong tinfo asm("a3") = (ulong)trap; \ register ulong mstatus = 0; \ - register ulong mtvec = (ulong)sbi_hart_expected_trap; \ trap->cause = 0; \ asm volatile( \ "csrrw %[mtvec], " STR(CSR_MTVEC) ", %[mtvec]\n" \ @@ -207,7 +207,7 @@ ulong sbi_get_insn(ulong mepc, struct sbi_trap_info *trap) register ulong tinfo asm("a3"); register ulong ttmp asm("a4"); register ulong mstatus = 0; - register ulong mtvec = (ulong)sbi_hart_expected_trap; + register ulong mtvec = sbi_hart_expected_trap_addr(); ulong insn = 0; trap->cause = 0;
The expected trap handler is selected once by the cold boot hart. This breaks heterogeneous systems where the cold boot hart implements H but another hart does not, because the H-aware handler accesses mtval2 and mtinst. Select the handler from the current hart's MISA at each use so every hart uses only the CSRs it implements. Fixes: 1de66d170e71 ("lib: Optimize unpriv load/store implementation") Reported-by: Bo Gan <ganboing@gmail.com> Link: https://lore.kernel.org/r/e702f291-dde8-4b99-a65e-182d2b847720@gmail.com Suggested-by: Bo Gan <ganboing@gmail.com> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com> Reviewed-by: Bo Gan <ganboing@gmail.com> --- include/sbi/sbi_csr_detect.h | 4 ++-- include/sbi/sbi_hart.h | 2 +- lib/sbi/sbi_hart.c | 9 +++++---- lib/sbi/sbi_illegal_atomic.c | 4 ++-- lib/sbi/sbi_unpriv.c | 6 +++--- 5 files changed, 13 insertions(+), 12 deletions(-)