[PATCH v4 00/19] Provide a command line option to choose how to handle SErrors

[PATCH v4 00/19] Provide a command line option to choose how to handle SErrors

[Wei Chen]

From XSA-201, we know that, a guest could trigger SErrors when accessing
memory mapped HW in a non-conventional way. In the patches for XSA-201,
we crash the guest when we captured such asynchronous aborts to avoid data
corruption.

In order to distinguish guest-generated SErrors from hypervisor-generated
SErrors. We have to place SError checking code in every EL1 -> EL2 paths.
That will be an overhead on entries caused by dsb/isb.

But not all platforms want to categorize the SErrors. For example, a host
that is running with trusted guests. The administrator can confirm that
all guests that are running on the host will not trigger such SErrors. In
this user scene, we should provide some options to administrator to avoid
categorizing the SErrors. And then reduce the overhead of dsb/isb.

We provided following 3 options to administrator to determine how to handle
the SErrors:

* `diverse`:
  The hypervisor will distinguish guest SErrors from hypervisor SErrors.
  The guest generated SErrors will be forwarded to guests, the hypervisor
  generated SErrors will cause the whole system crash.
  It requires:
  1. Place dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
     correctly.
  2. Place dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
     SErrors to guests.
  3. Place dsb/isb in context switch to isolate the SErrors between 2 vCPUs.

* `forward`:
  The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
  All SErrors will be forwarded to guests, except the SErrors generated when
  idle vCPU is running. The idle domain doesn't have the ability to hanle the
  SErrors, so we have to crash the whole system when we get SErros with 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
  of the dsb/isb

---
v3->v4:
1. Rename SKIP_CHECK_PENDING_VSERROR to SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT.
2. Add ASSERT in SYNCHRONIZING_SERROR macro to ensure abort is enabled.
3. Use one local_abort_is_enabled for ARM32 and ARM64.
4. Fix some grammer issues.
5. Add Reviewed-by tags from Julien and Stefano for most of this series.

Wei Chen (19):
  xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome
    check
  xen/arm: Introduce a helper to get default HCR_EL2 flags
  xen/arm: Set and restore HCR_EL2 register for each vCPU separately
  xen/arm: Avoid setting/clearing HCR_RW at every context switch
  xen/arm: Save HCR_EL2 when a guest took the SError
  xen/arm: Introduce a virtual abort injection helper
  xen/arm: Introduce a command line parameter for SErrors/Aborts
  xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op
  xen/arm64: Use alternative to skip the check of pending serrors
  xen/arm32: Use alternative to skip the check of pending serrors
  xen/arm: Move macro VABORT_GEN_BY_GUEST to common header
  xen/arm: Introduce new helpers to handle guest/hyp SErrors
  xen/arm: Replace do_trap_guest_serror with new helpers
  xen/arm: Unmask the Abort/SError bit in the exception entries
  xen/arm: Introduce a helper to check local abort is enabled
  xen/arm: Introduce a macro to synchronize SError
  xen/arm: Isolate the SError between the context switch of 2 vCPUs
  xen/arm: Prevent slipping hypervisor SError to guest
  xen/arm: Handle guest external abort as guest SError

 docs/misc/xen-command-line.markdown   |  44 ++++++++
 xen/arch/arm/arm32/asm-offsets.c      |   1 +
 xen/arch/arm/arm32/entry.S            |  28 ++++-
 xen/arch/arm/arm32/traps.c            |   5 +-
 xen/arch/arm/arm64/asm-offsets.c      |   1 +
 xen/arch/arm/arm64/domctl.c           |   6 ++
 xen/arch/arm/arm64/entry.S            | 105 +++++++++----------
 xen/arch/arm/arm64/traps.c            |   2 +-
 xen/arch/arm/domain.c                 |  19 ++++
 xen/arch/arm/domain_build.c           |   7 ++
 xen/arch/arm/p2m.c                    |  10 +-
 xen/arch/arm/traps.c                  | 187 +++++++++++++++++++++++++++++-----
 xen/include/asm-arm/arm32/processor.h |  12 +--
 xen/include/asm-arm/arm64/processor.h |   3 +-
 xen/include/asm-arm/cpufeature.h      |   4 +-
 xen/include/asm-arm/domain.h          |   4 +
 xen/include/asm-arm/processor.h       |  30 +++++-
 xen/include/asm-arm/system.h          |   7 ++
 18 files changed, 364 insertions(+), 111 deletions(-)

-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 01/19] xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome check

[Wei Chen]

Xen will do exception syndrome check while some types of exception
take place in EL2. The syndrome check code read the ESR_EL2 register
directly, but in some situation this register maybe overridden by
nested exception.

For example, if we re-enable IRQ before reading ESR_EL2 which means
Xen may enter in IRQ exception mode and return the processor with
clobbered ESR_EL2 (See ARM ARM DDI 0487A.j D7.2.25)

In this case the guest exception syndrome has been overridden, we will
check the syndrome for guest sync exception with an incorrect ESR_EL2
value. So we want to save ESR_EL2 to cpu_user_regs as soon as the
exception takes place in EL2 to avoid using an incorrect syndrome value.

In order to save ESR_EL2, we added a 32-bit member hsr to cpu_user_regs.
But while saving registers in trap entry, we use stp to save ELR and
CPSR at the same time through 64-bit general registers. If we keep this
code, the hsr will be overridden by upper 32-bit of CPSR. So adjust the
code to use str to save ELR in a separate instruction and use stp to
save CPSR and HSR at the same time through 32-bit general registers.
This change affects the registers restore in trap exit, we can't use the
ldp to restore ELR and CPSR from stack at the same time. We have to use
ldr to restore them separately.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/arm32/asm-offsets.c      |  1 +
 xen/arch/arm/arm32/entry.S            |  3 +++
 xen/arch/arm/arm64/asm-offsets.c      |  1 +
 xen/arch/arm/arm64/entry.S            | 13 +++++++++----
 xen/arch/arm/arm64/traps.c            |  2 +-
 xen/arch/arm/traps.c                  |  4 ++--
 xen/include/asm-arm/arm32/processor.h |  2 +-
 xen/include/asm-arm/arm64/processor.h |  3 +--
 8 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c
index f8e6b53..5b543ab 100644
--- a/xen/arch/arm/arm32/asm-offsets.c
+++ b/xen/arch/arm/arm32/asm-offsets.c
@@ -26,6 +26,7 @@ void __dummy__(void)
    OFFSET(UREGS_lr, struct cpu_user_regs, lr);
    OFFSET(UREGS_pc, struct cpu_user_regs, pc);
    OFFSET(UREGS_cpsr, struct cpu_user_regs, cpsr);
+   OFFSET(UREGS_hsr, struct cpu_user_regs, hsr);
 
    OFFSET(UREGS_LR_usr, struct cpu_user_regs, lr_usr);
    OFFSET(UREGS_SP_usr, struct cpu_user_regs, sp_usr);
diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 2a6f4f0..2187226 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -23,6 +23,9 @@
         add r11, sp, #UREGS_kernel_sizeof+4;                            \
         str r11, [sp, #UREGS_sp];                                       \
                                                                         \
+        mrc CP32(r11, HSR);             /* Save exception syndrome */   \
+        str r11, [sp, #UREGS_hsr];                                      \
+                                                                        \
         mrs r11, SPSR_hyp;                                              \
         str r11, [sp, #UREGS_cpsr];                                     \
         and r11, #PSR_MODE_MASK;                                        \
diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
index 69ea92a..ce24e44 100644
--- a/xen/arch/arm/arm64/asm-offsets.c
+++ b/xen/arch/arm/arm64/asm-offsets.c
@@ -27,6 +27,7 @@ void __dummy__(void)
    OFFSET(UREGS_SP, struct cpu_user_regs, sp);
    OFFSET(UREGS_PC, struct cpu_user_regs, pc);
    OFFSET(UREGS_CPSR, struct cpu_user_regs, cpsr);
+   OFFSET(UREGS_ESR_el2, struct cpu_user_regs, hsr);
 
    OFFSET(UREGS_SPSR_el1, struct cpu_user_regs, spsr_el1);
 
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index c181b5e..02802c0 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -121,9 +121,13 @@ lr      .req    x30             // link register
 
         stp     lr, x21, [sp, #UREGS_LR]
 
-        mrs     x22, elr_el2
-        mrs     x23, spsr_el2
-        stp     x22, x23, [sp, #UREGS_PC]
+        mrs     x21, elr_el2
+        str     x21, [sp, #UREGS_PC]
+
+        add     x21, sp, #UREGS_CPSR
+        mrs     x22, spsr_el2
+        mrs     x23, esr_el2
+        stp     w22, w23, [x21]
 
         .endm
 
@@ -307,7 +311,8 @@ ENTRY(return_to_new_vcpu64)
 return_from_trap:
         msr     daifset, #2 /* Mask interrupts */
 
-        ldp     x21, x22, [sp, #UREGS_PC]       // load ELR, SPSR
+        ldr     x21, [sp, #UREGS_PC]            // load ELR
+        ldr     w22, [sp, #UREGS_CPSR]          // load SPSR
 
         pop     x0, x1
         pop     x2, x3
diff --git a/xen/arch/arm/arm64/traps.c b/xen/arch/arm/arm64/traps.c
index 8e89376..36b3a30 100644
--- a/xen/arch/arm/arm64/traps.c
+++ b/xen/arch/arm/arm64/traps.c
@@ -32,7 +32,7 @@ static const char *handler[]= {
 
 asmlinkage void do_bad_mode(struct cpu_user_regs *regs, int reason)
 {
-    union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };
+    union hsr hsr = { .bits = regs->hsr };
 
     printk("Bad mode in %s handler detected\n", handler[reason]);
     printk("ESR=0x%08"PRIx32":  EC=%"PRIx32", IL=%"PRIx32", ISS=%"PRIx32"\n",
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index b7d5fb6..f7fca37 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -864,7 +864,7 @@ static void _show_registers(struct cpu_user_regs *regs,
     printk("   HCR_EL2: %016"PRIregister"\n", READ_SYSREG(HCR_EL2));
     printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
     printk("\n");
-    printk("   ESR_EL2: %08"PRIx32"\n", READ_SYSREG32(ESR_EL2));
+    printk("   ESR_EL2: %08"PRIx32"\n", regs->hsr);
     printk(" HPFAR_EL2: %016"PRIregister"\n", READ_SYSREG(HPFAR_EL2));
 
 #ifdef CONFIG_ARM_32
@@ -2694,7 +2694,7 @@ static void enter_hypervisor_head(struct cpu_user_regs *regs)
 
 asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
 {
-    const union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };
+    const union hsr hsr = { .bits = regs->hsr };
 
     enter_hypervisor_head(regs);
 
diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
index db3b17b..f6d5df3 100644
--- a/xen/include/asm-arm/arm32/processor.h
+++ b/xen/include/asm-arm/arm32/processor.h
@@ -37,7 +37,7 @@ struct cpu_user_regs
         uint32_t pc, pc32;
     };
     uint32_t cpsr; /* Return mode */
-    uint32_t pad0; /* Doubleword-align the kernel half of the frame */
+    uint32_t hsr;  /* Exception Syndrome */
 
     /* Outer guest frame only from here on... */
 
diff --git a/xen/include/asm-arm/arm64/processor.h b/xen/include/asm-arm/arm64/processor.h
index b0726ff..24f836b 100644
--- a/xen/include/asm-arm/arm64/processor.h
+++ b/xen/include/asm-arm/arm64/processor.h
@@ -66,8 +66,7 @@ struct cpu_user_regs
     /* Return address and mode */
     __DECL_REG(pc,           pc32);             /* ELR_EL2 */
     uint32_t cpsr;                              /* SPSR_EL2 */
-
-    uint32_t pad0; /* Align end of kernel frame. */
+    uint32_t hsr;                               /* ESR_EL2 */
 
     /* Outer guest frame only from here on... */
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 02/19] xen/arm: Introduce a helper to get default HCR_EL2 flags

[Wei Chen]

We want to add HCR_EL2 register to Xen context switch. And each copy
of HCR_EL2 in vcpu structure will be initialized with the same set
of trap flags as the HCR_EL2 register. We introduce a helper here to
represent these flags to be reused easily.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/traps.c            | 11 ++++++++---
 xen/include/asm-arm/processor.h |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f7fca37..ebe25c6 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -136,6 +136,13 @@ static int __init vwfi_init(void)
 }
 presmp_initcall(vwfi_init);
 
+register_t get_default_hcr_flags(void)
+{
+    return  (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
+             (vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) |
+             HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB);
+}
+
 void init_traps(void)
 {
     /* Setup Hyp vector base */
@@ -160,9 +167,7 @@ void init_traps(void)
                  CPTR_EL2);
 
     /* Setup hypervisor traps */
-    WRITE_SYSREG(HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
-                 (vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) |
-                 HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB,HCR_EL2);
+    WRITE_SYSREG(get_default_hcr_flags(), HCR_EL2);
     isb();
 }
 
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index afc0e9a..4b6338b 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -708,6 +708,8 @@ int call_smc(register_t function_id, register_t arg0, register_t arg1,
 
 void do_trap_guest_error(struct cpu_user_regs *regs);
 
+register_t get_default_hcr_flags(void);
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_ARM_PROCESSOR_H */
 /*
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 03/19] xen/arm: Set and restore HCR_EL2 register for each vCPU separately

[Wei Chen]

Different domains may have different HCR_EL2 flags. For example, the
64-bit domain needs HCR_RW flag but the 32-bit does not need it. So
we give each domain a default HCR_EL2 value and save it in the vCPU's
context.

HCR_EL2 register has only one bit can be updated automatically without
explicit write (HCR_VSE). But we haven't used this bit currently, so
we can consider that the HCR_EL2 register will not be modified while
the guest is running. So save the HCR_EL2 while guest exiting to
hypervisor is not neccessary. We just have to restore this register for
each vCPU while context switching.

The p2m_restore_state which will be invoked in context switch progress
has included the writing of HCR_EL2 already. It updates the HCR_EL2.RW
bit to tell the hardware how to interpret the stage-1 page table as the
encodings are different between AArch64 and AArch32. We can reuse this
write to restore the HCR_EL2 for each vCPU. Of course, the value of each
vCPU's HCR_EL2 should be adjusted to have proper HCR_EL2.RW bit in this
function. In the later patch of this series, we will set the HCR_EL2.RW
for each vCPU while the domain is creating.

Signed-off-by: wei chen <Wei.Chen@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/domain.c        | 2 ++
 xen/arch/arm/p2m.c           | 9 +++------
 xen/include/asm-arm/domain.h | 3 +++
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index bb327da..5d18bb0 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -513,6 +513,8 @@ int vcpu_initialise(struct vcpu *v)
 
     v->arch.actlr = READ_SYSREG32(ACTLR_EL1);
 
+    v->arch.hcr_el2 = get_default_hcr_flags();
+
     processor_vcpu_initialise(v);
 
     if ( (rc = vcpu_vgic_init(v)) != 0 )
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6263760..83c4b7d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -128,27 +128,24 @@ void p2m_save_state(struct vcpu *p)
 
 void p2m_restore_state(struct vcpu *n)
 {
-    register_t hcr;
     struct p2m_domain *p2m = &n->domain->arch.p2m;
     uint8_t *last_vcpu_ran;
 
     if ( is_idle_vcpu(n) )
         return;
 
-    hcr = READ_SYSREG(HCR_EL2);
-
     WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
     isb();
 
     if ( is_32bit_domain(n->domain) )
-        hcr &= ~HCR_RW;
+        n->arch.hcr_el2 &= ~HCR_RW;
     else
-        hcr |= HCR_RW;
+        n->arch.hcr_el2 |= HCR_RW;
 
     WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
     isb();
 
-    WRITE_SYSREG(hcr, HCR_EL2);
+    WRITE_SYSREG(n->arch.hcr_el2, HCR_EL2);
     isb();
 
     last_vcpu_ran = &p2m->last_vcpu_ran[smp_processor_id()];
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 09fe502..7b1dacc 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -204,6 +204,9 @@ struct arch_vcpu
     register_t tpidr_el1;
     register_t tpidrro_el0;
 
+    /* HYP configuration */
+    register_t hcr_el2;
+
     uint32_t teecr, teehbr; /* ThumbEE, 32-bit guests only */
 #ifdef CONFIG_ARM_32
     /*
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 04/19] xen/arm: Avoid setting/clearing HCR_RW at every context switch

[Wei Chen]

The HCR_EL2 flags for 64-bit and 32-bit domains are different. But
when we initialized the HCR_EL2 for vcpu0 of Dom0 and all vcpus of
DomU in vcpu_initialise, we didn't know the domain's address size
information. We had to use compatible flags to initialize HCR_EL2,
and set HCR_RW for 64-bit domain or clear HCR_RW for 32-bit domain
at every context switch.

But, after we added the HCR_EL2 to vcpu's context, this behaviour
seems a little fussy. We can update the HCR_RW bit in vcpu's context
as soon as we get the domain's address size to avoid setting/clearing
HCR_RW at every context switch.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/arm64/domctl.c  | 6 ++++++
 xen/arch/arm/domain.c        | 5 +++++
 xen/arch/arm/domain_build.c  | 7 +++++++
 xen/arch/arm/p2m.c           | 5 -----
 xen/include/asm-arm/domain.h | 1 +
 5 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
index 44e1e7b..ab8781f 100644
--- a/xen/arch/arm/arm64/domctl.c
+++ b/xen/arch/arm/arm64/domctl.c
@@ -14,6 +14,8 @@
 
 static long switch_mode(struct domain *d, enum domain_type type)
 {
+    struct vcpu *v;
+
     if ( d == NULL )
         return -EINVAL;
     if ( d->tot_pages != 0 )
@@ -23,6 +25,10 @@ static long switch_mode(struct domain *d, enum domain_type type)
 
     d->arch.type = type;
 
+    if ( is_64bit_domain(d) )
+        for_each_vcpu(d, v)
+            vcpu_switch_to_aarch64_mode(v);
+
     return 0;
 }
 
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 5d18bb0..69c2854 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -537,6 +537,11 @@ void vcpu_destroy(struct vcpu *v)
     free_xenheap_pages(v->arch.stack, STACK_ORDER);
 }
 
+void vcpu_switch_to_aarch64_mode(struct vcpu *v)
+{
+    v->arch.hcr_el2 |= HCR_RW;
+}
+
 int arch_domain_create(struct domain *d, unsigned int domcr_flags,
                        struct xen_arch_domainconfig *config)
 {
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index de59e5f..3abacc0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2148,6 +2148,10 @@ int construct_dom0(struct domain *d)
         return -EINVAL;
     }
     d->arch.type = kinfo.type;
+
+    if ( is_64bit_domain(d) )
+        vcpu_switch_to_aarch64_mode(v);
+
 #endif
 
     allocate_memory(d, &kinfo);
@@ -2240,6 +2244,9 @@ int construct_dom0(struct domain *d)
             printk("Failed to allocate dom0 vcpu %d on pcpu %d\n", i, cpu);
             break;
         }
+
+        if ( is_64bit_domain(d) )
+            vcpu_switch_to_aarch64_mode(d->vcpu[i]);
     }
 
     return 0;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 83c4b7d..34d5776 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -137,11 +137,6 @@ void p2m_restore_state(struct vcpu *n)
     WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
     isb();
 
-    if ( is_32bit_domain(n->domain) )
-        n->arch.hcr_el2 &= ~HCR_RW;
-    else
-        n->arch.hcr_el2 |= HCR_RW;
-
     WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
     isb();
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 7b1dacc..68185e2 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -268,6 +268,7 @@ struct arch_vcpu
 
 void vcpu_show_execution_state(struct vcpu *);
 void vcpu_show_registers(const struct vcpu *);
+void vcpu_switch_to_aarch64_mode(struct vcpu *);
 
 unsigned int domain_max_vcpus(const struct domain *);
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 05/19] xen/arm: Save HCR_EL2 when a guest took the SError

[Wei Chen]

The HCR_EL2.VSE (HCR.VA for aarch32) bit can be used to generate a
virtual abort to guest. The HCR_EL2.VSE bit has a peculiar feature
of getting cleared when the guest has taken the abort (this is the
only bit that behaves as such in HCR_EL2 register).

This means that if we set the HCR_EL2.VSE bit to signal such an abort,
we must preserve it in the guest context until it disappears from
HCR_EL2, and at which point it must be cleared from the context. This
is achieved by reading back from HCR_EL2 until the guest takes the
fault.

If we preserved a pending VSE in guest context, we have to restore
it to HCR_EL2 when context switch to this guest. This is achieved
by writing saved HCR_EL2 value in guest context back to HCR_EL2
register before return to guest. This had been done by the patch
of "Restore HCR_EL2 register".

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/traps.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ebe25c6..35ca0ed 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2694,7 +2694,18 @@ static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
 static void enter_hypervisor_head(struct cpu_user_regs *regs)
 {
     if ( guest_mode(regs) )
+    {
+        /*
+         * If we pended a virtual abort, preserve it until it gets cleared.
+         * See ARM ARM DDI 0487A.j D1.14.3 (Virtual Interrupts) for details,
+         * but the crucial bit is "On taking a vSError interrupt, HCR_EL2.VSE
+         * (alias of HCR.VA) is cleared to 0."
+         */
+        if ( current->arch.hcr_el2 & HCR_VA )
+            current->arch.hcr_el2 = READ_SYSREG(HCR_EL2);
+
         gic_clear_lrs(current);
+    }
 }
 
 asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 06/19] xen/arm: Introduce a virtual abort injection helper

[Wei Chen]

When guest triggers async aborts, in most platform, such aborts
will be routed to hypervisor. But we don't want the hypervisor
to handle such aborts, so we have to route such aborts back to
the guest.

This helper is using the HCR_EL2.VSE (HCR.VA for aarch32) bit to
route such aborts back to the guest. After updating HCR_EL2.VSE bit
in vCPU context, we write the value to HCR_EL2 immediately. In this
case we don't need to move the restoration of HCR_EL2 to other place,
and it worked regardless of whether we get preempted.

If the guest PC had been advanced by SVC/HVC/SMC instructions before
we caught the SError in hypervisor, we have to adjust the guest PC to
exact address while the SError generated.

About HSR_EC_SVC32/64, even thought we don't trap SVC32/64 today,
we would like them to be handled here. This would be useful when
VM introspection will gain support of SVC32/64 trapping.

After updating HCR_EL2.VSE bit of vCPU HCR_EL2, write the value
to HCR_EL2 immediately. In this case we don't need to move the
restoration of HCR_EL2 to leave_hypervisor_tail, and it worked
regardless of whether we get preempted.

This helper will be used by the later patches in this series, we
use #if 0 to disable it in this patch temporarily to remove the
warning message of unused function from compiler.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/traps.c            | 33 +++++++++++++++++++++++++++++++++
 xen/include/asm-arm/processor.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 35ca0ed..a24d986 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -639,6 +639,39 @@ static void inject_dabt_exception(struct cpu_user_regs *regs,
 #endif
 }
 
+#if 0
+/* Inject a virtual Abort/SError into the guest. */
+static void inject_vabt_exception(struct cpu_user_regs *regs)
+{
+    const union hsr hsr = { .bits = regs->hsr };
+
+    /*
+     * SVC/HVC/SMC already have an adjusted PC (See ARM ARM DDI 0487A.j
+     * D1.10.1 for more details), which we need to correct in order to
+     * return to after having injected the SError.
+     */
+    switch ( hsr.ec )
+    {
+    case HSR_EC_SVC32:
+    case HSR_EC_HVC32:
+    case HSR_EC_SMC32:
+#ifdef CONFIG_ARM_64
+    case HSR_EC_SVC64:
+    case HSR_EC_HVC64:
+    case HSR_EC_SMC64:
+#endif
+        regs->pc -= hsr.len ? 4 : 2;
+        break;
+
+    default:
+        break;
+    }
+
+    current->arch.hcr_el2 |= HCR_VA;
+    WRITE_SYSREG(current->arch.hcr_el2, HCR_EL2);
+}
+#endif
+
 struct reg_ctxt {
     /* Guest-side state */
     uint32_t sctlr_el1;
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 4b6338b..d7b0711 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -252,6 +252,7 @@
 #define HSR_EC_HVC32                0x12
 #define HSR_EC_SMC32                0x13
 #ifdef CONFIG_ARM_64
+#define HSR_EC_SVC64                0x15
 #define HSR_EC_HVC64                0x16
 #define HSR_EC_SMC64                0x17
 #define HSR_EC_SYSREG               0x18
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 07/19] xen/arm: Introduce a command line parameter for SErrors/Aborts

[Wei Chen]

In order to distinguish guest-generated SErrors from hypervisor-generated
SErrors we have to place SError checking code in every EL1 <-> EL2 paths.
That will cause overhead on entries and exits due to dsb/isb.

However, not all platforms want to categorize SErrors. For example, a host
that is running with trusted guests. The administrator can confirm that
all guests that are running on the host will not trigger such SErrors. In
this use-case, we should provide some options to administrators to avoid
categorizing SErrors and then reduce the overhead of dsb/isb.

We provided the following 3 options to administrators to determine how
the hypervisors handle SErrors:

* `diverse`:
  The hypervisor will distinguish guest SErrors from hypervisor SErrors.
  The guest generated SErrors will be forwarded to guests, the hypervisor
  generated SErrors will cause the whole system to crash.
  It requires:
  1. dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
     correctly.
  2. dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
     SErrors to guests.
  3. dsb/isb in context switch to isolate SErrors between 2 vCPUs.

* `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 of the dsb/isb pairs.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 docs/misc/xen-command-line.markdown | 44 +++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c                | 19 ++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 9eb85d6..9d42b6a 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1470,6 +1470,50 @@ enabling more sockets and cores to go into deeper sleep states.
 
 Set the serial transmit buffer size.
 
+### serrors (ARM)
+> `= diverse | forward | panic`
+
+> Default: `diverse`
+
+This parameter is provided to administrators to determine how the
+hypervisors handle SErrors.
+
+In order to distinguish guest-generated SErrors from hypervisor-generated
+SErrors we have to place SError checking code in every EL1 <-> EL2 paths.
+That will cause overhead on entries and exits due to dsb/isb. However, not all
+platforms need to categorize SErrors. For example, a host that is running with
+trusted guests. The administrator can confirm that all guests that are running
+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
+hypervisors handle SErrors:
+
+* `diverse`:
+  The hypervisor will distinguish guest SErrors from hypervisor SErrors.
+  The guest generated SErrors will be forwarded to guests, the hypervisor
+  generated SErrors will cause the whole system to crash.
+  It requires:
+  1. dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors correctly.
+  2. dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
+     SErrors to guests.
+  3. dsb/isb in context switch to isolate SErrors between 2 vCPUs.
+
+* `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
+  of the dsb/isb pairs.
+
 ### smap
 > `= <boolean> | hvm`
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a24d986..41955af 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -143,6 +143,25 @@ register_t get_default_hcr_flags(void)
              HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB);
 }
 
+static enum {
+    SERRORS_DIVERSE,
+    SERRORS_FORWARD,
+    SERRORS_PANIC,
+} serrors_op;
+
+static void __init parse_serrors_behavior(const char *str)
+{
+    if ( !strcmp(str, "forward") )
+        serrors_op = SERRORS_FORWARD;
+    else if ( !strcmp(str, "panic") )
+        serrors_op = SERRORS_PANIC;
+    else
+        serrors_op = SERRORS_DIVERSE;
+
+    return;
+}
+custom_param("serrors", parse_serrors_behavior);
+
 void init_traps(void)
 {
     /* Setup Hyp vector base */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 08/19] xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op

[Wei Chen]

In the later patches of this series, we want to use the alternative
patching framework to avoid synchronizing serror_op in every entries
and exits. So we define a new cpu feature "SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT"
for serror_op. When serror_op is not equal to SERROR_DIVERSE, this
feature will be set to cpu_hwcaps.

Currently, the default serror_op is SERROR_DIVERSE, if we want to
change the serror_op value we have to place the serror parameter
in command line. It seems no problem to update cpu_hwcaps directly
in the serror parameter parsing function.

While the default option will be diverse today, this may change in the
future. So we introduce this initcall to guarantee the cpu_hwcaps can be
updated no matter the serror parameter is placed in the command line
or not.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

---
v3->v4:
Rename SKIP_CHECK_PENDING_VSERROR to SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
in code an commit message.
---
 xen/arch/arm/traps.c             | 9 +++++++++
 xen/include/asm-arm/cpufeature.h | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 41955af..1e130fb 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -162,6 +162,15 @@ static void __init parse_serrors_behavior(const char *str)
 }
 custom_param("serrors", parse_serrors_behavior);
 
+static int __init update_serrors_cpu_caps(void)
+{
+    if ( serrors_op != SERRORS_DIVERSE )
+        cpus_set_cap(SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT);
+
+    return 0;
+}
+__initcall(update_serrors_cpu_caps);
+
 void init_traps(void)
 {
     /* Setup Hyp vector base */
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index c0a25ae..9eb72e1 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -40,8 +40,9 @@
 #define ARM32_WORKAROUND_766422 2
 #define ARM64_WORKAROUND_834220 3
 #define LIVEPATCH_FEATURE   4
+#define SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT 5
 
-#define ARM_NCAPS           5
+#define ARM_NCAPS           6
 
 #ifndef __ASSEMBLY__
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 09/19] xen/arm64: Use alternative to skip the check of pending serrors

[Wei Chen]

We have provided an option to administrator to determine how to
handle the SErrors. In order to skip the check of pending SError,
in conventional way, we have to read the option every time before
we try to check the pending SError. This will add overhead to check
the option at every trap.

The ARM64 supports the alternative patching feature. We can use an
ALTERNATIVE to avoid checking option at every trap. We added a new
cpufeature named "SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT". This feature
will be enabled when the option is not diverse.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/arm64/entry.S | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 02802c0..3d2fdfb 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -1,5 +1,6 @@
 #include <asm/asm_defns.h>
 #include <asm/regs.h>
+#include <asm/alternative.h>
 #include <public/xen.h>
 
 /*
@@ -229,12 +230,14 @@ hyp_irq:
 
 guest_sync:
         entry   hyp=0, compat=0
-        bl      check_pending_vserror
         /*
-         * If x0 is Non-zero, a vSError took place, the initial exception
-         * doesn't have any significance to be handled. Exit ASAP
+         * 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
          */
-        cbnz    x0, 1f
+        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
+                    "nop; nop",
+                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
         msr     daifclr, #2
         mov     x0, sp
         bl      do_trap_hypervisor
@@ -243,12 +246,14 @@ guest_sync:
 
 guest_irq:
         entry   hyp=0, compat=0
-        bl      check_pending_vserror
         /*
-         * If x0 is Non-zero, a vSError took place, the initial exception
-         * doesn't have any significance to be handled. Exit ASAP
+         * 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
          */
-        cbnz    x0, 1f
+        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
+                    "nop; nop",
+                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
         mov     x0, sp
         bl      do_trap_irq
 1:
@@ -267,12 +272,14 @@ guest_error:
 
 guest_sync_compat:
         entry   hyp=0, compat=1
-        bl      check_pending_vserror
         /*
-         * If x0 is Non-zero, a vSError took place, the initial exception
-         * doesn't have any significance to be handled. Exit ASAP
+         * 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
          */
-        cbnz    x0, 1f
+        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
+                    "nop; nop",
+                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
         msr     daifclr, #2
         mov     x0, sp
         bl      do_trap_hypervisor
@@ -281,12 +288,14 @@ guest_sync_compat:
 
 guest_irq_compat:
         entry   hyp=0, compat=1
-        bl      check_pending_vserror
         /*
-         * If x0 is Non-zero, a vSError took place, the initial exception
-         * doesn't have any significance to be handled. Exit ASAP
+         * 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
          */
-        cbnz    x0, 1f
+        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
+                    "nop; nop",
+                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
         mov     x0, sp
         bl      do_trap_irq
 1:
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 10/19] xen/arm32: Use alternative to skip the check of pending serrors

[Wei Chen]

We have provided an option to administrator to determine how to
handle the SErrors. In order to skip the check of pending SError,
in conventional way, we have to read the option every time before
we try to check the pending SError. This will add overhead to check
the option at every trap.

The ARM32 supports the alternative patching feature. We can use an
ALTERNATIVE to avoid checking option at every trap. We added a new
cpufeature named "SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT". This feature
will be enabled when the option is not diverse.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm32/entry.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 2187226..8cd2123 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -1,5 +1,6 @@
 #include <asm/asm_defns.h>
 #include <asm/regs.h>
+#include <asm/alternative.h>
 #include <public/xen.h>
 
 #define SAVE_ONE_BANKED(reg)    mrs r11, reg; str r11, [sp, #UREGS_##reg]
@@ -44,6 +45,14 @@ save_guest_regs:
         SAVE_BANKED(fiq)
         SAVE_ONE_BANKED(R8_fiq); SAVE_ONE_BANKED(R9_fiq); SAVE_ONE_BANKED(R10_fiq)
         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.
+         */
+        ALTERNATIVE("nop",
+                    "b skip_check",
+                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
         /*
          * Start to check pending virtual abort in the gap of Guest -> HYP
          * world switch.
@@ -99,6 +108,7 @@ abort_guest_exit_end:
          */
         bne return_from_trap
 
+skip_check:
         mov pc, lr
 
 #define DEFINE_TRAP_ENTRY(trap)                                         \
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 11/19] xen/arm: Move macro VABORT_GEN_BY_GUEST to common header

[Wei Chen]

We want to move part of SErrors checking code from hyp_error assembly code
to a function. This new function will use this macro to distinguish the
guest SErrors from hypervisor SErrors. So we have to move this macro to
common header.

The VABORT_GEN_BY_GUEST macro uses the symbols abort_guest_exit_start
and abort_guest_exit_end. After we move this macro to a common header,
we need to make sure that the two symbols are visible to other source
files. Currently, they are declared .global in arm32/entry.S, but not
arm64/entry.S. Fix that.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/arm64/entry.S            |  2 ++
 xen/include/asm-arm/arm32/processor.h | 10 ----------
 xen/include/asm-arm/processor.h       | 10 ++++++++++
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 3d2fdfb..d2ebf5b 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -380,10 +380,12 @@ check_pending_vserror:
          * exception handler, and the elr_el2 will be set to
          * abort_guest_exit_start or abort_guest_exit_end.
          */
+        .global abort_guest_exit_start
 abort_guest_exit_start:
 
         isb
 
+        .global abort_guest_exit_end
 abort_guest_exit_end:
         /* Mask PSTATE asynchronous abort bit, close the checking window. */
         msr     daifset, #4
diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
index f6d5df3..68cc821 100644
--- a/xen/include/asm-arm/arm32/processor.h
+++ b/xen/include/asm-arm/arm32/processor.h
@@ -56,16 +56,6 @@ struct cpu_user_regs
     uint32_t pad1; /* Doubleword-align the user half of the frame */
 };
 
-/* Functions for pending virtual abort checking window. */
-void abort_guest_exit_start(void);
-void abort_guest_exit_end(void);
-
-#define VABORT_GEN_BY_GUEST(r)  \
-( \
-    ( (unsigned long)abort_guest_exit_start == (r)->pc ) || \
-    ( (unsigned long)abort_guest_exit_end == (r)->pc ) \
-)
-
 #endif
 
 /* Layout as used in assembly, with src/dest registers mixed in */
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index d7b0711..163c39c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -711,6 +711,16 @@ void do_trap_guest_error(struct cpu_user_regs *regs);
 
 register_t get_default_hcr_flags(void);
 
+/* Functions for pending virtual abort checking window. */
+void abort_guest_exit_start(void);
+void abort_guest_exit_end(void);
+
+#define VABORT_GEN_BY_GUEST(r)  \
+( \
+    ( (unsigned long)abort_guest_exit_start == (r)->pc ) || \
+    ( (unsigned long)abort_guest_exit_end == (r)->pc ) \
+)
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_ARM_PROCESSOR_H */
 /*
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 12/19] xen/arm: Introduce new helpers to handle guest/hyp SErrors

[Wei Chen]

Currently, ARM32 and ARM64 has different SError exception handlers.
These handlers include lots of code to check SError handle options
and code to distinguish guest-generated SErrors from hypervisor
SErrors.

The new helpers: do_trap_guest_serror and do_trap_hyp_serror are
wrappers of __do_trap_serror with constant guest/hyp parameters.
__do_trap_serror moves the option checking code and SError checking
code from assembly to C source. This will make the code become more
readable and avoid placing check code in too many places.

These two helpers only handle the following 3 types of SErrors:
1) Guest-generated SError and had been delivered in EL1 and then
   been forwarded to EL2.
2) Guest-generated SError but hadn't been delivered in EL1 before
   trapping to EL2. This SError would be caught in EL2 as soon as
   we just unmasked the PSTATE.A bit.
3) Hypervisor generated native SError, that would be a bug.

In the new helpers, we have used the function "inject_vabt_exception"
which was disabled by "#if 0" before. Now, we can remove the "#if 0"
to make this function to be available.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/traps.c            | 69 +++++++++++++++++++++++++++++++++++++++--
 xen/include/asm-arm/processor.h |  4 +++
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 1e130fb..c5c5ff5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -667,7 +667,6 @@ static void inject_dabt_exception(struct cpu_user_regs *regs,
 #endif
 }
 
-#if 0
 /* Inject a virtual Abort/SError into the guest. */
 static void inject_vabt_exception(struct cpu_user_regs *regs)
 {
@@ -698,7 +697,59 @@ static void inject_vabt_exception(struct cpu_user_regs *regs)
     current->arch.hcr_el2 |= HCR_VA;
     WRITE_SYSREG(current->arch.hcr_el2, HCR_EL2);
 }
-#endif
+
+/*
+ * SError exception handler. We only handle the following 3 types of SErrors:
+ * 1) Guest-generated SError and had been delivered in EL1 and then
+ *    been forwarded to EL2.
+ * 2) Guest-generated SError but hadn't been delivered in EL1 before
+ *    trapping to EL2. This SError would be caught in EL2 as soon as
+ *    we just unmasked the PSTATE.A bit.
+ * 3) Hypervisor generated native SError, that would be a bug.
+ *
+ * A true parameter "guest" means that the SError is type#1 or type#2.
+ */
+static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
+{
+    /*
+     * Only "DIVERSE" option needs to distinguish the guest-generated SErrors
+     * from hypervisor SErrors.
+     */
+    if ( serrors_op == SERRORS_DIVERSE )
+    {
+        /* Forward the type#1 and type#2 SErrors to guests. */
+        if ( guest )
+            return inject_vabt_exception(regs);
+
+        /* Type#3 SErrors will panic the whole system */
+        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:
+     * 1) "DIVERSE" option with Hypervisor generated SErrors.
+     * 2) "FORWARD" option with Idle Domain generated SErrors.
+     * 3) "PANIC" option with all SErrors.
+     */
+    do_unexpected_trap("SError", regs);
+}
 
 struct reg_ctxt {
     /* Guest-side state */
@@ -2917,6 +2968,20 @@ asmlinkage void do_trap_guest_error(struct cpu_user_regs *regs)
     domain_crash_synchronous();
 }
 
