[Xen-devel] [PATCH for-4.13 v4 00/19] xen/arm: XSA-201 and XSA-263 fixes

[Xen-devel] [PATCH for-4.13 v4 00/19] xen/arm: XSA-201 and XSA-263 fixes

[Julien Grall]

Hi all,

This is v4 of the series. For those wondering why it is v4 and not v2, this
series is closely related to XSA-303 [1] and refrained to post a new version
publicly. To avoid delaying the series was reviewed privately on security@.

The series is now nearly fully reviewed. There are just a few missing tags
for patch #11, #12 and #19.

The series is based on XSA-303 which has not yet been committed. For
convenience, I have pushed a branch on my public git:

https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git
branch entry-rework/v4

@Juergen: On v1, you agreed this should be considered as a blocker for Xen 4.13.
Are you still happy to consider this series to go in Xen 4.13?This is mostly
fixing up the non-XSA part of XSA-303. This should allow to handle properly
SSBD workaround and receive safely SErrors.

Cheers,

[1] https://xenbits.xen.org/xsa/advisory-303.html

Cc: jgross@suse.com

Julien Grall (18):
  docs/misc: xen-command-line: Remove wrong statement from
    serrors=diverse
  xen/arm: Remove serrors=forward
  xen/arm: traps: Rework __do_serror() documentation
  docs/misc: xen-command-line: Rework documentation of the option
    'serrors'
  xen/arm: traps: Update the correct PC when inject a virtual SError to
    the guest
  xen/arm64: entry: Avoid open-coding interrupt flags
  xen/arm64: entry: Introduce a macro to generate guest vector and use
    it
  xen/arm64: entry: Check if an SError is pending when receiving a
    vSError
  xen/arm: traps: Rework entry/exit from the guest path
  xen/arm32: entry: Rename save_guest_regs()
  xen/arm: Ensure the SSBD workaround is re-enabled right after exiting
    a guest
  xen/arm: traps: Don't ignore invalid value for serrors=
  xen/arm: alternative: Remove unused parameter for
    alternative_if_not_cap
  xen/arm: Move ARCH_PATCH_INSN_SIZE out of the header livepatch.h
  xen/arm: Allow insn.h to be called from assembly
  xen/arm: asm: Replace use of ALTERNATIVE with alternative_if
  xen/arm: Update the ASSERT() in SYNCHRONIZE_SERROR()
  xen/arm: entry: Ensure the guest state is synced when receiving a
    vSError

Mark Rutland (1):
  xen/arm: alternative: add auto-nop infrastructure

 docs/misc/xen-command-line.pandoc |  45 +++-------
 xen/arch/arm/alternative.c        |   2 -
 xen/arch/arm/arm32/entry.S        |  80 ++++++++++++++----
 xen/arch/arm/arm32/traps.c        |  12 +--
 xen/arch/arm/arm64/entry.S        | 170 +++++++++++++++++++++-----------------
 xen/arch/arm/domain.c             |  11 ---
 xen/arch/arm/traps.c              | 166 +++++++++++++++++--------------------
 xen/include/asm-arm/alternative.h |  75 ++++++++++++-----
 xen/include/asm-arm/cpufeature.h  |  11 ++-
 xen/include/asm-arm/insn.h        |   7 ++
 xen/include/asm-arm/livepatch.h   |   4 +-
 xen/include/asm-arm/macros.h      |   7 ++
 xen/include/asm-arm/processor.h   |   2 +-
 13 files changed, 323 insertions(+), 269 deletions(-)

-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 01/19] docs/misc: xen-command-line: Remove wrong statement from serrors=diverse

[Julien Grall]

When serrors=diverse is selected by the user, we will only synchronize
the pending SErrors on entry to hypervisor from guest context and exit
from guest to hypervisor context.

We don't need synchronize SErrors between guest context switch as they
would be categorized to Hypervisor generated SErrors in any case.

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

---
    The documentation for the option serrors will be fully reworked in a
    follow-up patch. But the dsb/isb in the context switch will be
    removed before the documentation rework. So to avoid any question
    while reviewing the next patch, drop it now.

    Changes in v3:
        - Add Stefano's reviewed-by

    Changes in v2:
        - Patch added
---
 docs/misc/xen-command-line.pandoc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index ef56defef5..30a04df4db 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1877,7 +1877,6 @@ hypervisors handle SErrors:
   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.
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 02/19] xen/arm: Remove serrors=forward

[Julien Grall]

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

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

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

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

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

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

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

---
    Changes in v4:
        - Fix grammar
        - Add Stefano's reviewed-by

    Changes in v3:
        - Fix typo in the commit message
        - Rework comments in arm32/traps.c

    Changes in v2:
        - Patch added
---
 docs/misc/xen-command-line.pandoc | 13 ++-----------
 xen/arch/arm/arm32/traps.c        | 12 ++++++------
 xen/arch/arm/domain.c             | 11 -----------
 xen/arch/arm/traps.c              | 34 +++++++---------------------------
 xen/include/asm-arm/cpufeature.h  | 11 +++++------
 5 files changed, 20 insertions(+), 61 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 30a04df4db..b8a09ce5c4 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1850,7 +1850,7 @@ accidentally leaking secrets by releasing pages without proper sanitization.
 Set the serial transmit buffer size.
 
 ### serrors (ARM)
-> `= diverse | forward | panic`
+> `= diverse | panic`
 
 > Default: `diverse`
 
@@ -1866,7 +1866,7 @@ on the host will not trigger such SErrors. In this case, the administrator can
 use this parameter to skip categorizing SErrors and reduce the overhead of
 dsb/isb.
 
-We provided the following 3 options to administrators to determine how the
+We provided the following 2 options to administrators to determine how the
 hypervisors handle SErrors:
 
 * `diverse`:
@@ -1878,15 +1878,6 @@ hypervisors handle SErrors:
   2. dsb/isb on EL2 -> EL1 return paths to prevent slipping hypervisor
      SErrors to guests.
 
-* `forward`:
-  The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
-  All SErrors will be forwarded to guests, except the SErrors generated when
-  the idle vCPU is running. The idle domain doesn't have the ability to handle
-  SErrors, so we have to crash the whole system when we get SErros with the
-  idle vCPU. This option will avoid most overhead of the dsb/isb, except the
-  dsb/isb in context switch which is used to isolate the SErrors between 2
-  vCPUs.
-
 * `panic`:
   The hypervisor will not distinguish guest SErrors from hypervisor SErrors.
   All SErrors will crash the whole system. This option will avoid all overhead
diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
index 76f714a168..9c9790a6d1 100644
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -69,12 +69,12 @@ void do_trap_prefetch_abort(struct cpu_user_regs *regs)
 void do_trap_data_abort(struct cpu_user_regs *regs)
 {
     /*
-     * We cannot distinguish Xen SErrors from synchronous data aborts. We
-     * want to avoid treating any Xen synchronous aborts as SErrors and
-     * forwarding them to the guest. Instead, crash the system in all
-     * cases when the abort comes from Xen. Even if they are Xen SErrors
-     * it would be a reasonable thing to do, and the default behavior with
-     * serror_op == DIVERSE.
+     * We cannot distinguish between Asynchronous External Abort and
+     * Synchronous Data Abort.
+     *
+     * As asynchronous abort (aka SError) generated by the hypervisor will
+     * result in a crash of the system (see __do_trap_serror()), it is fine to
+     * do it here.
      */
     if ( VABORT_GEN_BY_GUEST(regs) )
         do_trap_guest_serror(regs);
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 460e968e97..53dc1d11c6 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -353,17 +353,6 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
 
     local_irq_disable();
 
-    /*
-     * If the serrors_op is "FORWARD", we have to prevent forwarding
-     * SError to wrong vCPU. So before context switch, we have to use
-     * the SYNCRONIZE_SERROR to guarantee that the pending SError would
-     * be caught by current vCPU.
-     *
-     * The SKIP_CTXT_SWITCH_SERROR_SYNC will be set to cpu_hwcaps when the
-     * serrors_op is NOT "FORWARD".
-     */
-    SYNCHRONIZE_SERROR(SKIP_CTXT_SWITCH_SERROR_SYNC);
-
     set_current(next);
 
     prev = __context_switch(prev, next);
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a3deb59372..6ed9e66710 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -103,15 +103,12 @@ register_t get_default_hcr_flags(void)
 
 static enum {
     SERRORS_DIVERSE,
-    SERRORS_FORWARD,
     SERRORS_PANIC,
 } serrors_op;
 
 static int __init parse_serrors_behavior(const char *str)
 {
-    if ( !strcmp(str, "forward") )
-        serrors_op = SERRORS_FORWARD;
-    else if ( !strcmp(str, "panic") )
+    if ( !strcmp(str, "panic") )
         serrors_op = SERRORS_PANIC;
     else
         serrors_op = SERRORS_DIVERSE;
@@ -125,9 +122,6 @@ static int __init update_serrors_cpu_caps(void)
     if ( serrors_op != SERRORS_DIVERSE )
         cpus_set_cap(SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT);
 
-    if ( serrors_op != SERRORS_FORWARD )
-        cpus_set_cap(SKIP_CTXT_SWITCH_SERROR_SYNC);
-
     return 0;
 }
 __initcall(update_serrors_cpu_caps);
@@ -675,6 +669,9 @@ static void inject_vabt_exception(struct cpu_user_regs *regs)
  * 3) Hypervisor generated native SError, that would be a bug.
  *
  * A true parameter "guest" means that the SError is type#1 or type#2.
+ *
+ * Note that Arm32 asynchronous external abort generated by the
+ * hypervisor will be handled in do_trap_data_abort().
  */
 static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
 {
@@ -692,28 +689,11 @@ static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
         goto crash_system;
     }
 
-    /*
-     * The "FORWARD" option will forward all SErrors to the guests, except
-     * idle domain generated SErrors.
-     */
-    if ( serrors_op == SERRORS_FORWARD )
-    {
-        /*
-         * Because the idle domain doesn't have the ability to handle the
-         * SErrors, we have to crash the whole system while we get a SError
-         * generated by idle domain.
-         */
-        if ( is_idle_vcpu(current) )
-            goto crash_system;
-
-        return inject_vabt_exception(regs);
-    }
-
 crash_system:
-    /* Three possibilities to crash the whole system:
+    /*
+     * Two possibilities to crash the whole system:
      * 1) "DIVERSE" option with Hypervisor generated SErrors.
-     * 2) "FORWARD" option with Idle Domain generated SErrors.
-     * 3) "PANIC" option with all SErrors.
+     * 2) "PANIC" option with all SErrors.
      */
     do_unexpected_trap("SError", regs);
 }
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index d06f09ecfa..9af5666628 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -41,13 +41,12 @@
 #define ARM64_WORKAROUND_834220 3
 #define LIVEPATCH_FEATURE   4
 #define SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT 5
-#define SKIP_CTXT_SWITCH_SERROR_SYNC 6
-#define ARM_HARDEN_BRANCH_PREDICTOR 7
-#define ARM_SSBD 8
-#define ARM_SMCCC_1_1 9
-#define ARM64_WORKAROUND_AT_SPECULATE 10
+#define ARM_HARDEN_BRANCH_PREDICTOR 6
+#define ARM_SSBD 7
+#define ARM_SMCCC_1_1 8
+#define ARM64_WORKAROUND_AT_SPECULATE 9
 
-#define ARM_NCAPS           11
+#define ARM_NCAPS           10
 
 #ifndef __ASSEMBLY__
 
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 03/19] xen/arm: traps: Rework __do_serror() documentation

[Julien Grall]

The documentation on top of __do_serror() is trying to describe all the
possibilities to receive an SErrors.

The description of type#2 is quite misleading because receiving an
SError in EL2 after unmasking SError interrupt ({PSTATE, CPSR}.A) does
not necessarily imply the SError were generated by the guest. You also
need to be in a special window (see abort_guest_exist_{guest, end}).

However, for the context of the function it does not matter how we
categorize the interrupts. What matter is to know whether this is a
guest-generated SError.

All the documentation of __do_serror() is now reworked to avoid
misleading information.

Take the opportunity to simplify the code after the forward option has
been dropped.

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

---
    Changes in v3:
        - Fix typo
        - Add Stefano's acked-by

    Changes in v2:
        - Patch added
---
 xen/arch/arm/traps.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6ed9e66710..3262052f47 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -660,41 +660,31 @@ static void inject_vabt_exception(struct cpu_user_regs *regs)
 }
 
 /*
- * 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.
+ * SError exception handler.
  *
  * A true parameter "guest" means that the SError is type#1 or type#2.
  *
+ * @guest indicates whether this is a SError generated by the guest.
+ *
+ * If true, the SError was generated by the guest, so it is safe to continue
+ * and forward to the guest (if requested).
+ *
+ * If false, the SError was likely generated by the hypervisor. As we cannot
+ * distinguish between precise and imprecise SErrors, it is not safe to
+ * continue.
+ *
  * Note that Arm32 asynchronous external abort generated by the
  * hypervisor will be handled in do_trap_data_abort().
  */
 static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
 {
     /*
-     * Only "DIVERSE" option needs to distinguish the guest-generated SErrors
-     * from hypervisor SErrors.
+     * When using "DIVERSE", the SErrors generated by the guest will be
+     * forwarded to the currently running vCPU.
      */
-    if ( serrors_op == SERRORS_DIVERSE )
-    {
-        /* Forward the type#1 and type#2 SErrors to guests. */
-        if ( guest )
+    if ( serrors_op == SERRORS_DIVERSE && guest )
             return inject_vabt_exception(regs);
 
-        /* Type#3 SErrors will panic the whole system */
-        goto crash_system;
-    }
-
-crash_system:
-    /*
-     * Two possibilities to crash the whole system:
-     * 1) "DIVERSE" option with Hypervisor generated SErrors.
-     * 2) "PANIC" option with all SErrors.
-     */
     do_unexpected_trap("SError", regs);
 }
 
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 04/19] docs/misc: xen-command-line: Rework documentation of the option 'serrors'

[Julien Grall]

The current documentation is misleading for a few reasons:
    1) The synchronization happens on all exit/entry from/to the guest.
       This includes from EL0 (i.e userspace).
    2) Trusted guest can also generate SErrors (e.g. memory failure)
    3) Without RAS support, SErrors are IMP DEFINED. Unless you have a
    complete TRM in hand, you can't really make a decision.
    4) The documentation is written around performance when this is not
    the first concern.

The documentation is now reworked to focus on the consequences of using
serrors="panic" and avoid to go in details on the exact implementation.

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

---

TBH, I think this was a mistake to introduce more options without
understanding the real use case from the users and the impact. I am not
totally against serrors="panic" but I don't think this can be safely
used by anyone withtout having a TRM in hand that exhaustively describes
all the SErrors.

    Changes in v4:
        - Add Stefano's acked-by

    Changes in v2:
        - Patch added
---
 docs/misc/xen-command-line.pandoc | 33 +++++++++------------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index b8a09ce5c4..451d213c8c 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1854,34 +1854,19 @@ Set the serial transmit buffer size.
 
 > 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 2 options to administrators to determine how the
-hypervisors handle SErrors:
+This parameter is provided to administrators to determine how the hypervisor
+handles 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.
+  The hypervisor will distinguish guest SErrors from hypervisor SErrors:
+    - The guest generated SErrors will be forwarded to the currently running
+      guest.
+    - The hypervisor generated SErrors will cause the whole system to crash
 
 * `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.
+  All SErrors will cause the whole system to crash. This option should only
+  be used if you trust all your guests and/or they don't have a gadget (e.g.
+  device) to generate SErrors in normal run.
 
 ### shim_mem (x86)
 > `= List of ( min:<size> | max:<size> | <size> )`
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 05/19] xen/arm: traps: Update the correct PC when inject a virtual SError to the guest

[Julien Grall]

When injecting a virtual Abort to the guest, we want to update the guest
PC so it can re-execute the HVC/SMC once it has handled the SError.

This is unfortunately not the case when the SError is synchronized on
entry from the guest. As the SError will be received while running in
hypervisor context, we will update the PC of hypervisor context (i.e
the trap).

Rework inject_vabt_exception so it uses the guest context rather than
the current one.

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

---

Technically, updating the PC is only necessary when guest SError are
received while running in hypervisor. The code should be reworked to get
the path a bit simpler, but this is post Xen 4.13 work.

    Changes in v4:
        - Add Stefano's acked-by

    Changes in v3:
        - s/vcpu_info/vcpu/

    Changes in v2:
        - Add patch
---
 xen/arch/arm/traps.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 3262052f47..12c52a3860 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -629,11 +629,18 @@ static void inject_dabt_exception(struct cpu_user_regs *regs,
 #endif
 }
 
-/* Inject a virtual Abort/SError into the guest. */
-static void inject_vabt_exception(struct cpu_user_regs *regs)
+/*
+ * Inject a virtual Abort/SError into the guest.
+ *
+ * This should only be called with 'current'.
+ */
+static void inject_vabt_exception(struct vcpu *v)
 {
+    struct cpu_user_regs *regs = guest_cpu_user_regs();
     const union hsr hsr = { .bits = regs->hsr };
 
+    ASSERT(v == current);
+
     /*
      * 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
@@ -656,7 +663,7 @@ static void inject_vabt_exception(struct cpu_user_regs *regs)
         break;
     }
 
-    vcpu_hcr_set_flags(current, HCR_VA);
+    vcpu_hcr_set_flags(v, HCR_VA);
 }
 
 /*
@@ -683,7 +690,7 @@ static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
      * forwarded to the currently running vCPU.
      */
     if ( serrors_op == SERRORS_DIVERSE && guest )
-            return inject_vabt_exception(regs);
+            return inject_vabt_exception(current);
 
     do_unexpected_trap("SError", regs);
 }
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 06/19] xen/arm64: entry: Avoid open-coding interrupt flags

[Julien Grall]

At the moment, the interrupts to mask/unmask are hardcoded in the code
making more difficult to find out what's going on.

A new series of short-hand specific to the file entry.S is now added.

The name of the short-hands should tell which interrupts will be
changed by the msr daif{set, clr} instructions.

Take the opportunity to replace the hardcoded values with the new
short-hands.

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

---
    Changes in v3:
        - Add Stefano's reviewed-by

    Changes in v2:
        - Patch added
---
 xen/arch/arm/arm64/entry.S | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 3e41ba65b6..fac4655bb9 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -6,6 +6,24 @@
 #include <asm/smccc.h>
 #include <public/xen.h>
 
+#define IFLAGS_D_BIT    8
+#define IFLAGS_A_BIT    4
+#define IFLAGS_I_BIT    2
+#define IFLAGS_F_BIT    1
+
+/*
+ * Short-hands to define the interrupts (D, A, I, F)
+ *
+ * _ means the interrupt state will not change
+ * X means the state of interrupt X will change
+ *
+ * To be used with msr daif{set, clr} only.
+ *
+ */
+#define IFLAGS__AI_     IFLAGS_A_BIT | IFLAGS_I_BIT
+#define IFLAGS__A__     IFLAGS_A_BIT
+#define IFLAGS___I_     IFLAGS_I_BIT
+
 /*
  * Stack pushing/popping (register pairs only). Equivalent to store decrement
  * before, load increment after.
@@ -338,7 +356,7 @@ guest_sync_slowpath:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #6
+        msr     daifclr, #IFLAGS__AI_
         mov     x0, sp
         bl      do_trap_guest_sync
 1:
@@ -354,7 +372,7 @@ guest_irq:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #4
+        msr     daifclr, #IFLAGS__A__
         mov     x0, sp
         bl      do_trap_irq
 1:
@@ -366,7 +384,7 @@ guest_fiq_invalid:
 
 guest_error:
         entry   hyp=0, compat=0
-        msr     daifclr, #6
+        msr     daifclr, #IFLAGS__AI_
         mov     x0, sp
         bl      do_trap_guest_serror
         exit    hyp=0, compat=0
@@ -381,7 +399,7 @@ guest_sync_compat:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #6
+        msr     daifclr, #IFLAGS__AI_
         mov     x0, sp
         bl      do_trap_guest_sync
 1:
@@ -397,7 +415,7 @@ guest_irq_compat:
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #4
+        msr     daifclr, #IFLAGS__A__
         mov     x0, sp
         bl      do_trap_irq
 1:
@@ -409,7 +427,7 @@ guest_fiq_invalid_compat:
 
 guest_error_compat:
         entry   hyp=0, compat=1
-        msr     daifclr, #6
+        msr     daifclr, #IFLAGS__AI_
         mov     x0, sp
         bl      do_trap_guest_serror
         exit    hyp=0, compat=1
@@ -420,7 +438,7 @@ ENTRY(return_to_new_vcpu64)
         exit    hyp=0, compat=0
 
 return_from_trap:
-        msr     daifset, #2 /* Mask interrupts */
+        msr     daifset, #IFLAGS___I_ /* Mask interrupts */
 
         ldr     x21, [sp, #UREGS_PC]            /* load ELR */
         ldr     w22, [sp, #UREGS_CPSR]          /* load SPSR */
@@ -471,7 +489,7 @@ check_pending_vserror:
          * SError, the EL2 error exception will happen after PSTATE.A
          * is cleared.
          */
-        msr     daifclr, #4
+        msr     daifclr, #IFLAGS__A__
 
         /*
          * This is our single instruction exception window. A pending
@@ -490,7 +508,7 @@ abort_guest_exit_start:
         .global abort_guest_exit_end
 abort_guest_exit_end:
         /* Mask PSTATE asynchronous abort bit, close the checking window. */
-        msr     daifset, #4
+        msr     daifset, #IFLAGS__A__
 
         /*
          * Compare elr_el2 and the saved value to check whether we are
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 07/19] xen/arm64: entry: Introduce a macro to generate guest vector and use it

[Julien Grall]

Most of the guest vectors are using the same pattern. This makes fairly
tedious to alter the pattern and risk introducing mistakes when updating
each path.

A new macro is introduced to generate the guest vectors and now use it
in the one that use the open-code version.

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

---
    Changes in v3:
        - Add Stefano's reviewed-by

    Changes in v2:
        - Use the new short-hands
---
 xen/arch/arm/arm64/entry.S | 84 ++++++++++++++++------------------------------
 1 file changed, 28 insertions(+), 56 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index fac4655bb9..2867c499da 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -175,6 +175,30 @@
 
         .endm
 
+        /*
+         * Generate a guest vector.
+         *
+         * iflags: Correspond to the list of interrupts to unmask
+         * save_x0_x1: See the description on top of the macro 'entry'
+         */
+        .macro  guest_vector compat, iflags, trap, save_x0_x1=1
+        entry   hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
+        /*
+         * The vSError will be checked while SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+         * is not set. If a vSError took place, the initial exception will be
+         * skipped. Exit ASAP
+         */
+        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
+                    "nop; nop",
+                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+        msr     daifclr, \iflags
+        mov     x0, sp
+        bl      do_trap_\trap
+1:
+        exit    hyp=0, compat=\compat
+        .endm
+
+
 /*
  * Bad Abort numbers
  *-----------------
@@ -347,36 +371,10 @@ guest_sync_slowpath:
          * x0/x1 may have been scratch by the fast path above, so avoid
          * to save them.
          */
-        entry   hyp=0, compat=0, save_x0_x1=0
-        /*
-         * 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
-         */
-        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
-                    "nop; nop",
-                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #IFLAGS__AI_
-        mov     x0, sp
-        bl      do_trap_guest_sync
-1:
-        exit    hyp=0, compat=0
+        guest_vector compat=0, iflags=IFLAGS__AI_, trap=guest_sync, save_x0_x1=0
 
 guest_irq:
-        entry   hyp=0, compat=0
-        /*
-         * 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
-         */
-        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
-                    "nop; nop",
-                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #IFLAGS__A__
-        mov     x0, sp
-        bl      do_trap_irq
-1:
-        exit    hyp=0, compat=0
+        guest_vector compat=0, iflags=IFLAGS__A__, trap=irq
 
 guest_fiq_invalid:
         entry   hyp=0, compat=0
@@ -390,36 +388,10 @@ guest_error:
         exit    hyp=0, compat=0
 
 guest_sync_compat:
-        entry   hyp=0, compat=1
-        /*
-         * 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
-         */
-        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
-                    "nop; nop",
-                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #IFLAGS__AI_
-        mov     x0, sp
-        bl      do_trap_guest_sync
-1:
-        exit    hyp=0, compat=1
+        guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_sync
 
 guest_irq_compat:
-        entry   hyp=0, compat=1
-        /*
-         * 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
-         */
-        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
-                    "nop; nop",
-                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
-        msr     daifclr, #IFLAGS__A__
-        mov     x0, sp
-        bl      do_trap_irq
-1:
-        exit    hyp=0, compat=1
+        guest_vector compat=1, iflags=IFLAGS__A__, trap=irq
 
 guest_fiq_invalid_compat:
         entry   hyp=0, compat=1
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 08/19] xen/arm64: entry: Check if an SError is pending when receiving a vSError

[Julien Grall]

At the moment, when we receive an SError exception from the guest, we
don't check if there are any other pending. For hardening the code, we
should ensure any pending SError are accounted to the guest before
executing any code with SError unmasked.

The recently introduced macro 'guest_vector' could used to generate the
two vectors and therefore take advantage of any change required in the
future.

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

---
    Changes in v3:
        - Add Stefano's reviewed-by

    Changes in v2:
        - Title: s/head/entry/
        - Use the new short-hands
---
 xen/arch/arm/arm64/entry.S | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 2867c499da..d94c13e7bf 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -381,11 +381,7 @@ guest_fiq_invalid:
         invalid BAD_FIQ
 
 guest_error:
-        entry   hyp=0, compat=0
-        msr     daifclr, #IFLAGS__AI_
-        mov     x0, sp
-        bl      do_trap_guest_serror
-        exit    hyp=0, compat=0
+        guest_vector compat=0, iflags=IFLAGS__AI_, trap=guest_serror
 
 guest_sync_compat:
         guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_sync
@@ -398,11 +394,7 @@ guest_fiq_invalid_compat:
         invalid BAD_FIQ
 
 guest_error_compat:
-        entry   hyp=0, compat=1
-        msr     daifclr, #IFLAGS__AI_
-        mov     x0, sp
-        bl      do_trap_guest_serror
-        exit    hyp=0, compat=1
+        guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror
 
 ENTRY(return_to_new_vcpu32)
         exit    hyp=0, compat=1
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 09/19] xen/arm: traps: Rework entry/exit from the guest path

[Julien Grall]

At the moment, enter_hypervisor_head() and leave_hypervisor_tail() are
used to deal with actions to be done before/after any guest request is
handled.

While they are meant to work in pair, the former is called for most of
the traps, including traps from the same exception level (i.e.
hypervisor) whilst the latter will only be called when returning to the
guest.

As pointed out, the enter_hypervisor_head() is not called from all the
traps, so this makes potentially difficult to extend it for the dealing
with same exception level.

Furthermore, some assembly only path will require to call
enter_hypervisor_tail(). So the function is now directly call by
assembly in for guest vector only. This means that the check whether we
are called in a guest trap can now be removed.

Take the opportunity to rename enter_hypervisor_tail() and
leave_hypervisor_tail() to something more meaningful and document them.
This should help everyone to understand the purpose of the two
functions.

Note that enter_hypervisor_tail() does not take any parameters anymore
as after the rework, the code does not use them anymore.

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

---
    Changes in v4:
        - Add Stefano's acked-by

    Changes in v3:
        - Update the documentation on the callee side of
        leave_hypervisor_to_guest().
        - Replace "interrupts" with "IRQ"

    Changes in v2:
        - Update in-code comment
        - Remove 'regs' parameter from enter_hypervisor_tail()
        - Add arm32 code
---
 xen/arch/arm/arm32/entry.S |  5 +++-
 xen/arch/arm/arm64/entry.S |  3 +-
 xen/arch/arm/traps.c       | 71 ++++++++++++++++++++++------------------------
 3 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index ec90cca093..5abff24e91 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -177,6 +177,9 @@ skip_check:
         .if     \guest_iflags != n
         cpsie   \guest_iflags
         .endif
+
+        bl      enter_hypervisor_from_guest
+
 2:
         /* We are ready to handle the trap, setup the registers and jump. */
         adr     lr, return_from_trap
@@ -332,7 +335,7 @@ ENTRY(return_to_new_vcpu32)
 return_to_guest:
         mov r11, sp
         bic sp, #7 /* Align the stack pointer */
-        bl leave_hypervisor_tail /* Disables interrupts on return */
+        bl leave_hypervisor_to_guest /* Mask IRQ on return */
         mov sp, r11
         RESTORE_ONE_BANKED(SP_usr)
         /* LR_usr is the same physical register as lr and is restored below */
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index d94c13e7bf..97dc60210d 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -165,7 +165,7 @@
 
         .if \hyp == 0         /* Guest mode */
 
-        bl      leave_hypervisor_tail /* Disables interrupts on return */
+        bl      leave_hypervisor_to_guest /* Mask IRQ on return */
 
         exit_guest \compat
 
@@ -192,6 +192,7 @@
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
         msr     daifclr, \iflags
+        bl      enter_hypervisor_from_guest
         mov     x0, sp
         bl      do_trap_\trap
 1:
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 12c52a3860..adbedc2d15 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1984,47 +1984,46 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
              cpu_require_ssbd_mitigation();
 }
 
-static void enter_hypervisor_head(struct cpu_user_regs *regs)
+/*
+ * Actions that needs to be done after entering the hypervisor from the
+ * guest and before we handle any request.
+ */
+void enter_hypervisor_from_guest(void)
 {
-    if ( guest_mode(regs) )
-    {
-        struct vcpu *v = current;
+    struct vcpu *v = current;
 
-        /* If the guest has disabled the workaround, bring it back on. */
-        if ( needs_ssbd_flip(v) )
-            arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
+    /* If the guest has disabled the workaround, bring it back on. */
+    if ( needs_ssbd_flip(v) )
+        arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
 
-        /*
-         * 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 ( v->arch.hcr_el2 & HCR_VA )
-            v->arch.hcr_el2 = READ_SYSREG(HCR_EL2);
+    /*
+     * 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 ( v->arch.hcr_el2 & HCR_VA )
+        v->arch.hcr_el2 = READ_SYSREG(HCR_EL2);
 
 #ifdef CONFIG_NEW_VGIC
-        /*
-         * We need to update the state of our emulated devices using level
-         * triggered interrupts before syncing back the VGIC state.
-         *
-         * TODO: Investigate whether this is necessary to do on every
-         * trap and how it can be optimised.
-         */
-        vtimer_update_irqs(v);
-        vcpu_update_evtchn_irq(v);
+    /*
+     * We need to update the state of our emulated devices using level
+     * triggered interrupts before syncing back the VGIC state.
+     *
+     * TODO: Investigate whether this is necessary to do on every
+     * trap and how it can be optimised.
+     */
+    vtimer_update_irqs(v);
+    vcpu_update_evtchn_irq(v);
 #endif
 
-        vgic_sync_from_lrs(v);
-    }
+    vgic_sync_from_lrs(v);
 }
 
 void do_trap_guest_sync(struct cpu_user_regs *regs)
 {
     const union hsr hsr = { .bits = regs->hsr };
 
-    enter_hypervisor_head(regs);
-
     switch ( hsr.ec )
     {
     case HSR_EC_WFI_WFE:
@@ -2158,8 +2157,6 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
 {
     const union hsr hsr = { .bits = regs->hsr };
 
-    enter_hypervisor_head(regs);
-
     switch ( hsr.ec )
     {
 #ifdef CONFIG_ARM_64
@@ -2196,27 +2193,21 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
 
 void do_trap_hyp_serror(struct cpu_user_regs *regs)
 {
-    enter_hypervisor_head(regs);
-
     __do_trap_serror(regs, VABORT_GEN_BY_GUEST(regs));
 }
 
 void do_trap_guest_serror(struct cpu_user_regs *regs)
 {
-    enter_hypervisor_head(regs);
-
     __do_trap_serror(regs, true);
 }
 
 void do_trap_irq(struct cpu_user_regs *regs)
 {
-    enter_hypervisor_head(regs);
     gic_interrupt(regs, 0);
 }
 
 void do_trap_fiq(struct cpu_user_regs *regs)
 {
-    enter_hypervisor_head(regs);
     gic_interrupt(regs, 1);
 }
 
@@ -2259,7 +2250,13 @@ static void check_for_vcpu_work(void)
     local_irq_disable();
 }
 
-void leave_hypervisor_tail(void)
+/*
+ * Actions that needs to be done before entering the guest. This is the
+ * last thing executed before the guest context is fully restored.
+ *
+ * The function will return with IRQ masked.
+ */
+void leave_hypervisor_to_guest(void)
 {
     local_irq_disable();
 
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 10/19] xen/arm32: entry: Rename save_guest_regs()

[Julien Grall]

The function save_guest_regs() is doing more than saving guest
registers. It also restore the vectors table and consume any pending
SErrors generated by the guest. So rename the function to
arch_enter_hypervisor_from_guest_preirq().

Take the opportunity to use ENDPROC() for the benefits of static
analizer and the reader.

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

---

The name of the label is quite long but this was the best I came up. Other
suggestions made were guest_vector() and guest_entry().

I think guest_vector() is a no-go because we are only implementing part of the
vector.

Regarding guest_entry(), this is debatable because from the guest PoV, you
exit it so it can be misleading. I actually got confused more than one time.

Another possibility would be enter_from_guest(), but this is fairly
close to enter_hypervisor_from_guest(). The code would look like:

bl enter_from_guest
csie ...
bl enter_hypervisor_from_guest

Any other suggestion for the name?

    Changes in v3:
        - Patch added
---
 xen/arch/arm/arm32/entry.S | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 5abff24e91..cea4e0e302 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -24,7 +24,11 @@
 #define RESTORE_BANKED(mode) \
         RESTORE_ONE_BANKED(SP_##mode) ; RESTORE_ONE_BANKED(LR_##mode) ; RESTORE_ONE_BANKED(SPSR_##mode)
 
-save_guest_regs:
+/*
+ * Actions that needs to be done after entering the hypervisor from the
+ * guest and before the interrupts are unmasked.
+ */
+arch_enter_hypervisor_from_guest_preirq:
 #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
         /*
          * Restore vectors table to the default as it may have been
@@ -115,6 +119,7 @@ abort_guest_exit_end:
 
 skip_check:
         mov pc, lr
+ENDPROC(arch_enter_hypervisor_from_guest_preirq)
 
         /*
          * Macro to define a trap entry.
@@ -173,7 +178,7 @@ skip_check:
 
 1:
         /* Trap from the guest */
-        bl      save_guest_regs
+        bl      arch_enter_hypervisor_from_guest_preirq
         .if     \guest_iflags != n
         cpsie   \guest_iflags
         .endif
@@ -363,7 +368,7 @@ return_to_hypervisor:
          * HVBAR very late.
          *
          * Default vectors table will be restored on exit (see
-         * save_guest_regs).
+         * arch_enter_hypervisor_from_guest_preirq).
          */
         mov r9, #0                      /* vector tables = NULL */
         /*
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 11/19] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Julien Grall]

At the moment, SSBD workaround is re-enabled for Xen after interrupts
are unmasked. This means we may end up to execute some part of the
hypervisor if an interrupt is received before the workaround is
re-enabled.

Each trap may require to unmask different interrupts.
As the rest of enter_hypervisor_from_guest() does not require to have
interrupts masked, the function is now split in two parts:
    1) enter_hypervisor_from_guest_preirq() called with interrupts
       masked.
    2) enter_hypervisor_from_guest() called with interrupts unmasked.

Note that while it might be possible to avoid spliting the function in
two parts, it requires a bit more work than I can currently invest to
avoid using indirect branch.

Furthermore, the function name is rather generic as there might be more
work to dob before interrupts are unmasked in the future.

Fixes: a7898e4c59 ("xen/arm: Add ARCH_WORKAROUND_2 support for guests")
Reported-by: Andrii Anisov <andrii_anisov@epam.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v4:
        - Remove spurious line

    Changes in v3:
        - Rework the arm32 part

    Changes in v2:
        - Add Arm32 code
        - Rename enter_hypervisor_from_guest_noirq() to
        enter_hypervisor_from_guest_preirq()
        - Update the commit message to explain the choice of splitting
        the code.
---
 xen/arch/arm/arm32/entry.S |  2 +-
 xen/arch/arm/arm64/entry.S |  1 +
 xen/arch/arm/traps.c       | 14 ++++++++++++--
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index cea4e0e302..0a9c248ee2 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -118,7 +118,7 @@ abort_guest_exit_end:
         bne return_from_trap
 
 skip_check:
-        mov pc, lr
+        b   enter_hypervisor_from_guest_preirq
 ENDPROC(arch_enter_hypervisor_from_guest_preirq)
 
         /*
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 97dc60210d..d4fb5fdc1c 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -191,6 +191,7 @@
         ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
                     "nop; nop",
                     SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+        bl      enter_hypervisor_from_guest_preirq
         msr     daifclr, \iflags
         bl      enter_hypervisor_from_guest
         mov     x0, sp
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index adbedc2d15..cb4e3b627b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1986,15 +1986,25 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
 
 /*
  * Actions that needs to be done after entering the hypervisor from the
- * guest and before we handle any request.
+ * guest and before the interrupts are unmasked.
  */
-void enter_hypervisor_from_guest(void)
+void enter_hypervisor_from_guest_preirq(void)
 {
     struct vcpu *v = current;
 
     /* If the guest has disabled the workaround, bring it back on. */
     if ( needs_ssbd_flip(v) )
         arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
+}
+
+/*
+ * Actions that needs to be done after entering the hypervisor from the
+ * guest and before we handle any request. Depending on the exception trap,
+ * this may be called with interrupts unmasked.
+ */
+void enter_hypervisor_from_guest(void)
+{
+    struct vcpu *v = current;
 
     /*
      * If we pended a virtual abort, preserve it until it gets cleared.
-- 
2.11.0


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

Re: [Xen-devel] [PATCH for-4.13 v4 11/19] xen/arm: Ensure the SSBD workaround is re-enabled right after exiting a guest

[Stefano Stabellini]

On Thu, 31 Oct 2019, Julien Grall wrote:
> At the moment, SSBD workaround is re-enabled for Xen after interrupts
> are unmasked. This means we may end up to execute some part of the
> hypervisor if an interrupt is received before the workaround is
> re-enabled.
> 
> Each trap may require to unmask different interrupts.
> As the rest of enter_hypervisor_from_guest() does not require to have
> interrupts masked, the function is now split in two parts:
>     1) enter_hypervisor_from_guest_preirq() called with interrupts
>        masked.
>     2) enter_hypervisor_from_guest() called with interrupts unmasked.
> 
> Note that while it might be possible to avoid spliting the function in
> two parts, it requires a bit more work than I can currently invest to
> avoid using indirect branch.
> 
> Furthermore, the function name is rather generic as there might be more
> work to dob before interrupts are unmasked in the future.
> 
> Fixes: a7898e4c59 ("xen/arm: Add ARCH_WORKAROUND_2 support for guests")
> Reported-by: Andrii Anisov <andrii_anisov@epam.com>
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v4:
>         - Remove spurious line
> 
>     Changes in v3:
>         - Rework the arm32 part
> 
>     Changes in v2:
>         - Add Arm32 code
>         - Rename enter_hypervisor_from_guest_noirq() to
>         enter_hypervisor_from_guest_preirq()
>         - Update the commit message to explain the choice of splitting
>         the code.
> ---
>  xen/arch/arm/arm32/entry.S |  2 +-
>  xen/arch/arm/arm64/entry.S |  1 +
>  xen/arch/arm/traps.c       | 14 ++++++++++++--
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> index cea4e0e302..0a9c248ee2 100644
> --- a/xen/arch/arm/arm32/entry.S
> +++ b/xen/arch/arm/arm32/entry.S
> @@ -118,7 +118,7 @@ abort_guest_exit_end:
>          bne return_from_trap
>  
>  skip_check:
> -        mov pc, lr
> +        b   enter_hypervisor_from_guest_preirq
>  ENDPROC(arch_enter_hypervisor_from_guest_preirq)
>  
>          /*
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 97dc60210d..d4fb5fdc1c 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -191,6 +191,7 @@
>          ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>                      "nop; nop",
>                      SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> +        bl      enter_hypervisor_from_guest_preirq
>          msr     daifclr, \iflags
>          bl      enter_hypervisor_from_guest
>          mov     x0, sp
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index adbedc2d15..cb4e3b627b 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1986,15 +1986,25 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
>  
>  /*
>   * Actions that needs to be done after entering the hypervisor from the
> - * guest and before we handle any request.
> + * guest and before the interrupts are unmasked.
>   */
> -void enter_hypervisor_from_guest(void)
> +void enter_hypervisor_from_guest_preirq(void)
>  {
>      struct vcpu *v = current;
>  
>      /* If the guest has disabled the workaround, bring it back on. */
>      if ( needs_ssbd_flip(v) )
>          arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> +}
> +
> +/*
> + * Actions that needs to be done after entering the hypervisor from the
> + * guest and before we handle any request. Depending on the exception trap,
> + * this may be called with interrupts unmasked.
> + */
> +void enter_hypervisor_from_guest(void)
> +{
> +    struct vcpu *v = current;
>  
>      /*
>       * If we pended a virtual abort, preserve it until it gets cleared.
> -- 
> 2.11.0
> 

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

[Xen-devel] [PATCH for-4.13 v4 12/19] xen/arm: traps: Don't ignore invalid value for serrors=

[Julien Grall]

serrors= only supports 3 values "diverse", "forward" and "panic".

The current implementation of parse_serrors_behavior() will default to
"diverse" for any invalid value and not tell the users.

Rather than ignore the invalid input, return an error to the caller so
it can decides the be approach.

This will be useful after a follow-up patch where the number of options
will be reduced.

Take the opportunity to initialize serrors_op to SERRORS_DIVERSE rather
than relying on the item to be the first in the enum and therefore
equal to 0.

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

---
    Changes in v3:
        - Add Stefano's reviewed-by

    Changes in v2:
        - Patch added
---
 xen/arch/arm/traps.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index cb4e3b627b..d028ec9224 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -104,14 +104,16 @@ register_t get_default_hcr_flags(void)
 static enum {
     SERRORS_DIVERSE,
     SERRORS_PANIC,
-} serrors_op;
+} serrors_op = SERRORS_DIVERSE;
 
 static int __init parse_serrors_behavior(const char *str)
 {
     if ( !strcmp(str, "panic") )
         serrors_op = SERRORS_PANIC;
-    else
+    else if ( !strcmp(str, "diverse") )
         serrors_op = SERRORS_DIVERSE;
+    else
+        return -EINVAL;
 
     return 0;
 }
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 13/19] xen/arm: alternative: Remove unused parameter for alternative_if_not_cap

[Julien Grall]

The macro alternative_if_not_cap is taking two parameters. The second
parameter is never used and it is hard to see how this can be used
correctly as it is only protecting the alternative section magic.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---

    Changes in v2:
        - Add Volodymyr's reviewed-by
        - Add Stefano's acked-by
---
 xen/include/asm-arm/alternative.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h
index dedb6dd001..2830a6da2d 100644
--- a/xen/include/asm-arm/alternative.h
+++ b/xen/include/asm-arm/alternative.h
@@ -116,13 +116,11 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en
  * The code that follows this macro will be assembled and linked as
  * normal. There are no restrictions on this code.
  */
-.macro alternative_if_not cap, enable = 1
-	.if \enable
+.macro alternative_if_not cap
 	.pushsection .altinstructions, "a"
 	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
 	.popsection
 661:
-	.endif
 .endm
 
 /*
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 14/19] xen/arm: Move ARCH_PATCH_INSN_SIZE out of the header livepatch.h

[Julien Grall]

At the moment, ARCH_PATCH_INSN_SIZE is defined in the header
livepatch.h. However, this is also used in the alternative code.

Rather than including livepatch.h just for using the define, move it in
the header insn.h which seems more suitable.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---

    Changes in v2:
        - Add Ross's reviewed-by
        - Add Stefano's acked-by
        - Add Volodymyr's reviewed-by
---
 xen/arch/arm/alternative.c      | 2 --
 xen/include/asm-arm/insn.h      | 3 +++
 xen/include/asm-arm/livepatch.h | 4 +---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
index 52ed7edf69..237c4e5642 100644
--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -30,8 +30,6 @@
 #include <asm/byteorder.h>
 #include <asm/cpufeature.h>
 #include <asm/insn.h>
-/* XXX: Move ARCH_PATCH_INSN_SIZE out of livepatch.h */
-#include <asm/livepatch.h>
 #include <asm/page.h>
 
 /* Override macros from asm/page.h to make them work with mfn_t */
diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
index 3489179826..19277212e1 100644
--- a/xen/include/asm-arm/insn.h
+++ b/xen/include/asm-arm/insn.h
@@ -11,6 +11,9 @@
 # error "unknown ARM variant"
 #endif
 
+/* On ARM32,64 instructions are always 4 bytes long. */
+#define ARCH_PATCH_INSN_SIZE 4
+
 #endif /* !__ARCH_ARM_INSN */
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/livepatch.h b/xen/include/asm-arm/livepatch.h
index 6bca79deb9..026af5e7dc 100644
--- a/xen/include/asm-arm/livepatch.h
+++ b/xen/include/asm-arm/livepatch.h
@@ -7,9 +7,7 @@
 #define __XEN_ARM_LIVEPATCH_H__
 
 #include <xen/sizes.h> /* For SZ_* macros. */
-
-/* On ARM32,64 instructions are always 4 bytes long. */
-#define ARCH_PATCH_INSN_SIZE 4
+#include <asm/insn.h>
 
 /*
  * The va of the hypervisor .text region. We need this as the
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 15/19] xen/arm: Allow insn.h to be called from assembly

[Julien Grall]

A follow-up patch will require to include insn.h from assembly code. So
we need to protect any C-specific definition to avoid compilation
errors when used in assembly code.

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

---

    Changes in v3:
        - Add Stefano's reviewed-by

    Changes in v2:
        - Fix typo
        - Remove in-code comment
---
 xen/include/asm-arm/insn.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
index 19277212e1..27271e95f9 100644
--- a/xen/include/asm-arm/insn.h
+++ b/xen/include/asm-arm/insn.h
@@ -1,6 +1,8 @@
 #ifndef __ARCH_ARM_INSN
 #define __ARCH_ARM_INSN
 
+#ifndef __ASSEMBLY__
+
 #include <xen/types.h>
 
 #if defined(CONFIG_ARM_64)
@@ -11,6 +13,8 @@
 # error "unknown ARM variant"
 #endif
 
+#endif /* __ASSEMBLY__ */
+
 /* On ARM32,64 instructions are always 4 bytes long. */
 #define ARCH_PATCH_INSN_SIZE 4
 
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 16/19] xen/arm: alternative: add auto-nop infrastructure

[Julien Grall]

From: Mark Rutland <mark.rutland@arm.com>

In some cases, one side of an alternative sequence is simply a number of
NOPs used to balance the other side. Keeping track of this manually is
tedious, and the presence of large chains of NOPs makes the code more
painful to read than necessary.

To ameliorate matters, this patch adds a new alternative_else_nop_endif,
which automatically balances an alternative sequence with a trivial NOP
sled.

In many cases, we would like a NOP-sled in the default case, and
instructions patched in in the presence of a feature. To enable the NOPs
to be generated automatically for this case, this patch also adds a new
alternative_if, and updates alternative_else and alternative_endif to
work with either alternative_if or alternative_endif.

The alternative infrastructure was originally ported from Linux. So this
is pretty much a straight backport from commit 792d47379f4d "arm64:
alternative: add auto-nop infrastructure". The only difference is the
nops macro added as not yet existing in Xen.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
[will: use new nops macro to generate nop sequences]
Signed-off-by: Will Deacon <will.deacon@arm.com>
[julien: Add nops and port to Xen]
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
---
    Changes in v3:
        - Add missing include to get auto-nop infrastructure build on
          arm32.

    Changes in v2:
        - Add Volodymyr's reviewed-by
        - Add Stefano's acked-by
---
 xen/include/asm-arm/alternative.h | 71 +++++++++++++++++++++++++++++----------
 xen/include/asm-arm/macros.h      |  7 ++++
 2 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h
index 2830a6da2d..92e888aff8 100644
--- a/xen/include/asm-arm/alternative.h
+++ b/xen/include/asm-arm/alternative.h
@@ -2,6 +2,7 @@
 #define __ASM_ALTERNATIVE_H
 
 #include <asm/cpufeature.h>
+#include <asm/insn.h>
 
 #define ARM_CB_PATCH ARM_NCAPS
 
@@ -87,6 +88,7 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en
 #else
 
 #include <asm/asm_defns.h>
+#include <asm/macros.h>
 
 .macro altinstruction_entry orig_offset alt_offset feature orig_len alt_len
 	.word \orig_offset - .
@@ -111,34 +113,55 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en
 .endm
 
 /*
- * Begin an alternative code sequence.
+ * Alternative sequences
+ *
+ * The code for the case where the capability is not present will be
+ * assembled and linked as normal. There are no restrictions on this
+ * code.
+ *
+ * The code for the case where the capability is present will be
+ * assembled into a special section to be used for dynamic patching.
+ * Code for that case must:
+ *
+ * 1. Be exactly the same length (in bytes) as the default code
+ *    sequence.
  *
- * The code that follows this macro will be assembled and linked as
- * normal. There are no restrictions on this code.
+ * 2. Not contain a branch target that is used outside of the
+ *    alternative sequence it is defined in (branches into an
+ *    alternative sequence are not fixed up).
+ */
+
+/*
+ * Begin an alternative code sequence.
  */
 .macro alternative_if_not cap
+	.set .Lasm_alt_mode, 0
 	.pushsection .altinstructions, "a"
 	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
 	.popsection
 661:
 .endm
 
+.macro alternative_if cap
+	.set .Lasm_alt_mode, 1
+	.pushsection .altinstructions, "a"
+	altinstruction_entry 663f, 661f, \cap, 664f-663f, 662f-661f
+	.popsection
+	.pushsection .altinstr_replacement, "ax"
+	.align 2	/* So GAS knows label 661 is suitably aligned */
+661:
+.endm
+
 /*
- * Provide the alternative code sequence.
- *
- * The code that follows this macro is assembled into a special
- * section to be used for dynamic patching. Code that follows this
- * macro must:
- *
- * 1. Be exactly the same length (in bytes) as the default code
- *    sequence.
- *
- * 2. Not contain a branch target that is used outside of the
- *    alternative sequence it is defined in (branches into an
- *    alternative sequence are not fixed up).
+ * Provide the other half of the alternative code sequence.
  */
 .macro alternative_else
-662:	.pushsection .altinstr_replacement, "ax"
+662:
+	.if .Lasm_alt_mode==0
+	.pushsection .altinstr_replacement, "ax"
+	.else
+	.popsection
+	.endif
 663:
 .endm
 
@@ -154,12 +177,26 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en
  * Complete an alternative code sequence.
  */
 .macro alternative_endif
-664:	.popsection
+664:
+	.if .Lasm_alt_mode==0
+	.popsection
+	.endif
 	.org	. - (664b-663b) + (662b-661b)
 	.org	. - (662b-661b) + (664b-663b)
 .endm
 
 /*
+ * Provides a trivial alternative or default sequence consisting solely
+ * of NOPs. The number of NOPs is chosen automatically to match the
+ * previous case.
+ */
+.macro alternative_else_nop_endif
+alternative_else
+	nops	(662b-661b) / ARCH_PATCH_INSN_SIZE
+alternative_endif
+.endm
+
+/*
  * Callback-based alternative epilogue
  */
 .macro alternative_cb_end
diff --git a/xen/include/asm-arm/macros.h b/xen/include/asm-arm/macros.h
index 1d4bb41d15..91ea3505e4 100644
--- a/xen/include/asm-arm/macros.h
+++ b/xen/include/asm-arm/macros.h
@@ -13,4 +13,11 @@
 # error "unknown ARM variant"
 #endif
 
+    /* NOP sequence  */
+    .macro nops, num
+    .rept   \num
+    nop
+    .endr
+    .endm
+
 #endif /* __ASM_ARM_MACROS_H */
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 17/19] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

[Julien Grall]

Using alternative_if makes the code a bit more streamlined.

Take the opportunity to use the new auto-nop infrastructure to avoid
counting the number of nop in the else part for arch/arm/arm64/entry.S

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

---
    This is pretty much a matter of taste, but at least for arm64 this
    allows us to use the auto-nop infrastructure. So the arm32 is more
    to keep inline with arm64.

    Changes in v4:
        - Add Stefano's reviewed-by

    Changes in v3:
        - Fix build on Arm32

    Changes in v2:
        - Correctly use alternative_if{, _not}
---
 xen/arch/arm/arm32/entry.S | 7 ++++---
 xen/arch/arm/arm64/entry.S | 8 +++++---
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 0a9c248ee2..34156c4404 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -59,9 +59,10 @@ arch_enter_hypervisor_from_guest_preirq:
          * 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)
+        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+        b   skip_check
+        alternative_else_nop_endif
+
         /*
          * Start to check pending virtual abort in the gap of Guest -> HYP
          * world switch.
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index d4fb5fdc1c..a8ba7ab961 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -188,9 +188,11 @@
          * is not set. If a vSError took place, the initial exception will be
          * skipped. Exit ASAP
          */
-        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
-                    "nop; nop",
-                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+        alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+        bl      check_pending_vserror
+        cbnz    x0, 1f
+        alternative_else_nop_endif
+
         bl      enter_hypervisor_from_guest_preirq
         msr     daifclr, \iflags
         bl      enter_hypervisor_from_guest
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 18/19] xen/arm: Update the ASSERT() in SYNCHRONIZE_SERROR()

[Julien Grall]

The macro SYNCHRONIZE_SERROR() has an assert to check whether it will
be called with Abort interrupt unmasked. However, this is only done if
a given cap is not enabled.

None of the callers will treat the abort interrupt differently
depending on a feature. Furthermore, it makes more difficult to check
whether SYNCHRONIZE_SERROR() is going to be called with abort interrupt
unmasked.

Therefore, we now require the abort interrupt to be unmasked regardless
the state of the cap.

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

---
    Changes in v3:
        - Add Stefano's acked-by

    Changes in v2:
        - Patch added
---
 xen/include/asm-arm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index e9d2ae2715..aa642e3ab2 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -542,7 +542,7 @@ register_t get_default_hcr_flags(void);
  */
 #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");                 \
-- 
2.11.0


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

[Xen-devel] [PATCH for-4.13 v4 19/19] xen/arm: entry: Ensure the guest state is synced when receiving a vSError

[Julien Grall]

When a SError/Asynchronous Abort generated by the guest has been
consumed, we will skip the handling of the initial exception.

This includes the calls to enter_hypervisor_from_guest{, _noirq} that
is used to synchronize part of the guest state with the internal
representation and re-enable workarounds (e.g. SSBD). However, we still
call leave_hypervisor_to_guest() which is used for preempting the guest
and synchronizing back part of the guest state.

enter_hypervisor_from_guest{, _noirq} works in pair with
leave_hypervisor_to_guest(), so skipping the first two may result
in a loss of some part of guest state.

An example is the new vGIC which will save the state of the LRs on exit
from the guest and rewrite all of them on entry to the guest.

A more worrying example is SSBD workaround may not be re-enabled. If
leave_hypervisor_to_guest() is rescheduling the vCPU, then we may end to
run a lot of code with SSBD workaroud disabled.

For now, calling leave_hypervisor_to_guest() is not necessary when
injecting a vSError to the guest. But it would still be good to give an
opportunity to reschedule. So both enter_hypervisor_from_guest() and
leave_hypervisor_to_guest() are called.

Note that on arm64, the return value for check_pending_vserror is now
stored in x19 instead of x0. This is because we want to keep the value
across call to C-functions (x0, unlike x19, will not be saved by the
callee).

Take the opportunity to rename check_pending_vserror() to
check_pending_guest_serror() as the function is dealing with host SError
and *not* virtual SError. The documentation is also updated accross
Arm32 and Arm64 to clarify how Xen is dealing with SError generated by
the guest.

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

---

    Changes in v4:
        - Rewording + typo

    Changes in v3:
        - Update comments in the code.
        - Update commit message
        - Add arm32 support

There are two known issues without this patch applied:
    * The state of the vGIC when using the new version may be lost.
    * SSBD workaround may be kept disabled while rescheduling the guest.
---
 xen/arch/arm/arm32/entry.S | 57 ++++++++++++++++++++++++++++++++++++++--------
 xen/arch/arm/arm64/entry.S | 54 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 88 insertions(+), 23 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 34156c4404..b31056a616 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -27,6 +27,10 @@
 /*
  * Actions that needs to be done after entering the hypervisor from the
  * guest and before the interrupts are unmasked.
+ *
+ * @return:
+ *  r4: Set to a non-zero value if a pending Abort exception took place.
+ *      Otherwise, it will be set to zero.
  */
 arch_enter_hypervisor_from_guest_preirq:
 #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
@@ -56,18 +60,35 @@ arch_enter_hypervisor_from_guest_preirq:
         SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
 
         /*
-         * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
-         * feature, the checking of pending SErrors will be skipped.
+         * We may have entered the hypervisor with pending asynchronous Abort
+         * generated by the guest. If we need to categorize them, then
+         * we need to consume any outstanding asynchronous Abort.
+         * Otherwise, they can be consumed later on.
          */
         alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+        mov r4, #0              /* r4 := No Abort was consumed */
         b   skip_check
         alternative_else_nop_endif
 
         /*
-         * Start to check pending virtual abort in the gap of Guest -> HYP
-         * world switch.
+         * Consume pending asynchronous Abort generated by the guest if any.
+         *
+         * The only way to consume an Abort interrupt is to unmask it. So
+         * Abort exception will be unmaked for a small window and then masked
+         * it again.
+         *
+         * It is fine to unmask asynchronous Abort exception as we fully
+         * control the state of the processor and only limited code will
+         * be executed if the exception returns (see do_trap_data_abort()).
          *
-         * Save ELR_hyp to check whether the pending virtual abort exception
+         * TODO: The asynchronous abort path should be reworked to
+         * inject the virtual asynchronous Abort in enter_hypervisor_*
+         * rather than do_trap_data_abort(). This should make easier to
+         * understand the path.
+         */
+
+        /*
+         * save elr_hyp to check whether the pending virtual abort exception
          * takes place while we are doing this trap exception.
          */
         mrs r1, ELR_hyp
@@ -112,11 +133,11 @@ abort_guest_exit_end:
         cmp r1, r2
 
         /*
-         * Not equal, the pending virtual abort exception took place, the
-         * initial exception does not have any significance to be handled.
-         * Exit ASAP.
+         * Set r4 depending on whether an asynchronous abort were
+         * consumed.
          */
-        bne return_from_trap
+        movne r4, #1
+        moveq r4, #0
 
 skip_check:
         b   enter_hypervisor_from_guest_preirq
@@ -179,12 +200,28 @@ ENDPROC(arch_enter_hypervisor_from_guest_preirq)
 
 1:
         /* Trap from the guest */
+        /*
+         * arch_enter_hypervisor_from_guest_preirq will return with r4 set to
+         * a non-zero value if an asynchronous Abort was consumed.
+         *
+         * When an asynchronous Abort has been consumed (r4 != 0), we may have
+         * injected a virtual asynchronous Abort to the guest.
+         *
+         * In this case, the initial exception will be discarded (PC has
+         * been adjusted by inject_vabt_exception()). However, we still
+         * want to give an opportunity to reschedule the vCPU. So we
+         * only want to skip the handling of the initial exception (i.e.
+         * do_trap_*()).
+         */
         bl      arch_enter_hypervisor_from_guest_preirq
         .if     \guest_iflags != n
         cpsie   \guest_iflags
         .endif
 
-        bl      enter_hypervisor_from_guest
+        adr     lr, 2f
+        cmp     r4, #0
+        adrne   lr, return_from_trap
+        b       enter_hypervisor_from_guest
 
 2:
         /* We are ready to handle the trap, setup the registers and jump. */
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index a8ba7ab961..d35855af96 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -184,18 +184,41 @@
         .macro  guest_vector compat, iflags, trap, save_x0_x1=1
         entry   hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
         /*
-         * The vSError will be checked while SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
-         * is not set. If a vSError took place, the initial exception will be
-         * skipped. Exit ASAP
+         * We may have entered the hypervisor with pending SErrors
+         * generated by the guest. If we need to categorize them, then
+         * we need to check any outstanding SErrors will be consumed.
+         *
+         * The function check_pending_guest_serror() will unmask SError
+         * exception temporarily. This is fine to do before enter_*
+         * helpers are called because we fully control the state of the
+         * processor and only limited code willl be executed (see
+         * do_trap_hyp_serror()).
+         *
+         * When a SError has been consumed (x19 != 0), we may have injected a
+         * virtual SError to the guest.
+         *
+         * In this case, the initial exception will be discarded (PC has
+         * been adjusted by inject_vabt_exception()). However, we still
+         * want to give an opportunity to reschedule the vCPU. So we
+         * only want to skip the handling of the initial exception (i.e.
+         * do_trap_*()).
+         *
+         * TODO: The SErrors path should be reworked to inject the vSError in
+         * enter_hypervisor_* rather than do_trap_hyp_serror. This should make
+         * easier to understand the path.
          */
         alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
-        bl      check_pending_vserror
-        cbnz    x0, 1f
+        bl      check_pending_guest_serror
         alternative_else_nop_endif
 
         bl      enter_hypervisor_from_guest_preirq
         msr     daifclr, \iflags
         bl      enter_hypervisor_from_guest
+
+        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+        cbnz    x19, 1f
+        alternative_else_nop_endif
+
         mov     x0, sp
         bl      do_trap_\trap
 1:
@@ -436,13 +459,17 @@ return_from_trap:
         eret
 
 /*
- * This function is used to check pending virtual SError in the gap of
- * EL1 -> EL2 world switch.
- * The x0 register will be used to indicate the results of detection.
- * x0 -- Non-zero indicates a pending virtual SError took place.
- * x0 -- Zero indicates no pending virtual SError took place.
+ * Consume pending SError generated by the guest if any.
+ *
+ * @return:
+ *  x19: Set to a non-zero value if a pending Abort exception took place.
+ *       Otherwise, it will be set to zero.
+ *
+ * Without RAS extension, the only way to consume a SError is to unmask
+ * it. So the function will unmask SError exception for a small window and
+ * then mask it again.
  */
-check_pending_vserror:
+check_pending_guest_serror:
         /*
          * Save elr_el2 to check whether the pending SError exception takes
          * place while we are doing this sync exception.
@@ -487,11 +514,12 @@ abort_guest_exit_end:
 
         /*
          * Not equal, the pending SError exception took place, set
-         * x0 to non-zero.
+         * x19 to non-zero.
          */
-        cset    x0, ne
+        cset    x19, ne
 
         ret
+ENDPROC(check_pending_guest_serror)
 
 /*
  * Exception vectors.
-- 
2.11.0


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

Re: [Xen-devel] [PATCH for-4.13 v4 19/19] xen/arm: entry: Ensure the guest state is synced when receiving a vSError

[Stefano Stabellini]

On Thu, 31 Oct 2019, Julien Grall wrote:
> When a SError/Asynchronous Abort generated by the guest has been
> consumed, we will skip the handling of the initial exception.
> 
> This includes the calls to enter_hypervisor_from_guest{, _noirq} that
> is used to synchronize part of the guest state with the internal
> representation and re-enable workarounds (e.g. SSBD). However, we still
> call leave_hypervisor_to_guest() which is used for preempting the guest
> and synchronizing back part of the guest state.
> 
> enter_hypervisor_from_guest{, _noirq} works in pair with
> leave_hypervisor_to_guest(), so skipping the first two may result
> in a loss of some part of guest state.
> 
> An example is the new vGIC which will save the state of the LRs on exit
> from the guest and rewrite all of them on entry to the guest.
> 
> A more worrying example is SSBD workaround may not be re-enabled. If
> leave_hypervisor_to_guest() is rescheduling the vCPU, then we may end to
> run a lot of code with SSBD workaroud disabled.
> 
> For now, calling leave_hypervisor_to_guest() is not necessary when
> injecting a vSError to the guest. But it would still be good to give an
> opportunity to reschedule. So both enter_hypervisor_from_guest() and
> leave_hypervisor_to_guest() are called.
> 
> Note that on arm64, the return value for check_pending_vserror is now
> stored in x19 instead of x0. This is because we want to keep the value
> across call to C-functions (x0, unlike x19, will not be saved by the
> callee).
> 
> Take the opportunity to rename check_pending_vserror() to
> check_pending_guest_serror() as the function is dealing with host SError
> and *not* virtual SError. The documentation is also updated accross
> Arm32 and Arm64 to clarify how Xen is dealing with SError generated by
> the guest.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> 
>     Changes in v4:
>         - Rewording + typo
> 
>     Changes in v3:
>         - Update comments in the code.
>         - Update commit message
>         - Add arm32 support
> 
> There are two known issues without this patch applied:
>     * The state of the vGIC when using the new version may be lost.
>     * SSBD workaround may be kept disabled while rescheduling the guest.
> ---
>  xen/arch/arm/arm32/entry.S | 57 ++++++++++++++++++++++++++++++++++++++--------
>  xen/arch/arm/arm64/entry.S | 54 ++++++++++++++++++++++++++++++++-----------
>  2 files changed, 88 insertions(+), 23 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> index 34156c4404..b31056a616 100644
> --- a/xen/arch/arm/arm32/entry.S
> +++ b/xen/arch/arm/arm32/entry.S
> @@ -27,6 +27,10 @@
>  /*
>   * Actions that needs to be done after entering the hypervisor from the
>   * guest and before the interrupts are unmasked.
> + *
> + * @return:
> + *  r4: Set to a non-zero value if a pending Abort exception took place.
> + *      Otherwise, it will be set to zero.
>   */
>  arch_enter_hypervisor_from_guest_preirq:
>  #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
> @@ -56,18 +60,35 @@ arch_enter_hypervisor_from_guest_preirq:
>          SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
>  
>          /*
> -         * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
> -         * feature, the checking of pending SErrors will be skipped.
> +         * We may have entered the hypervisor with pending asynchronous Abort
> +         * generated by the guest. If we need to categorize them, then
> +         * we need to consume any outstanding asynchronous Abort.
> +         * Otherwise, they can be consumed later on.
>           */
>          alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> +        mov r4, #0              /* r4 := No Abort was consumed */
>          b   skip_check
>          alternative_else_nop_endif
>  
>          /*
> -         * Start to check pending virtual abort in the gap of Guest -> HYP
> -         * world switch.
> +         * Consume pending asynchronous Abort generated by the guest if any.
> +         *
> +         * The only way to consume an Abort interrupt is to unmask it. So
> +         * Abort exception will be unmaked for a small window and then masked
> +         * it again.
> +         *
> +         * It is fine to unmask asynchronous Abort exception as we fully
> +         * control the state of the processor and only limited code will
> +         * be executed if the exception returns (see do_trap_data_abort()).
>           *
> -         * Save ELR_hyp to check whether the pending virtual abort exception
> +         * TODO: The asynchronous abort path should be reworked to
> +         * inject the virtual asynchronous Abort in enter_hypervisor_*
> +         * rather than do_trap_data_abort(). This should make easier to
> +         * understand the path.
> +         */
> +
> +        /*
> +         * save elr_hyp to check whether the pending virtual abort exception
>           * takes place while we are doing this trap exception.
>           */
>          mrs r1, ELR_hyp
> @@ -112,11 +133,11 @@ abort_guest_exit_end:
>          cmp r1, r2
>  
>          /*
> -         * Not equal, the pending virtual abort exception took place, the
> -         * initial exception does not have any significance to be handled.
> -         * Exit ASAP.
> +         * Set r4 depending on whether an asynchronous abort were
> +         * consumed.
>           */
> -        bne return_from_trap
> +        movne r4, #1
> +        moveq r4, #0
>  
>  skip_check:
>          b   enter_hypervisor_from_guest_preirq
> @@ -179,12 +200,28 @@ ENDPROC(arch_enter_hypervisor_from_guest_preirq)
>  
>  1:
>          /* Trap from the guest */
> +        /*
> +         * arch_enter_hypervisor_from_guest_preirq will return with r4 set to
> +         * a non-zero value if an asynchronous Abort was consumed.
> +         *
> +         * When an asynchronous Abort has been consumed (r4 != 0), we may have
> +         * injected a virtual asynchronous Abort to the guest.
> +         *
> +         * In this case, the initial exception will be discarded (PC has
> +         * been adjusted by inject_vabt_exception()). However, we still
> +         * want to give an opportunity to reschedule the vCPU. So we
> +         * only want to skip the handling of the initial exception (i.e.
> +         * do_trap_*()).
> +         */
>          bl      arch_enter_hypervisor_from_guest_preirq
>          .if     \guest_iflags != n
>          cpsie   \guest_iflags
>          .endif
>  
> -        bl      enter_hypervisor_from_guest
> +        adr     lr, 2f
> +        cmp     r4, #0
> +        adrne   lr, return_from_trap
> +        b       enter_hypervisor_from_guest
>  
>  2:
>          /* We are ready to handle the trap, setup the registers and jump. */
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index a8ba7ab961..d35855af96 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -184,18 +184,41 @@
>          .macro  guest_vector compat, iflags, trap, save_x0_x1=1
>          entry   hyp=0, compat=\compat, save_x0_x1=\save_x0_x1
>          /*
> -         * The vSError will be checked while SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> -         * is not set. If a vSError took place, the initial exception will be
> -         * skipped. Exit ASAP
> +         * We may have entered the hypervisor with pending SErrors
> +         * generated by the guest. If we need to categorize them, then
> +         * we need to check any outstanding SErrors will be consumed.
> +         *
> +         * The function check_pending_guest_serror() will unmask SError
> +         * exception temporarily. This is fine to do before enter_*
> +         * helpers are called because we fully control the state of the
> +         * processor and only limited code willl be executed (see
> +         * do_trap_hyp_serror()).
> +         *
> +         * When a SError has been consumed (x19 != 0), we may have injected a
> +         * virtual SError to the guest.
> +         *
> +         * In this case, the initial exception will be discarded (PC has
> +         * been adjusted by inject_vabt_exception()). However, we still
> +         * want to give an opportunity to reschedule the vCPU. So we
> +         * only want to skip the handling of the initial exception (i.e.
> +         * do_trap_*()).
> +         *
> +         * TODO: The SErrors path should be reworked to inject the vSError in
> +         * enter_hypervisor_* rather than do_trap_hyp_serror. This should make
> +         * easier to understand the path.
>           */
>          alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> -        bl      check_pending_vserror
> -        cbnz    x0, 1f
> +        bl      check_pending_guest_serror
>          alternative_else_nop_endif
>  
>          bl      enter_hypervisor_from_guest_preirq
>          msr     daifclr, \iflags
>          bl      enter_hypervisor_from_guest
> +
> +        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> +        cbnz    x19, 1f
> +        alternative_else_nop_endif
> +
>          mov     x0, sp
>          bl      do_trap_\trap
>  1:
> @@ -436,13 +459,17 @@ return_from_trap:
>          eret
>  
>  /*
> - * This function is used to check pending virtual SError in the gap of
> - * EL1 -> EL2 world switch.
> - * The x0 register will be used to indicate the results of detection.
> - * x0 -- Non-zero indicates a pending virtual SError took place.
> - * x0 -- Zero indicates no pending virtual SError took place.
> + * Consume pending SError generated by the guest if any.
> + *
> + * @return:
> + *  x19: Set to a non-zero value if a pending Abort exception took place.
> + *       Otherwise, it will be set to zero.
> + *
> + * Without RAS extension, the only way to consume a SError is to unmask
> + * it. So the function will unmask SError exception for a small window and
> + * then mask it again.
>   */
> -check_pending_vserror:
> +check_pending_guest_serror:
>          /*
>           * Save elr_el2 to check whether the pending SError exception takes
>           * place while we are doing this sync exception.
> @@ -487,11 +514,12 @@ abort_guest_exit_end:
>  
>          /*
>           * Not equal, the pending SError exception took place, set
> -         * x0 to non-zero.
> +         * x19 to non-zero.
>           */
> -        cset    x0, ne
> +        cset    x19, ne
>  
>          ret
> +ENDPROC(check_pending_guest_serror)
>  
>  /*
>   * Exception vectors.
> -- 
> 2.11.0
> 

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

Re: [Xen-devel] [PATCH for-4.13 v4 00/19] xen/arm: XSA-201 and XSA-263 fixes

[Jürgen Groß]

On 31.10.19 16:09, Julien Grall wrote:
> Hi all,
> 
> This is v4 of the series. For those wondering why it is v4 and not v2, this
> series is closely related to XSA-303 [1] and refrained to post a new version
> publicly. To avoid delaying the series was reviewed privately on security@.
> 
> The series is now nearly fully reviewed. There are just a few missing tags
> for patch #11, #12 and #19.
> 
> The series is based on XSA-303 which has not yet been committed. For
> convenience, I have pushed a branch on my public git:
> 
> https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git
> branch entry-rework/v4
> 
> @Juergen: On v1, you agreed this should be considered as a blocker for Xen 4.13.
> Are you still happy to consider this series to go in Xen 4.13?This is mostly
> fixing up the non-XSA part of XSA-303. This should allow to handle properly
> SSBD workaround and receive safely SErrors.

Yeah, still fine with me, so for the series:

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen

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

Re: [Xen-devel] [PATCH for-4.13 v4 00/19] xen/arm: XSA-201 and XSA-263 fixes

[Julien Grall]

Hi,

On 11/1/19 10:47 AM, Jürgen Groß wrote:
> On 31.10.19 16:09, Julien Grall wrote:
>> Hi all,
>>
>> This is v4 of the series. For those wondering why it is v4 and not v2, 
>> this
>> series is closely related to XSA-303 [1] and refrained to post a new 
>> version
>> publicly. To avoid delaying the series was reviewed privately on 
>> security@.
>>
>> The series is now nearly fully reviewed. There are just a few missing 
>> tags
>> for patch #11, #12 and #19.
>>
>> The series is based on XSA-303 which has not yet been committed. For
>> convenience, I have pushed a branch on my public git:
>>
>> https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git
>> branch entry-rework/v4
>>
>> @Juergen: On v1, you agreed this should be considered as a blocker for 
>> Xen 4.13.
>> Are you still happy to consider this series to go in Xen 4.13?This is 
>> mostly
>> fixing up the non-XSA part of XSA-303. This should allow to handle 
>> properly
>> SSBD workaround and receive safely SErrors.
> 
> Yeah, still fine with me, so for the series:
> 
> Release-acked-by: Juergen Gross <jgross@suse.com>

Thank you! I took the liberty to commit the series with the renaming 
Stefano and I discussed yesterday.

Hopefully this is the last big series for Arm for Xen 4.13 :).

Cheers,

-- 
Julien Grall

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