| Message ID | cebafe40b170589d52e2ef66f3bfac7396fa1f56.1710446682.git.ptosi@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Add support for hypervisor kCFI | expand |
On Thu, 14 Mar 2024 20:24:31 +0000, Pierre-Clément Tosi <ptosi@google.com> wrote: > > Introduce handlers for EL2{t,h} synchronous exceptions distinct from > handlers for other "invalid" exceptions when running with the nVHE host > vector. This will allow a future patch to handle CFI (synchronous) > errors without affecting other classes of exceptions. > > Remove superfluous SP overflow check from the non-synchronous > handlers. Why are they superfluous? Because we are panic'ing? Detecting a stack overflow is pretty valuable in any circumstances. > > Signed-off-by: Pierre-Clément Tosi <ptosi@google.com> > --- > arch/arm64/kvm/hyp/nvhe/host.S | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S > index 27c989c4976d..1b9111c2b480 100644 > --- a/arch/arm64/kvm/hyp/nvhe/host.S > +++ b/arch/arm64/kvm/hyp/nvhe/host.S > @@ -183,7 +183,7 @@ SYM_FUNC_END(__host_hvc) > .endif > .endm > > -.macro invalid_host_el2_vect > +.macro host_el2_sync_vect > .align 7 > > /* > @@ -221,6 +221,11 @@ SYM_FUNC_END(__host_hvc) > b __hyp_do_panic > .endm > > +.macro invalid_host_el2_vect > + .align 7 > + b __hyp_panic > +.endm > + > /* > * The host vector does not use an ESB instruction in order to avoid consuming > * SErrors that should only be consumed by the host. Guest entry is deferred by > @@ -233,12 +238,12 @@ SYM_FUNC_END(__host_hvc) > */ > .align 11 > SYM_CODE_START(__kvm_hyp_host_vector) > - invalid_host_el2_vect // Synchronous EL2t > + host_el2_sync_vect // Synchronous EL2t The real question is: under which circumstances would running with SP_EL0 be valid? I cannot see good reason for it. M.
Hi Marc, On Sun, Mar 17, 2024 at 11:42:44AM +0000, Marc Zyngier wrote: > On Thu, 14 Mar 2024 20:24:31 +0000, > Pierre-Clément Tosi <ptosi@google.com> wrote: > > > > Remove superfluous SP overflow check from the non-synchronous > > handlers. > > Why are they superfluous? Because we are panic'ing? Detecting a stack > overflow is pretty valuable in any circumstances. I've reverted to keeping these in v2. However, the rationale was based on the assumption that the stack overflows into an invalid mapping so that accessing it post-overflow triggers a page fault. If that is correct, can't handlers of non-synchronous exceptions just blindly access SP and rely on the synchronous exception handler to catch any overflow (and somehow handle it or panic, this isn't really relevant)? In particular, note that passing those checks doesn't guarantee that the SP won't actually overflow while the handler is running (as most push to the stack). In that case, they'll end up in the synchronous handler anyway, right? So, given that the checks seem (to me) to happen at completely arbitrary points in time (due to the nature of exceptions), it is therefore not clear how they make the code more robust than not having them? But I'm probably missing something?
diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S index 27c989c4976d..1b9111c2b480 100644 --- a/arch/arm64/kvm/hyp/nvhe/host.S +++ b/arch/arm64/kvm/hyp/nvhe/host.S @@ -183,7 +183,7 @@ SYM_FUNC_END(__host_hvc) .endif .endm -.macro invalid_host_el2_vect +.macro host_el2_sync_vect .align 7 /* @@ -221,6 +221,11 @@ SYM_FUNC_END(__host_hvc) b __hyp_do_panic .endm +.macro invalid_host_el2_vect + .align 7 + b __hyp_panic +.endm + /* * The host vector does not use an ESB instruction in order to avoid consuming * SErrors that should only be consumed by the host. Guest entry is deferred by @@ -233,12 +238,12 @@ SYM_FUNC_END(__host_hvc) */ .align 11 SYM_CODE_START(__kvm_hyp_host_vector) - invalid_host_el2_vect // Synchronous EL2t + host_el2_sync_vect // Synchronous EL2t invalid_host_el2_vect // IRQ EL2t invalid_host_el2_vect // FIQ EL2t invalid_host_el2_vect // Error EL2t - invalid_host_el2_vect // Synchronous EL2h + host_el2_sync_vect // Synchronous EL2h invalid_host_el2_vect // IRQ EL2h invalid_host_el2_vect // FIQ EL2h invalid_host_el2_vect // Error EL2h
Introduce handlers for EL2{t,h} synchronous exceptions distinct from handlers for other "invalid" exceptions when running with the nVHE host vector. This will allow a future patch to handle CFI (synchronous) errors without affecting other classes of exceptions. Remove superfluous SP overflow check from the non-synchronous handlers. Signed-off-by: Pierre-Clément Tosi <ptosi@google.com> --- arch/arm64/kvm/hyp/nvhe/host.S | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)