[PATCH 0/7] xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update

[PATCH 0/7] xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update

[Julien Grall]

Hi all,

Arm has recently published a SMC Calling Convention (SMCCC)
specification update [1] that provides an optimised calling convention
and optional, discoverable support for mitigating CVE-2017-5715 (XSA-254
variant 2). ARM Trusted Firmware (ATF) has already gained such an
implementation[2].

This series addresses a few things:

    - It provides a Xen implementation of PSCI v1.0, which is a prerequisite
      for being able to discover SMCCC v1.1.
    - It allows Xen to advertise SMCCC v1.1
    - It implements Xen support for the ARM_WORKAROUND_1 function that is used
      to mitigate CVE-2017-5715 (if such mitigation is available on the
      hypervisor).

This method is intended to fully replace the initial PSCI_GET_VERSION
approach. Although PSCI_GET_VERSION still works, it has an obvious
overhead and is called on some of the hottest paths. We expect
ARCH_WORKAROUND_1 to be much faster.

Another series will be sent to allow the hypervisor discovering SMCCC 1.1 and
use it for the mitigation.

This series is based on the "xen/arm: SMCCC fixes and PSCI clean-up" one [3].

Cheers,

[1]: https://developer.arm.com/-/media/developer/pdf/ARM%20DEN%200070A%20Firmware%20interfaces%20for%20mitigating%20CVE-2017-5715_V1.0.pdf

[2]: https://github.com/ARM-software/arm-trusted-firmware/pull/1240

[3] https://lists.xenproject.org/archives/html/xen-devel/2018-02/msg00117.html

Julien Grall (7):
  xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu
  xen/arm: psci: Rework the PSCI definitions
  xen/arm: vpsci: Add support for PSCI 1.1
  xen/arm: vsmc: Implement SMCCC 1.1
  xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support
  xen/arm: Adapt smccc.h to be able to use it in assembly code
  xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

 tools/libxl/libxl_arm.c          |  3 +-
 xen/arch/arm/arm64/entry.S       | 56 +++++++++++++++++++++++++-
 xen/arch/arm/domain_build.c      |  1 +
 xen/arch/arm/platforms/seattle.c |  4 +-
 xen/arch/arm/psci.c              | 10 ++---
 xen/arch/arm/vpsci.c             | 85 +++++++++++++++++++++++++++++-----------
 xen/arch/arm/vsmc.c              | 41 +++++++++++++++++++
 xen/include/asm-arm/perfc_defn.h |  1 +
 xen/include/asm-arm/processor.h  |  2 +
 xen/include/asm-arm/psci.h       | 38 ++++++++++--------
 xen/include/asm-arm/smccc.h      | 37 ++++++++++++++---
 xen/include/asm-arm/vpsci.h      |  2 +-
 12 files changed, 225 insertions(+), 55 deletions(-)

-- 
2.11.0


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

[PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Julien Grall]

Currently, the behavior of do_common_cpu will slightly change depending
on the PSCI version passed in parameter. Looking at the code, more the
specific 0.2 behavior could move out of the function or adapted for 0.1:

    - x0/r0 can be updated on PSCI 0.1 because general purpose registers
    are undefined upon CPU on.
    - PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
    safer to bail out if the CPU is already on.

Based on this, the parameter 'ver' is removed and do_psci_cpu_on
(implementation for PSCI 0.1) is adapted to avoid returning
PSCI_ALREADY_ON.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/vpsci.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 884f0fa710..359db884f9 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -22,7 +22,7 @@
 #include <public/sched.h>
 
 static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
-                       register_t context_id,int ver)
+                            register_t context_id)
 {
     struct vcpu *v;
     struct domain *d = current->domain;
@@ -40,8 +40,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
     if ( is_64bit_domain(d) && is_thumb )
         return PSCI_INVALID_PARAMETERS;
 
-    if ( (ver == PSCI_VERSION(0, 2)) &&
-            !test_bit(_VPF_down, &v->pause_flags) )
+    if ( !test_bit(_VPF_down, &v->pause_flags) )
         return PSCI_ALREADY_ON;
 
     if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
@@ -55,18 +54,21 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
     ctxt->ttbr0 = 0;
     ctxt->ttbr1 = 0;
     ctxt->ttbcr = 0; /* Defined Reset Value */
+
+    /*
+     * x0/r0_usr are always updated because for PSCI 0.1 the general
+     * purpose registers are undefined upon CPU_on.
+     */
     if ( is_32bit_domain(d) )
     {
         ctxt->user_regs.cpsr = PSR_GUEST32_INIT;
-        if ( ver == PSCI_VERSION(0, 2) )
-            ctxt->user_regs.r0_usr = context_id;
+        ctxt->user_regs.r0_usr = context_id;
     }
 #ifdef CONFIG_ARM_64
     else
     {
         ctxt->user_regs.cpsr = PSR_GUEST64_INIT;
-        if ( ver == PSCI_VERSION(0, 2) )
-            ctxt->user_regs.x0 = context_id;
+        ctxt->user_regs.x0 = context_id;
     }
 #endif
 
@@ -93,7 +95,14 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
 
 static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point)
 {
-    return do_common_cpu_on(vcpuid, entry_point, 0 , PSCI_VERSION(0, 1));
+    int32_t ret;
+
+    ret = do_common_cpu_on(vcpuid, entry_point, 0);
+    /*
+     * PSCI 0.1 does not define the return code PSCI_ALREADY_ON.
+     * Instead, return PSCI_INVALID_PARAMETERS.
+     */
+    return (ret == PSCI_ALREADY_ON) ? PSCI_INVALID_PARAMETERS : ret;
 }
 
 static int32_t do_psci_cpu_off(uint32_t power_state)
@@ -133,8 +142,7 @@ static int32_t do_psci_0_2_cpu_on(register_t target_cpu,
                                   register_t entry_point,
                                   register_t context_id)
 {
-    return do_common_cpu_on(target_cpu, entry_point, context_id,
-                            PSCI_VERSION(0, 2));
+    return do_common_cpu_on(target_cpu, entry_point, context_id);
 }
 
 static const unsigned long target_affinity_mask[] = {
-- 
2.11.0


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

[PATCH 2/7] xen/arm: psci: Rework the PSCI definitions

[Julien Grall]

Some PSCI functions are only available in the 32-bit version. After
recent changes, Xen always needs to know whether the call was made using
32-bit id or 64-bit id. So we don't emulate reserved one.

With the current naming scheme, it is not easy to know which call
supports 32-bit and 64-bit id. So rework the definitions to encode the
version in the name. From now the functions will be named PSCI_0_2_FNxx
where xx is 32 or 64.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/platforms/seattle.c |  4 ++--
 xen/arch/arm/psci.c              | 10 +++++-----
 xen/arch/arm/vpsci.c             | 22 +++++++++++-----------
 xen/include/asm-arm/psci.h       | 37 +++++++++++++++++++++----------------
 4 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
index 22c062293f..893cc17972 100644
--- a/xen/arch/arm/platforms/seattle.c
+++ b/xen/arch/arm/platforms/seattle.c
@@ -33,12 +33,12 @@ static const char * const seattle_dt_compat[] __initconst =
  */
 static void seattle_system_reset(void)
 {
-    call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
+    call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
 }
 
 static void seattle_system_off(void)
 {
-    call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
+    call_smc(PSCI_0_2_FN32_SYSTEM_OFF, 0, 0, 0);
 }
 
 PLATFORM_START(seattle, "SEATTLE")
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 1508a3be3a..5dda35cd7c 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -31,9 +31,9 @@
  * (native-width) function ID.
  */
 #ifdef CONFIG_ARM_64
-#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64(name)
+#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64_##name
 #else
-#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32(name)
+#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32_##name
 #endif
 
 uint32_t psci_ver;
@@ -48,13 +48,13 @@ int call_psci_cpu_on(int cpu)
 void call_psci_system_off(void)
 {
     if ( psci_ver > PSCI_VERSION(0, 1) )
-        call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
+        call_smc(PSCI_0_2_FN32_SYSTEM_OFF, 0, 0, 0);
 }
 
 void call_psci_system_reset(void)
 {
     if ( psci_ver > PSCI_VERSION(0, 1) )
-        call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
+        call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
 }
 
 int __init psci_is_smc_method(const struct dt_device_node *psci)
@@ -144,7 +144,7 @@ int __init psci_init_0_2(void)
         }
     }
 
-    psci_ver = call_smc(PSCI_0_2_FN32(PSCI_VERSION), 0, 0, 0);
+    psci_ver = call_smc(PSCI_0_2_FN32_PSCI_VERSION, 0, 0, 0);
 
     /* For the moment, we only support PSCI 0.2 and PSCI 1.x */
     if ( psci_ver != PSCI_VERSION(0, 2) && PSCI_VERSION_MAJOR(psci_ver) != 1 )
diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 359db884f9..17dab42cf4 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -250,35 +250,35 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
      */
     switch ( fid )
     {
-    case PSCI_0_2_FN32(PSCI_VERSION):
+    case PSCI_0_2_FN32_PSCI_VERSION:
         perfc_incr(vpsci_version);
         PSCI_SET_RESULT(regs, do_psci_0_2_version());
         return true;
 
-    case PSCI_0_2_FN32(CPU_OFF):
+    case PSCI_0_2_FN32_CPU_OFF:
         perfc_incr(vpsci_cpu_off);
         PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off());
         return true;
 
-    case PSCI_0_2_FN32(MIGRATE_INFO_TYPE):
+    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
         perfc_incr(vpsci_migrate_info_type);
         PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type());
         return true;
 
-    case PSCI_0_2_FN32(SYSTEM_OFF):
+    case PSCI_0_2_FN32_SYSTEM_OFF:
         perfc_incr(vpsci_system_off);
         do_psci_0_2_system_off();
         PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
         return true;
 
-    case PSCI_0_2_FN32(SYSTEM_RESET):
+    case PSCI_0_2_FN32_SYSTEM_RESET:
         perfc_incr(vpsci_system_reset);
         do_psci_0_2_system_reset();
         PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
         return true;
 
-    case PSCI_0_2_FN32(CPU_ON):
-    case PSCI_0_2_FN64(CPU_ON):
+    case PSCI_0_2_FN32_CPU_ON:
+    case PSCI_0_2_FN64_CPU_ON:
     {
         register_t vcpuid = PSCI_ARG(regs, 1);
         register_t epoint = PSCI_ARG(regs, 2);
@@ -289,8 +289,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
         return true;
     }
 
-    case PSCI_0_2_FN32(CPU_SUSPEND):
-    case PSCI_0_2_FN64(CPU_SUSPEND):
+    case PSCI_0_2_FN32_CPU_SUSPEND:
+    case PSCI_0_2_FN64_CPU_SUSPEND:
     {
         uint32_t pstate = PSCI_ARG32(regs, 1);
         register_t epoint = PSCI_ARG(regs, 2);
@@ -301,8 +301,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
         return true;
     }
 
