Hi Nick,
Thanks for reviewing and helping.
On Thu, Nov 14, 2024 at 02:17:30PM +0800, Nick Hu wrote:
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:
>> >> 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.
Got it thanks.
Since you already know what's the problem, can you send a patch to opensbi.
If you want rather have me do it, let me know. Thanks.
Regards,
Nick
>Regards,
>Nick
>> >
>> >Regards,
>> >Nick
>> >>
>> >> _______________________________________________
>> >> linux-riscv mailing list
>> >> linux-riscv@xxxxxxxxxxxxxxxxxxx
>> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