+asmlinkage void do_trap_hyp_serror(struct cpu_user_regs *regs)
+{
+    enter_hypervisor_head(regs);
+
+    __do_trap_serror(regs, VABORT_GEN_BY_GUEST(regs));
+}
+
+asmlinkage void do_trap_guest_serror(struct cpu_user_regs *regs)
+{
+    enter_hypervisor_head(regs);
+
+    __do_trap_serror(regs, true);
+}
+
 asmlinkage void do_trap_irq(struct cpu_user_regs *regs)
 {
     enter_hypervisor_head(regs);
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 163c39c..81227aa 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -709,6 +709,10 @@ int call_smc(register_t function_id, register_t arg0, register_t arg1,
 
 void do_trap_guest_error(struct cpu_user_regs *regs);
 
+void do_trap_hyp_serror(struct cpu_user_regs *regs);
+
+void do_trap_guest_serror(struct cpu_user_regs *regs);
+
 register_t get_default_hcr_flags(void);
 
 /* Functions for pending virtual abort checking window. */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 13/19] xen/arm: Replace do_trap_guest_serror with new helpers

[Wei Chen]

We have introduced two helpers to handle the guest/hyp SErrors:
do_trap_guest_serror and do_trap_guest_hyp_serror. These handlers
can take the role of do_trap_guest_serror and reduce the assembly
code in the same time. So we use these two helpers to replace it
and drop it now.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/arm32/traps.c      |  5 +----
 xen/arch/arm/arm64/entry.S      | 36 +++---------------------------------
 xen/arch/arm/traps.c            | 15 ---------------
 xen/include/asm-arm/processor.h |  2 --
 4 files changed, 4 insertions(+), 54 deletions(-)

diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
index 4176f0e..5bc5f64 100644
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -62,10 +62,7 @@ asmlinkage void do_trap_prefetch_abort(struct cpu_user_regs *regs)
 
 asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
 {
-    if ( VABORT_GEN_BY_GUEST(regs) )
-        do_trap_guest_error(regs);
-    else
-        do_unexpected_trap("Data Abort", regs);
+    do_trap_hyp_serror(regs);
 }
 
 /*
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index d2ebf5b..7b16850 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -178,40 +178,10 @@ hyp_error_invalid:
         invalid BAD_ERROR
 
 hyp_error:
-        /*
-         * Only two possibilities:
-         * 1) Either we come from the exit path, having just unmasked
-         *    PSTATE.A: change the return code to an EL2 fault, and
-         *    carry on, as we're already in a sane state to handle it.
-         * 2) Or we come from anywhere else, and that's a bug: we panic.
-         */
         entry   hyp=1
         msr     daifclr, #2
-
-        /*
-         * The ELR_EL2 may be modified by an interrupt, so we have to use the
-         * saved value in cpu_user_regs to check whether we come from 1) or
-         * not.
-         */
-        ldr     x0, [sp, #UREGS_PC]
-        adr     x1, abort_guest_exit_start
-        cmp     x0, x1
-        adr     x1, abort_guest_exit_end
-        ccmp    x0, x1, #4, ne
         mov     x0, sp
-        mov     x1, #BAD_ERROR
-
-        /*
-         * Not equal, the exception come from 2). It's a bug, we have to
-         * panic the hypervisor.
-         */
-        b.ne    do_bad_mode
-
-        /*
-         * Otherwise, the exception come from 1). It happened because of
-         * the guest. Crash this guest.
-         */
-        bl      do_trap_guest_error
+        bl      do_trap_hyp_serror
         exit    hyp=1
 
 /* Traps taken in Current EL with SP_ELx */
@@ -267,7 +237,7 @@ guest_error:
         entry   hyp=0, compat=0
         msr     daifclr, #2
         mov     x0, sp
-        bl      do_trap_guest_error
+        bl      do_trap_guest_serror
         exit    hyp=0, compat=0
 
 guest_sync_compat:
@@ -309,7 +279,7 @@ guest_error_compat:
         entry   hyp=0, compat=1
         msr     daifclr, #2
         mov     x0, sp
-        bl      do_trap_guest_error
+        bl      do_trap_guest_serror
         exit    hyp=0, compat=1
 
 ENTRY(return_to_new_vcpu32)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c5c5ff5..21cf922 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2953,21 +2953,6 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
     }
 }
 
-asmlinkage void do_trap_guest_error(struct cpu_user_regs *regs)
-{
-    enter_hypervisor_head(regs);
-
-    /*
-     * Currently, to ensure hypervisor safety, when we received a
-     * guest-generated vSerror/vAbort, we just crash the guest to protect
-     * the hypervisor. In future we can better handle this by injecting
-     * a vSerror/vAbort to the guest.
-     */
-    gdprintk(XENLOG_WARNING, "Guest(Dom-%u) will be crashed by vSError\n",
-             current->domain->domain_id);
-    domain_crash_synchronous();
-}
-
 asmlinkage void do_trap_hyp_serror(struct cpu_user_regs *regs)
 {
     enter_hypervisor_head(regs);
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 81227aa..bb24bee 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -707,8 +707,6 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
 int call_smc(register_t function_id, register_t arg0, register_t arg1,
              register_t arg2);
 
-void do_trap_guest_error(struct cpu_user_regs *regs);
-
 void do_trap_hyp_serror(struct cpu_user_regs *regs);
 
 void do_trap_guest_serror(struct cpu_user_regs *regs);
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 14/19] xen/arm: Unmask the Abort/SError bit in the exception entries

[Wei Chen]

Currently, we masked the Abort/SError bit in Xen exception entries.
So Xen could not capture any Abort/SError while it's running.
Now, Xen has the ability to handle the Abort/SError, we should unmask
the Abort/SError bit by default to let Xen capture Abort/SError while
it's running.

But in order to avoid receiving nested asynchronous abort, we don't
unmask Abort/SError bit in hyp_error and trap_data_abort.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/arm32/entry.S | 15 ++++++++++++++-
 xen/arch/arm/arm64/entry.S | 13 ++++++++-----
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 8cd2123..2b04a99 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -116,6 +116,7 @@ skip_check:
 trap_##trap:                                                            \
         SAVE_ALL;                                                       \
         cpsie i;        /* local_irq_enable */                          \
+        cpsie a;        /* asynchronous abort enable */                 \
         adr lr, return_from_trap;                                       \
         mov r0, sp;                                                     \
         mov r11, sp;                                                    \
@@ -126,6 +127,18 @@ trap_##trap:                                                            \
         ALIGN;                                                          \
 trap_##trap:                                                            \
         SAVE_ALL;                                                       \
+        cpsie a;        /* asynchronous abort enable */                 \
+        adr lr, return_from_trap;                                       \
+        mov r0, sp;                                                     \
+        mov r11, sp;                                                    \
+        bic sp, #7; /* Align the stack pointer (noop on guest trap) */  \
+        b do_trap_##trap
+
+#define DEFINE_TRAP_ENTRY_NOABORT(trap)                                 \
+        ALIGN;                                                          \
+trap_##trap:                                                            \
+        SAVE_ALL;                                                       \
+        cpsie i;        /* local_irq_enable */                          \
         adr lr, return_from_trap;                                       \
         mov r0, sp;                                                     \
         mov r11, sp;                                                    \
@@ -146,10 +159,10 @@ GLOBAL(hyp_traps_vector)
 DEFINE_TRAP_ENTRY(undefined_instruction)
 DEFINE_TRAP_ENTRY(supervisor_call)
 DEFINE_TRAP_ENTRY(prefetch_abort)
-DEFINE_TRAP_ENTRY(data_abort)
 DEFINE_TRAP_ENTRY(hypervisor)
 DEFINE_TRAP_ENTRY_NOIRQ(irq)
 DEFINE_TRAP_ENTRY_NOIRQ(fiq)
+DEFINE_TRAP_ENTRY_NOABORT(data_abort)
 
 return_from_trap:
         mov sp, r11
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 7b16850..137e67c 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -187,13 +187,14 @@ hyp_error:
 /* Traps taken in Current EL with SP_ELx */
 hyp_sync:
         entry   hyp=1
-        msr     daifclr, #2
+        msr     daifclr, #6
         mov     x0, sp
         bl      do_trap_hypervisor
         exit    hyp=1
 
 hyp_irq:
         entry   hyp=1
+        msr     daifclr, #4
         mov     x0, sp
         bl      do_trap_irq
         exit    hyp=1
@@ -208,7 +209,7 @@ guest_sync:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #2
+        msr     daifclr, #6
         mov     x0, sp
         bl      do_trap_hypervisor
 1:
@@ -224,6 +225,7 @@ guest_irq:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+        msr     daifclr, #4
         mov     x0, sp
         bl      do_trap_irq
 1:
@@ -235,7 +237,7 @@ guest_fiq_invalid:
 
 guest_error:
         entry   hyp=0, compat=0
-        msr     daifclr, #2
+        msr     daifclr, #6
         mov     x0, sp
         bl      do_trap_guest_serror
         exit    hyp=0, compat=0
@@ -250,7 +252,7 @@ guest_sync_compat:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #2
+        msr     daifclr, #6
         mov     x0, sp
         bl      do_trap_hypervisor
 1:
@@ -266,6 +268,7 @@ guest_irq_compat:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+        msr     daifclr, #4
         mov     x0, sp
         bl      do_trap_irq
 1:
@@ -277,7 +280,7 @@ guest_fiq_invalid_compat:
 
 guest_error_compat:
         entry   hyp=0, compat=1
-        msr     daifclr, #2
+        msr     daifclr, #6
         mov     x0, sp
         bl      do_trap_guest_serror
         exit    hyp=0, compat=1
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 15/19] xen/arm: Introduce a helper to check local abort is enabled

[Wei Chen]

In previous patch, we have umasked the Abort/SError bit for Xen
in most of its running time. So in some use-cases, we have to
check whether the abort is enabled in current context. For example,
while we want to synchronize SErrors, we have to confirm the abort
is enabled. Otherwise synchronize SErrors is pointless.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>

---
v3->v4:
Use one local_abort_is_enabled for ARM32 and ARM64.
---
 xen/include/asm-arm/system.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/include/asm-arm/system.h b/xen/include/asm-arm/system.h
index 2eb96e8..b94e56f 100644
--- a/xen/include/asm-arm/system.h
+++ b/xen/include/asm-arm/system.h
@@ -51,6 +51,13 @@
 # error "unknown ARM variant"
 #endif
 
+static inline int local_abort_is_enabled(void)
+{
+    unsigned long flags;
+    local_save_flags(flags);
+    return !(flags & PSR_ABT_MASK);
+}
+
 #define arch_fetch_and_add(x, v) __sync_fetch_and_add(x, v)
 
 extern struct vcpu *__context_switch(struct vcpu *prev, struct vcpu *next);
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 16/19] xen/arm: Introduce a macro to synchronize SError

[Wei Chen]

In previous patches, we have provided the ability to synchronize
SErrors in exception entries. But we haven't synchronized SErrors
while returning to guest and doing context switch.

So we still have two risks:
1. Slipping hypervisor SErrors to guest. For example, hypervisor
   triggers a SError while returning to guest, but this SError may be
   delivered after entering guest. In "DIVERSE" option, this SError
   would be routed back to guest and panic the guest. But actually,
   we should crash the whole system due to this hypervisor SError.
2. Slipping previous guest SErrors to the next guest. In "FORWARD"
   option, if hypervisor triggers a SError while context switching.
   This SError may be delivered after switching to next vCPU. In this
   case, this SError will be forwarded to next vCPU and may panic
   an incorrect guest.

So we have have to introduce this macro to synchronize SErrors while
returning to guest and doing context switch. In this macro, we use
ASSERT to make sure the abort is ummasked. Because we unmasked abort
in the entries, but we don't know whether someone will mask it in the
future.

We also added a barrier to this macro to prevent compiler reorder our
asm volatile code.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/include/asm-arm/processor.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index bb24bee..0ed6cac 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -723,6 +723,19 @@ void abort_guest_exit_end(void);
     ( (unsigned long)abort_guest_exit_end == (r)->pc ) \
 )
 
+/*
+ * Synchronize SError unless the feature is selected.
+ * This is relying on the SErrors are currently unmasked.
+ */
+#define SYNCHRONIZE_SERROR(feat)                                  \
+    do {                                                          \
+        ASSERT(!cpus_have_cap(feat) || local_abort_is_enabled()); \
+        ASSERT(local_abort_is_enabled());                         \
+        asm volatile(ALTERNATIVE("dsb sy; isb",                   \
+                                 "nop; nop", feat)                \
+                                 : : : "memory");                 \
+    } while (0)
+
 #endif /* __ASSEMBLY__ */
 #endif /* __ASM_ARM_PROCESSOR_H */
 /*
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 17/19] xen/arm: Isolate the SError between the context switch of 2 vCPUs

[Wei Chen]

If there is a pending SError while we are doing context switch, if the
SError handle option is "FORWARD", We have to guarantee this SError to
be caught by current vCPU, otherwise it will be caught by next vCPU and
be forwarded to this wrong vCPU.

So we have to synchronize SError before switch to next vCPU. But this is
only required by "FORWARD" option. In this case we added a new flag
SKIP_CTXT_SWITCH_SERROR_SYNC in cpu_hwcaps to skip synchronizing SError
in context switch for other options. In the meantime, we don't need to
export serror_op accessing to other source files.

Because we have umasked the Abort/SError bit in previous patch, we have
to disable Abort/SError before doing context switch as we have done for
IRQ.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/domain.c            | 12 ++++++++++++
 xen/arch/arm/traps.c             |  3 +++
 xen/include/asm-arm/cpufeature.h |  3 ++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 69c2854..76310ed 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -29,6 +29,7 @@
 #include <asm/cpufeature.h>
 #include <asm/vfp.h>
 #include <asm/procinfo.h>
+#include <asm/alternative.h>
 
 #include <asm/gic.h>
 #include <asm/vgic.h>
@@ -312,6 +313,17 @@ 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 21cf922..c092e66 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -167,6 +167,9 @@ 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);
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 9eb72e1..b3cf706 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -41,8 +41,9 @@
 #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_NCAPS           6
+#define ARM_NCAPS           7
 
 #ifndef __ASSEMBLY__
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 18/19] xen/arm: Prevent slipping hypervisor SError to guest

[Wei Chen]

If there is a pending SError while we're returning from trap. If the
SError handle option is "DIVERSE", we have to prevent slipping this
hypervisor SError to guest. So we have to use the dsb/isb to guarantee
that the pending hypervisor SError would be caught in hypervisor before
return to guest.

In previous patch, we will set SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT to
cpu_hwcaps when option is NOT "DIVERSE". This means we can use the
alternative to skip synchronizing SErrors for other SErrors handle options.

Because we have umasked the Abort/SError bit in previous patch. We have
to disable the Abort/SError before returning to guest as we have done
for IRQ.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
---
 xen/arch/arm/traps.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c092e66..c8163db 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2989,6 +2989,19 @@ asmlinkage void leave_hypervisor_tail(void)
         local_irq_disable();
         if (!softirq_pending(smp_processor_id())) {
             gic_inject();
+
+            /*
+             * If the SErrors handle option is "DIVERSE", we have to prevent
+             * slipping the hypervisor SError to guest. In this option, before
+             * returning from trap, we have to synchronize SErrors to guarantee
+             * that the pending SError would be caught in hypervisor.
+             *
+             * If option is NOT "DIVERSE", SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+             * will be set to cpu_hwcaps. This means we can use the alternative
+             * to skip synchronizing SErrors for other SErrors handle options.
+             */
+            SYNCHRONIZE_SERROR(SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT);
+
             return;
         }
         local_irq_enable();
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v4 19/19] xen/arm: Handle guest external abort as guest SError

[Wei Chen]

The guest generated external data/instruction aborts can be treated
as guest SErrors. We already have a handler to handle the SErrors,
so we can reuse this handler to handle guest external aborts.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/traps.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c8163db..3625e04 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2583,12 +2583,12 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
 
     /*
      * If this bit has been set, it means that this instruction abort is caused
-     * by a guest external abort. Currently we crash the guest to protect the
-     * hypervisor. In future one can better handle this by injecting a virtual
-     * abort to the guest.
+     * by a guest external abort. We can handle this instruction abort as guest
+     * SError.
      */
     if ( hsr.iabt.eat )
-        domain_crash_synchronous();
+        return __do_trap_serror(regs, true);
+
 
     if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
         gpa = get_faulting_ipa(gva);
@@ -2715,12 +2715,10 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
 
     /*
      * If this bit has been set, it means that this data abort is caused
-     * by a guest external abort. Currently we crash the guest to protect the
-     * hypervisor. In future one can better handle this by injecting a virtual
-     * abort to the guest.
+     * by a guest external abort. We treat this data abort as guest SError.
      */
     if ( dabt.eat )
-        domain_crash_synchronous();
+        return __do_trap_serror(regs, true);
 
     info.dabt = dabt;
 #ifdef CONFIG_ARM_32
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 15/19] xen/arm: Introduce a helper to check local abort is enabled

[Julien Grall]

Hi Wei,

On 05/04/17 10:09, Wei Chen wrote:
> In previous patch, we have umasked the Abort/SError bit for Xen
> in most of its running time. So in some use-cases, we have to
> check whether the abort is enabled in current context. For example,
> while we want to synchronize SErrors, we have to confirm the abort
> is enabled. Otherwise synchronize SErrors is pointless.
>
> Signed-off-by: Wei Chen <Wei.Chen@arm.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 16/19] xen/arm: Introduce a macro to synchronize SError

[Julien Grall]

Hi Wei,

On 05/04/17 10:09, Wei Chen wrote:
> In previous patches, we have provided the ability to synchronize
> SErrors in exception entries. But we haven't synchronized SErrors
> while returning to guest and doing context switch.
>
> So we still have two risks:
> 1. Slipping hypervisor SErrors to guest. For example, hypervisor
>    triggers a SError while returning to guest, but this SError may be
>    delivered after entering guest. In "DIVERSE" option, this SError
>    would be routed back to guest and panic the guest. But actually,
>    we should crash the whole system due to this hypervisor SError.
> 2. Slipping previous guest SErrors to the next guest. In "FORWARD"
>    option, if hypervisor triggers a SError while context switching.
>    This SError may be delivered after switching to next vCPU. In this
>    case, this SError will be forwarded to next vCPU and may panic
>    an incorrect guest.
>
> So we have have to introduce this macro to synchronize SErrors while
> returning to guest and doing context switch. In this macro, we use
> ASSERT to make sure the abort is ummasked. Because we unmasked abort
> in the entries, but we don't know whether someone will mask it in the
> future.
>
> We also added a barrier to this macro to prevent compiler reorder our
> asm volatile code.
>
> Signed-off-by: Wei Chen <Wei.Chen@arm.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>  xen/include/asm-arm/processor.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index bb24bee..0ed6cac 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -723,6 +723,19 @@ void abort_guest_exit_end(void);
>      ( (unsigned long)abort_guest_exit_end == (r)->pc ) \
>  )
>
> +/*
> + * Synchronize SError unless the feature is selected.
> + * This is relying on the SErrors are currently unmasked.
> + */
> +#define SYNCHRONIZE_SERROR(feat)                                  \
> +    do {                                                          \
> +        ASSERT(!cpus_have_cap(feat) || local_abort_is_enabled()); \
> +        ASSERT(local_abort_is_enabled());                         \

Only one of the ASSERT is enough here. I am easy on which one to keep.

> +        asm volatile(ALTERNATIVE("dsb sy; isb",                   \
> +                                 "nop; nop", feat)                \
> +                                 : : : "memory");                 \
> +    } while (0)
> +
>  #endif /* __ASSEMBLY__ */
>  #endif /* __ASM_ARM_PROCESSOR_H */
>  /*
>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 18/19] xen/arm: Prevent slipping hypervisor SError to guest

[Julien Grall]

Hi Wei,

On 05/04/17 10:09, Wei Chen wrote:
> If there is a pending SError while we're returning from trap. If the
> SError handle option is "DIVERSE", we have to prevent slipping this
> hypervisor SError to guest. So we have to use the dsb/isb to guarantee
> that the pending hypervisor SError would be caught in hypervisor before
> return to guest.
>
> In previous patch, we will set SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT to
> cpu_hwcaps when option is NOT "DIVERSE". This means we can use the
> alternative to skip synchronizing SErrors for other SErrors handle options.
>
> Because we have umasked the Abort/SError bit in previous patch. We have
> to disable the Abort/SError before returning to guest as we have done
> for IRQ.
>
> Signed-off-by: Wei Chen <Wei.Chen@arm.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,


-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 00/19] Provide a command line option to choose how to handle SErrors

[Stefano Stabellini]

Thank you Wei. I committed this series. I fixed on commit patch #16 that
has 2 asserts.

On Wed, 5 Apr 2017, Wei Chen wrote:
> From XSA-201, we know that, a guest could trigger SErrors when accessing
> memory mapped HW in a non-conventional way. In the patches for XSA-201,
> we crash the guest when we captured such asynchronous aborts to avoid data
> corruption.
> 
> In order to distinguish guest-generated SErrors from hypervisor-generated
> SErrors. We have to place SError checking code in every EL1 -> EL2 paths.
> That will be an overhead on entries caused by dsb/isb.
> 
> But not all platforms want to categorize the SErrors. For example, a host
> that is running with trusted guests. The administrator can confirm that
> all guests that are running on the host will not trigger such SErrors. In
> this user scene, we should provide some options to administrator to avoid
> categorizing the SErrors. And then reduce the overhead of dsb/isb.
> 
> We provided following 3 options to administrator to determine how to handle
> the SErrors:
> 
> * `diverse`:
>   The hypervisor will distinguish guest SErrors from hypervisor SErrors.
>   The guest generated SErrors will be forwarded to guests, the hypervisor
>   generated SErrors will cause the whole system crash.
>   It requires:
>   1. Place dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
>      correctly.
>   2. Place dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
>      SErrors to guests.
>   3. Place dsb/isb in context switch to isolate the SErrors between 2 vCPUs.
> 
> * `forward`:
>   The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
>   All SErrors will be forwarded to guests, except the SErrors generated when
>   idle vCPU is running. The idle domain doesn't have the ability to hanle the
>   SErrors, so we have to crash the whole system when we get SErros with 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
>   of the dsb/isb
> 
> ---
> v3->v4:
> 1. Rename SKIP_CHECK_PENDING_VSERROR to SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT.
> 2. Add ASSERT in SYNCHRONIZING_SERROR macro to ensure abort is enabled.
> 3. Use one local_abort_is_enabled for ARM32 and ARM64.
> 4. Fix some grammer issues.
> 5. Add Reviewed-by tags from Julien and Stefano for most of this series.
> 
> Wei Chen (19):
>   xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome
>     check
>   xen/arm: Introduce a helper to get default HCR_EL2 flags
>   xen/arm: Set and restore HCR_EL2 register for each vCPU separately
>   xen/arm: Avoid setting/clearing HCR_RW at every context switch
>   xen/arm: Save HCR_EL2 when a guest took the SError
>   xen/arm: Introduce a virtual abort injection helper
>   xen/arm: Introduce a command line parameter for SErrors/Aborts
>   xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op
>   xen/arm64: Use alternative to skip the check of pending serrors
>   xen/arm32: Use alternative to skip the check of pending serrors
>   xen/arm: Move macro VABORT_GEN_BY_GUEST to common header
>   xen/arm: Introduce new helpers to handle guest/hyp SErrors
>   xen/arm: Replace do_trap_guest_serror with new helpers
>   xen/arm: Unmask the Abort/SError bit in the exception entries
>   xen/arm: Introduce a helper to check local abort is enabled
>   xen/arm: Introduce a macro to synchronize SError
>   xen/arm: Isolate the SError between the context switch of 2 vCPUs
>   xen/arm: Prevent slipping hypervisor SError to guest
>   xen/arm: Handle guest external abort as guest SError
> 
>  docs/misc/xen-command-line.markdown   |  44 ++++++++
>  xen/arch/arm/arm32/asm-offsets.c      |   1 +
>  xen/arch/arm/arm32/entry.S            |  28 ++++-
>  xen/arch/arm/arm32/traps.c            |   5 +-
>  xen/arch/arm/arm64/asm-offsets.c      |   1 +
>  xen/arch/arm/arm64/domctl.c           |   6 ++
>  xen/arch/arm/arm64/entry.S            | 105 +++++++++----------
>  xen/arch/arm/arm64/traps.c            |   2 +-
>  xen/arch/arm/domain.c                 |  19 ++++
>  xen/arch/arm/domain_build.c           |   7 ++
>  xen/arch/arm/p2m.c                    |  10 +-
>  xen/arch/arm/traps.c                  | 187 +++++++++++++++++++++++++++++-----
>  xen/include/asm-arm/arm32/processor.h |  12 +--
>  xen/include/asm-arm/arm64/processor.h |   3 +-
>  xen/include/asm-arm/cpufeature.h      |   4 +-
>  xen/include/asm-arm/domain.h          |   4 +
>  xen/include/asm-arm/processor.h       |  30 +++++-
>  xen/include/asm-arm/system.h          |   7 ++
>  18 files changed, 364 insertions(+), 111 deletions(-)
> 
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 00/19] Provide a command line option to choose how to handle SErrors

[Wei Chen]

Thanks to you and Julien :)

On 2017/4/6 3:18, Stefano Stabellini wrote:
> Thank you Wei. I committed this series. I fixed on commit patch #16 that
> has 2 asserts.
> 
> On Wed, 5 Apr 2017, Wei Chen wrote:
>>  From XSA-201, we know that, a guest could trigger SErrors when accessing
>> memory mapped HW in a non-conventional way. In the patches for XSA-201,
>> we crash the guest when we captured such asynchronous aborts to avoid data
>> corruption.
>>
>> In order to distinguish guest-generated SErrors from hypervisor-generated
>> SErrors. We have to place SError checking code in every EL1 -> EL2 paths.
>> That will be an overhead on entries caused by dsb/isb.
>>
>> But not all platforms want to categorize the SErrors. For example, a host
>> that is running with trusted guests. The administrator can confirm that
>> all guests that are running on the host will not trigger such SErrors. In
>> this user scene, we should provide some options to administrator to avoid
>> categorizing the SErrors. And then reduce the overhead of dsb/isb.
>>
>> We provided following 3 options to administrator to determine how to handle
>> the SErrors:
>>
>> * `diverse`:
>>    The hypervisor will distinguish guest SErrors from hypervisor SErrors.
>>    The guest generated SErrors will be forwarded to guests, the hypervisor
>>    generated SErrors will cause the whole system crash.
>>    It requires:
>>    1. Place dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
>>       correctly.
>>    2. Place dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
>>       SErrors to guests.
>>    3. Place dsb/isb in context switch to isolate the SErrors between 2 vCPUs.
>>
>> * `forward`:
>>    The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
>>    All SErrors will be forwarded to guests, except the SErrors generated when
>>    idle vCPU is running. The idle domain doesn't have the ability to hanle the
>>    SErrors, so we have to crash the whole system when we get SErros with 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
>>    of the dsb/isb
>>
>> ---
>> v3->v4:
>> 1. Rename SKIP_CHECK_PENDING_VSERROR to SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT.
>> 2. Add ASSERT in SYNCHRONIZING_SERROR macro to ensure abort is enabled.
>> 3. Use one local_abort_is_enabled for ARM32 and ARM64.
>> 4. Fix some grammer issues.
>> 5. Add Reviewed-by tags from Julien and Stefano for most of this series.
>>
>> Wei Chen (19):
>>    xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome
>>      check
>>    xen/arm: Introduce a helper to get default HCR_EL2 flags
>>    xen/arm: Set and restore HCR_EL2 register for each vCPU separately
>>    xen/arm: Avoid setting/clearing HCR_RW at every context switch
>>    xen/arm: Save HCR_EL2 when a guest took the SError
>>    xen/arm: Introduce a virtual abort injection helper
>>    xen/arm: Introduce a command line parameter for SErrors/Aborts
>>    xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op
>>    xen/arm64: Use alternative to skip the check of pending serrors
>>    xen/arm32: Use alternative to skip the check of pending serrors
>>    xen/arm: Move macro VABORT_GEN_BY_GUEST to common header
>>    xen/arm: Introduce new helpers to handle guest/hyp SErrors
>>    xen/arm: Replace do_trap_guest_serror with new helpers
>>    xen/arm: Unmask the Abort/SError bit in the exception entries
>>    xen/arm: Introduce a helper to check local abort is enabled
>>    xen/arm: Introduce a macro to synchronize SError
>>    xen/arm: Isolate the SError between the context switch of 2 vCPUs
>>    xen/arm: Prevent slipping hypervisor SError to guest
>>    xen/arm: Handle guest external abort as guest SError
>>
>>   docs/misc/xen-command-line.markdown   |  44 ++++++++
>>   xen/arch/arm/arm32/asm-offsets.c      |   1 +
>>   xen/arch/arm/arm32/entry.S            |  28 ++++-
>>   xen/arch/arm/arm32/traps.c            |   5 +-
>>   xen/arch/arm/arm64/asm-offsets.c      |   1 +
>>   xen/arch/arm/arm64/domctl.c           |   6 ++
>>   xen/arch/arm/arm64/entry.S            | 105 +++++++++----------
>>   xen/arch/arm/arm64/traps.c            |   2 +-
>>   xen/arch/arm/domain.c                 |  19 ++++
>>   xen/arch/arm/domain_build.c           |   7 ++
>>   xen/arch/arm/p2m.c                    |  10 +-
>>   xen/arch/arm/traps.c                  | 187 +++++++++++++++++++++++++++++-----
>>   xen/include/asm-arm/arm32/processor.h |  12 +--
>>   xen/include/asm-arm/arm64/processor.h |   3 +-
>>   xen/include/asm-arm/cpufeature.h      |   4 +-
>>   xen/include/asm-arm/domain.h          |   4 +
>>   xen/include/asm-arm/processor.h       |  30 +++++-
>>   xen/include/asm-arm/system.h          |   7 ++
>>   18 files changed, 364 insertions(+), 111 deletions(-)
>>
>> -- 
>> 2.7.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
>>
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 00/19] Provide a command line option to choose how to handle SErrors

[Julien Grall]

Hi,

I just noticed that commit b32d442abd "setup vwfi correctly on cpu0" has 
not been revered as asked by Stefano (see [1]).

Stefano, can you revert it?

Cheers,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2017-04/msg00264.html

On 05/04/17 20:18, Stefano Stabellini wrote:
> Thank you Wei. I committed this series. I fixed on commit patch #16 that
> has 2 asserts.
>
> On Wed, 5 Apr 2017, Wei Chen wrote:
>> From XSA-201, we know that, a guest could trigger SErrors when accessing
>> memory mapped HW in a non-conventional way. In the patches for XSA-201,
>> we crash the guest when we captured such asynchronous aborts to avoid data
>> corruption.
>>
>> In order to distinguish guest-generated SErrors from hypervisor-generated
>> SErrors. We have to place SError checking code in every EL1 -> EL2 paths.
>> That will be an overhead on entries caused by dsb/isb.
>>
>> But not all platforms want to categorize the SErrors. For example, a host
>> that is running with trusted guests. The administrator can confirm that
>> all guests that are running on the host will not trigger such SErrors. In
>> this user scene, we should provide some options to administrator to avoid
>> categorizing the SErrors. And then reduce the overhead of dsb/isb.
>>
>> We provided following 3 options to administrator to determine how to handle
>> the SErrors:
>>
>> * `diverse`:
>>   The hypervisor will distinguish guest SErrors from hypervisor SErrors.
>>   The guest generated SErrors will be forwarded to guests, the hypervisor
>>   generated SErrors will cause the whole system crash.
>>   It requires:
>>   1. Place dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
>>      correctly.
>>   2. Place dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
>>      SErrors to guests.
>>   3. Place dsb/isb in context switch to isolate the SErrors between 2 vCPUs.
>>
>> * `forward`:
>>   The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
>>   All SErrors will be forwarded to guests, except the SErrors generated when
>>   idle vCPU is running. The idle domain doesn't have the ability to hanle the
>>   SErrors, so we have to crash the whole system when we get SErros with 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
>>   of the dsb/isb
>>
>> ---
>> v3->v4:
>> 1. Rename SKIP_CHECK_PENDING_VSERROR to SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT.
>> 2. Add ASSERT in SYNCHRONIZING_SERROR macro to ensure abort is enabled.
>> 3. Use one local_abort_is_enabled for ARM32 and ARM64.
>> 4. Fix some grammer issues.
>> 5. Add Reviewed-by tags from Julien and Stefano for most of this series.
>>
>> Wei Chen (19):
>>   xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome
>>     check
>>   xen/arm: Introduce a helper to get default HCR_EL2 flags
>>   xen/arm: Set and restore HCR_EL2 register for each vCPU separately
>>   xen/arm: Avoid setting/clearing HCR_RW at every context switch
>>   xen/arm: Save HCR_EL2 when a guest took the SError
>>   xen/arm: Introduce a virtual abort injection helper
>>   xen/arm: Introduce a command line parameter for SErrors/Aborts
>>   xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op
>>   xen/arm64: Use alternative to skip the check of pending serrors
>>   xen/arm32: Use alternative to skip the check of pending serrors
>>   xen/arm: Move macro VABORT_GEN_BY_GUEST to common header
>>   xen/arm: Introduce new helpers to handle guest/hyp SErrors
>>   xen/arm: Replace do_trap_guest_serror with new helpers
>>   xen/arm: Unmask the Abort/SError bit in the exception entries
>>   xen/arm: Introduce a helper to check local abort is enabled
>>   xen/arm: Introduce a macro to synchronize SError
>>   xen/arm: Isolate the SError between the context switch of 2 vCPUs
>>   xen/arm: Prevent slipping hypervisor SError to guest
>>   xen/arm: Handle guest external abort as guest SError
>>
>>  docs/misc/xen-command-line.markdown   |  44 ++++++++
>>  xen/arch/arm/arm32/asm-offsets.c      |   1 +
>>  xen/arch/arm/arm32/entry.S            |  28 ++++-
>>  xen/arch/arm/arm32/traps.c            |   5 +-
>>  xen/arch/arm/arm64/asm-offsets.c      |   1 +
>>  xen/arch/arm/arm64/domctl.c           |   6 ++
>>  xen/arch/arm/arm64/entry.S            | 105 +++++++++----------
>>  xen/arch/arm/arm64/traps.c            |   2 +-
>>  xen/arch/arm/domain.c                 |  19 ++++
>>  xen/arch/arm/domain_build.c           |   7 ++
>>  xen/arch/arm/p2m.c                    |  10 +-
>>  xen/arch/arm/traps.c                  | 187 +++++++++++++++++++++++++++++-----
>>  xen/include/asm-arm/arm32/processor.h |  12 +--
>>  xen/include/asm-arm/arm64/processor.h |   3 +-
>>  xen/include/asm-arm/cpufeature.h      |   4 +-
>>  xen/include/asm-arm/domain.h          |   4 +
>>  xen/include/asm-arm/processor.h       |  30 +++++-
>>  xen/include/asm-arm/system.h          |   7 ++
>>  18 files changed, 364 insertions(+), 111 deletions(-)
>>
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
>>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v4 00/19] Provide a command line option to choose how to handle SErrors

[Stefano Stabellini]

Done. I have also made the backports to the stable trees.

On Fri, 7 Apr 2017, Julien Grall wrote:
> Hi,
> 
> I just noticed that commit b32d442abd "setup vwfi correctly on cpu0" has not
> been revered as asked by Stefano (see [1]).
> 
> Stefano, can you revert it?
> 
> Cheers,
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2017-04/msg00264.html
> 
> On 05/04/17 20:18, Stefano Stabellini wrote:
> > Thank you Wei. I committed this series. I fixed on commit patch #16 that
> > has 2 asserts.
> > 
> > On Wed, 5 Apr 2017, Wei Chen wrote:
> > > From XSA-201, we know that, a guest could trigger SErrors when accessing
> > > memory mapped HW in a non-conventional way. In the patches for XSA-201,
> > > we crash the guest when we captured such asynchronous aborts to avoid data
> > > corruption.
> > > 
> > > In order to distinguish guest-generated SErrors from hypervisor-generated
> > > SErrors. We have to place SError checking code in every EL1 -> EL2 paths.
> > > That will be an overhead on entries caused by dsb/isb.
> > > 
> > > But not all platforms want to categorize the SErrors. For example, a host
> > > that is running with trusted guests. The administrator can confirm that
> > > all guests that are running on the host will not trigger such SErrors. In
> > > this user scene, we should provide some options to administrator to avoid
> > > categorizing the SErrors. And then reduce the overhead of dsb/isb.
> > > 
> > > We provided following 3 options to administrator to determine how to
> > > handle
> > > the SErrors:
> > > 
> > > * `diverse`:
> > >   The hypervisor will distinguish guest SErrors from hypervisor SErrors.
> > >   The guest generated SErrors will be forwarded to guests, the hypervisor
> > >   generated SErrors will cause the whole system crash.
> > >   It requires:
> > >   1. Place dsb/isb on all EL1 -> EL2 trap entries to categorize SErrors
> > >      correctly.
> > >   2. Place dsb/isb on EL2 -> EL1 return paths to prevent slipping
> > > hypervisor
> > >      SErrors to guests.
> > >   3. Place dsb/isb in context switch to isolate the SErrors between 2
> > > vCPUs.
> > > 
> > > * `forward`:
> > >   The hypervisor will not distinguish guest SErrors from hypervisor
> > > SErrors.
> > >   All SErrors will be forwarded to guests, except the SErrors generated
> > > when
> > >   idle vCPU is running. The idle domain doesn't have the ability to hanle
> > > the
> > >   SErrors, so we have to crash the whole system when we get SErros with
> > > 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
> > >   of the dsb/isb
> > > 
> > > ---
> > > v3->v4:
> > > 1. Rename SKIP_CHECK_PENDING_VSERROR to
> > > SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT.
> > > 2. Add ASSERT in SYNCHRONIZING_SERROR macro to ensure abort is enabled.
> > > 3. Use one local_abort_is_enabled for ARM32 and ARM64.
> > > 4. Fix some grammer issues.
> > > 5. Add Reviewed-by tags from Julien and Stefano for most of this series.
> > > 
> > > Wei Chen (19):
> > >   xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome
> > >     check
> > >   xen/arm: Introduce a helper to get default HCR_EL2 flags
> > >   xen/arm: Set and restore HCR_EL2 register for each vCPU separately
> > >   xen/arm: Avoid setting/clearing HCR_RW at every context switch
> > >   xen/arm: Save HCR_EL2 when a guest took the SError
> > >   xen/arm: Introduce a virtual abort injection helper
> > >   xen/arm: Introduce a command line parameter for SErrors/Aborts
> > >   xen/arm: Introduce a initcall to update cpu_hwcaps by serror_op
> > >   xen/arm64: Use alternative to skip the check of pending serrors
> > >   xen/arm32: Use alternative to skip the check of pending serrors
> > >   xen/arm: Move macro VABORT_GEN_BY_GUEST to common header
> > >   xen/arm: Introduce new helpers to handle guest/hyp SErrors
> > >   xen/arm: Replace do_trap_guest_serror with new helpers
> > >   xen/arm: Unmask the Abort/SError bit in the exception entries
> > >   xen/arm: Introduce a helper to check local abort is enabled
> > >   xen/arm: Introduce a macro to synchronize SError
> > >   xen/arm: Isolate the SError between the context switch of 2 vCPUs
> > >   xen/arm: Prevent slipping hypervisor SError to guest
> > >   xen/arm: Handle guest external abort as guest SError
> > > 
> > >  docs/misc/xen-command-line.markdown   |  44 ++++++++
> > >  xen/arch/arm/arm32/asm-offsets.c      |   1 +
> > >  xen/arch/arm/arm32/entry.S            |  28 ++++-
> > >  xen/arch/arm/arm32/traps.c            |   5 +-
> > >  xen/arch/arm/arm64/asm-offsets.c      |   1 +
> > >  xen/arch/arm/arm64/domctl.c           |   6 ++
> > >  xen/arch/arm/arm64/entry.S            | 105 +++++++++----------
> > >  xen/arch/arm/arm64/traps.c            |   2 +-
> > >  xen/arch/arm/domain.c                 |  19 ++++
> > >  xen/arch/arm/domain_build.c           |   7 ++
> > >  xen/arch/arm/p2m.c                    |  10 +-
> > >  xen/arch/arm/traps.c                  | 187
> > > +++++++++++++++++++++++++++++-----
> > >  xen/include/asm-arm/arm32/processor.h |  12 +--
> > >  xen/include/asm-arm/arm64/processor.h |   3 +-
> > >  xen/include/asm-arm/cpufeature.h      |   4 +-
> > >  xen/include/asm-arm/domain.h          |   4 +
> > >  xen/include/asm-arm/processor.h       |  30 +++++-
> > >  xen/include/asm-arm/system.h          |   7 ++
> > >  18 files changed, 364 insertions(+), 111 deletions(-)
> > > 
> > > --
> > > 2.7.4
> > > 
> > > 
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xen.org
> > > https://lists.xen.org/xen-devel
> > > 
> 
> -- 
> Julien Grall
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel