|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen master] xen/arm: entry: Ensure the guest state is synced when receiving a vSError
commit a458d3bd0d2585275c128556ec0cbd818c6a7b0d
Author: Julien Grall <julien.grall@xxxxxxx>
AuthorDate: Tue Sep 24 18:58:39 2019 +0100
Commit: Julien Grall <julien.grall@xxxxxxx>
CommitDate: Fri Nov 1 14:34:39 2019 +0000
xen/arm: entry: Ensure the guest state is synced when receiving a vSError
When a SError/Asynchronous Abort generated by the guest has been
consumed, we will skip the handling of the initial exception.
This includes the calls to enter_hypervisor_from_guest{, _noirq} that
is used to synchronize part of the guest state with the internal
representation and re-enable workarounds (e.g. SSBD). However, we still
call leave_hypervisor_to_guest() which is used for preempting the guest
and synchronizing back part of the guest state.
enter_hypervisor_from_guest{, _noirq} works in pair with
leave_hypervisor_to_guest(), so skipping the first two may result
in a loss of some part of guest state.
An example is the new vGIC which will save the state of the LRs on exit
from the guest and rewrite all of them on entry to the guest.
A more worrying example is SSBD workaround may not be re-enabled. If
leave_hypervisor_to_guest() is rescheduling the vCPU, then we may end to
run a lot of code with SSBD workaroud disabled.
For now, calling leave_hypervisor_to_guest() is not necessary when
injecting a vSError to the guest. But it would still be good to give an
opportunity to reschedule. So both enter_hypervisor_from_guest() and
leave_hypervisor_to_guest() are called.
Note that on arm64, the return value for check_pending_vserror is now
stored in x19 instead of x0. This is because we want to keep the value
across call to C-functions (x0, unlike x19, will not be saved by the
callee).
Take the opportunity to rename check_pending_vserror() to
check_pending_guest_serror() as the function is dealing with host SError
and *not* virtual SError. The documentation is also updated accross
Arm32 and Arm64 to clarify how Xen is dealing with SError generated by
the guest.
Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Release-acked-by: Juergen Gross <jgross@xxxxxxxx>
---
xen/arch/arm/arm32/entry.S | 57 ++++++++++++++++++++++++++++++++++++++--------
xen/arch/arm/arm64/entry.S | 54 ++++++++++++++++++++++++++++++++-----------
2 files changed, 88 insertions(+), 23 deletions(-)
diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 6185f46114..31ccfb2631 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -27,6 +27,10 @@
/*
* Actions that needs to be done after entering the hypervisor from the
* guest and before the interrupts are unmasked.
+ *
+ * @return:
+ * r4: Set to a non-zero value if a pending Abort exception took place.
+ * Otherwise, it will be set to zero.
*/
prepare_context_from_guest:
#ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
@@ -56,18 +60,35 @@ prepare_context_from_guest:
SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
/*
- * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
- * feature, the checking of pending SErrors will be skipped.
+ * We may have entered the hypervisor with pending asynchronous Abort
+ * generated by the guest. If we need to categorize them, then
+ * we need to consume any outstanding asynchronous Abort.
+ * Otherwise, they can be consumed later on.
*/
alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+ mov r4, #0 /* r4 := No Abort was consumed */
b skip_check
alternative_else_nop_endif
/*
- * Start to check pending virtual abort in the gap of Guest -> HYP
- * world switch.
+ * Consume pending asynchronous Abort generated by the guest if any.
+ *
+ * The only way to consume an Abort interrupt is to unmask it. So
+ * Abort exception will be unmaked for a small window and then masked
+ * it again.
+ *
+ * It is fine to unmask asynchronous Abort exception as we fully
+ * control the state of the processor and only limited code will
+ * be executed if the exception returns (see do_trap_data_abort()).
*
- * Save ELR_hyp to check whether the pending virtual abort exception
+ * TODO: The asynchronous abort path should be reworked to
+ * inject the virtual asynchronous Abort in enter_hypervisor_*
+ * rather than do_trap_data_abort(). This should make easier to
+ * understand the path.
+ */
+
+ /*
+ * save elr_hyp to check whether the pending virtual abort exception
* takes place while we are doing this trap exception.
*/
mrs r1, ELR_hyp
@@ -112,11 +133,11 @@ abort_guest_exit_end:
cmp r1, r2
/*
- * Not equal, the pending virtual abort exception took place, the
- * initial exception does not have any significance to be handled.
- * Exit ASAP.
+ * Set r4 depending on whether an asynchronous abort were
+ * consumed.
*/
- bne return_from_trap
+ movne r4, #1
+ moveq r4, #0
skip_check:
b enter_hypervisor_from_guest_preirq
@@ -179,12 +200,28 @@ ENDPROC(prepare_context_from_guest)
1:
/* Trap from the guest */
+ /*
+ * prepare_context_from_guest will return with r4 set to
+ * a non-zero value if an asynchronous Abort was consumed.
+ *
+ * When an asynchronous Abort has been consumed (r4 != 0), we may have
+ * injected a virtual asynchronous Abort to the guest.
+ *
+ * In this case, the initial exception will be discarded (PC has
+ * been adjusted by inject_vabt_exception()). However, we still
+ * want to give an opportunity to reschedule the vCPU. So we
+ * only want to skip the handling of the initial exception (i.e.
+ * do_trap_*()).
+ */
bl prepare_context_from_guest
.if \guest_iflags != n
cpsie \guest_iflags
.endif
- bl enter_hypervisor_from_guest
+ adr lr, 2f
+ cmp r4, #0
+ adrne lr, return_from_trap
+ b enter_hypervisor_from_guest
2:
/* We are ready to handle the trap, setup the registers and jump. */
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index a8ba7ab961..d35855af96 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -184,18 +184,41 @@
.macro guest_vector compat, iflags, trap, save_x0_x1=1
entry hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
/*
- * The vSError will be checked while SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
- * is not set. If a vSError took place, the initial exception will be
- * skipped. Exit ASAP
+ * We may have entered the hypervisor with pending SErrors
+ * generated by the guest. If we need to categorize them, then
+ * we need to check any outstanding SErrors will be consumed.
+ *
+ * The function check_pending_guest_serror() will unmask SError
+ * exception temporarily. This is fine to do before enter_*
+ * helpers are called because we fully control the state of the
+ * processor and only limited code willl be executed (see
+ * do_trap_hyp_serror()).
+ *
+ * When a SError has been consumed (x19 != 0), we may have injected a
+ * virtual SError to the guest.
+ *
+ * In this case, the initial exception will be discarded (PC has
+ * been adjusted by inject_vabt_exception()). However, we still
+ * want to give an opportunity to reschedule the vCPU. So we
+ * only want to skip the handling of the initial exception (i.e.
+ * do_trap_*()).
+ *
+ * TODO: The SErrors path should be reworked to inject the vSError in
+ * enter_hypervisor_* rather than do_trap_hyp_serror. This should make
+ * easier to understand the path.
*/
alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
- bl check_pending_vserror
- cbnz x0, 1f
+ bl check_pending_guest_serror
alternative_else_nop_endif
bl enter_hypervisor_from_guest_preirq
msr daifclr, \iflags
bl enter_hypervisor_from_guest
+
+ alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+ cbnz x19, 1f
+ alternative_else_nop_endif
+
mov x0, sp
bl do_trap_\trap
1:
@@ -436,13 +459,17 @@ return_from_trap:
eret
/*
- * This function is used to check pending virtual SError in the gap of
- * EL1 -> EL2 world switch.
- * The x0 register will be used to indicate the results of detection.
- * x0 -- Non-zero indicates a pending virtual SError took place.
- * x0 -- Zero indicates no pending virtual SError took place.
+ * Consume pending SError generated by the guest if any.
+ *
+ * @return:
+ * x19: Set to a non-zero value if a pending Abort exception took place.
+ * Otherwise, it will be set to zero.
+ *
+ * Without RAS extension, the only way to consume a SError is to unmask
+ * it. So the function will unmask SError exception for a small window and
+ * then mask it again.
*/
-check_pending_vserror:
+check_pending_guest_serror:
/*
* Save elr_el2 to check whether the pending SError exception takes
* place while we are doing this sync exception.
@@ -487,11 +514,12 @@ abort_guest_exit_end:
/*
* Not equal, the pending SError exception took place, set
- * x0 to non-zero.
+ * x19 to non-zero.
*/
- cset x0, ne
+ cset x19, ne
ret
+ENDPROC(check_pending_guest_serror)
/*
* Exception vectors.
--
generated by git-patchbot for /home/xen/git/xen.git#master
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |