[Xen-devel] [PATCH RFC for-4.13 10/10] xen/arm64: entry: Ensure the guest state is synced when receiving a vSError

[Julien Grall <julien.grall@xxxxxxx>]

At the moment, when a SError is received while checking for a pending
one, we will skip the handling the initial exception.

This includes call to exit_from_guest{, _noirq} that is used to
synchronize part of the guest state with the internal representation.
However, we still call leave_hypervisor_tail() which is used for preempting
the guest and synchronizing back part of the guest state.

exit_from_guest{, _noirq} works in pair with leave_hypervisor_tail(), so
skipping if former may result to 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.

For now, calling leave_hypervisor_tail() is not necessary when injecting
a vSError to the guest. But as the path is spread accross multiple file,
it is hard to enforce that for the future (someone we may want to crash the
domain). Therefore it is best to call exit_from_guest{, _noirq} in the
vSError path as well.

Note that the return value of check_pending_vserror is now set in x19
instead of x0. This is because we want to keep the value across call to
C-function and x0, unlike x19, will not be saved by the callee.

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

---

I am not aware of any issues other than with the new vGIC. But I
haven't looked hard enough so I think it would be worth to try to fix it
for Xen 4.13.
---
 xen/arch/arm/arm64/entry.S | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 91cf6ee6f4..f5350247e1 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -168,11 +168,13 @@
         /*
          * 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
+         * skipped.
+         *
+         * However, we still need to call exit_from_guest{,_noirq} as the
+         * return path to the guest may rely on state saved by them.
          */
         alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
         bl      check_pending_vserror
-        cbnz    x0, 1f
         alternative_else_nop_endif
 
         mov     x0, sp
@@ -180,6 +182,11 @@
         msr     daifclr, \iflags
         mov     x0, sp
         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:
@@ -383,9 +390,9 @@ return_from_trap:
 /*
  * 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.
+ * The register x19 will be used to indicate the results of detection.
+ * x19 -- Non-zero indicates a pending virtual SError took place.
+ * x19 -- Zero indicates no pending virtual SError took place.
  */
 check_pending_vserror:
         /*
@@ -432,9 +439,9 @@ 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
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel