Allocate and switch to 16-byte aligned secondary stack on overflow. This
provides us stack space to better handle overflows; and is used in
a subsequent patch to dump the hypervisor stacktrace.
Signed-off-by: Kalesh Singh <[email protected]>
---
arch/arm64/kernel/stacktrace.c | 3 +++
arch/arm64/kvm/hyp/nvhe/host.S | 9 ++-------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index a84e38d41d38..f346b4c66f1c 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -242,4 +242,7 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
unwind(task, &state, consume_entry, cookie);
}
+#else /* __KVM_NVHE_HYPERVISOR__ */
+DEFINE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], overflow_stack)
+ __aligned(16);
#endif /* !__KVM_NVHE_HYPERVISOR__ */
diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
index ea6a397b64a6..4e3032a244e1 100644
--- a/arch/arm64/kvm/hyp/nvhe/host.S
+++ b/arch/arm64/kvm/hyp/nvhe/host.S
@@ -177,13 +177,8 @@ SYM_FUNC_END(__host_hvc)
b hyp_panic
.L__hyp_sp_overflow\@:
- /*
- * Reset SP to the top of the stack, to allow handling the hyp_panic.
- * This corrupts the stack but is ok, since we won't be attempting
- * any unwinding here.
- */
- ldr_this_cpu x0, kvm_init_params + NVHE_INIT_STACK_HYP_VA, x1
- mov sp, x0
+ /* Switch to the overflow stack */
+ adr_this_cpu sp, overflow_stack + PAGE_SIZE, x0
b hyp_panic_bad_stack
ASM_BUG()
--
2.36.1.255.ge46751e96f-goog
On Tue, 07 Jun 2022 17:50:45 +0100,
Kalesh Singh <[email protected]> wrote:
>
> Allocate and switch to 16-byte aligned secondary stack on overflow. This
> provides us stack space to better handle overflows; and is used in
> a subsequent patch to dump the hypervisor stacktrace.
>
> Signed-off-by: Kalesh Singh <[email protected]>
> ---
> arch/arm64/kernel/stacktrace.c | 3 +++
> arch/arm64/kvm/hyp/nvhe/host.S | 9 ++-------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index a84e38d41d38..f346b4c66f1c 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -242,4 +242,7 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
>
> unwind(task, &state, consume_entry, cookie);
> }
> +#else /* __KVM_NVHE_HYPERVISOR__ */
> +DEFINE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], overflow_stack)
> + __aligned(16);
Does this need to be a whole page? With 64kB pages, this is
potentially a lot of memory for something that will hardly ever be
used. The rest of the kernel limits this to 4kB, which seems more
reasonable. There is no guard page anyway, so PAGE_SIZE doesn't
provide any extra protection.
> #endif /* !__KVM_NVHE_HYPERVISOR__ */
> diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
> index ea6a397b64a6..4e3032a244e1 100644
> --- a/arch/arm64/kvm/hyp/nvhe/host.S
> +++ b/arch/arm64/kvm/hyp/nvhe/host.S
> @@ -177,13 +177,8 @@ SYM_FUNC_END(__host_hvc)
> b hyp_panic
>
> .L__hyp_sp_overflow\@:
> - /*
> - * Reset SP to the top of the stack, to allow handling the hyp_panic.
> - * This corrupts the stack but is ok, since we won't be attempting
> - * any unwinding here.
> - */
> - ldr_this_cpu x0, kvm_init_params + NVHE_INIT_STACK_HYP_VA, x1
> - mov sp, x0
> + /* Switch to the overflow stack */
> + adr_this_cpu sp, overflow_stack + PAGE_SIZE, x0
>
> b hyp_panic_bad_stack
> ASM_BUG()
> --
> 2.36.1.255.ge46751e96f-goog
>
>
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Wed, Jun 8, 2022 at 12:34 AM Marc Zyngier <[email protected]> wrote:
>
> On Tue, 07 Jun 2022 17:50:45 +0100,
> Kalesh Singh <[email protected]> wrote:
> >
> > Allocate and switch to 16-byte aligned secondary stack on overflow. This
> > provides us stack space to better handle overflows; and is used in
> > a subsequent patch to dump the hypervisor stacktrace.
> >
> > Signed-off-by: Kalesh Singh <[email protected]>
> > ---
> > arch/arm64/kernel/stacktrace.c | 3 +++
> > arch/arm64/kvm/hyp/nvhe/host.S | 9 ++-------
> > 2 files changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> > index a84e38d41d38..f346b4c66f1c 100644
> > --- a/arch/arm64/kernel/stacktrace.c
> > +++ b/arch/arm64/kernel/stacktrace.c
> > @@ -242,4 +242,7 @@ noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
> >
> > unwind(task, &state, consume_entry, cookie);
> > }
> > +#else /* __KVM_NVHE_HYPERVISOR__ */
> > +DEFINE_PER_CPU(unsigned long [PAGE_SIZE/sizeof(long)], overflow_stack)
> > + __aligned(16);
>
> Does this need to be a whole page? With 64kB pages, this is
> potentially a lot of memory for something that will hardly ever be
> used. The rest of the kernel limits this to 4kB, which seems more
> reasonable. There is no guard page anyway, so PAGE_SIZE doesn't
> provide any extra protection.
My oversight on the !4kB page sizes. I think this could be as small as:
(STACK_SIZE - 1) / 2 + sizeof(long)
'/ 2' : Min frame size (x29, x30)
'+ sizeof(long)' : To round up
since we only save the one address (PC) for each frame. WDYT?
Thanks,
Kalesh
>
> > #endif /* !__KVM_NVHE_HYPERVISOR__ */
> > diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
> > index ea6a397b64a6..4e3032a244e1 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/host.S
> > +++ b/arch/arm64/kvm/hyp/nvhe/host.S
> > @@ -177,13 +177,8 @@ SYM_FUNC_END(__host_hvc)
> > b hyp_panic
> >
> > .L__hyp_sp_overflow\@:
> > - /*
> > - * Reset SP to the top of the stack, to allow handling the hyp_panic.
> > - * This corrupts the stack but is ok, since we won't be attempting
> > - * any unwinding here.
> > - */
> > - ldr_this_cpu x0, kvm_init_params + NVHE_INIT_STACK_HYP_VA, x1
> > - mov sp, x0
> > + /* Switch to the overflow stack */
> > + adr_this_cpu sp, overflow_stack + PAGE_SIZE, x0
> >
> > b hyp_panic_bad_stack
> > ASM_BUG()
> > --
> > 2.36.1.255.ge46751e96f-goog
> >
> >
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.