xen/arm: Remove serrors=forward

Per the Arm ARM (D4.5 in ARM DDI 0487E.a), SError may be precise or
imprecise.

Imprecise means the state presented to the exception handler is not
guaranteed to be consistent with any point in the excution stream from
which the exception was taken. In other words, they are likely to be
fatal as you can't return safely from them.

Without the RAS extension, the Arm architecture does not provide a way
to differentiate between imprecise and precise SError. Furthermore Xen
has no support for RAS yet. So from a software POV, there is not much
we can do.

More generally, forwarding blindly SErrors to the guest is likely to be
the wrong thing to do. Indeed, Xen is not able to know what is the
content of the SError. This may be a critical device used by the
hypervisor that is about to fail.

In a nutshell, the option serrors=forward is not safe to use in any
environment with the current state of Xen. Therefore the option and any
code related to it are completely removed.

Take the opportunity to rework the comment in do_trap_data_abort() as
all SErrors/External Abort generated by the hypervisor will result in
a crash of the system no matter what the user passed on the command
line.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-acked-by: Juergen Gross <jgross@suse.com>

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 30a04df4db19d2df169faa87b95df2eabf85e07f..b8a09ce5c497b733146929cdd67d74795126c543 100644 (file)
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1850,7 +1850,7 @@ accidentally leaking secrets by releasing pages without proper sanitization.
 Set the serial transmit buffer size.
 
 ### serrors (ARM)
-> `= diverse | forward | panic`
+> `= diverse | panic`
 
 > Default: `diverse`
 
@@ -1866,7 +1866,7 @@ on the host will not trigger such SErrors. In this case, the administrator can
 use this parameter to skip categorizing SErrors and reduce the overhead of
 dsb/isb.
 
-We provided the following 3 options to administrators to determine how the
+We provided the following 2 options to administrators to determine how the
 hypervisors handle SErrors:
 
 * `diverse`:
@@ -1878,15 +1878,6 @@ hypervisors handle SErrors:
   2. dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
      SErrors to guests.
 
-* `forward`:
-  The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
-  All SErrors will be forwarded to guests, except the SErrors generated when
-  the idle vCPU is running. The idle domain doesn't have the ability to handle
-  SErrors, so we have to crash the whole system when we get SErros with the
-  idle vCPU. This option will avoid most overhead of the dsb/isb, except the
-  dsb/isb in context switch which is used to isolate the SErrors between 2
-  vCPUs.
-
 * `panic`:
   The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
   All SErrors will crash the whole system. This option will avoid all overhead

diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
index 76f714a168199044e7e5039ea4f118567fd822cd..9c9790a6d1e2ede7001ab6b861f94b3333fe0367 100644 (file)
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -69,12 +69,12 @@ void do_trap_prefetch_abort(struct cpu_user_regs *regs)
 void do_trap_data_abort(struct cpu_user_regs *regs)
 {
     /*
-     * We cannot distinguish Xen SErrors from synchronous data aborts. We
-     * want to avoid treating any Xen synchronous aborts as SErrors and
-     * forwarding them to the guest. Instead, crash the system in all
-     * cases when the abort comes from Xen. Even if they are Xen SErrors
-     * it would be a reasonable thing to do, and the default behavior with
-     * serror_op == DIVERSE.
+     * We cannot distinguish between Asynchronous External Abort and
+     * Synchronous Data Abort.
+     *
+     * As asynchronous abort (aka SError) generated by the hypervisor will
+     * result in a crash of the system (see __do_trap_serror()), it is fine to
+     * do it here.
      */
     if ( VABORT_GEN_BY_GUEST(regs) )
         do_trap_guest_serror(regs);

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 5380fbb081dfa7282cfbecd5d49baaefc1d75792..9e8e9d921d2a0e41889b45ab71ba460d8c32d0dd 100644 (file)
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -353,17 +353,6 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
 
     local_irq_disable();
 
-    /*
-     * If the serrors_op is "FORWARD", we have to prevent forwarding
-     * SError to wrong vCPU. So before context switch, we have to use
-     * the SYNCRONIZE_SERROR to guarantee that the pending SError would
-     * be caught by current vCPU.
-     *
-     * The SKIP_CTXT_SWITCH_SERROR_SYNC will be set to cpu_hwcaps when the
-     * serrors_op is NOT "FORWARD".
-     */
-    SYNCHRONIZE_SERROR(SKIP_CTXT_SWITCH_SERROR_SYNC);
-
     set_current(next);
 
     prev = __context_switch(prev, next);

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a3deb59372a47bc22e8647b47b69271f8db27163..6ed9e66710d59843ae97776d467064f0b6e210b0 100644 (file)
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -103,15 +103,12 @@ register_t get_default_hcr_flags(void)
 
 static enum {
     SERRORS_DIVERSE,
-    SERRORS_FORWARD,
     SERRORS_PANIC,
 } serrors_op;
 
 static int __init parse_serrors_behavior(const char *str)
 {
-    if ( !strcmp(str, "forward") )
-        serrors_op = SERRORS_FORWARD;
-    else if ( !strcmp(str, "panic") )
+    if ( !strcmp(str, "panic") )
         serrors_op = SERRORS_PANIC;
     else
         serrors_op = SERRORS_DIVERSE;
@@ -125,9 +122,6 @@ static int __init update_serrors_cpu_caps(void)
     if ( serrors_op != SERRORS_DIVERSE )
         cpus_set_cap(SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT);
 
-    if ( serrors_op != SERRORS_FORWARD )
-        cpus_set_cap(SKIP_CTXT_SWITCH_SERROR_SYNC);
-
     return 0;
 }
 __initcall(update_serrors_cpu_caps);
@@ -675,6 +669,9 @@ static void inject_vabt_exception(struct cpu_user_regs *regs)
  * 3) Hypervisor generated native SError, that would be a bug.
  *
  * A true parameter "guest" means that the SError is type#1 or type#2.
+ *
+ * Note that Arm32 asynchronous external abort generated by the
+ * hypervisor will be handled in do_trap_data_abort().
  */
 static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
 {
@@ -692,28 +689,11 @@ static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
         goto crash_system;
     }
 
-    /*
-     * The "FORWARD" option will forward all SErrors to the guests, except
-     * idle domain generated SErrors.
-     */
-    if ( serrors_op == SERRORS_FORWARD )
-    {
-        /*
-         * Because the idle domain doesn't have the ability to handle the
-         * SErrors, we have to crash the whole system while we get a SError
-         * generated by idle domain.
-         */
-        if ( is_idle_vcpu(current) )
-            goto crash_system;
-
-        return inject_vabt_exception(regs);
-    }
-
 crash_system:
-    /* Three possibilities to crash the whole system:
+    /*
+     * Two possibilities to crash the whole system:
      * 1) "DIVERSE" option with Hypervisor generated SErrors.
-     * 2) "FORWARD" option with Idle Domain generated SErrors.
-     * 3) "PANIC" option with all SErrors.
+     * 2) "PANIC" option with all SErrors.
      */
     do_unexpected_trap("SError", regs);
 }

diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index d06f09ecfa3497a9e0d01b71c975b07d463817eb..9af566662848996c3159ac2d1594535edea983bd 100644 (file)
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -41,13 +41,12 @@
 #define ARM64_WORKAROUND_834220 3
 #define LIVEPATCH_FEATURE   4
 #define SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT 5
-#define SKIP_CTXT_SWITCH_SERROR_SYNC 6
-#define ARM_HARDEN_BRANCH_PREDICTOR 7
-#define ARM_SSBD 8
-#define ARM_SMCCC_1_1 9
-#define ARM64_WORKAROUND_AT_SPECULATE 10
+#define ARM_HARDEN_BRANCH_PREDICTOR 6
+#define ARM_SSBD 7
+#define ARM_SMCCC_1_1 8
+#define ARM64_WORKAROUND_AT_SPECULATE 9
 
-#define ARM_NCAPS           11
+#define ARM_NCAPS           10
 
 #ifndef __ASSEMBLY__