Messages in this thread | | | From | Sami Tolvanen <> | Date | Mon, 30 Nov 2020 13:13:07 -0800 | Subject | Re: [PATCH v2 2/2] arm64: scs: use vmapped IRQ and SDEI shadow stacks |
| |
On Mon, Nov 30, 2020 at 3:49 AM Will Deacon <will@kernel.org> wrote: > > On Tue, Nov 24, 2020 at 11:59:40AM -0800, Sami Tolvanen wrote: > > Use scs_alloc() to allocate also IRQ and SDEI shadow stacks instead of > > using statically allocated stacks. > > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > --- > > arch/arm64/kernel/Makefile | 1 - > > arch/arm64/kernel/entry.S | 6 ++-- > > arch/arm64/kernel/irq.c | 19 ++++++++++ > > arch/arm64/kernel/scs.c | 16 --------- > > arch/arm64/kernel/sdei.c | 71 +++++++++++++++++++++++++++++++------- > > include/linux/scs.h | 4 --- > > 6 files changed, 81 insertions(+), 36 deletions(-) > > delete mode 100644 arch/arm64/kernel/scs.c > > [...] > > > @@ -70,18 +97,40 @@ static int _init_sdei_stack(unsigned long * __percpu *ptr, int cpu) > > return 0; > > } > > > > +static int _init_sdei_scs(unsigned long * __percpu *ptr, int cpu) > > +{ > > + void *s; > > + > > + s = scs_alloc(cpu_to_node(cpu)); > > + if (!s) > > + return -ENOMEM; > > + per_cpu(*ptr, cpu) = s; > > + > > + return 0; > > +} > > + > > static int init_sdei_stacks(void) > > { > > int cpu; > > int err = 0; > > > > for_each_possible_cpu(cpu) { > > - err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > - if (err) > > - break; > > - err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > - if (err) > > - break; > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > + err = _init_sdei_stack(&sdei_stack_normal_ptr, cpu); > > + if (err) > > + break; > > + err = _init_sdei_stack(&sdei_stack_critical_ptr, cpu); > > + if (err) > > + break; > > + } > > + if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { > > + err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu); > > + if (err) > > + break; > > + err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu); > > + if (err) > > + break; > > This looks ok to me, but I think it would be better to follow the same > approach as you have for the IRQ stacks and instead have a separate > init_sdei_scs() function (similarly for the free() path), which means > you can simply the IS_ENABLED() checks too.
OK, I can change this in v3. It makes error handling in sdei_arch_get_entry_point() a bit more awkward though. We'll need something like this:
if (IS_ENABLED(CONFIG_VMAP_STACK)) { if (init_sdei_stacks()) return 0; }
if (IS_ENABLED(CONFIG_SHADOW_CALL_STACK)) { if (init_sdei_scs()) { if (IS_ENABLED(CONFIG_VMAP_STACK)) free_sdei_stacks(); return 0; } }
Sami
| |