-    case PSCI_0_2_FN32(AFFINITY_INFO):
-    case PSCI_0_2_FN64(AFFINITY_INFO):
+    case PSCI_0_2_FN32_AFFINITY_INFO:
+    case PSCI_0_2_FN64_AFFINITY_INFO:
     {
         register_t taff = PSCI_ARG(regs, 1);
         uint32_t laff = PSCI_ARG32(regs, 2);
diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
index 3c44468e72..becc9f9ded 100644
--- a/xen/include/asm-arm/psci.h
+++ b/xen/include/asm-arm/psci.h
@@ -23,22 +23,27 @@ void call_psci_system_off(void);
 void call_psci_system_reset(void);
 
 /* PSCI v0.2 interface */
-#define PSCI_0_2_FN32(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
-                                               ARM_SMCCC_CONV_32,               \
-                                               ARM_SMCCC_OWNER_STANDARD,        \
-                                               PSCI_0_2_FN_##name)
-#define PSCI_0_2_FN64(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
-                                               ARM_SMCCC_CONV_64,               \
-                                               ARM_SMCCC_OWNER_STANDARD,        \
-                                               PSCI_0_2_FN_##name)
-#define PSCI_0_2_FN_PSCI_VERSION        0
-#define PSCI_0_2_FN_CPU_SUSPEND         1
-#define PSCI_0_2_FN_CPU_OFF             2
-#define PSCI_0_2_FN_CPU_ON              3
-#define PSCI_0_2_FN_AFFINITY_INFO       4
-#define PSCI_0_2_FN_MIGRATE_INFO_TYPE   6
-#define PSCI_0_2_FN_SYSTEM_OFF          8
-#define PSCI_0_2_FN_SYSTEM_RESET        9
+#define PSCI_0_2_FN32(nr) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
+                                             ARM_SMCCC_CONV_32,               \
+                                             ARM_SMCCC_OWNER_STANDARD,        \
+                                             nr)
+#define PSCI_0_2_FN64(nr) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
+                                             ARM_SMCCC_CONV_64,               \
+                                             ARM_SMCCC_OWNER_STANDARD,        \
+                                             nr)
+
+#define PSCI_0_2_FN32_PSCI_VERSION        PSCI_0_2_FN32(0)
+#define PSCI_0_2_FN32_CPU_SUSPEND         PSCI_0_2_FN32(1)
+#define PSCI_0_2_FN32_CPU_OFF             PSCI_0_2_FN32(2)
+#define PSCI_0_2_FN32_CPU_ON              PSCI_0_2_FN32(3)
+#define PSCI_0_2_FN32_AFFINITY_INFO       PSCI_0_2_FN32(4)
+#define PSCI_0_2_FN32_MIGRATE_INFO_TYPE   PSCI_0_2_FN32(6)
+#define PSCI_0_2_FN32_SYSTEM_OFF          PSCI_0_2_FN32(8)
+#define PSCI_0_2_FN32_SYSTEM_RESET        PSCI_0_2_FN32(9)
+
+#define PSCI_0_2_FN64_CPU_SUSPEND         PSCI_0_2_FN64(1)
+#define PSCI_0_2_FN64_CPU_ON              PSCI_0_2_FN64(3)
+#define PSCI_0_2_FN64_AFFINITY_INFO       PSCI_0_2_FN64(4)
 
 /* PSCI v0.2 affinity level state returned by AFFINITY_INFO */
 #define PSCI_0_2_AFFINITY_LEVEL_ON      0
-- 
2.11.0


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

[PATCH 3/7] xen/arm: vpsci: Add support for PSCI 1.1

[Julien Grall]

At the moment, Xen provides virtual PSCI interface compliant with 0.1
and 0.2. Since them, the specification has been updated and the latest
version is 1.1 (see ARM DEN 0022D).

From an implementation point of view, only PSCI_FEATURES is mandatory.
The rest is optional and can be left unimplemented for now.

At the same time, the compatible for PSCI node have been updated to
expose "arm,psci-1.0".

Signed-off-by: Julien Grall <julien.grall@arm.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: mirela.simonovic@aggios.com

---
    We may want to provide a way for the toolstack to specify a PSCI
    version. This could be useful if a guest is expecting a given
    version.
---
 tools/libxl/libxl_arm.c          |  3 ++-
 xen/arch/arm/domain_build.c      |  1 +
 xen/arch/arm/vpsci.c             | 34 +++++++++++++++++++++++++++++++++-
 xen/include/asm-arm/perfc_defn.h |  1 +
 xen/include/asm-arm/psci.h       |  1 +
 xen/include/asm-arm/vpsci.h      |  2 +-
 6 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 3e46554301..86f59c0d80 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -410,7 +410,8 @@ static int make_psci_node(libxl__gc *gc, void *fdt)
     res = fdt_begin_node(fdt, "psci");
     if (res) return res;
 
-    res = fdt_property_compat(gc, fdt, 2, "arm,psci-0.2","arm,psci");
+    res = fdt_property_compat(gc, fdt, 3, "arm,psci-1.0",
+                              "arm,psci-0.2", "arm,psci");
     if (res) return res;
 
     res = fdt_property_string(fdt, "method", "hvc");
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 155c952349..941688a2ce 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -637,6 +637,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
 {
     int res;
     const char compat[] =
+        "arm,psci-1.0""\0"
         "arm,psci-0.2""\0"
         "arm,psci";
 
diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 17dab42cf4..025250a119 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -115,7 +115,7 @@ static int32_t do_psci_cpu_off(uint32_t power_state)
 
 static uint32_t do_psci_0_2_version(void)
 {
-    return PSCI_VERSION(0, 2);
+    return PSCI_VERSION(1, 0);
 }
 
 static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
@@ -199,6 +199,28 @@ static void do_psci_0_2_system_reset(void)
     domain_shutdown(d,SHUTDOWN_reboot);
 }
 
+static int32_t do_psci_1_0_features(uint32_t psci_func_id)
+{
+    switch ( psci_func_id )
+    {
+    case PSCI_0_2_FN32_PSCI_VERSION:
+    case PSCI_0_2_FN32_CPU_OFF:
+    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
+    case PSCI_0_2_FN32_SYSTEM_OFF:
+    case PSCI_0_2_FN32_SYSTEM_RESET:
+    case PSCI_0_2_FN32_CPU_ON:
+    case PSCI_0_2_FN64_CPU_ON:
+    case PSCI_0_2_FN32_CPU_SUSPEND:
+    case PSCI_0_2_FN64_CPU_SUSPEND:
+    case PSCI_0_2_FN32_AFFINITY_INFO:
+    case PSCI_0_2_FN64_AFFINITY_INFO:
+    case PSCI_1_0_FN32_PSCI_FEATURES:
+        return 0;
+    default:
+        return PSCI_NOT_SUPPORTED;
+    }
+}
+
 #define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
 #define PSCI_ARG(reg, n) get_user_reg(reg, n)
 
@@ -311,6 +333,16 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
         PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
         return true;
     }
+
+    case PSCI_1_0_FN32_PSCI_FEATURES:
+    {
+        uint32_t psci_func_id = PSCI_ARG32(regs, 1);
+
+        perfc_incr(vpsci_features);
+        PSCI_SET_RESULT(regs, do_psci_1_0_features(psci_func_id));
+        return true;
+    }
+
     default:
         return false;
     }
diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
index a7acb7d21c..87866264ca 100644
--- a/xen/include/asm-arm/perfc_defn.h
+++ b/xen/include/asm-arm/perfc_defn.h
@@ -31,6 +31,7 @@ PERFCOUNTER(vpsci_system_off,          "vpsci: system_off")
 PERFCOUNTER(vpsci_system_reset,        "vpsci: system_reset")
 PERFCOUNTER(vpsci_cpu_suspend,         "vpsci: cpu_suspend")
 PERFCOUNTER(vpsci_cpu_affinity_info,   "vpsci: cpu_affinity_info")
+PERFCOUNTER(vpsci_features,            "vpsci: features")
 
 PERFCOUNTER(vgicd_reads,                "vgicd: read")
 PERFCOUNTER(vgicd_writes,               "vgicd: write")
diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
index becc9f9ded..e2629eed01 100644
--- a/xen/include/asm-arm/psci.h
+++ b/xen/include/asm-arm/psci.h
@@ -40,6 +40,7 @@ void call_psci_system_reset(void);
 #define PSCI_0_2_FN32_MIGRATE_INFO_TYPE   PSCI_0_2_FN32(6)
 #define PSCI_0_2_FN32_SYSTEM_OFF          PSCI_0_2_FN32(8)
 #define PSCI_0_2_FN32_SYSTEM_RESET        PSCI_0_2_FN32(9)
+#define PSCI_1_0_FN32_PSCI_FEATURES       PSCI_0_2_FN32(10)
 
 #define PSCI_0_2_FN64_CPU_SUSPEND         PSCI_0_2_FN64(1)
 #define PSCI_0_2_FN64_CPU_ON              PSCI_0_2_FN64(3)
diff --git a/xen/include/asm-arm/vpsci.h b/xen/include/asm-arm/vpsci.h
index d6a890f6a2..6d98c3651c 100644
--- a/xen/include/asm-arm/vpsci.h
+++ b/xen/include/asm-arm/vpsci.h
@@ -4,7 +4,7 @@
 #include <asm/psci.h>
 
 /* Number of function implemented by virtual PSCI (only 0.2 or later) */
-#define VPSCI_NR_FUNCS  11
+#define VPSCI_NR_FUNCS  12
 
 /* Functions handle PSCI calls from the guests */
 bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid);
-- 
2.11.0


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

[PATCH 4/7] xen/arm: vsmc: Implement SMCCC 1.1

[Julien Grall]

The new SMC Calling Convention (v1.1) allows for a reduced overhead when
calling into the firmware, and provides a new feature discovery
mechanism. See ARM DEN 00070A.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/vpsci.c        |  1 +
 xen/arch/arm/vsmc.c         | 23 +++++++++++++++++++++++
 xen/include/asm-arm/smccc.h | 15 +++++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 025250a119..e40ba5c5ad 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -215,6 +215,7 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id)
     case PSCI_0_2_FN32_AFFINITY_INFO:
     case PSCI_0_2_FN64_AFFINITY_INFO:
     case PSCI_1_0_FN32_PSCI_FEATURES:
+    case ARM_SMCCC_VERSION_FID:
         return 0;
     default:
         return PSCI_NOT_SUPPORTED;
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index 3d3bd95fee..a708aa5e81 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -81,6 +81,26 @@ static bool fill_function_call_count(struct cpu_user_regs *regs, uint32_t cnt)
     return true;
 }
 
+/* SMCCC interface for ARM Architecture */
+static bool handle_arch(struct cpu_user_regs *regs)
+{
+    uint32_t fid = (uint32_t)get_user_reg(regs, 0);
+
+    switch ( fid )
+    {
+    case ARM_SMCCC_VERSION_FID:
+        set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
+        return true;
+
+    case ARM_SMCCC_ARCH_FEATURES_FID:
+        /* Nothing supported yet */
+        set_user_reg(regs, 0, -1);
+        return true;
+    }
+
+    return false;
+}
+
 /* SMCCC interface for hypervisor. Tell about itself. */
 static bool handle_hypervisor(struct cpu_user_regs *regs)
 {
@@ -188,6 +208,9 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
     {
         switch ( smccc_get_owner(funcid) )
         {
+        case ARM_SMCCC_OWNER_ARCH:
+            handled = handle_arch(regs);
+            break;
         case ARM_SMCCC_OWNER_HYPERVISOR:
             handled = handle_hypervisor(regs);
             break;
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 62b3a8cdf5..431389c118 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -16,6 +16,9 @@
 #ifndef __ASM_ARM_SMCCC_H__
 #define __ASM_ARM_SMCCC_H__
 
+#define ARM_SMCCC_VERSION_1_0   0x10000
+#define ARM_SMCCC_VERSION_1_1   0x10001
+
 /*
  * This file provides common defines for ARM SMC Calling Convention as
  * specified in
@@ -100,6 +103,18 @@ static inline uint32_t smccc_get_owner(register_t funcid)
                        ARM_SMCCC_OWNER_##owner,     \
                        0xFF03)
 
+#define ARM_SMCCC_VERSION_FID                       \
+    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
+                       ARM_SMCCC_CONV_32,           \
+                       ARM_SMCCC_OWNER_ARCH,        \
+                       0x0)                         \
+
+#define ARM_SMCCC_ARCH_FEATURES_FID                 \
+    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
+                       ARM_SMCCC_CONV_32,           \
+                       ARM_SMCCC_OWNER_ARCH,        \
+                       0x1)
+
 /* Only one error code defined in SMCCC */
 #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
 
-- 
2.11.0


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

[PATCH 5/7] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Julien Grall]

SMCCC 1.1 offers firmware-based CPU workarounds. In particular,
SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254
(CVE-2017-5715).

If the hypervisor has some mitigation for this issue, report that we
deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor
workaround on every guest exit.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/vsmc.c         | 22 ++++++++++++++++++++--
 xen/include/asm-arm/smccc.h |  6 ++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index a708aa5e81..144a1cd761 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -18,6 +18,7 @@
 #include <xen/lib.h>
 #include <xen/types.h>
 #include <public/arch-arm/smccc.h>
+#include <asm/cpufeature.h>
 #include <asm/monitor.h>
 #include <asm/regs.h>
 #include <asm/smccc.h>
@@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs)
         return true;
 
     case ARM_SMCCC_ARCH_FEATURES_FID:
-        /* Nothing supported yet */
-        set_user_reg(regs, 0, -1);
+    {
+        uint32_t arch_func_id = get_user_reg(regs, 1);
+        int ret = -1;
+
+        switch ( arch_func_id )
+        {
+        case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
+            if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
+                ret = 0;
+            break;
+        }
+
+        set_user_reg(regs, 0, ret);
+
+        return true;
+    }
+
+    case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
+        /* No return value */
         return true;
     }
 
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 431389c118..b790fac17c 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -115,6 +115,12 @@ static inline uint32_t smccc_get_owner(register_t funcid)
                        ARM_SMCCC_OWNER_ARCH,        \
                        0x1)
 
+#define ARM_SMCCC_ARCH_WORKAROUND_1_FID             \
+    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
+                      ARM_SMCCC_CONV_32,            \
+                      ARM_SMCCC_OWNER_ARCH,         \
+                      0x8000)
+
 /* Only one error code defined in SMCCC */
 #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
 
-- 
2.11.0


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

[PATCH 6/7] xen/arm: Adapt smccc.h to be able to use it in assembly code

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/smccc.h | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index b790fac17c..d24ccb51d8 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -25,18 +25,20 @@
  * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
  */
 
-#define ARM_SMCCC_STD_CALL              0U
-#define ARM_SMCCC_FAST_CALL             1U
+#define ARM_SMCCC_STD_CALL              _AC(0,U)
+#define ARM_SMCCC_FAST_CALL             _AC(1,U)
 #define ARM_SMCCC_TYPE_SHIFT            31
 
-#define ARM_SMCCC_CONV_32               0U
-#define ARM_SMCCC_CONV_64               1U
+#define ARM_SMCCC_CONV_32               _AC(0,U)
+#define ARM_SMCCC_CONV_64               _AC(1,U)
 #define ARM_SMCCC_CONV_SHIFT            30
 
-#define ARM_SMCCC_OWNER_MASK            0x3FU
+#define ARM_SMCCC_OWNER_MASK            _AC(0x3F,U)
 #define ARM_SMCCC_OWNER_SHIFT           24
 
-#define ARM_SMCCC_FUNC_MASK             0xFFFFU
+#define ARM_SMCCC_FUNC_MASK             _AC(0xFFFF,U)
+
+#ifndef __ASSEMBLY__
 
 /* Check if this is fast call. */
 static inline bool smccc_is_fast_call(register_t funcid)
@@ -62,6 +64,8 @@ static inline uint32_t smccc_get_owner(register_t funcid)
     return (funcid >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK;
 }
 
+#endif
+
 /*
  * Construct function identifier from call type (fast or standard),
  * calling convention (32 or 64 bit), service owner and function number.
-- 
2.11.0


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

[PATCH 7/7] xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

[Julien Grall]

The function SMCCC_ARCH_WORKAROUND_1 will be called by the guest for
hardening the branch predictor. So we want the handling to be as fast as
possible.

As the mitigation is applied on every guest exit, we can check for the
call before saving all the context and return very early.

For now, only provide a fast path for HVC64 call. Because the code rely
on 2 registers, x0 and x1 are saved in advanced.

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

---
    guest_sync only handle 64-bit guest, so I have only implemented the
    64-bit side for now. We can discuss whether it is useful to
    implement it for 32-bit guests.

    We could also consider to implement the fast path for SMC64,
    althought a guest should always use HVC.
---
 xen/arch/arm/arm64/entry.S      | 56 +++++++++++++++++++++++++++++++++++++++--
 xen/include/asm-arm/processor.h |  2 ++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 6d99e46f0f..67f96d518f 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -1,6 +1,7 @@
 #include <asm/asm_defns.h>
 #include <asm/regs.h>
 #include <asm/alternative.h>
+#include <asm/smccc.h>
 #include <public/xen.h>
 
 /*
@@ -90,8 +91,12 @@ lr      .req    x30             /* link register */
         .endm
 /*
  * Save state on entry to hypervisor, restore on exit
+ *
+ * save_x0_x1: Does the macro needs to save x0/x1 (default 1). If 0,
+ * we rely on the on x0/x1 to have been saved at the correct position on
+ * the stack before.
  */
-        .macro  entry, hyp, compat
+        .macro  entry, hyp, compat, save_x0_x1=1
         sub     sp, sp, #(UREGS_SPSR_el1 - UREGS_LR) /* CPSR, PC, SP, LR */
         push    x28, x29
         push    x26, x27
@@ -107,7 +112,16 @@ lr      .req    x30             /* link register */
         push    x6, x7
         push    x4, x5
         push    x2, x3
+        /*
+         * The caller may already have saved x0/x1 on the stack at the
+         * correct address and corrupt them with another value. Only
+         * save them if save_x0_x1 == 1.
+         */
+        .if \save_x0_x1 == 1
         push    x0, x1
+        .else
+        sub     sp, sp, #16
+        .endif
 
         .if \hyp == 1        /* Hypervisor mode */
 
@@ -200,7 +214,45 @@ hyp_irq:
         exit    hyp=1
 
 guest_sync:
-        entry   hyp=0, compat=0
+        /*
+         * Save x0, x1 in advance
+         */
+        stp     x0, x1, [sp, #-(UREGS_kernel_sizeof - UREGS_X0)]
+
+        /*
+         * x1 is used because x0 may contain the function identifier.
+         * This avoids to restore x0 from the stack.
+         */
+        mrs     x1, esr_el2
+        lsr     x1, x1, #HSR_EC_SHIFT           /* x1 = ESR_EL2.EC */
+        cmp     x1, #HSR_EC_HVC64
+        b.ne    1f                              /* Not a HVC skip fastpath. */
+
+        mrs     x1, esr_el2
+        and     x1, x1, #0xffff                 /* Check the immediate [0:16] */
+        cbnz    x1, 1f                          /* should be 0 for HVC #0 */
+
+        /*
+         * Fastest path possible for ARM_SMCCC_ARCH_WORKAROUND_1.
+         * The workaround has already been applied on the exception
+         * entry from the guest, so let's quickly get back to the guest.
+         */
+        eor     w0, w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
+        cbnz    w0, 1f
+
+        /*
+         * Clobber both x0 and x1 to prevent leakage. Note that thanks
+         * the eor, x0 = 0.
+         */
+        mov     x1, x0
+        eret
+
+1:
+        /*
+         * 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
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index c0f79d0093..222a02dd99 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -306,6 +306,8 @@
 #define HDCR_TPM        (_AC(1,U)<<6)           /* Trap Performance Monitors accesses */
 #define HDCR_TPMCR      (_AC(1,U)<<5)           /* Trap PMCR accesses */
 
+#define HSR_EC_SHIFT                26
+
 #define HSR_EC_UNKNOWN              0x00
 #define HSR_EC_WFI_WFE              0x01
 #define HSR_EC_CP15_32              0x03
-- 
2.11.0


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

[PATCH 8/9] xen/arm: Park CPUs with a MIDR different from the boot CPU.

[Julien Grall]

Xen does not properly support big.LITTLE platform. All vCPUs of a guest
will always have the MIDR of the boot CPU (see arch_domain_create).
At best the guest may see unreliable performance (vCPU switching between
big and LITTLE), at worst the guest will become unreliable or insecure.

This is becoming more apparent with branch predictor hardening in Linux
because they target a specific kind of CPUs and may not work on other
CPUs.

For the time being, park any CPUs with a MDIR different from the boot
CPU. This will be revisited in the future once Xen gains understanding
of big.LITTLE.

[1] https://lists.xenproject.org/archives/html/xen-devel/2016-12/msg00826.html

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

---

We probably want to backport this as part of XSA-254. Using big.LITTLE
on Xen has never been supported but we didn't make it clearly. This is
becoming more apparent with code targeting specific CPUs.
---
 xen/arch/arm/smpboot.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 1255185a9c..2c2815f9ee 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -292,6 +292,21 @@ void start_secondary(unsigned long boot_phys_offset,
 
     init_traps();
 
+    /*
+     * Currently Xen assumes the platform has only one kind of CPUs.
+     * This assumption does not hold on big.LITTLE platform and may
+     * result to unstability. Better to park them for now.
+     *
+     * TODO: Add big.LITTLE support.
+     */
+    if ( current_cpu_data.midr.bits != boot_cpu_data.midr.bits )
+    {
+        printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR (0x%x).\n",
+               smp_processor_id(), current_cpu_data.midr.bits,
+               boot_cpu_data.midr.bits);
+        stop_cpu();
+    }
+
     mmu_init_secondary_cpu();
 
     gic_init_secondary_cpu();
-- 
2.11.0


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

[PATCH 9/9] xen/arm: Help to know the hardening provided for a CPU

[Julien Grall]

---
 xen/arch/arm/cpuerrata.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 9c7458ef06..6704648b26 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -79,7 +79,8 @@ static bool copy_hyp_vect_bpi(unsigned int slot, const char *hyp_vec_start,
 static bool __maybe_unused
 install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
                          const char *hyp_vec_start,
-                         const char *hyp_vec_end)
+                         const char *hyp_vec_end,
+                         const char *desc)
 {
     static int last_slot = -1;
     static DEFINE_SPINLOCK(bp_lock);
@@ -94,6 +95,9 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
     if ( !entry->matches(entry) )
         return true;
 
+    printk(XENLOG_INFO "CPU%u will %s on exception entry\n",
+           smp_processor_id(), desc);
+
     /*
      * No need to install hardened vector when the processor has
      * ID_AA64PRF0_EL1.CSV2 set.
@@ -157,7 +161,8 @@ static int enable_psci_bp_hardening(void *data)
      */
     if ( psci_ver >= PSCI_VERSION(0, 2) )
         ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
-                                       __psci_hyp_bp_inval_end);
+                                       __psci_hyp_bp_inval_end,
+                                       "call PSCI get version");
     else if ( !warned )
     {
         ASSERT(system_state < SYS_STATE_active);
-- 
2.11.0


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

Re: [PATCH 8/9] xen/arm: Park CPUs with a MIDR different from the boot CPU.

[Julien Grall]

Hmmm sorry, I was not meant to be sent with this series. It is also a 
duplicate of the same patch sent last week.

On 05/02/18 13:20, Julien Grall wrote:
> Xen does not properly support big.LITTLE platform. All vCPUs of a guest
> will always have the MIDR of the boot CPU (see arch_domain_create).
> At best the guest may see unreliable performance (vCPU switching between
> big and LITTLE), at worst the guest will become unreliable or insecure.
> 
> This is becoming more apparent with branch predictor hardening in Linux
> because they target a specific kind of CPUs and may not work on other
> CPUs.
> 
> For the time being, park any CPUs with a MDIR different from the boot
> CPU. This will be revisited in the future once Xen gains understanding
> of big.LITTLE.
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2016-12/msg00826.html
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
> 
> We probably want to backport this as part of XSA-254. Using big.LITTLE
> on Xen has never been supported but we didn't make it clearly. This is
> becoming more apparent with code targeting specific CPUs.
> ---
>   xen/arch/arm/smpboot.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index 1255185a9c..2c2815f9ee 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -292,6 +292,21 @@ void start_secondary(unsigned long boot_phys_offset,
>   
>       init_traps();
>   
> +    /*
> +     * Currently Xen assumes the platform has only one kind of CPUs.
> +     * This assumption does not hold on big.LITTLE platform and may
> +     * result to unstability. Better to park them for now.
> +     *
> +     * TODO: Add big.LITTLE support.
> +     */
> +    if ( current_cpu_data.midr.bits != boot_cpu_data.midr.bits )
> +    {
> +        printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR (0x%x).\n",
> +               smp_processor_id(), current_cpu_data.midr.bits,
> +               boot_cpu_data.midr.bits);
> +        stop_cpu();
> +    }
> +
>       mmu_init_secondary_cpu();
>   
>       gic_init_secondary_cpu();
> 

-- 
Julien Grall

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

Re: [PATCH 9/9] xen/arm: Help to know the hardening provided for a CPU

[Julien Grall]

Hmmm sorry, I was not meant to be sent with this series. I will resend 
it separately with a proper commit message.

On 05/02/18 13:20, Julien Grall wrote:
> ---
>   xen/arch/arm/cpuerrata.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 9c7458ef06..6704648b26 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -79,7 +79,8 @@ static bool copy_hyp_vect_bpi(unsigned int slot, const char *hyp_vec_start,
>   static bool __maybe_unused
>   install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>                            const char *hyp_vec_start,
> -                         const char *hyp_vec_end)
> +                         const char *hyp_vec_end,
> +                         const char *desc)
>   {
>       static int last_slot = -1;
>       static DEFINE_SPINLOCK(bp_lock);
> @@ -94,6 +95,9 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>       if ( !entry->matches(entry) )
>           return true;
>   
> +    printk(XENLOG_INFO "CPU%u will %s on exception entry\n",
> +           smp_processor_id(), desc);
> +
>       /*
>        * No need to install hardened vector when the processor has
>        * ID_AA64PRF0_EL1.CSV2 set.
> @@ -157,7 +161,8 @@ static int enable_psci_bp_hardening(void *data)
>        */
>       if ( psci_ver >= PSCI_VERSION(0, 2) )
>           ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
> -                                       __psci_hyp_bp_inval_end);
> +                                       __psci_hyp_bp_inval_end,
> +                                       "call PSCI get version");
>       else if ( !warned )
>       {
>           ASSERT(system_state < SYS_STATE_active);
> 

-- 
Julien Grall

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

Re: [PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Volodymyr Babchuk]

Hi Julien,

On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
> Currently, the behavior of do_common_cpu will slightly change depending
> on the PSCI version passed in parameter. Looking at the code, more the
> specific 0.2 behavior could move out of the function or adapted for 0.1:
>
>     - x0/r0 can be updated on PSCI 0.1 because general purpose registers
>     are undefined upon CPU on.
>     - PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
>     safer to bail out if the CPU is already on.
>
> Based on this, the parameter 'ver' is removed and do_psci_cpu_on
> (implementation for PSCI 0.1) is adapted to avoid returning
> PSCI_ALREADY_ON.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> ---
>  xen/arch/arm/vpsci.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 884f0fa710..359db884f9 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -22,7 +22,7 @@
>  #include <public/sched.h>
>
>  static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
> -                       register_t context_id,int ver)
> +                            register_t context_id)
>  {
>      struct vcpu *v;
>      struct domain *d = current->domain;
> @@ -40,8 +40,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>      if ( is_64bit_domain(d) && is_thumb )
>          return PSCI_INVALID_PARAMETERS;
>
> -    if ( (ver == PSCI_VERSION(0, 2)) &&
> -            !test_bit(_VPF_down, &v->pause_flags) )
> +    if ( !test_bit(_VPF_down, &v->pause_flags) )
>          return PSCI_ALREADY_ON;
>
>      if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
> @@ -55,18 +54,21 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>      ctxt->ttbr0 = 0;
>      ctxt->ttbr1 = 0;
>      ctxt->ttbcr = 0; /* Defined Reset Value */
> +
> +    /*
> +     * x0/r0_usr are always updated because for PSCI 0.1 the general
> +     * purpose registers are undefined upon CPU_on.
> +     */
>      if ( is_32bit_domain(d) )
>      {
>          ctxt->user_regs.cpsr = PSR_GUEST32_INIT;
> -        if ( ver == PSCI_VERSION(0, 2) )
> -            ctxt->user_regs.r0_usr = context_id;
> +        ctxt->user_regs.r0_usr = context_id;
>      }
>  #ifdef CONFIG_ARM_64
>      else
>      {
>          ctxt->user_regs.cpsr = PSR_GUEST64_INIT;
> -        if ( ver == PSCI_VERSION(0, 2) )
> -            ctxt->user_regs.x0 = context_id;
> +        ctxt->user_regs.x0 = context_id;
>      }
>  #endif
>
> @@ -93,7 +95,14 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>
>  static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point)
>  {
> -    return do_common_cpu_on(vcpuid, entry_point, 0 , PSCI_VERSION(0, 1));
> +    int32_t ret;
> +
> +    ret = do_common_cpu_on(vcpuid, entry_point, 0);
> +    /*
> +     * PSCI 0.1 does not define the return code PSCI_ALREADY_ON.
> +     * Instead, return PSCI_INVALID_PARAMETERS.
> +     */
> +    return (ret == PSCI_ALREADY_ON) ? PSCI_INVALID_PARAMETERS : ret;
>  }
>
>  static int32_t do_psci_cpu_off(uint32_t power_state)
> @@ -133,8 +142,7 @@ static int32_t do_psci_0_2_cpu_on(register_t target_cpu,
>                                    register_t entry_point,
>                                    register_t context_id)
>  {
> -    return do_common_cpu_on(target_cpu, entry_point, context_id,
> -                            PSCI_VERSION(0, 2));
> +    return do_common_cpu_on(target_cpu, entry_point, context_id);
>  }
>
>  static const unsigned long target_affinity_mask[] = {
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 2/7] xen/arm: psci: Rework the PSCI definitions

[Volodymyr Babchuk]

Hi,

On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
> Some PSCI functions are only available in the 32-bit version. After
> recent changes, Xen always needs to know whether the call was made using
> 32-bit id or 64-bit id. So we don't emulate reserved one.
>
> With the current naming scheme, it is not easy to know which call
> supports 32-bit and 64-bit id. So rework the definitions to encode the
> version in the name. From now the functions will be named PSCI_0_2_FNxx
> where xx is 32 or 64.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> ---
>  xen/arch/arm/platforms/seattle.c |  4 ++--
>  xen/arch/arm/psci.c              | 10 +++++-----
>  xen/arch/arm/vpsci.c             | 22 +++++++++++-----------
>  xen/include/asm-arm/psci.h       | 37 +++++++++++++++++++++----------------
>  4 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
> index 22c062293f..893cc17972 100644
> --- a/xen/arch/arm/platforms/seattle.c
> +++ b/xen/arch/arm/platforms/seattle.c
> @@ -33,12 +33,12 @@ static const char * const seattle_dt_compat[] __initconst =
>   */
>  static void seattle_system_reset(void)
>  {
> -    call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
> +    call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
>  }
>
>  static void seattle_system_off(void)
>  {
> -    call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
> +    call_smc(PSCI_0_2_FN32_SYSTEM_OFF, 0, 0, 0);
>  }
>
>  PLATFORM_START(seattle, "SEATTLE")
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 1508a3be3a..5dda35cd7c 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -31,9 +31,9 @@
>   * (native-width) function ID.
>   */
>  #ifdef CONFIG_ARM_64
> -#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64(name)
> +#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN64_##name
>  #else
> -#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32(name)
> +#define PSCI_0_2_FN_NATIVE(name)    PSCI_0_2_FN32_##name
>  #endif
>
>  uint32_t psci_ver;
> @@ -48,13 +48,13 @@ int call_psci_cpu_on(int cpu)
>  void call_psci_system_off(void)
>  {
>      if ( psci_ver > PSCI_VERSION(0, 1) )
> -        call_smc(PSCI_0_2_FN32(SYSTEM_OFF), 0, 0, 0);
> +        call_smc(PSCI_0_2_FN32_SYSTEM_OFF, 0, 0, 0);
>  }
>
>  void call_psci_system_reset(void)
>  {
>      if ( psci_ver > PSCI_VERSION(0, 1) )
> -        call_smc(PSCI_0_2_FN32(SYSTEM_RESET), 0, 0, 0);
> +        call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
>  }
>
>  int __init psci_is_smc_method(const struct dt_device_node *psci)
> @@ -144,7 +144,7 @@ int __init psci_init_0_2(void)
>          }
>      }
>
> -    psci_ver = call_smc(PSCI_0_2_FN32(PSCI_VERSION), 0, 0, 0);
> +    psci_ver = call_smc(PSCI_0_2_FN32_PSCI_VERSION, 0, 0, 0);
>
>      /* For the moment, we only support PSCI 0.2 and PSCI 1.x */
>      if ( psci_ver != PSCI_VERSION(0, 2) && PSCI_VERSION_MAJOR(psci_ver) != 1 )
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 359db884f9..17dab42cf4 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -250,35 +250,35 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
>       */
>      switch ( fid )
>      {
> -    case PSCI_0_2_FN32(PSCI_VERSION):
> +    case PSCI_0_2_FN32_PSCI_VERSION:
>          perfc_incr(vpsci_version);
>          PSCI_SET_RESULT(regs, do_psci_0_2_version());
>          return true;
>
> -    case PSCI_0_2_FN32(CPU_OFF):
> +    case PSCI_0_2_FN32_CPU_OFF:
>          perfc_incr(vpsci_cpu_off);
>          PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off());
>          return true;
>
> -    case PSCI_0_2_FN32(MIGRATE_INFO_TYPE):
> +    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
>          perfc_incr(vpsci_migrate_info_type);
>          PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type());
>          return true;
>
> -    case PSCI_0_2_FN32(SYSTEM_OFF):
> +    case PSCI_0_2_FN32_SYSTEM_OFF:
>          perfc_incr(vpsci_system_off);
>          do_psci_0_2_system_off();
>          PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
>          return true;
>
> -    case PSCI_0_2_FN32(SYSTEM_RESET):
> +    case PSCI_0_2_FN32_SYSTEM_RESET:
>          perfc_incr(vpsci_system_reset);
>          do_psci_0_2_system_reset();
>          PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE);
>          return true;
>
> -    case PSCI_0_2_FN32(CPU_ON):
> -    case PSCI_0_2_FN64(CPU_ON):
> +    case PSCI_0_2_FN32_CPU_ON:
> +    case PSCI_0_2_FN64_CPU_ON:
>      {
>          register_t vcpuid = PSCI_ARG(regs, 1);
>          register_t epoint = PSCI_ARG(regs, 2);
> @@ -289,8 +289,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
>          return true;
>      }
>
> -    case PSCI_0_2_FN32(CPU_SUSPEND):
> -    case PSCI_0_2_FN64(CPU_SUSPEND):
> +    case PSCI_0_2_FN32_CPU_SUSPEND:
> +    case PSCI_0_2_FN64_CPU_SUSPEND:
>      {
>          uint32_t pstate = PSCI_ARG32(regs, 1);
>          register_t epoint = PSCI_ARG(regs, 2);
> @@ -301,8 +301,8 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
>          return true;
>      }
>
> -    case PSCI_0_2_FN32(AFFINITY_INFO):
> -    case PSCI_0_2_FN64(AFFINITY_INFO):
> +    case PSCI_0_2_FN32_AFFINITY_INFO:
> +    case PSCI_0_2_FN64_AFFINITY_INFO:
>      {
>          register_t taff = PSCI_ARG(regs, 1);
>          uint32_t laff = PSCI_ARG32(regs, 2);
> diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
> index 3c44468e72..becc9f9ded 100644
> --- a/xen/include/asm-arm/psci.h
> +++ b/xen/include/asm-arm/psci.h
> @@ -23,22 +23,27 @@ void call_psci_system_off(void);
>  void call_psci_system_reset(void);
>
>  /* PSCI v0.2 interface */
> -#define PSCI_0_2_FN32(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
> -                                               ARM_SMCCC_CONV_32,               \
> -                                               ARM_SMCCC_OWNER_STANDARD,        \
> -                                               PSCI_0_2_FN_##name)
> -#define PSCI_0_2_FN64(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
> -                                               ARM_SMCCC_CONV_64,               \
> -                                               ARM_SMCCC_OWNER_STANDARD,        \
> -                                               PSCI_0_2_FN_##name)
> -#define PSCI_0_2_FN_PSCI_VERSION        0
> -#define PSCI_0_2_FN_CPU_SUSPEND         1
> -#define PSCI_0_2_FN_CPU_OFF             2
> -#define PSCI_0_2_FN_CPU_ON              3
> -#define PSCI_0_2_FN_AFFINITY_INFO       4
> -#define PSCI_0_2_FN_MIGRATE_INFO_TYPE   6
> -#define PSCI_0_2_FN_SYSTEM_OFF          8
> -#define PSCI_0_2_FN_SYSTEM_RESET        9
> +#define PSCI_0_2_FN32(nr) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
> +                                             ARM_SMCCC_CONV_32,               \
> +                                             ARM_SMCCC_OWNER_STANDARD,        \
> +                                             nr)
> +#define PSCI_0_2_FN64(nr) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,             \
> +                                             ARM_SMCCC_CONV_64,               \
> +                                             ARM_SMCCC_OWNER_STANDARD,        \
> +                                             nr)
> +
> +#define PSCI_0_2_FN32_PSCI_VERSION        PSCI_0_2_FN32(0)
> +#define PSCI_0_2_FN32_CPU_SUSPEND         PSCI_0_2_FN32(1)
> +#define PSCI_0_2_FN32_CPU_OFF             PSCI_0_2_FN32(2)
> +#define PSCI_0_2_FN32_CPU_ON              PSCI_0_2_FN32(3)
> +#define PSCI_0_2_FN32_AFFINITY_INFO       PSCI_0_2_FN32(4)
> +#define PSCI_0_2_FN32_MIGRATE_INFO_TYPE   PSCI_0_2_FN32(6)
> +#define PSCI_0_2_FN32_SYSTEM_OFF          PSCI_0_2_FN32(8)
> +#define PSCI_0_2_FN32_SYSTEM_RESET        PSCI_0_2_FN32(9)
> +
> +#define PSCI_0_2_FN64_CPU_SUSPEND         PSCI_0_2_FN64(1)
> +#define PSCI_0_2_FN64_CPU_ON              PSCI_0_2_FN64(3)
> +#define PSCI_0_2_FN64_AFFINITY_INFO       PSCI_0_2_FN64(4)
>
>  /* PSCI v0.2 affinity level state returned by AFFINITY_INFO */
>  #define PSCI_0_2_AFFINITY_LEVEL_ON      0
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 3/7] xen/arm: vpsci: Add support for PSCI 1.1

[Volodymyr Babchuk]

Hi,

On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
> At the moment, Xen provides virtual PSCI interface compliant with 0.1
> and 0.2. Since them, the specification has been updated and the latest
> version is 1.1 (see ARM DEN 0022D).
>
> From an implementation point of view, only PSCI_FEATURES is mandatory.
> The rest is optional and can be left unimplemented for now.
>
> At the same time, the compatible for PSCI node have been updated to
> expose "arm,psci-1.0".
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: mirela.simonovic@aggios.com
>
> ---
>     We may want to provide a way for the toolstack to specify a PSCI
>     version. This could be useful if a guest is expecting a given
>     version.
> ---
>  tools/libxl/libxl_arm.c          |  3 ++-
>  xen/arch/arm/domain_build.c      |  1 +
>  xen/arch/arm/vpsci.c             | 34 +++++++++++++++++++++++++++++++++-
>  xen/include/asm-arm/perfc_defn.h |  1 +
>  xen/include/asm-arm/psci.h       |  1 +
>  xen/include/asm-arm/vpsci.h      |  2 +-
>  6 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 3e46554301..86f59c0d80 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -410,7 +410,8 @@ static int make_psci_node(libxl__gc *gc, void *fdt)
>      res = fdt_begin_node(fdt, "psci");
>      if (res) return res;
>
> -    res = fdt_property_compat(gc, fdt, 2, "arm,psci-0.2","arm,psci");
> +    res = fdt_property_compat(gc, fdt, 3, "arm,psci-1.0",
> +                              "arm,psci-0.2", "arm,psci");
>      if (res) return res;
>
>      res = fdt_property_string(fdt, "method", "hvc");
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 155c952349..941688a2ce 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -637,6 +637,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
>  {
>      int res;
>      const char compat[] =
> +        "arm,psci-1.0""\0"
>          "arm,psci-0.2""\0"
>          "arm,psci";
>
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 17dab42cf4..025250a119 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -115,7 +115,7 @@ static int32_t do_psci_cpu_off(uint32_t power_state)
>
>  static uint32_t do_psci_0_2_version(void)
>  {
> -    return PSCI_VERSION(0, 2);
> +    return PSCI_VERSION(1, 0);
>  }
I'm confused a bit with the versions. In the commit message you
mentioned version 1.1. But there you return version 1,0 from a
function named do_psci_0_2_version().

>
>  static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
> @@ -199,6 +199,28 @@ static void do_psci_0_2_system_reset(void)
>      domain_shutdown(d,SHUTDOWN_reboot);
>  }
>
> +static int32_t do_psci_1_0_features(uint32_t psci_func_id)
> +{
> +    switch ( psci_func_id )
> +    {
> +    case PSCI_0_2_FN32_PSCI_VERSION:
> +    case PSCI_0_2_FN32_CPU_OFF:
> +    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
> +    case PSCI_0_2_FN32_SYSTEM_OFF:
> +    case PSCI_0_2_FN32_SYSTEM_RESET:
> +    case PSCI_0_2_FN32_CPU_ON:
> +    case PSCI_0_2_FN64_CPU_ON:
> +    case PSCI_0_2_FN32_CPU_SUSPEND:
> +    case PSCI_0_2_FN64_CPU_SUSPEND:
> +    case PSCI_0_2_FN32_AFFINITY_INFO:
> +    case PSCI_0_2_FN64_AFFINITY_INFO:
> +    case PSCI_1_0_FN32_PSCI_FEATURES:
Are those functions sorted in a some order?

> +        return 0;
> +    default:
> +        return PSCI_NOT_SUPPORTED;
> +    }
> +}
> +
>  #define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
>  #define PSCI_ARG(reg, n) get_user_reg(reg, n)
>
> @@ -311,6 +333,16 @@ bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid)
>          PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
>          return true;
>      }
> +
> +    case PSCI_1_0_FN32_PSCI_FEATURES:
> +    {
> +        uint32_t psci_func_id = PSCI_ARG32(regs, 1);
> +
> +        perfc_incr(vpsci_features);
> +        PSCI_SET_RESULT(regs, do_psci_1_0_features(psci_func_id));
> +        return true;
> +    }
> +
>      default:
>          return false;
>      }
> diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
> index a7acb7d21c..87866264ca 100644
> --- a/xen/include/asm-arm/perfc_defn.h
> +++ b/xen/include/asm-arm/perfc_defn.h
> @@ -31,6 +31,7 @@ PERFCOUNTER(vpsci_system_off,          "vpsci: system_off")
>  PERFCOUNTER(vpsci_system_reset,        "vpsci: system_reset")
>  PERFCOUNTER(vpsci_cpu_suspend,         "vpsci: cpu_suspend")
>  PERFCOUNTER(vpsci_cpu_affinity_info,   "vpsci: cpu_affinity_info")
> +PERFCOUNTER(vpsci_features,            "vpsci: features")
>
>  PERFCOUNTER(vgicd_reads,                "vgicd: read")
>  PERFCOUNTER(vgicd_writes,               "vgicd: write")
> diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
> index becc9f9ded..e2629eed01 100644
> --- a/xen/include/asm-arm/psci.h
> +++ b/xen/include/asm-arm/psci.h
> @@ -40,6 +40,7 @@ void call_psci_system_reset(void);
>  #define PSCI_0_2_FN32_MIGRATE_INFO_TYPE   PSCI_0_2_FN32(6)
>  #define PSCI_0_2_FN32_SYSTEM_OFF          PSCI_0_2_FN32(8)
>  #define PSCI_0_2_FN32_SYSTEM_RESET        PSCI_0_2_FN32(9)
> +#define PSCI_1_0_FN32_PSCI_FEATURES       PSCI_0_2_FN32(10)
>
>  #define PSCI_0_2_FN64_CPU_SUSPEND         PSCI_0_2_FN64(1)
>  #define PSCI_0_2_FN64_CPU_ON              PSCI_0_2_FN64(3)
> diff --git a/xen/include/asm-arm/vpsci.h b/xen/include/asm-arm/vpsci.h
> index d6a890f6a2..6d98c3651c 100644
> --- a/xen/include/asm-arm/vpsci.h
> +++ b/xen/include/asm-arm/vpsci.h
> @@ -4,7 +4,7 @@
>  #include <asm/psci.h>
>
>  /* Number of function implemented by virtual PSCI (only 0.2 or later) */
> -#define VPSCI_NR_FUNCS  11
> +#define VPSCI_NR_FUNCS  12
>
>  /* Functions handle PSCI calls from the guests */
>  bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid);
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 4/7] xen/arm: vsmc: Implement SMCCC 1.1

[Volodymyr Babchuk]

On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
> The new SMC Calling Convention (v1.1) allows for a reduced overhead when
> calling into the firmware, and provides a new feature discovery
> mechanism. See ARM DEN 00070A.
Сould you please use also a human-readable document name? I remember
that I read "Firmware interfaces for mitigating CVE-2017-5715", but I
can't remember what is ARM DEN 00070A about.

> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/vpsci.c        |  1 +
>  xen/arch/arm/vsmc.c         | 23 +++++++++++++++++++++++
>  xen/include/asm-arm/smccc.h | 15 +++++++++++++++
>  3 files changed, 39 insertions(+)
>
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 025250a119..e40ba5c5ad 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -215,6 +215,7 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id)
>      case PSCI_0_2_FN32_AFFINITY_INFO:
>      case PSCI_0_2_FN64_AFFINITY_INFO:
>      case PSCI_1_0_FN32_PSCI_FEATURES:
> +    case ARM_SMCCC_VERSION_FID:
>          return 0;
>      default:
>          return PSCI_NOT_SUPPORTED;
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index 3d3bd95fee..a708aa5e81 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -81,6 +81,26 @@ static bool fill_function_call_count(struct cpu_user_regs *regs, uint32_t cnt)
>      return true;
>  }
>
> +/* SMCCC interface for ARM Architecture */
> +static bool handle_arch(struct cpu_user_regs *regs)
> +{
> +    uint32_t fid = (uint32_t)get_user_reg(regs, 0);
> +
> +    switch ( fid )
> +    {
> +    case ARM_SMCCC_VERSION_FID:
> +        set_user_reg(regs, 0, ARM_SMCCC_VERSION_1_1);
> +        return true;
> +
> +    case ARM_SMCCC_ARCH_FEATURES_FID:
> +        /* Nothing supported yet */
> +        set_user_reg(regs, 0, -1);
> +        return true;
> +    }
> +
> +    return false;
> +}
> +
>  /* SMCCC interface for hypervisor. Tell about itself. */
>  static bool handle_hypervisor(struct cpu_user_regs *regs)
>  {
> @@ -188,6 +208,9 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
>      {
>          switch ( smccc_get_owner(funcid) )
>          {
> +        case ARM_SMCCC_OWNER_ARCH:
> +            handled = handle_arch(regs);
> +            break;
>          case ARM_SMCCC_OWNER_HYPERVISOR:
>              handled = handle_hypervisor(regs);
>              break;
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 62b3a8cdf5..431389c118 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -16,6 +16,9 @@
>  #ifndef __ASM_ARM_SMCCC_H__
>  #define __ASM_ARM_SMCCC_H__
>
> +#define ARM_SMCCC_VERSION_1_0   0x10000
> +#define ARM_SMCCC_VERSION_1_1   0x10001
> +
>  /*
>   * This file provides common defines for ARM SMC Calling Convention as
>   * specified in
> @@ -100,6 +103,18 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>                         ARM_SMCCC_OWNER_##owner,     \
>                         0xFF03)
>
> +#define ARM_SMCCC_VERSION_FID                       \
> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
> +                       ARM_SMCCC_CONV_32,           \
> +                       ARM_SMCCC_OWNER_ARCH,        \
> +                       0x0)                         \
> +
> +#define ARM_SMCCC_ARCH_FEATURES_FID                 \
> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
> +                       ARM_SMCCC_CONV_32,           \
> +                       ARM_SMCCC_OWNER_ARCH,        \
> +                       0x1)
> +
>  /* Only one error code defined in SMCCC */
>  #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 5/7] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Volodymyr Babchuk]

Hi,

On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
> SMCCC 1.1 offers firmware-based CPU workarounds. In particular,
> SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254
> (CVE-2017-5715).
>
> If the hypervisor has some mitigation for this issue, report that we
> deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor
> workaround on every guest exit.
Just to be sure: is there some way to disable this workaround?


>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/vsmc.c         | 22 ++++++++++++++++++++--
>  xen/include/asm-arm/smccc.h |  6 ++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index a708aa5e81..144a1cd761 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -18,6 +18,7 @@
>  #include <xen/lib.h>
>  #include <xen/types.h>
>  #include <public/arch-arm/smccc.h>
> +#include <asm/cpufeature.h>
>  #include <asm/monitor.h>
>  #include <asm/regs.h>
>  #include <asm/smccc.h>
> @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs)
>          return true;
>
>      case ARM_SMCCC_ARCH_FEATURES_FID:
> -        /* Nothing supported yet */
> -        set_user_reg(regs, 0, -1);
> +    {
> +        uint32_t arch_func_id = get_user_reg(regs, 1);
> +        int ret = -1;
I think that register_t will suit better in this case.

> +
> +        switch ( arch_func_id )
> +        {
> +        case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
> +            if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
> +                ret = 0;
> +            break;
> +        }
> +
> +        set_user_reg(regs, 0, ret);
> +
> +        return true;
> +    }
> +
> +    case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
> +        /* No return value */
>          return true;
>      }
>
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 431389c118..b790fac17c 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -115,6 +115,12 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>                         ARM_SMCCC_OWNER_ARCH,        \
>                         0x1)
>
> +#define ARM_SMCCC_ARCH_WORKAROUND_1_FID             \
> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
> +                      ARM_SMCCC_CONV_32,            \
> +                      ARM_SMCCC_OWNER_ARCH,         \
> +                      0x8000)
> +
>  /* Only one error code defined in SMCCC */
>  #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 6/7] xen/arm: Adapt smccc.h to be able to use it in assembly code

[Volodymyr Babchuk]

Hi,

On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> ---
>  xen/include/asm-arm/smccc.h | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index b790fac17c..d24ccb51d8 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -25,18 +25,20 @@
>   * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
>   */
>
> -#define ARM_SMCCC_STD_CALL              0U
> -#define ARM_SMCCC_FAST_CALL             1U
> +#define ARM_SMCCC_STD_CALL              _AC(0,U)
> +#define ARM_SMCCC_FAST_CALL             _AC(1,U)
>  #define ARM_SMCCC_TYPE_SHIFT            31
>
> -#define ARM_SMCCC_CONV_32               0U
> -#define ARM_SMCCC_CONV_64               1U
> +#define ARM_SMCCC_CONV_32               _AC(0,U)
> +#define ARM_SMCCC_CONV_64               _AC(1,U)
>  #define ARM_SMCCC_CONV_SHIFT            30
>
> -#define ARM_SMCCC_OWNER_MASK            0x3FU
> +#define ARM_SMCCC_OWNER_MASK            _AC(0x3F,U)
>  #define ARM_SMCCC_OWNER_SHIFT           24
>
> -#define ARM_SMCCC_FUNC_MASK             0xFFFFU
> +#define ARM_SMCCC_FUNC_MASK             _AC(0xFFFF,U)
> +
> +#ifndef __ASSEMBLY__
>
>  /* Check if this is fast call. */
>  static inline bool smccc_is_fast_call(register_t funcid)
> @@ -62,6 +64,8 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>      return (funcid >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK;
>  }
>
> +#endif
> +
>  /*
>   * Construct function identifier from call type (fast or standard),
>   * calling convention (32 or 64 bit), service owner and function number.
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 7/7] xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

[Volodymyr Babchuk]

Hi,

On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
> The function SMCCC_ARCH_WORKAROUND_1 will be called by the guest for
> hardening the branch predictor. So we want the handling to be as fast as
> possible.
>
> As the mitigation is applied on every guest exit, we can check for the
> call before saving all the context and return very early.
Have you tried any benchmarks? How big is the benefit?

>
> For now, only provide a fast path for HVC64 call. Because the code rely
> on 2 registers, x0 and x1 are saved in advanced.
Is there a typo? Should it be "advance"?

>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> ---
>     guest_sync only handle 64-bit guest, so I have only implemented the
>     64-bit side for now. We can discuss whether it is useful to
>     implement it for 32-bit guests.
>
>     We could also consider to implement the fast path for SMC64,
>     althought a guest should always use HVC.
I can imagine a guest that know nothing about virtualization and use
SMC as a conduit. But I can't provide real world example, thou.

> ---
>  xen/arch/arm/arm64/entry.S      | 56 +++++++++++++++++++++++++++++++++++++++--
>  xen/include/asm-arm/processor.h |  2 ++
>  2 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 6d99e46f0f..67f96d518f 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -1,6 +1,7 @@
>  #include <asm/asm_defns.h>
>  #include <asm/regs.h>
>  #include <asm/alternative.h>
> +#include <asm/smccc.h>
>  #include <public/xen.h>
>
>  /*
> @@ -90,8 +91,12 @@ lr      .req    x30             /* link register */
>          .endm
>  /*
>   * Save state on entry to hypervisor, restore on exit
> + *
> + * save_x0_x1: Does the macro needs to save x0/x1 (default 1). If 0,
> + * we rely on the on x0/x1 to have been saved at the correct position on
> + * the stack before.
>   */
> -        .macro  entry, hyp, compat
> +        .macro  entry, hyp, compat, save_x0_x1=1
>          sub     sp, sp, #(UREGS_SPSR_el1 - UREGS_LR) /* CPSR, PC, SP, LR */
>          push    x28, x29
>          push    x26, x27
> @@ -107,7 +112,16 @@ lr      .req    x30             /* link register */
>          push    x6, x7
>          push    x4, x5
>          push    x2, x3
> +        /*
> +         * The caller may already have saved x0/x1 on the stack at the
> +         * correct address and corrupt them with another value. Only
> +         * save them if save_x0_x1 == 1.
> +         */
> +        .if \save_x0_x1 == 1
>          push    x0, x1
> +        .else
> +        sub     sp, sp, #16
> +        .endif
>
>          .if \hyp == 1        /* Hypervisor mode */
>
> @@ -200,7 +214,45 @@ hyp_irq:
>          exit    hyp=1
>
>  guest_sync:
> -        entry   hyp=0, compat=0
> +        /*
> +         * Save x0, x1 in advance
> +         */
> +        stp     x0, x1, [sp, #-(UREGS_kernel_sizeof - UREGS_X0)]
> +
> +        /*
> +         * x1 is used because x0 may contain the function identifier.
> +         * This avoids to restore x0 from the stack.
> +         */
> +        mrs     x1, esr_el2
> +        lsr     x1, x1, #HSR_EC_SHIFT           /* x1 = ESR_EL2.EC */
> +        cmp     x1, #HSR_EC_HVC64
> +        b.ne    1f                              /* Not a HVC skip fastpath. */
> +
> +        mrs     x1, esr_el2
> +        and     x1, x1, #0xffff                 /* Check the immediate [0:16] */
> +        cbnz    x1, 1f                          /* should be 0 for HVC #0 */
> +
> +        /*
> +         * Fastest path possible for ARM_SMCCC_ARCH_WORKAROUND_1.
> +         * The workaround has already been applied on the exception
> +         * entry from the guest, so let's quickly get back to the guest.
> +         */
> +        eor     w0, w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
> +        cbnz    w0, 1f
> +
> +        /*
> +         * Clobber both x0 and x1 to prevent leakage. Note that thanks
> +         * the eor, x0 = 0.
> +         */
> +        mov     x1, x0
> +        eret
> +
> +1:
> +        /*
> +         * 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
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index c0f79d0093..222a02dd99 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -306,6 +306,8 @@
>  #define HDCR_TPM        (_AC(1,U)<<6)           /* Trap Performance Monitors accesses */
>  #define HDCR_TPMCR      (_AC(1,U)<<5)           /* Trap PMCR accesses */
>
> +#define HSR_EC_SHIFT                26
> +
>  #define HSR_EC_UNKNOWN              0x00
>  #define HSR_EC_WFI_WFE              0x01
>  #define HSR_EC_CP15_32              0x03
> --
> 2.11.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 3/7] xen/arm: vpsci: Add support for PSCI 1.1

[Julien Grall]

On 02/06/2018 04:07 PM, Volodymyr Babchuk wrote:
> Hi,

Hi Volodymyr,

Thank you for the review.

> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>> At the moment, Xen provides virtual PSCI interface compliant with 0.1
>> and 0.2. Since them, the specification has been updated and the latest
>> version is 1.1 (see ARM DEN 0022D).
>>
>>  From an implementation point of view, only PSCI_FEATURES is mandatory.
>> The rest is optional and can be left unimplemented for now.
>>
>> At the same time, the compatible for PSCI node have been updated to
>> expose "arm,psci-1.0".
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: mirela.simonovic@aggios.com
>>
>> ---
>>      We may want to provide a way for the toolstack to specify a PSCI
>>      version. This could be useful if a guest is expecting a given
>>      version.
>> ---
>>   tools/libxl/libxl_arm.c          |  3 ++-
>>   xen/arch/arm/domain_build.c      |  1 +
>>   xen/arch/arm/vpsci.c             | 34 +++++++++++++++++++++++++++++++++-
>>   xen/include/asm-arm/perfc_defn.h |  1 +
>>   xen/include/asm-arm/psci.h       |  1 +
>>   xen/include/asm-arm/vpsci.h      |  2 +-
>>   6 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index 3e46554301..86f59c0d80 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -410,7 +410,8 @@ static int make_psci_node(libxl__gc *gc, void *fdt)
>>       res = fdt_begin_node(fdt, "psci");
>>       if (res) return res;
>>
>> -    res = fdt_property_compat(gc, fdt, 2, "arm,psci-0.2","arm,psci");
>> +    res = fdt_property_compat(gc, fdt, 3, "arm,psci-1.0",
>> +                              "arm,psci-0.2", "arm,psci");
>>       if (res) return res;
>>
>>       res = fdt_property_string(fdt, "method", "hvc");
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 155c952349..941688a2ce 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -637,6 +637,7 @@ static int make_psci_node(void *fdt, const struct dt_device_node *parent)
>>   {
>>       int res;
>>       const char compat[] =
>> +        "arm,psci-1.0""\0"
>>           "arm,psci-0.2""\0"
>>           "arm,psci";
>>
>> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
>> index 17dab42cf4..025250a119 100644
>> --- a/xen/arch/arm/vpsci.c
>> +++ b/xen/arch/arm/vpsci.c
>> @@ -115,7 +115,7 @@ static int32_t do_psci_cpu_off(uint32_t power_state)
>>
>>   static uint32_t do_psci_0_2_version(void)
>>   {
>> -    return PSCI_VERSION(0, 2);
>> +    return PSCI_VERSION(1, 0);
>>   }
> I'm confused a bit with the versions. In the commit message you
> mentioned version 1.1. But there you return version 1,0 from a
> function named do_psci_0_2_version().

So the function names are implemented based on when they were added in 
the specification. This makes slightly easier if we ever decide to 
expose 0.2 (or 1.0/1.1) only PSCI in the guest.

Also, good catch for the returned version. I first implemented the patch 
for 1.0 and forgot to update it.

> 
>>
>>   static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
>> @@ -199,6 +199,28 @@ static void do_psci_0_2_system_reset(void)
>>       domain_shutdown(d,SHUTDOWN_reboot);
>>   }
>>
>> +static int32_t do_psci_1_0_features(uint32_t psci_func_id)
>> +{
>> +    switch ( psci_func_id )
>> +    {
>> +    case PSCI_0_2_FN32_PSCI_VERSION:
>> +    case PSCI_0_2_FN32_CPU_OFF:
>> +    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
>> +    case PSCI_0_2_FN32_SYSTEM_OFF:
>> +    case PSCI_0_2_FN32_SYSTEM_RESET:
>> +    case PSCI_0_2_FN32_CPU_ON:
>> +    case PSCI_0_2_FN64_CPU_ON:
>> +    case PSCI_0_2_FN32_CPU_SUSPEND:
>> +    case PSCI_0_2_FN64_CPU_SUSPEND:
>> +    case PSCI_0_2_FN32_AFFINITY_INFO:
>> +    case PSCI_0_2_FN64_AFFINITY_INFO:
>> +    case PSCI_1_0_FN32_PSCI_FEATURES:
> Are those functions sorted in a some order?

They were meant to be sorted by the function ID. Thought it looks like 
it is not the case, I will fix that in the next version.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 4/7] xen/arm: vsmc: Implement SMCCC 1.1

[Julien Grall]

Hi,

On 02/06/2018 04:18 PM, Volodymyr Babchuk wrote:
> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>> The new SMC Calling Convention (v1.1) allows for a reduced overhead when
>> calling into the firmware, and provides a new feature discovery
>> mechanism. See ARM DEN 00070A.
> Сould you please use also a human-readable document name? I remember
> that I read "Firmware interfaces for mitigating CVE-2017-5715", but I
> can't remember what is ARM DEN 00070A about.

The reason I am using ARM DEN 0070A is because the name does not give 
you revision of the specification. So you can't know whether you use rev 
A or B. As new revision may introduce/change behavior, this is very 
helpful to know which specific revision that was used to write the code.

It is also much easier to find on the web the identifier than the title 
as you directly reach to a given version

Anyway, I can mention the full name of the specification in the commit.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 5/7] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Julien Grall]

On 02/06/2018 04:23 PM, Volodymyr Babchuk wrote:
> Hi,

Hi,

> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>> SMCCC 1.1 offers firmware-based CPU workarounds. In particular,
>> SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254
>> (CVE-2017-5715).
>>
>> If the hypervisor has some mitigation for this issue, report that we
>> deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor
>> workaround on every guest exit.
> Just to be sure: is there some way to disable this workaround?

In which context? If the platform does not have any processor affected 
by variant 2, then the workaround will not be enabled.

In case of Linux, this workaround will only be called on affected 
processors.

> 
> 
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>   xen/arch/arm/vsmc.c         | 22 ++++++++++++++++++++--
>>   xen/include/asm-arm/smccc.h |  6 ++++++
>>   2 files changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
>> index a708aa5e81..144a1cd761 100644
>> --- a/xen/arch/arm/vsmc.c
>> +++ b/xen/arch/arm/vsmc.c
>> @@ -18,6 +18,7 @@
>>   #include <xen/lib.h>
>>   #include <xen/types.h>
>>   #include <public/arch-arm/smccc.h>
>> +#include <asm/cpufeature.h>
>>   #include <asm/monitor.h>
>>   #include <asm/regs.h>
>>   #include <asm/smccc.h>
>> @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs)
>>           return true;
>>
>>       case ARM_SMCCC_ARCH_FEATURES_FID:
>> -        /* Nothing supported yet */
>> -        set_user_reg(regs, 0, -1);
>> +    {
>> +        uint32_t arch_func_id = get_user_reg(regs, 1);
>> +        int ret = -1;
> I think that register_t will suit better in this case.

Well no. The return in the spec is int32 and will fit in w0. register_t 
is either 32-bit or 64-bit. So int is the right type here.

> 
>> +
>> +        switch ( arch_func_id )
>> +        {
>> +        case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
>> +            if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
>> +                ret = 0;
>> +            break;
>> +        }
>> +
>> +        set_user_reg(regs, 0, ret);
>> +
>> +        return true;
>> +    }
>> +
>> +    case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
>> +        /* No return value */
>>           return true;
>>       }
>>
>> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
>> index 431389c118..b790fac17c 100644
>> --- a/xen/include/asm-arm/smccc.h
>> +++ b/xen/include/asm-arm/smccc.h
>> @@ -115,6 +115,12 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>>                          ARM_SMCCC_OWNER_ARCH,        \
>>                          0x1)
>>
>> +#define ARM_SMCCC_ARCH_WORKAROUND_1_FID             \
>> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
>> +                      ARM_SMCCC_CONV_32,            \
>> +                      ARM_SMCCC_OWNER_ARCH,         \
>> +                      0x8000)
>> +
>>   /* Only one error code defined in SMCCC */
>>   #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>>
>> --
>> 2.11.0
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xenproject.org
>> https://lists.xenproject.org/mailman/listinfo/xen-devel

Cheers,

-- 
Julien Grall

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

Re: [PATCH 7/7] xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

[Julien Grall]

Hi,

On 02/06/2018 04:36 PM, Volodymyr Babchuk wrote:
> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>> The function SMCCC_ARCH_WORKAROUND_1 will be called by the guest for
>> hardening the branch predictor. So we want the handling to be as fast as
>> possible.
>>
>> As the mitigation is applied on every guest exit, we can check for the
>> call before saving all the context and return very early.
> Have you tried any benchmarks? How big is the benefit?

I have benchmarked but I can't share the result. I can give you an idea 
on how this could benefits Xen.

Linux will call the workaround on every context switch between process. 
So imagine for each context switch, you have will enter in Xen and in 
the following order:
	1) enter Xen
	2) apply the workaround which means calling EL3.
	3) save part of the guest context
	4) call enter_hypervisor_head that will sync the vGIC state
	5) detect you actually call SMCCC_ARCH_WORKAROUND_1 that will do nothing
	6) call leave_hypervisor_tail that will sync back the vGIC state and 
execute softirq (that could reschedule the vCPU)
  	7) restore the guest context
	8) return to the guest

So effectively, instead of executing hundreds (if not thousands) 
instructions each time, you will end up only executing less than 50 
instructions.

>>
>> For now, only provide a fast path for HVC64 call. Because the code rely
>> on 2 registers, x0 and x1 are saved in advanced.
> Is there a typo? Should it be "advance"?
> 
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      guest_sync only handle 64-bit guest, so I have only implemented the
>>      64-bit side for now. We can discuss whether it is useful to
>>      implement it for 32-bit guests.
>>
>>      We could also consider to implement the fast path for SMC64,
>>      althought a guest should always use HVC.
> I can imagine a guest that know nothing about virtualization and use
> SMC as a conduit. But I can't provide real world example, thou.

Someone can easily send a follow-up patch for that if it is deemed 
necessary.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 4/7] xen/arm: vsmc: Implement SMCCC 1.1

[Volodymyr Babchuk]

Hi Julien,

On 6 February 2018 at 20:04, Julien Grall <julien.grall@arm.com> wrote:
> Hi,
>
> On 02/06/2018 04:18 PM, Volodymyr Babchuk wrote:
>>
>> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>>>
>>> The new SMC Calling Convention (v1.1) allows for a reduced overhead when
>>> calling into the firmware, and provides a new feature discovery
>>> mechanism. See ARM DEN 00070A.
>>
>> Сould you please use also a human-readable document name? I remember
>> that I read "Firmware interfaces for mitigating CVE-2017-5715", but I
>> can't remember what is ARM DEN 00070A about.
>
>
> The reason I am using ARM DEN 0070A is because the name does not give you
> revision of the specification. So you can't know whether you use rev A or B.
> As new revision may introduce/change behavior, this is very helpful to know
> which specific revision that was used to write the code.

Yes, this is true.
> It is also much easier to find on the web the identifier than the title as
> you directly reach to a given version
>

And this is also true.
> Anyway, I can mention the full name of the specification in the commit.
Yes, this is exactly what I asked. It would be good to have *both*
document ID and human readable name so one can quickly understood
about what document you are speaking, without googling its ID.


-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 7/7] xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

[Volodymyr Babchuk]

Hi Julien,


On 6 February 2018 at 20:33, Julien Grall <julien.grall@arm.com> wrote:
> Hi,
>
> On 02/06/2018 04:36 PM, Volodymyr Babchuk wrote:
>>
>> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>>>
>>> The function SMCCC_ARCH_WORKAROUND_1 will be called by the guest for
>>> hardening the branch predictor. So we want the handling to be as fast as
>>> possible.
>>>
>>> As the mitigation is applied on every guest exit, we can check for the
>>> call before saving all the context and return very early.
>>
>> Have you tried any benchmarks? How big is the benefit?
>
>
> I have benchmarked but I can't share the result. I can give you an idea on
> how this could benefits Xen.
>
> Linux will call the workaround on every context switch between process. So
> imagine for each context switch, you have will enter in Xen and in the
> following order:
>         1) enter Xen
>         2) apply the workaround which means calling EL3.
>         3) save part of the guest context
>         4) call enter_hypervisor_head that will sync the vGIC state
>         5) detect you actually call SMCCC_ARCH_WORKAROUND_1 that will do
> nothing
>         6) call leave_hypervisor_tail that will sync back the vGIC state and
> execute softirq (that could reschedule the vCPU)
>         7) restore the guest context
>         8) return to the guest
>
> So effectively, instead of executing hundreds (if not thousands)
> instructions each time, you will end up only executing less than 50
> instructions.
Sounds fine.

>
>>>
>>> For now, only provide a fast path for HVC64 call. Because the code rely
>>> on 2 registers, x0 and x1 are saved in advanced.
>>
>> Is there a typo? Should it be "advance"?
>>
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
Provided that above typo is fixed:

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>>
>>> ---
>>>      guest_sync only handle 64-bit guest, so I have only implemented the
>>>      64-bit side for now. We can discuss whether it is useful to
>>>      implement it for 32-bit guests.
>>>
>>>      We could also consider to implement the fast path for SMC64,
>>>      althought a guest should always use HVC.
>>
>> I can imagine a guest that know nothing about virtualization and use
>> SMC as a conduit. But I can't provide real world example, thou.
I'm okay with this.


-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 5/7] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Volodymyr Babchuk]

Hi,

On 6 February 2018 at 20:12, Julien Grall <julien.grall@arm.com> wrote:
> On 02/06/2018 04:23 PM, Volodymyr Babchuk wrote:
>>
>> Hi,
>
>
> Hi,
>
>> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>>>
>>> SMCCC 1.1 offers firmware-based CPU workarounds. In particular,
>>> SMCCC_ARCH_WORKAROUND_1 provides BP hardening for variant 2 of XSA-254
>>> (CVE-2017-5715).
>>>
>>> If the hypervisor has some mitigation for this issue, report that we
>>> deal with it using SMCCC_ARCH_WORKAROUND_1, as we apply the hypervisor
>>> workaround on every guest exit.
>>
>> Just to be sure: is there some way to disable this workaround?
>
>
> In which context? If the platform does not have any processor affected by
> variant 2, then the workaround will not be enabled.
Yes, right. I missed CPU caps check below.

> In case of Linux, this workaround will only be called on affected
> processors.
>
>
>>
>>
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

>>> ---
>>>   xen/arch/arm/vsmc.c         | 22 ++++++++++++++++++++--
>>>   xen/include/asm-arm/smccc.h |  6 ++++++
>>>   2 files changed, 26 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
>>> index a708aa5e81..144a1cd761 100644
>>> --- a/xen/arch/arm/vsmc.c
>>> +++ b/xen/arch/arm/vsmc.c
>>> @@ -18,6 +18,7 @@
>>>   #include <xen/lib.h>
>>>   #include <xen/types.h>
>>>   #include <public/arch-arm/smccc.h>
>>> +#include <asm/cpufeature.h>
>>>   #include <asm/monitor.h>
>>>   #include <asm/regs.h>
>>>   #include <asm/smccc.h>
>>> @@ -93,8 +94,25 @@ static bool handle_arch(struct cpu_user_regs *regs)
>>>           return true;
>>>
>>>       case ARM_SMCCC_ARCH_FEATURES_FID:
>>> -        /* Nothing supported yet */
>>> -        set_user_reg(regs, 0, -1);
>>> +    {
>>> +        uint32_t arch_func_id = get_user_reg(regs, 1);
>>> +        int ret = -1;
>>
>> I think that register_t will suit better in this case.
>
>
> Well no. The return in the spec is int32 and will fit in w0. register_t is
> either 32-bit or 64-bit. So int is the right type here.
Ah, yes, you are right.

>
>>
>>> +
>>> +        switch ( arch_func_id )
>>> +        {
>>> +        case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
>>> +            if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
>>> +                ret = 0;
>>> +            break;
>>> +        }
>>> +
>>> +        set_user_reg(regs, 0, ret);
>>> +
>>> +        return true;
>>> +    }
>>> +
>>> +    case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
>>> +        /* No return value */
>>>           return true;
>>>       }
>>>
>>> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
>>> index 431389c118..b790fac17c 100644
>>> --- a/xen/include/asm-arm/smccc.h
>>> +++ b/xen/include/asm-arm/smccc.h
>>> @@ -115,6 +115,12 @@ static inline uint32_t smccc_get_owner(register_t
>>> funcid)
>>>                          ARM_SMCCC_OWNER_ARCH,        \
>>>                          0x1)
>>>
>>> +#define ARM_SMCCC_ARCH_WORKAROUND_1_FID             \
>>> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,         \
>>> +                      ARM_SMCCC_CONV_32,            \
>>> +                      ARM_SMCCC_OWNER_ARCH,         \
>>> +                      0x8000)
>>> +
>>>   /* Only one error code defined in SMCCC */
>>>   #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>>>
>>> --
>>> 2.11.0
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xenproject.org
>>> https://lists.xenproject.org/mailman/listinfo/xen-devel
>
>
> Cheers,
>
> --
> Julien Grall



-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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

Re: [PATCH 0/7] xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update

[Manish Jaggi]

Hi Julien,


On 02/05/2018 06:50 PM, Julien Grall wrote:
> Hi all,
>
> Arm has recently published a SMC Calling Convention (SMCCC)
> specification update [1] that provides an optimised calling convention
> and optional, discoverable support for mitigating CVE-2017-5715 (XSA-254
> variant 2). ARM Trusted Firmware (ATF) has already gained such an
> implementation[2].
>
> This series addresses a few things:
>
>      - It provides a Xen implementation of PSCI v1.0, which is a prerequisite
>        for being able to discover SMCCC v1.1.
>      - It allows Xen to advertise SMCCC v1.1
>      - It implements Xen support for the ARM_WORKAROUND_1 function that is used
>        to mitigate CVE-2017-5715 (if such mitigation is available on the
>        hypervisor).
>
> This method is intended to fully replace the initial PSCI_GET_VERSION
> approach. Although PSCI_GET_VERSION still works, it has an obvious
> overhead and is called on some of the hottest paths. We expect
> ARCH_WORKAROUND_1 to be much faster.
>
> Another series will be sent to allow the hypervisor discovering SMCCC 1.1 and
> use it for the mitigation.
>
> This series is based on the "xen/arm: SMCCC fixes and PSCI clean-up" one [3].
>
> Cheers,
>
> [1]: https://developer.arm.com/-/media/developer/pdf/ARM%20DEN%200070A%20Firmware%20interfaces%20for%20mitigating%20CVE-2017-5715_V1.0.pdf
This link is not working.
>
> [2]: https://github.com/ARM-software/arm-trusted-firmware/pull/1240
>
> [3] https://lists.xenproject.org/archives/html/xen-devel/2018-02/msg00117.html
>
> Julien Grall (7):
>    xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu
>    xen/arm: psci: Rework the PSCI definitions
>    xen/arm: vpsci: Add support for PSCI 1.1
>    xen/arm: vsmc: Implement SMCCC 1.1
>    xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support
>    xen/arm: Adapt smccc.h to be able to use it in assembly code
>    xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1
>
>   tools/libxl/libxl_arm.c          |  3 +-
>   xen/arch/arm/arm64/entry.S       | 56 +++++++++++++++++++++++++-
>   xen/arch/arm/domain_build.c      |  1 +
>   xen/arch/arm/platforms/seattle.c |  4 +-
>   xen/arch/arm/psci.c              | 10 ++---
>   xen/arch/arm/vpsci.c             | 85 +++++++++++++++++++++++++++++-----------
>   xen/arch/arm/vsmc.c              | 41 +++++++++++++++++++
>   xen/include/asm-arm/perfc_defn.h |  1 +
>   xen/include/asm-arm/processor.h  |  2 +
>   xen/include/asm-arm/psci.h       | 38 ++++++++++--------
>   xen/include/asm-arm/smccc.h      | 37 ++++++++++++++---
>   xen/include/asm-arm/vpsci.h      |  2 +-
>   12 files changed, 225 insertions(+), 55 deletions(-)
>


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

Re: [PATCH 0/7] xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update

[Julien Grall]

On 08/02/18 16:26, Manish Jaggi wrote:
> On 02/05/2018 06:50 PM, Julien Grall wrote:
>> Hi all,
>>
>> Arm has recently published a SMC Calling Convention (SMCCC)
>> specification update [1] that provides an optimised calling convention
>> and optional, discoverable support for mitigating CVE-2017-5715 (XSA-254
>> variant 2). ARM Trusted Firmware (ATF) has already gained such an
>> implementation[2].
>>
>> This series addresses a few things:
>>
>>      - It provides a Xen implementation of PSCI v1.0, which is a 
>> prerequisite
>>        for being able to discover SMCCC v1.1.
>>      - It allows Xen to advertise SMCCC v1.1
>>      - It implements Xen support for the ARM_WORKAROUND_1 function 
>> that is used
>>        to mitigate CVE-2017-5715 (if such mitigation is available on the
>>        hypervisor).
>>
>> This method is intended to fully replace the initial PSCI_GET_VERSION
>> approach. Although PSCI_GET_VERSION still works, it has an obvious
>> overhead and is called on some of the hottest paths. We expect
>> ARCH_WORKAROUND_1 to be much faster.
>>
>> Another series will be sent to allow the hypervisor discovering SMCCC 
>> 1.1 and
>> use it for the mitigation.
>>
>> This series is based on the "xen/arm: SMCCC fixes and PSCI clean-up" 
>> one [3].
>>
>> Cheers,
>>
>> [1]: 
>> https://developer.arm.com/-/media/developer/pdf/ARM%20DEN%200070A%20Firmware%20interfaces%20for%20mitigating%20CVE-2017-5715_V1.0.pdf 
>>
> This link is not working.

Because the link has been relocated since then. You can find it at:

https://developer.arm.com/support/security-update/downloads

Cheers,

-- 
Julien Grall

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

Re: [PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Julien Grall]

On 06/02/18 15:42, Volodymyr Babchuk wrote:
> Hi Julien,

Hi Volodymyr,

> On 5 February 2018 at 15:20, Julien Grall <julien.grall@arm.com> wrote:
>> Currently, the behavior of do_common_cpu will slightly change depending
>> on the PSCI version passed in parameter. Looking at the code, more the
>> specific 0.2 behavior could move out of the function or adapted for 0.1:
>>
>>      - x0/r0 can be updated on PSCI 0.1 because general purpose registers
>>      are undefined upon CPU on.
>>      - PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
>>      safer to bail out if the CPU is already on.
>>
>> Based on this, the parameter 'ver' is removed and do_psci_cpu_on
>> (implementation for PSCI 0.1) is adapted to avoid returning
>> PSCI_ALREADY_ON.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

Thank you for the reviewed. FIY, I moved that patch towards the end of 
the series as it is not necessary for backporting. I kept your 
reviewed-by because there are no clash.

I hope that is fine for you.

Cheers,

-- 
Julien Grall

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

Re: [PATCH 1/7] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Volodymyr Babchuk]

Hi Julien,

On 8 February 2018 at 20:12, Julien Grall <julien.grall@arm.com> wrote:

Currently, the behavior of do_common_cpu will slightly change depending
>>> on the PSCI version passed in parameter. Looking at the code, more the
>>> specific 0.2 behavior could move out of the function or adapted for 0.1:
>>>
>>>      - x0/r0 can be updated on PSCI 0.1 because general purpose registers
>>>      are undefined upon CPU on.
>>>      - PSCI 0.1 does not defined PSCI_ALREADY_ON. However, it would be
>>>      safer to bail out if the CPU is already on.
>>>
>>> Based on this, the parameter 'ver' is removed and do_psci_cpu_on
>>> (implementation for PSCI 0.1) is adapted to avoid returning
>>> PSCI_ALREADY_ON.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>
>
> Thank you for the reviewed. FIY, I moved that patch towards the end of the
> series as it is not necessary for backporting. I kept your reviewed-by
> because there are no clash.
>
> I hope that is fine for you.
Yes, I'm perfectly fine with this.


-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@gmail.com

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