Hi Deepak On Thu, Nov 14, 2024 at 9:25 AM Deepak Gupta <debug@xxxxxxxxxxxx> wrote: > > On Thu, Nov 14, 2024 at 09:20:14AM +0800, Nick Hu wrote: > >Hi Deepak > > > >On Thu, Nov 14, 2024 at 9:06 AM Deepak Gupta <debug@xxxxxxxxxxxx> wrote: > >> > >> On Thu, Nov 14, 2024 at 12:13:38AM +0800, Nick Hu wrote: > >> >Hi Deepak > >> > > >> >On Tue, Nov 12, 2024 at 5:08 AM Deepak Gupta <debug@xxxxxxxxxxxx> wrote: > >> >> > >> >> Kernel will have to perform shadow stack operations on user shadow stack. > >> >> Like during signal delivery and sigreturn, shadow stack token must be > >> >> created and validated respectively. Thus shadow stack access for kernel > >> >> must be enabled. > >> >> > >> >> In future when kernel shadow stacks are enabled for linux kernel, it must > >> >> be enabled as early as possible for better coverage and prevent imbalance > >> >> between regular stack and shadow stack. After `relocate_enable_mmu` has > >> >> been done, this is as early as possible it can enabled. > >> >> > >> >> Signed-off-by: Deepak Gupta <debug@xxxxxxxxxxxx> > >> >> --- > >> >> arch/riscv/kernel/asm-offsets.c | 4 ++++ > >> >> arch/riscv/kernel/head.S | 12 ++++++++++++ > >> >> 2 files changed, 16 insertions(+) > >> >> > >> >> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c > >> >> index 766bd33f10cb..a22ab8a41672 100644 > >> >> --- a/arch/riscv/kernel/asm-offsets.c > >> >> +++ b/arch/riscv/kernel/asm-offsets.c > >> >> @@ -517,4 +517,8 @@ void asm_offsets(void) > >> >> DEFINE(FREGS_A6, offsetof(struct ftrace_regs, a6)); > >> >> DEFINE(FREGS_A7, offsetof(struct ftrace_regs, a7)); > >> >> #endif > >> >> + DEFINE(SBI_EXT_FWFT, SBI_EXT_FWFT); > >> >> + DEFINE(SBI_EXT_FWFT_SET, SBI_EXT_FWFT_SET); > >> >> + DEFINE(SBI_FWFT_SHADOW_STACK, SBI_FWFT_SHADOW_STACK); > >> >> + DEFINE(SBI_FWFT_SET_FLAG_LOCK, SBI_FWFT_SET_FLAG_LOCK); > >> >> } > >> >> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S > >> >> index 356d5397b2a2..6244408ca917 100644 > >> >> --- a/arch/riscv/kernel/head.S > >> >> +++ b/arch/riscv/kernel/head.S > >> >> @@ -164,6 +164,12 @@ secondary_start_sbi: > >> >> call relocate_enable_mmu > >> >> #endif > >> >> call .Lsetup_trap_vector > >> >> + li a7, SBI_EXT_FWFT > >> >> + li a6, SBI_EXT_FWFT_SET > >> >> + li a0, SBI_FWFT_SHADOW_STACK > >> >> + li a1, 1 /* enable supervisor to access shadow stack access */ > >> >> + li a2, SBI_FWFT_SET_FLAG_LOCK > >> >> + ecall > >> >> scs_load_current > >> >> call smp_callin > >> >> #endif /* CONFIG_SMP */ > >> >> @@ -320,6 +326,12 @@ SYM_CODE_START(_start_kernel) > >> >> la tp, init_task > >> >> la sp, init_thread_union + THREAD_SIZE > >> >> addi sp, sp, -PT_SIZE_ON_STACK > >> >> + li a7, SBI_EXT_FWFT > >> >> + li a6, SBI_EXT_FWFT_SET > >> >> + li a0, SBI_FWFT_SHADOW_STACK > >> >> + li a1, 1 /* enable supervisor to access shadow stack access */ > >> >> + li a2, SBI_FWFT_SET_FLAG_LOCK > >> >> + ecall > >> >> scs_load_current > >> >> > >> >> #ifdef CONFIG_KASAN > >> >> > >> >> -- > >> >> 2.45.0 > >> >> > >> >Should we clear the SBI_FWFT_SET_FLAG_LOCK before the cpu hotplug > >> >otherwise the menvcfg.sse won't be set by the fwft set sbi call when > >> >the hotplug cpu back to kernel? > >> > >> Hmm... > >> > >> An incoming hotplug CPU has no features setup on it. > >> I see that `sbi_cpu_start` will supply `secondary_start_sbi` as start > >> up code for incoming CPU. `secondary_start_sbi` is in head.S which converges > >> in `.Lsecondary_start_common`. And thus hotplugged CPU should be > >> issuing shadow stack set FWFT sbi as well. > >> > >> Am I missing something ? > >> > >This is the correct flow. However the opensbi will deny it due to the > >SBI_FWFT_SET_FLAG_LOCK already being set. > >So the menvcfg.sse will not set by this flow. > > > >if (conf->flags & SBI_FWFT_SET_FLAG_LOCK) > > return SBI_EDENIED; > > > > hmm... Why? > > `conf` is pointing to per-hart state in firmware. > > On this incoming cpu, opensbi (or equivalent) firmware must have > ensured that this per-hart state doesn't have lock set. > > Am I missing something? > Current OpenSBI doesn't clear the lock in the warm init of the hotplug path. It seems like we need a patch to address it. Regards, Nick > >Regards, > >Nick > >> > > >> >Regards, > >> >Nick > >> >> > >> >> _______________________________________________ > >> >> linux-riscv mailing list > >> >> linux-riscv@xxxxxxxxxxxxxxxxxxx > >> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
