|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH for-4.13 v4 02/19] 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@xxxxxxx>
Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
---
Changes in v4:
- Fix grammar
- Add Stefano's reviewed-by
Changes in v3:
- Fix typo in the commit message
- Rework comments in arm32/traps.c
Changes in v2:
- Patch added
---
docs/misc/xen-command-line.pandoc | 13 ++-----------
xen/arch/arm/arm32/traps.c | 12 ++++++------
xen/arch/arm/domain.c | 11 -----------
xen/arch/arm/traps.c | 34 +++++++---------------------------
xen/include/asm-arm/cpufeature.h | 11 +++++------
5 files changed, 20 insertions(+), 61 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index 30a04df4db..b8a09ce5c4 100644
--- 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 76f714a168..9c9790a6d1 100644
--- 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 460e968e97..53dc1d11c6 100644
--- 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 a3deb59372..6ed9e66710 100644
--- 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 d06f09ecfa..9af5666628 100644
--- 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__
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |