[PATCH v3 00/17] xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update

[PATCH v3 00/17] 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 guest support for the ARM_WORKAROUND_1 function that is used
    to mitigate CVE-2017-5715 (if such mitigation is available on the
    hypervisor).
    - It adds Xen support for branch predictor hardening via
    ARM_WORKAROUND_1 if the firmware supports it.

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.

Cheers,

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

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

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

Julien Grall (17):
  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
  xen/arm64: Print a per-CPU message with the BP hardening method used
  xen/arm: smccc: Add macros SMCCC_VERSION, SMCCC_VERSION_{MINOR, MAJOR}
  xen/arm: psci: Detect SMCCC version
  xen/arm: smccc: Implement SMCCC v1.1 inline primitive
  xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support
  xen/arm64: Kill PSCI_GET_VERSION as a variant-2 workaround
  xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu
  xen/arm: psci: Consolidate PSCI version print
  xen/arm: psci: Prefix with static any functions not exported
  xen/arm: vpsci: Update the return type for MIGRATE_INFO_TYPE
  xen/arm: vpsci: Introduce and use PSCI_INVALID_ADDRESS
  xen/arm: vpsci: Rework the logic to start AArch32 vCPU in Thumb mode

 tools/libxl/libxl_arm.c          |   3 +-
 xen/arch/arm/arm64/bpi.S         |  34 +++-----
 xen/arch/arm/arm64/entry.S       |  56 ++++++++++++-
 xen/arch/arm/cpuerrata.c         |  55 +++++++++----
 xen/arch/arm/domain_build.c      |   1 +
 xen/arch/arm/psci.c              |  48 +++++++++--
 xen/arch/arm/vpsci.c             |  89 ++++++++++++++++----
 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       |   2 +
 xen/include/asm-arm/smccc.h      | 174 +++++++++++++++++++++++++++++++++++++--
 xen/include/asm-arm/vpsci.h      |   2 +-
 13 files changed, 429 insertions(+), 79 deletions(-)

-- 
2.11.0


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

[PATCH v3 01/17] 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>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.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.

    Changes in v3:
        - Add Wei's acked-by
        - Add Volodymyr's reviewed-by

    Changes in v2:
        - Return v1.1 on GET_VERSION call as claimed by this patch
        - Order by function ID the calls in FEATURES call
---
 tools/libxl/libxl_arm.c          |  3 ++-
 xen/arch/arm/domain_build.c      |  1 +
 xen/arch/arm/vpsci.c             | 39 ++++++++++++++++++++++++++++++++++++++-
 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, 44 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 6ab8ab64d0..e82b62db1a 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -106,7 +106,11 @@ 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);
+    /*
+     * PSCI is backward compatible from 0.2. So we can bump the version
+     * without any issue.
+     */
+    return PSCI_VERSION(1, 1);
 }
 
 static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
@@ -191,6 +195,29 @@ 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)
+{
+    /* /!\ Ordered by function ID and not name */
+    switch ( psci_func_id )
+    {
+    case PSCI_0_2_FN32_PSCI_VERSION:
+    case PSCI_0_2_FN32_CPU_SUSPEND:
+    case PSCI_0_2_FN64_CPU_SUSPEND:
+    case PSCI_0_2_FN32_CPU_OFF:
+    case PSCI_0_2_FN32_CPU_ON:
+    case PSCI_0_2_FN64_CPU_ON:
+    case PSCI_0_2_FN32_AFFINITY_INFO:
+    case PSCI_0_2_FN64_AFFINITY_INFO:
+    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
+    case PSCI_0_2_FN32_SYSTEM_OFF:
+    case PSCI_0_2_FN32_SYSTEM_RESET:
+    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)
 
@@ -304,6 +331,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 035a41e812..0cca5e6830 100644
--- a/xen/include/asm-arm/vpsci.h
+++ b/xen/include/asm-arm/vpsci.h
@@ -23,7 +23,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

Re: [PATCH v3 01/17] xen/arm: vpsci: Add support for PSCI 1.1

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall 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>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: mirela.simonovic@aggios.com

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


> ---
>     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.
> 
>     Changes in v3:
>         - Add Wei's acked-by
>         - Add Volodymyr's reviewed-by
> 
>     Changes in v2:
>         - Return v1.1 on GET_VERSION call as claimed by this patch
>         - Order by function ID the calls in FEATURES call
> ---
>  tools/libxl/libxl_arm.c          |  3 ++-
>  xen/arch/arm/domain_build.c      |  1 +
>  xen/arch/arm/vpsci.c             | 39 ++++++++++++++++++++++++++++++++++++++-
>  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, 44 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 6ab8ab64d0..e82b62db1a 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -106,7 +106,11 @@ 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);
> +    /*
> +     * PSCI is backward compatible from 0.2. So we can bump the version
> +     * without any issue.
> +     */
> +    return PSCI_VERSION(1, 1);
>  }
>  
>  static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
> @@ -191,6 +195,29 @@ 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)
> +{
> +    /* /!\ Ordered by function ID and not name */
> +    switch ( psci_func_id )
> +    {
> +    case PSCI_0_2_FN32_PSCI_VERSION:
> +    case PSCI_0_2_FN32_CPU_SUSPEND:
> +    case PSCI_0_2_FN64_CPU_SUSPEND:
> +    case PSCI_0_2_FN32_CPU_OFF:
> +    case PSCI_0_2_FN32_CPU_ON:
> +    case PSCI_0_2_FN64_CPU_ON:
> +    case PSCI_0_2_FN32_AFFINITY_INFO:
> +    case PSCI_0_2_FN64_AFFINITY_INFO:
> +    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
> +    case PSCI_0_2_FN32_SYSTEM_OFF:
> +    case PSCI_0_2_FN32_SYSTEM_RESET:
> +    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)
>  
> @@ -304,6 +331,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 035a41e812..0cca5e6830 100644
> --- a/xen/include/asm-arm/vpsci.h
> +++ b/xen/include/asm-arm/vpsci.h
> @@ -23,7 +23,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

Re: [PATCH v3 01/17] xen/arm: vpsci: Add support for PSCI 1.1

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall 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>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: mirela.simonovic@aggios.com

This patch doesn't apply cleanly to staging. Am I missing a
prerequisite?


> ---
>     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.
> 
>     Changes in v3:
>         - Add Wei's acked-by
>         - Add Volodymyr's reviewed-by
> 
>     Changes in v2:
>         - Return v1.1 on GET_VERSION call as claimed by this patch
>         - Order by function ID the calls in FEATURES call
> ---
>  tools/libxl/libxl_arm.c          |  3 ++-
>  xen/arch/arm/domain_build.c      |  1 +
>  xen/arch/arm/vpsci.c             | 39 ++++++++++++++++++++++++++++++++++++++-
>  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, 44 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 6ab8ab64d0..e82b62db1a 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -106,7 +106,11 @@ 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);
> +    /*
> +     * PSCI is backward compatible from 0.2. So we can bump the version
> +     * without any issue.
> +     */
> +    return PSCI_VERSION(1, 1);
>  }
>  
>  static register_t do_psci_0_2_cpu_suspend(uint32_t power_state,
> @@ -191,6 +195,29 @@ 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)
> +{
> +    /* /!\ Ordered by function ID and not name */
> +    switch ( psci_func_id )
> +    {
> +    case PSCI_0_2_FN32_PSCI_VERSION:
> +    case PSCI_0_2_FN32_CPU_SUSPEND:
> +    case PSCI_0_2_FN64_CPU_SUSPEND:
> +    case PSCI_0_2_FN32_CPU_OFF:
> +    case PSCI_0_2_FN32_CPU_ON:
> +    case PSCI_0_2_FN64_CPU_ON:
> +    case PSCI_0_2_FN32_AFFINITY_INFO:
> +    case PSCI_0_2_FN64_AFFINITY_INFO:
> +    case PSCI_0_2_FN32_MIGRATE_INFO_TYPE:
> +    case PSCI_0_2_FN32_SYSTEM_OFF:
> +    case PSCI_0_2_FN32_SYSTEM_RESET:
> +    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)
>  
> @@ -304,6 +331,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 035a41e812..0cca5e6830 100644
> --- a/xen/include/asm-arm/vpsci.h
> +++ b/xen/include/asm-arm/vpsci.h
> @@ -23,7 +23,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

Re: [PATCH v3 01/17] xen/arm: vpsci: Add support for PSCI 1.1

[Julien Grall]

Hi Stefano,

On 21/02/2018 00:37, Stefano Stabellini wrote:
> On Thu, 15 Feb 2018, Julien Grall 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>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: mirela.simonovic@aggios.com
> 
> This patch doesn't apply cleanly to staging. Am I missing a
> prerequisite?

I messed up my git-format command and forgot to include "xen/arm: psci: 
Rework the PSCI definitions". It was included in v2.

I will resend the series with the ack/reviewed-by collected.

Cheers,

-- 
Julien Grall

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

[PATCH v3 02/17] 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 "Firmware interfaces for mitigating CVE-2017-5715"
ARM DEN 00070A.

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

---
    Changes in v3:
        - Use ARM_SMCCC_NOT_SUPPORTED rather than hardcoded return

    Changes in v2:
        - Add a humand readable name for the specification
---
 xen/arch/arm/vpsci.c        |  1 +
 xen/arch/arm/vsmc.c         | 23 +++++++++++++++++++++++
 xen/include/asm-arm/smccc.h | 18 +++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index e82b62db1a..19ee7caeb4 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -212,6 +212,7 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id)
     case PSCI_0_2_FN32_SYSTEM_OFF:
     case PSCI_0_2_FN32_SYSTEM_RESET:
     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..7ec492741b 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, ARM_SMCCC_NOT_SUPPORTED);
+        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..629cc5150b 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,8 +103,21 @@ static inline uint32_t smccc_get_owner(register_t funcid)
                        ARM_SMCCC_OWNER_##owner,     \
                        0xFF03)
 
-/* Only one error code defined in SMCCC */
+#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)
+
+/* SMCCC error codes */
 #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
+#define ARM_SMCCC_NOT_SUPPORTED         (-1)
 
 /* SMCCC function identifier range which is reserved for existing APIs */
 #define ARM_SMCCC_RESERVED_RANGE_START  0x0
-- 
2.11.0


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

Re: [PATCH v3 02/17] xen/arm: vsmc: Implement SMCCC 1.1

[Volodymyr Babchuk]

Hi Julien,

On 15.02.18 17:02, Julien Grall 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 "Firmware interfaces for mitigating CVE-2017-5715"
> ARM DEN 00070A.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> 
> ---
>      Changes in v3:
>          - Use ARM_SMCCC_NOT_SUPPORTED rather than hardcoded return
> 
>      Changes in v2:
>          - Add a humand readable name for the specification
> ---
>   xen/arch/arm/vpsci.c        |  1 +
>   xen/arch/arm/vsmc.c         | 23 +++++++++++++++++++++++
>   xen/include/asm-arm/smccc.h | 18 +++++++++++++++++-
>   3 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index e82b62db1a..19ee7caeb4 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -212,6 +212,7 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id)
>       case PSCI_0_2_FN32_SYSTEM_OFF:
>       case PSCI_0_2_FN32_SYSTEM_RESET:
>       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..7ec492741b 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, ARM_SMCCC_NOT_SUPPORTED);
> +        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..629cc5150b 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,8 +103,21 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>                          ARM_SMCCC_OWNER_##owner,     \
>                          0xFF03)
>   
> -/* Only one error code defined in SMCCC */
> +#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)
> +
> +/* SMCCC error codes */
>   #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
> +#define ARM_SMCCC_NOT_SUPPORTED         (-1)
>   
>   /* SMCCC function identifier range which is reserved for existing APIs */
>   #define ARM_SMCCC_RESERVED_RANGE_START  0x0
> 

-- 
Volodymyr Babchuk

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

Re: [PATCH v3 02/17] xen/arm: vsmc: Implement SMCCC 1.1

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall 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 "Firmware interfaces for mitigating CVE-2017-5715"
> ARM DEN 00070A.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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


> ---
>     Changes in v3:
>         - Use ARM_SMCCC_NOT_SUPPORTED rather than hardcoded return
> 
>     Changes in v2:
>         - Add a humand readable name for the specification
> ---
>  xen/arch/arm/vpsci.c        |  1 +
>  xen/arch/arm/vsmc.c         | 23 +++++++++++++++++++++++
>  xen/include/asm-arm/smccc.h | 18 +++++++++++++++++-
>  3 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index e82b62db1a..19ee7caeb4 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -212,6 +212,7 @@ static int32_t do_psci_1_0_features(uint32_t psci_func_id)
>      case PSCI_0_2_FN32_SYSTEM_OFF:
>      case PSCI_0_2_FN32_SYSTEM_RESET:
>      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..7ec492741b 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, ARM_SMCCC_NOT_SUPPORTED);
> +        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..629cc5150b 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,8 +103,21 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>                         ARM_SMCCC_OWNER_##owner,     \
>                         0xFF03)
>  
> -/* Only one error code defined in SMCCC */
> +#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)
> +
> +/* SMCCC error codes */
>  #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
> +#define ARM_SMCCC_NOT_SUPPORTED         (-1)
>  
>  /* SMCCC function identifier range which is reserved for existing APIs */
>  #define ARM_SMCCC_RESERVED_RANGE_START  0x0
> -- 
> 2.11.0
> 

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

[PATCH v3 03/17] 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>
Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>

---
    Changes in v3:
        - Fix minor conflict during rebase

    Changes in v2:
        - Add Volodymyr's reviewed-by
---
 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 7ec492741b..40a80d5760 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, ARM_SMCCC_NOT_SUPPORTED);
+    {
+        uint32_t arch_func_id = get_user_reg(regs, 1);
+        int ret = ARM_SMCCC_NOT_SUPPORTED;
+
+        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 629cc5150b..2951caa49d 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)
+
 /* SMCCC error codes */
 #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
 #define ARM_SMCCC_NOT_SUPPORTED         (-1)
-- 
2.11.0


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

Re: [PATCH v3 03/17] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall 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.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>

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

> ---
>     Changes in v3:
>         - Fix minor conflict during rebase
> 
>     Changes in v2:
>         - Add Volodymyr's reviewed-by
> ---
>  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 7ec492741b..40a80d5760 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, ARM_SMCCC_NOT_SUPPORTED);
> +    {
> +        uint32_t arch_func_id = get_user_reg(regs, 1);
> +        int ret = ARM_SMCCC_NOT_SUPPORTED;
> +
> +        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 629cc5150b..2951caa49d 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)
> +
>  /* SMCCC error codes */
>  #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>  #define ARM_SMCCC_NOT_SUPPORTED         (-1)
> -- 
> 2.11.0
> 

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

Re: [PATCH v3 03/17] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Andre Przywara]

Hi,

On 15/02/18 15:02, Julien Grall 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.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>
> 
> ---
>     Changes in v3:
>         - Fix minor conflict during rebase
> 
>     Changes in v2:
>         - Add Volodymyr's reviewed-by
> ---
>  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 7ec492741b..40a80d5760 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, ARM_SMCCC_NOT_SUPPORTED);
> +    {
> +        uint32_t arch_func_id = get_user_reg(regs, 1);

Shouldn't the function identifier be in x0/r0?

Cheers,
Andre.

> +        int ret = ARM_SMCCC_NOT_SUPPORTED;
> +
> +        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 629cc5150b..2951caa49d 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)
> +
>  /* SMCCC error codes */
>  #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>  #define ARM_SMCCC_NOT_SUPPORTED         (-1)
> 

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

Re: [PATCH v3 03/17] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Julien Grall]

On 21/02/18 16:34, Andre Przywara wrote:
> Hi,

Hi,

> On 15/02/18 15:02, Julien Grall 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.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>
>>
>> ---
>>      Changes in v3:
>>          - Fix minor conflict during rebase
>>
>>      Changes in v2:
>>          - Add Volodymyr's reviewed-by
>> ---
>>   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 7ec492741b..40a80d5760 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, ARM_SMCCC_NOT_SUPPORTED);
>> +    {
>> +        uint32_t arch_func_id = get_user_reg(regs, 1);
> 
> Shouldn't the function identifier be in x0/r0?

x0/r0 contain the function identifier of the actual function (i.e 
ARM_SMCCC_ARCH_FEATURES_FID). What we want here is the arch_func_id to 
check if the firmware implement it. This is the first parameter of the 
function, according to the calling convention this will be stored in x1/r1.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v3 03/17] xen/arm: vsmc: Implement SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Andre Przywara]

Hi,

On 21/02/18 16:41, Julien Grall wrote:
> 
> 
> On 21/02/18 16:34, Andre Przywara wrote:
>> Hi,
> 
> Hi,
> 
>> On 15/02/18 15:02, Julien Grall 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.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>
>>>
>>> ---
>>>      Changes in v3:
>>>          - Fix minor conflict during rebase
>>>
>>>      Changes in v2:
>>>          - Add Volodymyr's reviewed-by
>>> ---
>>>   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 7ec492741b..40a80d5760 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, ARM_SMCCC_NOT_SUPPORTED);
>>> +    {
>>> +        uint32_t arch_func_id = get_user_reg(regs, 1);
>>
>> Shouldn't the function identifier be in x0/r0?
> 
> x0/r0 contain the function identifier of the actual function (i.e
> ARM_SMCCC_ARCH_FEATURES_FID). What we want here is the arch_func_id to
> check if the firmware implement it. This is the first parameter of the
> function, according to the calling convention this will be stored in x1/r1.

Ah, right. I guess the missing context in this patch confused me.
Looks alright then:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

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

[PATCH v3 04/17] 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>
Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>

---
    Changes in v2:
        - Add Volodymyr's reviewed-by
---
 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 2951caa49d..30208d12ca 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

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

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>

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


> ---
>     Changes in v2:
>         - Add Volodymyr's reviewed-by
> ---
>  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 2951caa49d..30208d12ca 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 v3 05/17] 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 advance.

Signed-off-by: Julien Grall <julien.grall@arm.com>
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.

    Changes in v2:
        - Add Volodymyr's reviewed-by
---
 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

Re: [PATCH v3 05/17] xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall 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.
> 
> For now, only provide a fast path for HVC64 call. Because the code rely
> on 2 registers, x0 and x1 are saved in advance.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>

Nice!

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


> ---
>     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.
> 
>     Changes in v2:
>         - Add Volodymyr's reviewed-by
> ---
>  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

Re: [PATCH v3 05/17] xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

[Andre Przywara]

Hi,

On 15/02/18 15:02, Julien Grall 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.
> 
> For now, only provide a fast path for HVC64 call. Because the code rely
> on 2 registers, x0 and x1 are saved in advance.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 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.
> 
>     Changes in v2:
>         - Add Volodymyr's reviewed-by
> ---
>  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,

A bit confusing. What about:
    * save_x0_x1: Does the macro needs to save x0 and x1? Defaults to 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

It would be good to mention that this function identifier can't be
encoded as an immediate for the "cmp" instruction, but it can with
arithmetic instructions. Hence the exclusive OR.

> +
> +        /*
> +         * Clobber both x0 and x1 to prevent leakage. Note that thanks
> +         * the eor, x0 = 0.
> +         */
> +        mov     x1, x0
> +        eret

I think this is more readable:

	   /* Clobber x1 to prevent leakage. x0 is already 0. */
	   mov     x1, xzr
	   eret

Cheers,
Andre.

> +
> +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
> 

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

Re: [PATCH v3 05/17] xen/arm64: Implement a fast path for handling SMCCC_ARCH_WORKAROUND_1

[Julien Grall]

On 21/02/18 14:27, Andre Przywara wrote:
> Hi,

Hi Andre,

> On 15/02/18 15:02, Julien Grall 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.
>>
>> For now, only provide a fast path for HVC64 call. Because the code rely
>> on 2 registers, x0 and x1 are saved in advance.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> 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.
>>
>>      Changes in v2:
>>          - Add Volodymyr's reviewed-by
>> ---
>>   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,
> 
> A bit confusing. What about:
>      * save_x0_x1: Does the macro needs to save x0 and x1? Defaults to 1.
>      * If 0, ....

Sounds good to me. I will update the comment.

> 
>> + * 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
> 
> It would be good to mention that this function identifier can't be
> encoded as an immediate for the "cmp" instruction, but it can with
> arithmetic instructions. Hence the exclusive OR.

I will.

> 
>> +
>> +        /*
>> +         * Clobber both x0 and x1 to prevent leakage. Note that thanks
>> +         * the eor, x0 = 0.
>> +         */
>> +        mov     x1, x0
>> +        eret
> 
> I think this is more readable:
> 
> 	   /* Clobber x1 to prevent leakage. x0 is already 0. */

I would prefer to say "clobber x0 and x1 to prevent leakage. Note that 
x0 is already 0 thanks to the eor". So it is clear that we expect both 
registers to be clobbered.

> 	   mov     x1, xzr
> 	   eret
Cheers,

-- 
Julien Grall

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

[PATCH v3 06/17] xen/arm64: Print a per-CPU message with the BP hardening method used

[Julien Grall]

This will make easier to know whether BP hardening has been enabled for
a CPU and which method is used.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babcuk <volodymyr_babchuk@epam.com>

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

    Changes in v2:
        - Patch added
---
 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 c243521ed4..8d5f8d372a 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 v3 06/17] xen/arm64: Print a per-CPU message with the BP hardening method used

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> This will make easier to know whether BP hardening has been enabled for
> a CPU and which method is used.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babcuk <volodymyr_babchuk@epam.com>

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


> ---
>     Changes in v3:
>         - Add Volodymyr's reviewed-by
> 
>     Changes in v2:
>         - Patch added
> ---
>  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 c243521ed4..8d5f8d372a 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

[PATCH v3 07/17] xen/arm: smccc: Add macros SMCCC_VERSION, SMCCC_VERSION_{MINOR, MAJOR}

[Julien Grall]

Add macros SMCCC_VERSION, SMCCC_VERSION_{MINOR, MAJOR} to easily convert
between a 32-bit value and a version number. The encoding is based on
2.2.2 in "Firmware interfaces for mitigation CVE-2017-5715" (ARM DEN 0070A).

Also re-use them to define ARM_SMCCC_VERSION_1_0 and ARM_SMCCC_VERSION_1_1.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

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

    Changes in v2:
        - Patch added
---
 xen/include/asm-arm/smccc.h | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 30208d12ca..d0240d64bf 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -16,8 +16,20 @@
 #ifndef __ASM_ARM_SMCCC_H__
 #define __ASM_ARM_SMCCC_H__
 
-#define ARM_SMCCC_VERSION_1_0   0x10000
-#define ARM_SMCCC_VERSION_1_1   0x10001
+#define SMCCC_VERSION_MAJOR_SHIFT            16
+#define SMCCC_VERSION_MINOR_MASK             \
+        ((1U << SMCCC_VERSION_MAJOR_SHIFT) - 1)
+#define SMCCC_VERSION_MAJOR_MASK             ~SMCCC_VERSION_MINOR_MASK
+#define SMCCC_VERSION_MAJOR(ver)             \
+        (((ver) & SMCCC_VERSION_MAJOR_MASK) >> SMCCC_VERSION_MAJOR_SHIFT)
+#define SMCCC_VERSION_MINOR(ver)             \
+        ((ver) & SMCCC_VERSION_MINOR_MASK)
+
+#define SMCCC_VERSION(major, minor)          \
+    (((major) << SMCCC_VERSION_MAJOR_SHIFT) | (minor))
+
+#define ARM_SMCCC_VERSION_1_0   SMCCC_VERSION(1, 0)
+#define ARM_SMCCC_VERSION_1_1   SMCCC_VERSION(1, 1)
 
 /*
  * This file provides common defines for ARM SMC Calling Convention as
-- 
2.11.0


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

Re: [PATCH v3 07/17] xen/arm: smccc: Add macros SMCCC_VERSION, SMCCC_VERSION_{MINOR, MAJOR}

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> Add macros SMCCC_VERSION, SMCCC_VERSION_{MINOR, MAJOR} to easily convert
> between a 32-bit value and a version number. The encoding is based on
> 2.2.2 in "Firmware interfaces for mitigation CVE-2017-5715" (ARM DEN 0070A).
> 
> Also re-use them to define ARM_SMCCC_VERSION_1_0 and ARM_SMCCC_VERSION_1_1.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

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


> ---
>     Changes in v3:
>         - Add Volodymyr's reviewed-by
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/include/asm-arm/smccc.h | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 30208d12ca..d0240d64bf 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -16,8 +16,20 @@
>  #ifndef __ASM_ARM_SMCCC_H__
>  #define __ASM_ARM_SMCCC_H__
>  
> -#define ARM_SMCCC_VERSION_1_0   0x10000
> -#define ARM_SMCCC_VERSION_1_1   0x10001
> +#define SMCCC_VERSION_MAJOR_SHIFT            16
> +#define SMCCC_VERSION_MINOR_MASK             \
> +        ((1U << SMCCC_VERSION_MAJOR_SHIFT) - 1)
> +#define SMCCC_VERSION_MAJOR_MASK             ~SMCCC_VERSION_MINOR_MASK
> +#define SMCCC_VERSION_MAJOR(ver)             \
> +        (((ver) & SMCCC_VERSION_MAJOR_MASK) >> SMCCC_VERSION_MAJOR_SHIFT)
> +#define SMCCC_VERSION_MINOR(ver)             \
> +        ((ver) & SMCCC_VERSION_MINOR_MASK)
> +
> +#define SMCCC_VERSION(major, minor)          \
> +    (((major) << SMCCC_VERSION_MAJOR_SHIFT) | (minor))
> +
> +#define ARM_SMCCC_VERSION_1_0   SMCCC_VERSION(1, 0)
> +#define ARM_SMCCC_VERSION_1_1   SMCCC_VERSION(1, 1)
>  
>  /*
>   * This file provides common defines for ARM SMC Calling Convention as
> -- 
> 2.11.0
> 

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

[PATCH v3 08/17] xen/arm: psci: Detect SMCCC version

[Julien Grall]

PSCI 1.0 and later allows the SMCCC version to be (indirectly) probed
via PSCI_FEATURES. If the PSCI_FEATURES does not exist (PSCI 0.2 or
earlier) and the function return an error, then we considered SMCCC 1.0
is implemented.

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

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/psci.c         | 34 +++++++++++++++++++++++++++++++++-
 xen/include/asm-arm/smccc.h |  2 ++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 5dda35cd7c..bc7b2260e8 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -37,6 +37,7 @@
 #endif
 
 uint32_t psci_ver;
+uint32_t smccc_ver;
 
 static uint32_t psci_cpu_on_nr;
 
@@ -57,6 +58,14 @@ void call_psci_system_reset(void)
         call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
 }
 
+static int __init psci_features(uint32_t psci_func_id)
+{
+    if ( psci_ver < PSCI_VERSION(1, 0) )
+        return PSCI_NOT_SUPPORTED;
+
+    return call_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, 0, 0);
+}
+
 int __init psci_is_smc_method(const struct dt_device_node *psci)
 {
     int ret;
@@ -82,6 +91,24 @@ int __init psci_is_smc_method(const struct dt_device_node *psci)
     return 0;
 }
 
+static void __init psci_init_smccc(void)
+{
+    /* PSCI is using at least SMCC 1.0 calling convention. */
+    smccc_ver = ARM_SMCCC_VERSION_1_0;
+
+    if ( psci_features(ARM_SMCCC_VERSION_FID) != PSCI_NOT_SUPPORTED )
+    {
+        uint32_t ret;
+
+        ret = call_smc(ARM_SMCCC_VERSION_FID, 0, 0, 0);
+        if ( ret != ARM_SMCCC_NOT_SUPPORTED )
+            smccc_ver = ret;
+    }
+
+    printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
+           SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
+}
+
 int __init psci_init_0_1(void)
 {
     int ret;
@@ -173,7 +200,12 @@ int __init psci_init(void)
     if ( ret )
         ret = psci_init_0_1();
 
-    return ret;
+    if ( ret )
+        return ret;
+
+    psci_init_smccc();
+
+    return 0;
 }
 
 /*
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index d0240d64bf..bc067892c7 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -52,6 +52,8 @@
 
 #ifndef __ASSEMBLY__
 
+extern uint32_t smccc_ver;
+
 /* Check if this is fast call. */
 static inline bool smccc_is_fast_call(register_t funcid)
 {
-- 
2.11.0


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

Re: [PATCH v3 08/17] xen/arm: psci: Detect SMCCC version

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> PSCI 1.0 and later allows the SMCCC version to be (indirectly) probed
> via PSCI_FEATURES. If the PSCI_FEATURES does not exist (PSCI 0.2 or
> earlier) and the function return an error, then we considered SMCCC 1.0
> is implemented.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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


> ---
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/psci.c         | 34 +++++++++++++++++++++++++++++++++-
>  xen/include/asm-arm/smccc.h |  2 ++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 5dda35cd7c..bc7b2260e8 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -37,6 +37,7 @@
>  #endif
>  
>  uint32_t psci_ver;
> +uint32_t smccc_ver;
>  
>  static uint32_t psci_cpu_on_nr;
>  
> @@ -57,6 +58,14 @@ void call_psci_system_reset(void)
>          call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
>  }
>  
> +static int __init psci_features(uint32_t psci_func_id)
> +{
> +    if ( psci_ver < PSCI_VERSION(1, 0) )
> +        return PSCI_NOT_SUPPORTED;
> +
> +    return call_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, 0, 0);
> +}
> +
>  int __init psci_is_smc_method(const struct dt_device_node *psci)
>  {
>      int ret;
> @@ -82,6 +91,24 @@ int __init psci_is_smc_method(const struct dt_device_node *psci)
>      return 0;
>  }
>  
> +static void __init psci_init_smccc(void)
> +{
> +    /* PSCI is using at least SMCC 1.0 calling convention. */
> +    smccc_ver = ARM_SMCCC_VERSION_1_0;
> +
> +    if ( psci_features(ARM_SMCCC_VERSION_FID) != PSCI_NOT_SUPPORTED )
> +    {
> +        uint32_t ret;
> +
> +        ret = call_smc(ARM_SMCCC_VERSION_FID, 0, 0, 0);
> +        if ( ret != ARM_SMCCC_NOT_SUPPORTED )
> +            smccc_ver = ret;
> +    }
> +
> +    printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
> +           SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
> +}
> +
>  int __init psci_init_0_1(void)
>  {
>      int ret;
> @@ -173,7 +200,12 @@ int __init psci_init(void)
>      if ( ret )
>          ret = psci_init_0_1();
>  
> -    return ret;
> +    if ( ret )
> +        return ret;
> +
> +    psci_init_smccc();
> +
> +    return 0;
>  }
>  
>  /*
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index d0240d64bf..bc067892c7 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -52,6 +52,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +extern uint32_t smccc_ver;
> +
>  /* Check if this is fast call. */
>  static inline bool smccc_is_fast_call(register_t funcid)
>  {
> -- 
> 2.11.0
> 

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

Re: [PATCH v3 08/17] xen/arm: psci: Detect SMCCC version

[Andre Przywara]

Hi,

On 15/02/18 15:02, Julien Grall wrote:
> PSCI 1.0 and later allows the SMCCC version to be (indirectly) probed
> via PSCI_FEATURES. If the PSCI_FEATURES does not exist (PSCI 0.2 or
> earlier) and the function return an error, then we considered SMCCC 1.0

                            returns                  assume
> is implemented.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/psci.c         | 34 +++++++++++++++++++++++++++++++++-
>  xen/include/asm-arm/smccc.h |  2 ++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 5dda35cd7c..bc7b2260e8 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -37,6 +37,7 @@
>  #endif
>  
>  uint32_t psci_ver;
> +uint32_t smccc_ver;
>  
>  static uint32_t psci_cpu_on_nr;
>  
> @@ -57,6 +58,14 @@ void call_psci_system_reset(void)
>          call_smc(PSCI_0_2_FN32_SYSTEM_RESET, 0, 0, 0);
>  }
>  
> +static int __init psci_features(uint32_t psci_func_id)
> +{
> +    if ( psci_ver < PSCI_VERSION(1, 0) )
> +        return PSCI_NOT_SUPPORTED;
> +
> +    return call_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, 0, 0);
> +}
> +
>  int __init psci_is_smc_method(const struct dt_device_node *psci)
>  {
>      int ret;
> @@ -82,6 +91,24 @@ int __init psci_is_smc_method(const struct dt_device_node *psci)
>      return 0;
>  }
>  
> +static void __init psci_init_smccc(void)
> +{
> +    /* PSCI is using at least SMCC 1.0 calling convention. */

                                 SMCCC

Other than those nits it looks good.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

> +    smccc_ver = ARM_SMCCC_VERSION_1_0;
> +
> +    if ( psci_features(ARM_SMCCC_VERSION_FID) != PSCI_NOT_SUPPORTED )
> +    {
> +        uint32_t ret;
> +
> +        ret = call_smc(ARM_SMCCC_VERSION_FID, 0, 0, 0);
> +        if ( ret != ARM_SMCCC_NOT_SUPPORTED )
> +            smccc_ver = ret;
> +    }
> +
> +    printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
> +           SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
> +}
> +
>  int __init psci_init_0_1(void)
>  {
>      int ret;
> @@ -173,7 +200,12 @@ int __init psci_init(void)
>      if ( ret )
>          ret = psci_init_0_1();
>  
> -    return ret;
> +    if ( ret )
> +        return ret;
> +
> +    psci_init_smccc();
> +
> +    return 0;
>  }
>  
>  /*
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index d0240d64bf..bc067892c7 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -52,6 +52,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +extern uint32_t smccc_ver;
> +
>  /* Check if this is fast call. */
>  static inline bool smccc_is_fast_call(register_t funcid)
>  {
> 

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

[PATCH v3 09/17] xen/arm: smccc: Implement SMCCC v1.1 inline primitive

[Julien Grall]

One of the major improvement of SMCCC v1.1 is that it only clobbers the
first 4 registers, both on 32 and 64bit. This means that it becomes very
easy to provide an inline version of the SMC call primitive, and avoid
performing a function call to stash the registers that woudl otherwise
be clobbered by SMCCC v1.0.

This patch has been adapted to Xen from Linux commit f2d3b2e8759a. The
changes mades are:
    - Using Xen coding style
    - Remove HVC as not used by Xen
    - Add arm_smccc_res structure

 Reviewed-by: Robin Murphy <robin.murphy@arm.com>
 Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
 Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
 Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

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

---

    Note that the patch is in arm64/for-next/core and should be merged
    in master soon.

    Changes in v2:
        - Patch added
---
 xen/include/asm-arm/smccc.h | 119 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index bc067892c7..154772b728 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -78,6 +78,125 @@ static inline uint32_t smccc_get_owner(register_t funcid)
     return (funcid >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK;
 }
 
+/*
+ * struct arm_smccc_res - Result from SMC call
+ * @a0 - @a3 result values from registers 0 to 3
+ */
+struct arm_smccc_res {
+    unsigned long a0;
+    unsigned long a1;
+    unsigned long a2;
+    unsigned long a3;
+};
+
+/* SMCCC v1.1 implementation madness follows */
+#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
+
+#define __count_args(...)                               \
+    ___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
+
+#define __constraint_write_0                        \
+    "+r" (r0), "=&r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_1                        \
+    "+r" (r0), "+r" (r1), "=&r" (r2), "=&r" (r3)
+#define __constraint_write_2                        \
+    "+r" (r0), "+r" (r1), "+r" (r2), "=&r" (r3)
+#define __constraint_write_3                        \
+    "+r" (r0), "+r" (r1), "+r" (r2), "+r" (r3)
+#define __constraint_write_4    __constraint_write_3
+#define __constraint_write_5    __constraint_write_4
+#define __constraint_write_6    __constraint_write_5
+#define __constraint_write_7    __constraint_write_6
+
+#define __constraint_read_0
+#define __constraint_read_1
+#define __constraint_read_2
+#define __constraint_read_3
+#define __constraint_read_4 "r" (r4)
+#define __constraint_read_5 __constraint_read_4, "r" (r5)
+#define __constraint_read_6 __constraint_read_5, "r" (r6)
+#define __constraint_read_7 __constraint_read_6, "r" (r7)
+
+#define __declare_arg_0(a0, res)                        \
+    struct arm_smccc_res    *___res = res;              \
+    register uin32_t        r0 asm("r0") = a0;          \
+    register unsigned long  r1 asm("r1");               \
+    register unsigned long  r2 asm("r2");               \
+    register unsigned long  r3 asm("r3")
+
+#define __declare_arg_1(a0, a1, res)                    \
+    struct arm_smccc_res    *___res = res;              \
+    register uint32_t       r0 asm("r0") = a0;          \
+    register typeof(a1)     r1 asm("r1") = a1;          \
+    register unsigned long  r2 asm("r2");               \
+    register unsigned long  r3 asm("r3")
+
+#define __declare_arg_2(a0, a1, a2, res)                \
+    struct arm_smccc_res    *___res = res;				\
+    register u32            r0 asm("r0") = a0;          \
+    register typeof(a1)     r1 asm("r1") = a1;          \
+    register typeof(a2)     r2 asm("r2") = a2;          \
+    register unsigned long  r3 asm("r3")
+
+#define __declare_arg_3(a0, a1, a2, a3, res)            \
+    struct arm_smccc_res    *___res = res;              \
+    register u32            r0 asm("r0") = a0;          \
+    register typeof(a1)     r1 asm("r1") = a1;          \
+    register typeof(a2)     r2 asm("r2") = a2;          \
+    register typeof(a3)     r3 asm("r3") = a3
+
+#define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
+    __declare_arg_3(a0, a1, a2, a3, res);               \
+    register typeof(a4) r4 asm("r4") = a4
+
+#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
+    __declare_arg_4(a0, a1, a2, a3, a4, res);           \
+    register typeof(a5) r5 asm("r5") = a5
+
+#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
+    __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
+    register typeof(a6) r6 asm("r6") = a6
+
+#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
+    __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
+    register typeof(a7) r7 asm("r7") = a7
+
+#define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
+#define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
+
+#define ___constraints(count)                       \
+    : __constraint_write_ ## count                  \
+    : __constraint_read_ ## count                   \
+    : "memory"
+#define __constraints(count)    ___constraints(count)
+
+/*
+ * arm_smccc_1_1_smc() - make an SMCCC v1.1 compliant SMC call
+ *
+ * This is a variadic macro taking one to eight source arguments, and
+ * an optional return structure.
+ *
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 3
+ *
+ * This macro is used to make SMC calls following SMC Calling Convention v1.1.
+ * The content of the supplied param are copied to registers 0 to 7 prior
+ * to the SMC instruction. The return values are updated with the content
+ * from register 0 to 3 on return from the SMC instruction if not NULL.
+ *
+ * We have an output list that is not necessarily used, and GCC feels
+ * entitled to optimise the whole sequence away. "volatile" is what
+ * makes it stick.
+ */
+#define arm_smccc_1_1_smc(...)                                  \
+    do {                                                        \
+        __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
+        asm volatile("smc #0\n"                                 \
+                     __constraints(__count_args(__VA_ARGS__))); \
+        if ( ___res )                                           \
+        *___res = (typeof(*___res)){r0, r1, r2, r3};            \
+    } while ( 0 )
+
 #endif
 
 /*
-- 
2.11.0


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

Re: [PATCH v3 09/17] xen/arm: smccc: Implement SMCCC v1.1 inline primitive

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> One of the major improvement of SMCCC v1.1 is that it only clobbers the
> first 4 registers, both on 32 and 64bit. This means that it becomes very
> easy to provide an inline version of the SMC call primitive, and avoid
> performing a function call to stash the registers that woudl otherwise
> be clobbered by SMCCC v1.0.
> 
> This patch has been adapted to Xen from Linux commit f2d3b2e8759a. The
> changes mades are:
>     - Using Xen coding style
>     - Remove HVC as not used by Xen
>     - Add arm_smccc_res structure
> 
>  Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>  Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>  Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>  Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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


> ---
> 
>     Note that the patch is in arm64/for-next/core and should be merged
>     in master soon.
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/include/asm-arm/smccc.h | 119 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index bc067892c7..154772b728 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -78,6 +78,125 @@ static inline uint32_t smccc_get_owner(register_t funcid)
>      return (funcid >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK;
>  }
>  
> +/*
> + * struct arm_smccc_res - Result from SMC call
> + * @a0 - @a3 result values from registers 0 to 3
> + */
> +struct arm_smccc_res {
> +    unsigned long a0;
> +    unsigned long a1;
> +    unsigned long a2;
> +    unsigned long a3;
> +};
> +
> +/* SMCCC v1.1 implementation madness follows */
> +#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
> +
> +#define __count_args(...)                               \
> +    ___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
> +
> +#define __constraint_write_0                        \
> +    "+r" (r0), "=&r" (r1), "=&r" (r2), "=&r" (r3)
> +#define __constraint_write_1                        \
> +    "+r" (r0), "+r" (r1), "=&r" (r2), "=&r" (r3)
> +#define __constraint_write_2                        \
> +    "+r" (r0), "+r" (r1), "+r" (r2), "=&r" (r3)
> +#define __constraint_write_3                        \
> +    "+r" (r0), "+r" (r1), "+r" (r2), "+r" (r3)
> +#define __constraint_write_4    __constraint_write_3
> +#define __constraint_write_5    __constraint_write_4
> +#define __constraint_write_6    __constraint_write_5
> +#define __constraint_write_7    __constraint_write_6
> +
> +#define __constraint_read_0
> +#define __constraint_read_1
> +#define __constraint_read_2
> +#define __constraint_read_3
> +#define __constraint_read_4 "r" (r4)
> +#define __constraint_read_5 __constraint_read_4, "r" (r5)
> +#define __constraint_read_6 __constraint_read_5, "r" (r6)
> +#define __constraint_read_7 __constraint_read_6, "r" (r7)
> +
> +#define __declare_arg_0(a0, res)                        \
> +    struct arm_smccc_res    *___res = res;              \
> +    register uin32_t        r0 asm("r0") = a0;          \
> +    register unsigned long  r1 asm("r1");               \
> +    register unsigned long  r2 asm("r2");               \
> +    register unsigned long  r3 asm("r3")
> +
> +#define __declare_arg_1(a0, a1, res)                    \
> +    struct arm_smccc_res    *___res = res;              \
> +    register uint32_t       r0 asm("r0") = a0;          \
> +    register typeof(a1)     r1 asm("r1") = a1;          \
> +    register unsigned long  r2 asm("r2");               \
> +    register unsigned long  r3 asm("r3")
> +
> +#define __declare_arg_2(a0, a1, a2, res)                \
> +    struct arm_smccc_res    *___res = res;				\
> +    register u32            r0 asm("r0") = a0;          \
> +    register typeof(a1)     r1 asm("r1") = a1;          \
> +    register typeof(a2)     r2 asm("r2") = a2;          \
> +    register unsigned long  r3 asm("r3")
> +
> +#define __declare_arg_3(a0, a1, a2, a3, res)            \
> +    struct arm_smccc_res    *___res = res;              \
> +    register u32            r0 asm("r0") = a0;          \
> +    register typeof(a1)     r1 asm("r1") = a1;          \
> +    register typeof(a2)     r2 asm("r2") = a2;          \
> +    register typeof(a3)     r3 asm("r3") = a3
> +
> +#define __declare_arg_4(a0, a1, a2, a3, a4, res)        \
> +    __declare_arg_3(a0, a1, a2, a3, res);               \
> +    register typeof(a4) r4 asm("r4") = a4
> +
> +#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)    \
> +    __declare_arg_4(a0, a1, a2, a3, a4, res);           \
> +    register typeof(a5) r5 asm("r5") = a5
> +
> +#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)    \
> +    __declare_arg_5(a0, a1, a2, a3, a4, a5, res);           \
> +    register typeof(a6) r6 asm("r6") = a6
> +
> +#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)    \
> +    __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);           \
> +    register typeof(a7) r7 asm("r7") = a7
> +
> +#define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
> +#define __declare_args(count, ...)  ___declare_args(count, __VA_ARGS__)
> +
> +#define ___constraints(count)                       \
> +    : __constraint_write_ ## count                  \
> +    : __constraint_read_ ## count                   \
> +    : "memory"
> +#define __constraints(count)    ___constraints(count)
> +
> +/*
> + * arm_smccc_1_1_smc() - make an SMCCC v1.1 compliant SMC call
> + *
> + * This is a variadic macro taking one to eight source arguments, and
> + * an optional return structure.
> + *
> + * @a0-a7: arguments passed in registers 0 to 7
> + * @res: result values from registers 0 to 3
> + *
> + * This macro is used to make SMC calls following SMC Calling Convention v1.1.
> + * The content of the supplied param are copied to registers 0 to 7 prior
> + * to the SMC instruction. The return values are updated with the content
> + * from register 0 to 3 on return from the SMC instruction if not NULL.
> + *
> + * We have an output list that is not necessarily used, and GCC feels
> + * entitled to optimise the whole sequence away. "volatile" is what
> + * makes it stick.
> + */
> +#define arm_smccc_1_1_smc(...)                                  \
> +    do {                                                        \
> +        __declare_args(__count_args(__VA_ARGS__), __VA_ARGS__); \
> +        asm volatile("smc #0\n"                                 \
> +                     __constraints(__count_args(__VA_ARGS__))); \
> +        if ( ___res )                                           \
> +        *___res = (typeof(*___res)){r0, r1, r2, r3};            \
> +    } while ( 0 )
> +
>  #endif
>  
>  /*
> -- 
> 2.11.0
> 

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

[PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Julien Grall]

Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.

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

---
    Changes in v3:
        - Add the missing call to smc #0.

    Changes in v2:
        - Patch added
---
 xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
 xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
 xen/include/asm-arm/smccc.h |  1 +
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
index 4b7f1dc21f..981fb83a88 100644
--- a/xen/arch/arm/arm64/bpi.S
+++ b/xen/arch/arm/arm64/bpi.S
@@ -16,6 +16,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <asm/smccc.h>
+
 .macro ventry target
     .rept 31
     nop
@@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
     add     sp, sp, #(8 * 18)
 ENTRY(__psci_hyp_bp_inval_end)
 
+ENTRY(__smccc_workaround_1_smc_start)
+    sub     sp, sp, #(8 * 4)
+    stp     x2, x3, [sp, #(8 * 0)]
+    stp     x0, x1, [sp, #(8 * 2)]
+    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
+    smc     #0
+    ldp     x2, x3, [sp, #(8 * 0)]
+    ldp     x0, x1, [sp, #(8 * 2)]
+    add     sp, sp, #(8 * 4)
+ENTRY(__smccc_workaround_1_smc_end)
+
 /*
  * Local variables:
  * mode: ASM
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 8d5f8d372a..dec9074422 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
     return ret;
 }
 
+extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
+
+static bool
+check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
+{
+    struct arm_smccc_res res;
+
+    /*
+     * Enable callbacks are called on every CPU based on the
+     * capabilities. So double-check whether the CPU matches the
+     * entry.
+     */
+    if ( !entry->matches(entry) )
+        return false;
+
+    if ( smccc_ver < SMCCC_VERSION(1, 1) )
+        return false;
+
+    arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
+                      ARM_SMCCC_ARCH_WORKAROUND_1_FID, &res);
+    if ( res.a0 != ARM_SMCCC_SUCCESS )
+        return false;
+
+    return install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
+                                    __smccc_workaround_1_smc_end,
+                                    "call ARM_SMCCC_ARCH_WORKAROUND_1");
+}
+
 extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
 
 static int enable_psci_bp_hardening(void *data)
@@ -154,12 +182,14 @@ static int enable_psci_bp_hardening(void *data)
     bool ret = true;
     static bool warned = false;
 
+    if ( check_smccc_arch_workaround_1(data) )
+        return 0;
     /*
      * The mitigation is using PSCI version function to invalidate the
      * branch predictor. This function is only available with PSCI 0.2
      * and later.
      */
-    if ( psci_ver >= PSCI_VERSION(0, 2) )
+    else if ( psci_ver >= PSCI_VERSION(0, 2) )
         ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
                                        __psci_hyp_bp_inval_end,
                                        "call PSCI get version");
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 154772b728..8342cc33fe 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -261,6 +261,7 @@ struct arm_smccc_res {
 /* SMCCC error codes */
 #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
 #define ARM_SMCCC_NOT_SUPPORTED         (-1)
+#define ARM_SMCCC_SUCCESS               (0)
 
 /* SMCCC function identifier range which is reserved for existing APIs */
 #define ARM_SMCCC_RESERVED_RANGE_START  0x0
-- 
2.11.0


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

Re: [PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Volodymyr Babchuk]

Julien,

On 15.02.18 17:02, Julien Grall wrote:
> Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> ---
>      Changes in v3:
>          - Add the missing call to smc #0.
> 
>      Changes in v2:
>          - Patch added
> ---
>   xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
>   xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
>   xen/include/asm-arm/smccc.h |  1 +
>   3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> index 4b7f1dc21f..981fb83a88 100644
> --- a/xen/arch/arm/arm64/bpi.S
> +++ b/xen/arch/arm/arm64/bpi.S
> @@ -16,6 +16,8 @@
>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> +#include <asm/smccc.h>
> +
>   .macro ventry target
>       .rept 31
>       nop
> @@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
>       add     sp, sp, #(8 * 18)
>   ENTRY(__psci_hyp_bp_inval_end)
>   
> +ENTRY(__smccc_workaround_1_smc_start)
> +    sub     sp, sp, #(8 * 4)
> +    stp     x2, x3, [sp, #(8 * 0)]
> +    stp     x0, x1, [sp, #(8 * 2)]
> +    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
> +    smc     #0
> +    ldp     x2, x3, [sp, #(8 * 0)]
> +    ldp     x0, x1, [sp, #(8 * 2)]
> +    add     sp, sp, #(8 * 4)
> +ENTRY(__smccc_workaround_1_smc_end)
> +
>   /*
>    * Local variables:
>    * mode: ASM
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 8d5f8d372a..dec9074422 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>       return ret;
>   }
>   
> +extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
> +
> +static bool
> +check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
> +{
> +    struct arm_smccc_res res;
> +
> +    /*
> +     * Enable callbacks are called on every CPU based on the
> +     * capabilities. So double-check whether the CPU matches the
> +     * entry.
> +     */
> +    if ( !entry->matches(entry) )
> +        return false;
> +
> +    if ( smccc_ver < SMCCC_VERSION(1, 1) )
> +        return false;
> +
> +    arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
> +                      ARM_SMCCC_ARCH_WORKAROUND_1_FID, &res);
> +    if ( res.a0 != ARM_SMCCC_SUCCESS )
> +        return false;
> +
> +    return install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
> +                                    __smccc_workaround_1_smc_end,
> +                                    "call ARM_SMCCC_ARCH_WORKAROUND_1");
> +}
> +
>   extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
>   
>   static int enable_psci_bp_hardening(void *data)
> @@ -154,12 +182,14 @@ static int enable_psci_bp_hardening(void *data)
>       bool ret = true;
>       static bool warned = false;
>   
> +    if ( check_smccc_arch_workaround_1(data) )
> +        return 0;
>       /*
>        * The mitigation is using PSCI version function to invalidate the
>        * branch predictor. This function is only available with PSCI 0.2
>        * and later.
>        */
> -    if ( psci_ver >= PSCI_VERSION(0, 2) )
> +    else if ( psci_ver >= PSCI_VERSION(0, 2) )
>           ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
>                                          __psci_hyp_bp_inval_end,
>                                          "call PSCI get version");
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 154772b728..8342cc33fe 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -261,6 +261,7 @@ struct arm_smccc_res {
>   /* SMCCC error codes */
>   #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>   #define ARM_SMCCC_NOT_SUPPORTED         (-1)
> +#define ARM_SMCCC_SUCCESS               (0)
>   
>   /* SMCCC function identifier range which is reserved for existing APIs */
>   #define ARM_SMCCC_RESERVED_RANGE_START  0x0
> 

-- 
Volodymyr Babchuk

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

Re: [PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     Changes in v3:
>         - Add the missing call to smc #0.
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
>  xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
>  xen/include/asm-arm/smccc.h |  1 +
>  3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> index 4b7f1dc21f..981fb83a88 100644
> --- a/xen/arch/arm/arm64/bpi.S
> +++ b/xen/arch/arm/arm64/bpi.S
> @@ -16,6 +16,8 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <asm/smccc.h>
> +
>  .macro ventry target
>      .rept 31
>      nop
> @@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
>      add     sp, sp, #(8 * 18)
>  ENTRY(__psci_hyp_bp_inval_end)
>  
> +ENTRY(__smccc_workaround_1_smc_start)
> +    sub     sp, sp, #(8 * 4)
> +    stp     x2, x3, [sp, #(8 * 0)]
> +    stp     x0, x1, [sp, #(8 * 2)]
> +    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
> +    smc     #0
> +    ldp     x2, x3, [sp, #(8 * 0)]
> +    ldp     x0, x1, [sp, #(8 * 2)]
> +    add     sp, sp, #(8 * 4)
> +ENTRY(__smccc_workaround_1_smc_end)
> +
>  /*
>   * Local variables:
>   * mode: ASM
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 8d5f8d372a..dec9074422 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>      return ret;
>  }
>  
> +extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
> +
> +static bool
> +check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
> +{
> +    struct arm_smccc_res res;
> +
> +    /*
> +     * Enable callbacks are called on every CPU based on the
> +     * capabilities. So double-check whether the CPU matches the
> +     * entry.
> +     */
> +    if ( !entry->matches(entry) )
> +        return false;

I think this should be return true?


> +    if ( smccc_ver < SMCCC_VERSION(1, 1) )
> +        return false;
> +
> +    arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
> +                      ARM_SMCCC_ARCH_WORKAROUND_1_FID, &res);
> +    if ( res.a0 != ARM_SMCCC_SUCCESS )
> +        return false;
> +
> +    return install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
> +                                    __smccc_workaround_1_smc_end,
> +                                    "call ARM_SMCCC_ARCH_WORKAROUND_1");
> +}
> +
>  extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
>  
>  static int enable_psci_bp_hardening(void *data)
> @@ -154,12 +182,14 @@ static int enable_psci_bp_hardening(void *data)
>      bool ret = true;
>      static bool warned = false;
>  
> +    if ( check_smccc_arch_workaround_1(data) )
> +        return 0;
>      /*
>       * The mitigation is using PSCI version function to invalidate the
>       * branch predictor. This function is only available with PSCI 0.2
>       * and later.
>       */
> -    if ( psci_ver >= PSCI_VERSION(0, 2) )
> +    else if ( psci_ver >= PSCI_VERSION(0, 2) )
>          ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
>                                         __psci_hyp_bp_inval_end,
>                                         "call PSCI get version");
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 154772b728..8342cc33fe 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -261,6 +261,7 @@ struct arm_smccc_res {
>  /* SMCCC error codes */
>  #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>  #define ARM_SMCCC_NOT_SUPPORTED         (-1)
> +#define ARM_SMCCC_SUCCESS               (0)
>  
>  /* SMCCC function identifier range which is reserved for existing APIs */
>  #define ARM_SMCCC_RESERVED_RANGE_START  0x0
> -- 
> 2.11.0
> 

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

Re: [PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Julien Grall]

Hi Stefano,

On 21/02/2018 00:35, Stefano Stabellini wrote:
> On Thu, 15 Feb 2018, Julien Grall wrote:
>> Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      Changes in v3:
>>          - Add the missing call to smc #0.
>>
>>      Changes in v2:
>>          - Patch added
>> ---
>>   xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
>>   xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
>>   xen/include/asm-arm/smccc.h |  1 +
>>   3 files changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
>> index 4b7f1dc21f..981fb83a88 100644
>> --- a/xen/arch/arm/arm64/bpi.S
>> +++ b/xen/arch/arm/arm64/bpi.S
>> @@ -16,6 +16,8 @@
>>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>    */
>>   
>> +#include <asm/smccc.h>
>> +
>>   .macro ventry target
>>       .rept 31
>>       nop
>> @@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
>>       add     sp, sp, #(8 * 18)
>>   ENTRY(__psci_hyp_bp_inval_end)
>>   
>> +ENTRY(__smccc_workaround_1_smc_start)
>> +    sub     sp, sp, #(8 * 4)
>> +    stp     x2, x3, [sp, #(8 * 0)]
>> +    stp     x0, x1, [sp, #(8 * 2)]
>> +    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
>> +    smc     #0
>> +    ldp     x2, x3, [sp, #(8 * 0)]
>> +    ldp     x0, x1, [sp, #(8 * 2)]
>> +    add     sp, sp, #(8 * 4)
>> +ENTRY(__smccc_workaround_1_smc_end)
>> +
>>   /*
>>    * Local variables:
>>    * mode: ASM
>> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
>> index 8d5f8d372a..dec9074422 100644
>> --- a/xen/arch/arm/cpuerrata.c
>> +++ b/xen/arch/arm/cpuerrata.c
>> @@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>>       return ret;
>>   }
>>   
>> +extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
>> +
>> +static bool
>> +check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
>> +{
>> +    struct arm_smccc_res res;
>> +
>> +    /*
>> +     * Enable callbacks are called on every CPU based on the
>> +     * capabilities. So double-check whether the CPU matches the
>> +     * entry.
>> +     */
>> +    if ( !entry->matches(entry) )
>> +        return false;
> 
> I think this should be return true?

Both are valid. It depends how you consider the workflow here. If you 
return:
	- true: You say that this helper already took care of that CPU. So no 
need to continue further.
	- false: This CPU does not match, let's fallback to a different method. 
That method will bailout later (see install_bp_hardening_vec).

I choose the latte because the SMCCC workaround is considered as an 
alternative method. So we want to fallback to the other one if it does 
not work at the cost of few extra instructions. But that's boot and 
going to be reworked in patch #11. Indeed this is just a temporary 
solution to plumb the new hardening method before we kill the 
PSCI_GET_VERSION one.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Stefano Stabellini]

On Wed, 21 Feb 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 21/02/2018 00:35, Stefano Stabellini wrote:
> > On Thu, 15 Feb 2018, Julien Grall wrote:
> > > Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > > 
> > > ---
> > >      Changes in v3:
> > >          - Add the missing call to smc #0.
> > > 
> > >      Changes in v2:
> > >          - Patch added
> > > ---
> > >   xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
> > >   xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
> > >   xen/include/asm-arm/smccc.h |  1 +
> > >   3 files changed, 45 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> > > index 4b7f1dc21f..981fb83a88 100644
> > > --- a/xen/arch/arm/arm64/bpi.S
> > > +++ b/xen/arch/arm/arm64/bpi.S
> > > @@ -16,6 +16,8 @@
> > >    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > >    */
> > >   +#include <asm/smccc.h>
> > > +
> > >   .macro ventry target
> > >       .rept 31
> > >       nop
> > > @@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
> > >       add     sp, sp, #(8 * 18)
> > >   ENTRY(__psci_hyp_bp_inval_end)
> > >   +ENTRY(__smccc_workaround_1_smc_start)
> > > +    sub     sp, sp, #(8 * 4)
> > > +    stp     x2, x3, [sp, #(8 * 0)]
> > > +    stp     x0, x1, [sp, #(8 * 2)]
> > > +    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
> > > +    smc     #0
> > > +    ldp     x2, x3, [sp, #(8 * 0)]
> > > +    ldp     x0, x1, [sp, #(8 * 2)]
> > > +    add     sp, sp, #(8 * 4)
> > > +ENTRY(__smccc_workaround_1_smc_end)
> > > +
> > >   /*
> > >    * Local variables:
> > >    * mode: ASM
> > > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> > > index 8d5f8d372a..dec9074422 100644
> > > --- a/xen/arch/arm/cpuerrata.c
> > > +++ b/xen/arch/arm/cpuerrata.c
> > > @@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct
> > > arm_cpu_capabilities *entry,
> > >       return ret;
> > >   }
> > >   +extern char __smccc_workaround_1_smc_start[],
> > > __smccc_workaround_1_smc_end[];
> > > +
> > > +static bool
> > > +check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
> > > +{
> > > +    struct arm_smccc_res res;
> > > +
> > > +    /*
> > > +     * Enable callbacks are called on every CPU based on the
> > > +     * capabilities. So double-check whether the CPU matches the
> > > +     * entry.
> > > +     */
> > > +    if ( !entry->matches(entry) )
> > > +        return false;
> > 
> > I think this should be return true?
> 
> Both are valid. It depends how you consider the workflow here. If you return:
> 	- true: You say that this helper already took care of that CPU. So no
> need to continue further.
> 	- false: This CPU does not match, let's fallback to a different
> method. That method will bailout later (see install_bp_hardening_vec).
> 
> I choose the latte because the SMCCC workaround is considered as an
> alternative method. So we want to fallback to the other one if it does not
> work at the cost of few extra instructions. But that's boot and going to be
> reworked in patch #11. Indeed this is just a temporary solution to plumb the
> new hardening method before we kill the PSCI_GET_VERSION one.

Yeah, I noticed that this is moot given the next patches in the series.
Given that you are already resending the series, I would also change this
to return true because I think it makes more sense, but it is
unimportant so either way:


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

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

Re: [PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Julien Grall]

Hi,

On 21/02/18 17:35, Stefano Stabellini wrote:
> On Wed, 21 Feb 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 21/02/2018 00:35, Stefano Stabellini wrote:
>>> On Thu, 15 Feb 2018, Julien Grall wrote:
>>>> Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
>>>>
>>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>>>
>>>> ---
>>>>       Changes in v3:
>>>>           - Add the missing call to smc #0.
>>>>
>>>>       Changes in v2:
>>>>           - Patch added
>>>> ---
>>>>    xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
>>>>    xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
>>>>    xen/include/asm-arm/smccc.h |  1 +
>>>>    3 files changed, 45 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
>>>> index 4b7f1dc21f..981fb83a88 100644
>>>> --- a/xen/arch/arm/arm64/bpi.S
>>>> +++ b/xen/arch/arm/arm64/bpi.S
>>>> @@ -16,6 +16,8 @@
>>>>     * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>>     */
>>>>    +#include <asm/smccc.h>
>>>> +
>>>>    .macro ventry target
>>>>        .rept 31
>>>>        nop
>>>> @@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
>>>>        add     sp, sp, #(8 * 18)
>>>>    ENTRY(__psci_hyp_bp_inval_end)
>>>>    +ENTRY(__smccc_workaround_1_smc_start)
>>>> +    sub     sp, sp, #(8 * 4)
>>>> +    stp     x2, x3, [sp, #(8 * 0)]
>>>> +    stp     x0, x1, [sp, #(8 * 2)]
>>>> +    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
>>>> +    smc     #0
>>>> +    ldp     x2, x3, [sp, #(8 * 0)]
>>>> +    ldp     x0, x1, [sp, #(8 * 2)]
>>>> +    add     sp, sp, #(8 * 4)
>>>> +ENTRY(__smccc_workaround_1_smc_end)
>>>> +
>>>>    /*
>>>>     * Local variables:
>>>>     * mode: ASM
>>>> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
>>>> index 8d5f8d372a..dec9074422 100644
>>>> --- a/xen/arch/arm/cpuerrata.c
>>>> +++ b/xen/arch/arm/cpuerrata.c
>>>> @@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct
>>>> arm_cpu_capabilities *entry,
>>>>        return ret;
>>>>    }
>>>>    +extern char __smccc_workaround_1_smc_start[],
>>>> __smccc_workaround_1_smc_end[];
>>>> +
>>>> +static bool
>>>> +check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
>>>> +{
>>>> +    struct arm_smccc_res res;
>>>> +
>>>> +    /*
>>>> +     * Enable callbacks are called on every CPU based on the
>>>> +     * capabilities. So double-check whether the CPU matches the
>>>> +     * entry.
>>>> +     */
>>>> +    if ( !entry->matches(entry) )
>>>> +        return false;
>>>
>>> I think this should be return true?
>>
>> Both are valid. It depends how you consider the workflow here. If you return:
>> 	- true: You say that this helper already took care of that CPU. So no
>> need to continue further.
>> 	- false: This CPU does not match, let's fallback to a different
>> method. That method will bailout later (see install_bp_hardening_vec).
>>
>> I choose the latte because the SMCCC workaround is considered as an
>> alternative method. So we want to fallback to the other one if it does not
>> work at the cost of few extra instructions. But that's boot and going to be
>> reworked in patch #11. Indeed this is just a temporary solution to plumb the
>> new hardening method before we kill the PSCI_GET_VERSION one.
> 
> Yeah, I noticed that this is moot given the next patches in the series.
> Given that you are already resending the series, I would also change this
> to return true because I think it makes more sense, but it is
> unimportant so either way:

I will keep false, because it make little sense to return true here, It 
will actually bring more confusion as we return false just after for a 
similar case.

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

I am going to drop it as I need some rework base on Andre's comment.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Andre Przywara]

Hi,

On 15/02/18 15:02, Julien Grall wrote:
> Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     Changes in v3:
>         - Add the missing call to smc #0.
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
>  xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
>  xen/include/asm-arm/smccc.h |  1 +
>  3 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> index 4b7f1dc21f..981fb83a88 100644
> --- a/xen/arch/arm/arm64/bpi.S
> +++ b/xen/arch/arm/arm64/bpi.S
> @@ -16,6 +16,8 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <asm/smccc.h>
> +
>  .macro ventry target
>      .rept 31
>      nop
> @@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
>      add     sp, sp, #(8 * 18)
>  ENTRY(__psci_hyp_bp_inval_end)
>  
> +ENTRY(__smccc_workaround_1_smc_start)
> +    sub     sp, sp, #(8 * 4)
> +    stp     x2, x3, [sp, #(8 * 0)]
> +    stp     x0, x1, [sp, #(8 * 2)]
> +    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
> +    smc     #0
> +    ldp     x2, x3, [sp, #(8 * 0)]
> +    ldp     x0, x1, [sp, #(8 * 2)]

I was expecting the restore to *mirror* the saving order, so x0, x1
first, then x2, x3. The code you have is correct, but somewhat
surprising. I wonder if you could just swap those two lines.

Or even better: you swap the store commands above, so that they match
what a push sequence would look like (higher addresses first).

> +    add     sp, sp, #(8 * 4)
> +ENTRY(__smccc_workaround_1_smc_end)
> +
>  /*
>   * Local variables:
>   * mode: ASM
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 8d5f8d372a..dec9074422 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>      return ret;
>  }
>  
> +extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
> +
> +static bool
> +check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
> +{
> +    struct arm_smccc_res res;
> +
> +    /*
> +     * Enable callbacks are called on every CPU based on the
> +     * capabilities. So double-check whether the CPU matches the
> +     * entry.
> +     */
> +    if ( !entry->matches(entry) )
> +        return false;
> +
> +    if ( smccc_ver < SMCCC_VERSION(1, 1) )
> +        return false;
> +

I guess we are calling the actual workaround function here to ultimately
know if that is implemented? And we know that this function isn't
harmful to call in any case?
Can you add a comment stating this here? Otherwise it's slightly
confusing to see the actual call in the function actually called check_
and installing the workaround.

Cheers,
Andre.

> +    arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
> +                      ARM_SMCCC_ARCH_WORKAROUND_1_FID, &res);
> +    if ( res.a0 != ARM_SMCCC_SUCCESS )
> +        return false;
> +
> +    return install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
> +                                    __smccc_workaround_1_smc_end,
> +                                    "call ARM_SMCCC_ARCH_WORKAROUND_1");
> +}
> +
>  extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
>  
>  static int enable_psci_bp_hardening(void *data)
> @@ -154,12 +182,14 @@ static int enable_psci_bp_hardening(void *data)
>      bool ret = true;
>      static bool warned = false;
>  
> +    if ( check_smccc_arch_workaround_1(data) )
> +        return 0;
>      /*
>       * The mitigation is using PSCI version function to invalidate the
>       * branch predictor. This function is only available with PSCI 0.2
>       * and later.
>       */
> -    if ( psci_ver >= PSCI_VERSION(0, 2) )
> +    else if ( psci_ver >= PSCI_VERSION(0, 2) )
>          ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
>                                         __psci_hyp_bp_inval_end,
>                                         "call PSCI get version");
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index 154772b728..8342cc33fe 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -261,6 +261,7 @@ struct arm_smccc_res {
>  /* SMCCC error codes */
>  #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION  (-1)
>  #define ARM_SMCCC_NOT_SUPPORTED         (-1)
> +#define ARM_SMCCC_SUCCESS               (0)
>  
>  /* SMCCC function identifier range which is reserved for existing APIs */
>  #define ARM_SMCCC_RESERVED_RANGE_START  0x0
> 

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

Re: [PATCH v3 10/17] xen/arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Julien Grall]

On 21/02/18 16:07, Andre Przywara wrote:
> Hi,

Hi Andre,

> On 15/02/18 15:02, Julien Grall wrote:
>> Add the detection and runtime code for ARM_SMCCC_ARCH_WORKAROUND_1.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      Changes in v3:
>>          - Add the missing call to smc #0.
>>
>>      Changes in v2:
>>          - Patch added
>> ---
>>   xen/arch/arm/arm64/bpi.S    | 13 +++++++++++++
>>   xen/arch/arm/cpuerrata.c    | 32 +++++++++++++++++++++++++++++++-
>>   xen/include/asm-arm/smccc.h |  1 +
>>   3 files changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
>> index 4b7f1dc21f..981fb83a88 100644
>> --- a/xen/arch/arm/arm64/bpi.S
>> +++ b/xen/arch/arm/arm64/bpi.S
>> @@ -16,6 +16,8 @@
>>    * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>    */
>>   
>> +#include <asm/smccc.h>
>> +
>>   .macro ventry target
>>       .rept 31
>>       nop
>> @@ -81,6 +83,17 @@ ENTRY(__psci_hyp_bp_inval_start)
>>       add     sp, sp, #(8 * 18)
>>   ENTRY(__psci_hyp_bp_inval_end)
>>   
>> +ENTRY(__smccc_workaround_1_smc_start)
>> +    sub     sp, sp, #(8 * 4)
>> +    stp     x2, x3, [sp, #(8 * 0)]
>> +    stp     x0, x1, [sp, #(8 * 2)]
>> +    mov     w0, #ARM_SMCCC_ARCH_WORKAROUND_1_FID
>> +    smc     #0
>> +    ldp     x2, x3, [sp, #(8 * 0)]
>> +    ldp     x0, x1, [sp, #(8 * 2)]
> 
> I was expecting the restore to *mirror* the saving order, so x0, x1
> first, then x2, x3. The code you have is correct, but somewhat
> surprising. I wonder if you could just swap those two lines.
> 
> Or even better: you swap the store commands above, so that they match
> what a push sequence would look like (higher addresses first).

I will choose this solution.

> 
>> +    add     sp, sp, #(8 * 4)
>> +ENTRY(__smccc_workaround_1_smc_end)
>> +
>>   /*
>>    * Local variables:
>>    * mode: ASM
>> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
>> index 8d5f8d372a..dec9074422 100644
>> --- a/xen/arch/arm/cpuerrata.c
>> +++ b/xen/arch/arm/cpuerrata.c
>> @@ -147,6 +147,34 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>>       return ret;
>>   }
>>   
>> +extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
>> +
>> +static bool
>> +check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
>> +{
>> +    struct arm_smccc_res res;
>> +
>> +    /*
>> +     * Enable callbacks are called on every CPU based on the
>> +     * capabilities. So double-check whether the CPU matches the
>> +     * entry.
>> +     */
>> +    if ( !entry->matches(entry) )
>> +        return false;
>> +
>> +    if ( smccc_ver < SMCCC_VERSION(1, 1) )
>> +        return false;
>> +
> 
> I guess we are calling the actual workaround function here to ultimately
> know if that is implemented? And we know that this function isn't
> harmful to call in any case?

What do you mean? This is very similar to what we do in 
enable_psci_bp_hardening. Except here we say the platform does not have 
SMCCC 1.1, so fallback to another solution.

> Can you add a comment stating this here?

Stating what? It is clear enough that you can't call arm_smccc_1_1_smc 
if the SMCCC version is not 1.1 (or later).

> Otherwise it's slightly
> confusing to see the actual call in the function actually called check_
> and installing the workaround.

Please see the follow-up patch. The current naming makes sense because 
we will fallback the PSCI one if not working.

Cheers,

-- 
Julien Grall

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

[PATCH v3 11/17] xen/arm64: Kill PSCI_GET_VERSION as a variant-2 workaround

[Julien Grall]

Now that we've standardised on SMCCC v1.1 to perform the branch
prediction invalidation, let's drop the previous band-aid. If vendors
haven't updated their firmware to do SMCCC 1.1, they haven't updated
PSCI either, so we don't loose anything.

This is aligned with the Linux commit 3a0a397ff5ff.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---
    Note that the patch is in arm64/for-next/core and should be merged
    in master soon.

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

    Changes in v2:
        - Patch added
---
 xen/arch/arm/arm64/bpi.S | 25 ----------------------
 xen/arch/arm/cpuerrata.c | 54 +++++++++++++++++-------------------------------
 2 files changed, 19 insertions(+), 60 deletions(-)

diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
index 981fb83a88..27ff801ed3 100644
--- a/xen/arch/arm/arm64/bpi.S
+++ b/xen/arch/arm/arm64/bpi.S
@@ -58,31 +58,6 @@ ENTRY(__bp_harden_hyp_vecs_start)
     .endr
 ENTRY(__bp_harden_hyp_vecs_end)
 
-ENTRY(__psci_hyp_bp_inval_start)
-    sub     sp, sp, #(8 * 18)
-    stp     x16, x17, [sp, #(16 * 0)]
-    stp     x14, x15, [sp, #(16 * 1)]
-    stp     x12, x13, [sp, #(16 * 2)]
-    stp     x10, x11, [sp, #(16 * 3)]
-    stp     x8, x9, [sp, #(16 * 4)]
-    stp     x6, x7, [sp, #(16 * 5)]
-    stp     x4, x5, [sp, #(16 * 6)]
-    stp     x2, x3, [sp, #(16 * 7)]
-    stp     x0, x1, [sp, #(16 * 8)]
-    mov     x0, #0x84000000
-    smc     #0
-    ldp     x16, x17, [sp, #(16 * 0)]
-    ldp     x14, x15, [sp, #(16 * 1)]
-    ldp     x12, x13, [sp, #(16 * 2)]
-    ldp     x10, x11, [sp, #(16 * 3)]
-    ldp     x8, x9, [sp, #(16 * 4)]
-    ldp     x6, x7, [sp, #(16 * 5)]
-    ldp     x4, x5, [sp, #(16 * 6)]
-    ldp     x2, x3, [sp, #(16 * 7)]
-    ldp     x0, x1, [sp, #(16 * 8)]
-    add     sp, sp, #(8 * 18)
-ENTRY(__psci_hyp_bp_inval_end)
-
 ENTRY(__smccc_workaround_1_smc_start)
     sub     sp, sp, #(8 * 4)
     stp     x2, x3, [sp, #(8 * 0)]
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index dec9074422..4eb1567589 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -149,10 +149,11 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
 
 extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
 
-static bool
-check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
+static int enable_smccc_arch_workaround_1(void *data)
 {
     struct arm_smccc_res res;
+    static bool warned = false;
+    const struct arm_cpu_capabilities *entry = data;
 
     /*
      * Enable callbacks are called on every CPU based on the
@@ -160,47 +161,30 @@ check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
      * entry.
      */
     if ( !entry->matches(entry) )
-        return false;
+        return 0;
 
     if ( smccc_ver < SMCCC_VERSION(1, 1) )
-        return false;
+        goto warn;
 
     arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
                       ARM_SMCCC_ARCH_WORKAROUND_1_FID, &res);
     if ( res.a0 != ARM_SMCCC_SUCCESS )
-        return false;
-
-    return install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
-                                    __smccc_workaround_1_smc_end,
-                                    "call ARM_SMCCC_ARCH_WORKAROUND_1");
-}
+        goto warn;
 
-extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
+    return !install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
+                                     __smccc_workaround_1_smc_end,
+                                     "call ARM_SMCCC_ARCH_WORKAROUND_1");
 
-static int enable_psci_bp_hardening(void *data)
-{
-    bool ret = true;
-    static bool warned = false;
-
-    if ( check_smccc_arch_workaround_1(data) )
-        return 0;
-    /*
-     * The mitigation is using PSCI version function to invalidate the
-     * branch predictor. This function is only available with PSCI 0.2
-     * and later.
-     */
-    else if ( psci_ver >= PSCI_VERSION(0, 2) )
-        ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
-                                       __psci_hyp_bp_inval_end,
-                                       "call PSCI get version");
-    else if ( !warned )
+warn:
+    if ( !warned )
     {
         ASSERT(system_state < SYS_STATE_active);
-        warning_add("PSCI 0.2 or later is required for the branch predictor hardening.\n");
-        warned = true;
+        warning_add("No support for ARM_SMCCC_ARCH_WORKAROUND_1.\n"
+                    "Please update your firmware.\n");
+        warned = false;
     }
 
-    return !ret;
+    return 0;
 }
 
 #endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */
@@ -316,22 +300,22 @@ static const struct arm_cpu_capabilities arm_errata[] = {
     {
         .capability = ARM_HARDEN_BRANCH_PREDICTOR,
         MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
-        .enable = enable_psci_bp_hardening,
+        .enable = enable_smccc_arch_workaround_1,
     },
     {
         .capability = ARM_HARDEN_BRANCH_PREDICTOR,
         MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
-        .enable = enable_psci_bp_hardening,
+        .enable = enable_smccc_arch_workaround_1,
     },
     {
         .capability = ARM_HARDEN_BRANCH_PREDICTOR,
         MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
-        .enable = enable_psci_bp_hardening,
+        .enable = enable_smccc_arch_workaround_1,
     },
     {
         .capability = ARM_HARDEN_BRANCH_PREDICTOR,
         MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
-        .enable = enable_psci_bp_hardening,
+        .enable = enable_smccc_arch_workaround_1,
     },
 #endif
 #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
-- 
2.11.0


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

Re: [PATCH v3 11/17] xen/arm64: Kill PSCI_GET_VERSION as a variant-2 workaround

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> Now that we've standardised on SMCCC v1.1 to perform the branch
> prediction invalidation, let's drop the previous band-aid. If vendors
> haven't updated their firmware to do SMCCC 1.1, they haven't updated
> PSCI either, so we don't loose anything.
> 
> This is aligned with the Linux commit 3a0a397ff5ff.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

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


> ---
>     Note that the patch is in arm64/for-next/core and should be merged
>     in master soon.
> 
>     Changes in v3:
>         - Add Volodymyr's reviewed-by
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/arm64/bpi.S | 25 ----------------------
>  xen/arch/arm/cpuerrata.c | 54 +++++++++++++++++-------------------------------
>  2 files changed, 19 insertions(+), 60 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> index 981fb83a88..27ff801ed3 100644
> --- a/xen/arch/arm/arm64/bpi.S
> +++ b/xen/arch/arm/arm64/bpi.S
> @@ -58,31 +58,6 @@ ENTRY(__bp_harden_hyp_vecs_start)
>      .endr
>  ENTRY(__bp_harden_hyp_vecs_end)
>  
> -ENTRY(__psci_hyp_bp_inval_start)
> -    sub     sp, sp, #(8 * 18)
> -    stp     x16, x17, [sp, #(16 * 0)]
> -    stp     x14, x15, [sp, #(16 * 1)]
> -    stp     x12, x13, [sp, #(16 * 2)]
> -    stp     x10, x11, [sp, #(16 * 3)]
> -    stp     x8, x9, [sp, #(16 * 4)]
> -    stp     x6, x7, [sp, #(16 * 5)]
> -    stp     x4, x5, [sp, #(16 * 6)]
> -    stp     x2, x3, [sp, #(16 * 7)]
> -    stp     x0, x1, [sp, #(16 * 8)]
> -    mov     x0, #0x84000000
> -    smc     #0
> -    ldp     x16, x17, [sp, #(16 * 0)]
> -    ldp     x14, x15, [sp, #(16 * 1)]
> -    ldp     x12, x13, [sp, #(16 * 2)]
> -    ldp     x10, x11, [sp, #(16 * 3)]
> -    ldp     x8, x9, [sp, #(16 * 4)]
> -    ldp     x6, x7, [sp, #(16 * 5)]
> -    ldp     x4, x5, [sp, #(16 * 6)]
> -    ldp     x2, x3, [sp, #(16 * 7)]
> -    ldp     x0, x1, [sp, #(16 * 8)]
> -    add     sp, sp, #(8 * 18)
> -ENTRY(__psci_hyp_bp_inval_end)
> -
>  ENTRY(__smccc_workaround_1_smc_start)
>      sub     sp, sp, #(8 * 4)
>      stp     x2, x3, [sp, #(8 * 0)]
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index dec9074422..4eb1567589 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -149,10 +149,11 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>  
>  extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
>  
> -static bool
> -check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
> +static int enable_smccc_arch_workaround_1(void *data)
>  {
>      struct arm_smccc_res res;
> +    static bool warned = false;
> +    const struct arm_cpu_capabilities *entry = data;
>  
>      /*
>       * Enable callbacks are called on every CPU based on the
> @@ -160,47 +161,30 @@ check_smccc_arch_workaround_1(const struct arm_cpu_capabilities *entry)
>       * entry.
>       */
>      if ( !entry->matches(entry) )
> -        return false;
> +        return 0;
>  
>      if ( smccc_ver < SMCCC_VERSION(1, 1) )
> -        return false;
> +        goto warn;
>  
>      arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID,
>                        ARM_SMCCC_ARCH_WORKAROUND_1_FID, &res);
>      if ( res.a0 != ARM_SMCCC_SUCCESS )
> -        return false;
> -
> -    return install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
> -                                    __smccc_workaround_1_smc_end,
> -                                    "call ARM_SMCCC_ARCH_WORKAROUND_1");
> -}
> +        goto warn;
>  
> -extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
> +    return !install_bp_hardening_vec(entry,__smccc_workaround_1_smc_start,
> +                                     __smccc_workaround_1_smc_end,
> +                                     "call ARM_SMCCC_ARCH_WORKAROUND_1");
>  
> -static int enable_psci_bp_hardening(void *data)
> -{
> -    bool ret = true;
> -    static bool warned = false;
> -
> -    if ( check_smccc_arch_workaround_1(data) )
> -        return 0;
> -    /*
> -     * The mitigation is using PSCI version function to invalidate the
> -     * branch predictor. This function is only available with PSCI 0.2
> -     * and later.
> -     */
> -    else if ( psci_ver >= PSCI_VERSION(0, 2) )
> -        ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
> -                                       __psci_hyp_bp_inval_end,
> -                                       "call PSCI get version");
> -    else if ( !warned )
> +warn:
> +    if ( !warned )
>      {
>          ASSERT(system_state < SYS_STATE_active);
> -        warning_add("PSCI 0.2 or later is required for the branch predictor hardening.\n");
> -        warned = true;
> +        warning_add("No support for ARM_SMCCC_ARCH_WORKAROUND_1.\n"
> +                    "Please update your firmware.\n");
> +        warned = false;
>      }
>  
> -    return !ret;
> +    return 0;
>  }
>  
>  #endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */
> @@ -316,22 +300,22 @@ static const struct arm_cpu_capabilities arm_errata[] = {
>      {
>          .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>          MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
> -        .enable = enable_psci_bp_hardening,
> +        .enable = enable_smccc_arch_workaround_1,
>      },
>      {
>          .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>          MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
> -        .enable = enable_psci_bp_hardening,
> +        .enable = enable_smccc_arch_workaround_1,
>      },
>      {
>          .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>          MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
> -        .enable = enable_psci_bp_hardening,
> +        .enable = enable_smccc_arch_workaround_1,
>      },
>      {
>          .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>          MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
> -        .enable = enable_psci_bp_hardening,
> +        .enable = enable_smccc_arch_workaround_1,
>      },
>  #endif
>  #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
> -- 
> 2.11.0
> 

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

[PATCH v3 12/17] 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>
Reviewed-by: Volodymyr Babchuk <volodymyr.babchuk@epam.com>

---
    The reviewed-by was kept despite move this patch towards the end
    of the series because there was no clash with the rest of the series.

    Changes in v2:
        - Move the patch towards the end of the series as not strictly
        necessary for SP2.
        - Add Volodymyr's reviewed-by
---
 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 19ee7caeb4..7ea3ea58e3 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)
@@ -137,8 +146,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

Re: [PATCH v3 12/17] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall 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>

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

> ---
>     The reviewed-by was kept despite move this patch towards the end
>     of the series because there was no clash with the rest of the series.
> 
>     Changes in v2:
>         - Move the patch towards the end of the series as not strictly
>         necessary for SP2.
>         - Add Volodymyr's reviewed-by
> ---
>  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 19ee7caeb4..7ea3ea58e3 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)
> @@ -137,8 +146,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

Re: [PATCH v3 12/17] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Andre Przywara]

Hi,

On 15/02/18 15:02, Julien Grall 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.

Is that explicitly mentioned somewhere in the spec? I couldn't find
anything in the original DEN0022A. Or is that because it does *not*
mention anything about the GPR state that we are safe to put anything in
there?

>     - 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>

Given that it's safe to clobber x0/r0 on CPU_ON in PSCI 0.1, the rest
looks correct to me:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> ---
>     The reviewed-by was kept despite move this patch towards the end
>     of the series because there was no clash with the rest of the series.
> 
>     Changes in v2:
>         - Move the patch towards the end of the series as not strictly
>         necessary for SP2.
>         - Add Volodymyr's reviewed-by
> ---
>  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 19ee7caeb4..7ea3ea58e3 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)
> @@ -137,8 +146,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[] = {
> 

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

Re: [PATCH v3 12/17] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Julien Grall]

On 21/02/18 16:27, Andre Przywara wrote:
> Hi,

Hi,

> On 15/02/18 15:02, Julien Grall 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.
> 
> Is that explicitly mentioned somewhere in the spec? I couldn't find
> anything in the original DEN0022A. Or is that because it does *not*
> mention anything about the GPR state that we are safe to put anything in
> there?

Because nothing tells you what is the GPR state when booting a secondary 
CPUs in the ARM ARM. This is done to the specification to decide what 
would be the value.

Today Xen will zero them, but it is not because of specific requirements 
just to avoid leak hypervisor content in those registers.

Cheers,

> 
>>      - 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>
> 
> Given that it's safe to clobber x0/r0 on CPU_ON in PSCI 0.1, the rest
> looks correct to me:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Cheers,
> Andre.
> 
>> ---
>>      The reviewed-by was kept despite move this patch towards the end
>>      of the series because there was no clash with the rest of the series.
>>
>>      Changes in v2:
>>          - Move the patch towards the end of the series as not strictly
>>          necessary for SP2.
>>          - Add Volodymyr's reviewed-by
>> ---
>>   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 19ee7caeb4..7ea3ea58e3 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)
>> @@ -137,8 +146,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[] = {
>>

-- 
Julien Grall

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

Re: [PATCH v3 12/17] xen/arm: vpsci: Remove parameter 'ver' from do_common_cpu

[Julien Grall]

On 21/02/18 16:37, Julien Grall wrote:
> 
> 
> On 21/02/18 16:27, Andre Przywara wrote:
>> Hi,
> 
> Hi,
> 
>> On 15/02/18 15:02, Julien Grall 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.
>>
>> Is that explicitly mentioned somewhere in the spec? I couldn't find
>> anything in the original DEN0022A. Or is that because it does *not*
>> mention anything about the GPR state that we are safe to put anything in
>> there?
> 
> Because nothing tells you what is the GPR state when booting a secondary 
> CPUs in the ARM ARM. This is done to the specification to decide what 
> would be the value.
> 
> Today Xen will zero them, but it is not because of specific requirements 
> just to avoid leak hypervisor content in those registers.

I have added in the commit message:
"This was deduced from the spec not mentioning the state of general 
purpose registers on CPU on."

Cheers,

-- 
Julien Grall

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

[PATCH v3 13/17] xen/arm: psci: Consolidate PSCI version print

[Julien Grall]

Xen is printing the same way the PSCI version for 0.1, 0.2 and later.
The only different is the former is hardcoded.

Furthermore PSCI is now used for other things than SMP bring up. So only
print the PSCI version in psci_init.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

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

    Changes in v2:
        - Patch added
---
 xen/arch/arm/psci.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index bc7b2260e8..7a8cf54e6d 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -136,8 +136,6 @@ int __init psci_init_0_1(void)
 
     psci_ver = PSCI_VERSION(0, 1);
 
-    printk(XENLOG_INFO "Using PSCI-0.1 for SMP bringup\n");
-
     return 0;
 }
 
@@ -183,9 +181,6 @@ int __init psci_init_0_2(void)
 
     psci_cpu_on_nr = PSCI_0_2_FN_NATIVE(CPU_ON);
 
-    printk(XENLOG_INFO "Using PSCI-%u.%u for SMP bringup\n",
-           PSCI_VERSION_MAJOR(psci_ver), PSCI_VERSION_MINOR(psci_ver));
-
     return 0;
 }
 
@@ -205,6 +200,9 @@ int __init psci_init(void)
 
     psci_init_smccc();
 
+    printk(XENLOG_INFO "Using PSCI v%u.%u\n",
+           PSCI_VERSION_MAJOR(psci_ver), PSCI_VERSION_MINOR(psci_ver));
+
     return 0;
 }
 
-- 
2.11.0


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

Re: [PATCH v3 13/17] xen/arm: psci: Consolidate PSCI version print

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> Xen is printing the same way the PSCI version for 0.1, 0.2 and later.
> The only different is the former is hardcoded.
> 
> Furthermore PSCI is now used for other things than SMP bring up. So only
> print the PSCI version in psci_init.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

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


> ---
>     Changes in v3:
>         - Add Volodymyr's reviewed-by
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/psci.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index bc7b2260e8..7a8cf54e6d 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -136,8 +136,6 @@ int __init psci_init_0_1(void)
>  
>      psci_ver = PSCI_VERSION(0, 1);
>  
> -    printk(XENLOG_INFO "Using PSCI-0.1 for SMP bringup\n");
> -
>      return 0;
>  }
>  
> @@ -183,9 +181,6 @@ int __init psci_init_0_2(void)
>  
>      psci_cpu_on_nr = PSCI_0_2_FN_NATIVE(CPU_ON);
>  
> -    printk(XENLOG_INFO "Using PSCI-%u.%u for SMP bringup\n",
> -           PSCI_VERSION_MAJOR(psci_ver), PSCI_VERSION_MINOR(psci_ver));
> -
>      return 0;
>  }
>  
> @@ -205,6 +200,9 @@ int __init psci_init(void)
>  
>      psci_init_smccc();
>  
> +    printk(XENLOG_INFO "Using PSCI v%u.%u\n",
> +           PSCI_VERSION_MAJOR(psci_ver), PSCI_VERSION_MINOR(psci_ver));
> +
>      return 0;
>  }
>  
> -- 
> 2.11.0
> 

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

[PATCH v3 14/17] xen/arm: psci: Prefix with static any functions not exported

[Julien Grall]

A bunch of PSCI functions are not prefixed with static despite no one is
using them outside the file and the prototype is not available in
psci.h.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---

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

diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 7a8cf54e6d..5d94a9a9ae 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -66,7 +66,7 @@ static int __init psci_features(uint32_t psci_func_id)
     return call_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, 0, 0);
 }
 
-int __init psci_is_smc_method(const struct dt_device_node *psci)
+static int __init psci_is_smc_method(const struct dt_device_node *psci)
 {
     int ret;
     const char *prop_str;
@@ -109,7 +109,7 @@ static void __init psci_init_smccc(void)
            SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
 }
 
-int __init psci_init_0_1(void)
+static int __init psci_init_0_1(void)
 {
     int ret;
     const struct dt_device_node *psci;
@@ -139,7 +139,7 @@ int __init psci_init_0_1(void)
     return 0;
 }
 
-int __init psci_init_0_2(void)
+static int __init psci_init_0_2(void)
 {
     static const struct dt_device_match psci_ids[] __initconst =
     {
-- 
2.11.0


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

Re: [PATCH v3 14/17] xen/arm: psci: Prefix with static any functions not exported

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> A bunch of PSCI functions are not prefixed with static despite no one is
> using them outside the file and the prototype is not available in
> psci.h.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

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

> ---
> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/psci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> index 7a8cf54e6d..5d94a9a9ae 100644
> --- a/xen/arch/arm/psci.c
> +++ b/xen/arch/arm/psci.c
> @@ -66,7 +66,7 @@ static int __init psci_features(uint32_t psci_func_id)
>      return call_smc(PSCI_1_0_FN32_PSCI_FEATURES, psci_func_id, 0, 0);
>  }
>  
> -int __init psci_is_smc_method(const struct dt_device_node *psci)
> +static int __init psci_is_smc_method(const struct dt_device_node *psci)
>  {
>      int ret;
>      const char *prop_str;
> @@ -109,7 +109,7 @@ static void __init psci_init_smccc(void)
>             SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
>  }
>  
> -int __init psci_init_0_1(void)
> +static int __init psci_init_0_1(void)
>  {
>      int ret;
>      const struct dt_device_node *psci;
> @@ -139,7 +139,7 @@ int __init psci_init_0_1(void)
>      return 0;
>  }
>  
> -int __init psci_init_0_2(void)
> +static int __init psci_init_0_2(void)
>  {
>      static const struct dt_device_match psci_ids[] __initconst =
>      {
> -- 
> 2.11.0
> 

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

[PATCH v3 15/17] xen/arm: vpsci: Update the return type for MIGRATE_INFO_TYPE

[Julien Grall]

From the specification, the PSCI call MIGRATE_INFO_TYPE will return an
int32_t. Update the function return type to match it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Cc: mirela.simonovic@aggios.com

---
    Changes in v3:
        - Patch added
---
 xen/arch/arm/vpsci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 7ea3ea58e3..9a082aa6ee 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -186,7 +186,7 @@ static int32_t do_psci_0_2_affinity_info(register_t target_affinity,
     return PSCI_0_2_AFFINITY_LEVEL_OFF;
 }
 
-static uint32_t do_psci_0_2_migrate_info_type(void)
+static int32_t do_psci_0_2_migrate_info_type(void)
 {
     return PSCI_0_2_TOS_MP_OR_NOT_PRESENT;
 }
-- 
2.11.0


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

Re: [PATCH v3 15/17] xen/arm: vpsci: Update the return type for MIGRATE_INFO_TYPE

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> >From the specification, the PSCI call MIGRATE_INFO_TYPE will return an
> int32_t. Update the function return type to match it.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Cc: mirela.simonovic@aggios.com

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

> ---
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/vpsci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 7ea3ea58e3..9a082aa6ee 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -186,7 +186,7 @@ static int32_t do_psci_0_2_affinity_info(register_t target_affinity,
>      return PSCI_0_2_AFFINITY_LEVEL_OFF;
>  }
>  
> -static uint32_t do_psci_0_2_migrate_info_type(void)
> +static int32_t do_psci_0_2_migrate_info_type(void)
>  {
>      return PSCI_0_2_TOS_MP_OR_NOT_PRESENT;
>  }
 

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

[PATCH v3 16/17] xen/arm: vpsci: Introduce and use PSCI_INVALID_ADDRESS

[Julien Grall]

PSCI 1.0 added the error return PSCI_INVALID_ADDRESS. It is used to
indicate the entry point address is known to be invalid.

In Xen case, this error could be returned when a 64-bit vCPU is using a
Thumb entry address.

For PSCI 0.1 implementation, return PSCI_INVALID_PARAMETERS instead.

Suggested-by: mirela.simonovic@aggios.com
Signed-off-by: Julien Grall <julien.grall@arm.com>
Cc: mirela.simonovic@aggios.com

---
    Changes in v3:
        - Patch added
---
 xen/arch/arm/vpsci.c       | 10 +++++++---
 xen/include/asm-arm/psci.h |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 9a082aa6ee..1729f7071e 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -38,7 +38,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
 
     /* THUMB set is not allowed with 64-bit domain */
     if ( is_64bit_domain(d) && is_thumb )
-        return PSCI_INVALID_PARAMETERS;
+        return PSCI_INVALID_ADDRESS;
 
     if ( !test_bit(_VPF_down, &v->pause_flags) )
         return PSCI_ALREADY_ON;
@@ -99,10 +99,14 @@ static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point)
 
     ret = do_common_cpu_on(vcpuid, entry_point, 0);
     /*
-     * PSCI 0.1 does not define the return code PSCI_ALREADY_ON.
+     * PSCI 0.1 does not define the return codes PSCI_ALREADY_ON and
+     * PSCI_INVALID_ADDRESS.
      * Instead, return PSCI_INVALID_PARAMETERS.
      */
-    return (ret == PSCI_ALREADY_ON) ? PSCI_INVALID_PARAMETERS : ret;
+    if ( ret == PSCI_ALREADY_ON || ret == PSCI_INVALID_ADDRESS )
+        ret = PSCI_INVALID_PARAMETERS;
+
+    return ret;
 }
 
 static int32_t do_psci_cpu_off(uint32_t power_state)
diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
index e2629eed01..9ac820e94a 100644
--- a/xen/include/asm-arm/psci.h
+++ b/xen/include/asm-arm/psci.h
@@ -13,6 +13,7 @@
 #define PSCI_INTERNAL_FAILURE       -6
 #define PSCI_NOT_PRESENT            -7
 #define PSCI_DISABLED               -8
+#define PSCI_INVALID_ADDRESS        -9
 
 /* availability of PSCI on the host for SMP bringup */
 extern uint32_t psci_ver;
-- 
2.11.0


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

Re: [PATCH v3 16/17] xen/arm: vpsci: Introduce and use PSCI_INVALID_ADDRESS

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> PSCI 1.0 added the error return PSCI_INVALID_ADDRESS. It is used to
> indicate the entry point address is known to be invalid.
> 
> In Xen case, this error could be returned when a 64-bit vCPU is using a
> Thumb entry address.
> 
> For PSCI 0.1 implementation, return PSCI_INVALID_PARAMETERS instead.
> 
> Suggested-by: mirela.simonovic@aggios.com
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Cc: mirela.simonovic@aggios.com

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


> ---
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/vpsci.c       | 10 +++++++---
>  xen/include/asm-arm/psci.h |  1 +
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 9a082aa6ee..1729f7071e 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -38,7 +38,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>  
>      /* THUMB set is not allowed with 64-bit domain */
>      if ( is_64bit_domain(d) && is_thumb )
> -        return PSCI_INVALID_PARAMETERS;
> +        return PSCI_INVALID_ADDRESS;
>  
>      if ( !test_bit(_VPF_down, &v->pause_flags) )
>          return PSCI_ALREADY_ON;
> @@ -99,10 +99,14 @@ static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point)
>  
>      ret = do_common_cpu_on(vcpuid, entry_point, 0);
>      /*
> -     * PSCI 0.1 does not define the return code PSCI_ALREADY_ON.
> +     * PSCI 0.1 does not define the return codes PSCI_ALREADY_ON and
> +     * PSCI_INVALID_ADDRESS.
>       * Instead, return PSCI_INVALID_PARAMETERS.
>       */
> -    return (ret == PSCI_ALREADY_ON) ? PSCI_INVALID_PARAMETERS : ret;
> +    if ( ret == PSCI_ALREADY_ON || ret == PSCI_INVALID_ADDRESS )
> +        ret = PSCI_INVALID_PARAMETERS;
> +
> +    return ret;
>  }
>  
>  static int32_t do_psci_cpu_off(uint32_t power_state)
> diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
> index e2629eed01..9ac820e94a 100644
> --- a/xen/include/asm-arm/psci.h
> +++ b/xen/include/asm-arm/psci.h
> @@ -13,6 +13,7 @@
>  #define PSCI_INTERNAL_FAILURE       -6
>  #define PSCI_NOT_PRESENT            -7
>  #define PSCI_DISABLED               -8
> +#define PSCI_INVALID_ADDRESS        -9
>  
>  /* availability of PSCI on the host for SMP bringup */
>  extern uint32_t psci_ver;
> -- 
> 2.11.0
> 

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

[PATCH v3 17/17] xen/arm: vpsci: Rework the logic to start AArch32 vCPU in Thumb mode

[Julien Grall]

32-bit domain is able to select the instruction (ARM vs Thumb) to use
when boot a new vCPU via CPU_ON. This is indicated via bit[0] of the
entry point address (see "T32 support" in PSCI v1.1 DEN0022D). bit[0]
must be cleared when setting the PC.

At the moment, Xen is setting the CPSR.T but never clear bit[0]. Clear
it to match the specification.

At the same time, slighlty rework the code to make clear thumb is only for
32-bit domain. Lastly, take the opportunity to switch is_thumb from int
to bool.

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

---
    Changes in v3:
        - Patch added
---
 xen/arch/arm/vpsci.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
index 1729f7071e..9f4e5b8844 100644
--- a/xen/arch/arm/vpsci.c
+++ b/xen/arch/arm/vpsci.c
@@ -28,7 +28,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
     struct domain *d = current->domain;
     struct vcpu_guest_context *ctxt;
     int rc;
-    int is_thumb = entry_point & 1;
+    bool is_thumb = entry_point & 1;
     register_t vcpuid;
 
     vcpuid = vaffinity_to_vcpuid(target_cpu);
@@ -62,6 +62,13 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
     if ( is_32bit_domain(d) )
     {
         ctxt->user_regs.cpsr = PSR_GUEST32_INIT;
+        /* Start the VCPU with THUMB set if it's requested by the kernel */
+        if ( is_thumb )
+        {
+            ctxt->user_regs.cpsr |= PSR_THUMB;
+            ctxt->user_regs.pc64 &= ~(u64)1;
+        }
+
         ctxt->user_regs.r0_usr = context_id;
     }
 #ifdef CONFIG_ARM_64
@@ -71,10 +78,6 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
         ctxt->user_regs.x0 = context_id;
     }
 #endif
-
-    /* Start the VCPU with THUMB set if it's requested by the kernel */
-    if ( is_thumb )
-        ctxt->user_regs.cpsr |= PSR_THUMB;
     ctxt->flags = VGCF_online;
 
     domain_lock(d);
-- 
2.11.0


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

Re: [PATCH v3 17/17] xen/arm: vpsci: Rework the logic to start AArch32 vCPU in Thumb mode

[Stefano Stabellini]

On Thu, 15 Feb 2018, Julien Grall wrote:
> 32-bit domain is able to select the instruction (ARM vs Thumb) to use
> when boot a new vCPU via CPU_ON. This is indicated via bit[0] of the
> entry point address (see "T32 support" in PSCI v1.1 DEN0022D). bit[0]
> must be cleared when setting the PC.
> 
> At the moment, Xen is setting the CPSR.T but never clear bit[0]. Clear
> it to match the specification.
> 
> At the same time, slighlty rework the code to make clear thumb is only for
> 32-bit domain. Lastly, take the opportunity to switch is_thumb from int
> to bool.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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


> ---
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/vpsci.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 1729f7071e..9f4e5b8844 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -28,7 +28,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>      struct domain *d = current->domain;
>      struct vcpu_guest_context *ctxt;
>      int rc;
> -    int is_thumb = entry_point & 1;
> +    bool is_thumb = entry_point & 1;
>      register_t vcpuid;
>  
>      vcpuid = vaffinity_to_vcpuid(target_cpu);
> @@ -62,6 +62,13 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>      if ( is_32bit_domain(d) )
>      {
>          ctxt->user_regs.cpsr = PSR_GUEST32_INIT;
> +        /* Start the VCPU with THUMB set if it's requested by the kernel */
> +        if ( is_thumb )
> +        {
> +            ctxt->user_regs.cpsr |= PSR_THUMB;
> +            ctxt->user_regs.pc64 &= ~(u64)1;
> +        }
> +
>          ctxt->user_regs.r0_usr = context_id;
>      }
>  #ifdef CONFIG_ARM_64
> @@ -71,10 +78,6 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>          ctxt->user_regs.x0 = context_id;
>      }
>  #endif
> -
> -    /* Start the VCPU with THUMB set if it's requested by the kernel */
> -    if ( is_thumb )
> -        ctxt->user_regs.cpsr |= PSR_THUMB;
>      ctxt->flags = VGCF_online;
>  
>      domain_lock(d);

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

Re: [PATCH v3 17/17] xen/arm: vpsci: Rework the logic to start AArch32 vCPU in Thumb mode

[Andre Przywara]

Hi,

On 15/02/18 15:02, Julien Grall wrote:
> 32-bit domain is able to select the instruction (ARM vs Thumb) to use
> when boot a new vCPU via CPU_ON. This is indicated via bit[0] of the
> entry point address (see "T32 support" in PSCI v1.1 DEN0022D). bit[0]
> must be cleared when setting the PC.
> 
> At the moment, Xen is setting the CPSR.T but never clear bit[0]. Clear
> it to match the specification.

Yes, that is the right thing to do, as the spec requires this.

> At the same time, slighlty rework the code to make clear thumb is only for
> 32-bit domain. Lastly, take the opportunity to switch is_thumb from int
> to bool.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre.

> 
> ---
>     Changes in v3:
>         - Patch added
> ---
>  xen/arch/arm/vpsci.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c
> index 1729f7071e..9f4e5b8844 100644
> --- a/xen/arch/arm/vpsci.c
> +++ b/xen/arch/arm/vpsci.c
> @@ -28,7 +28,7 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>      struct domain *d = current->domain;
>      struct vcpu_guest_context *ctxt;
>      int rc;
> -    int is_thumb = entry_point & 1;
> +    bool is_thumb = entry_point & 1;
>      register_t vcpuid;
>  
>      vcpuid = vaffinity_to_vcpuid(target_cpu);
> @@ -62,6 +62,13 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>      if ( is_32bit_domain(d) )
>      {
>          ctxt->user_regs.cpsr = PSR_GUEST32_INIT;
> +        /* Start the VCPU with THUMB set if it's requested by the kernel */
> +        if ( is_thumb )
> +        {
> +            ctxt->user_regs.cpsr |= PSR_THUMB;
> +            ctxt->user_regs.pc64 &= ~(u64)1;
> +        }
> +
>          ctxt->user_regs.r0_usr = context_id;
>      }
>  #ifdef CONFIG_ARM_64
> @@ -71,10 +78,6 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point,
>          ctxt->user_regs.x0 = context_id;
>      }
>  #endif
> -
> -    /* Start the VCPU with THUMB set if it's requested by the kernel */
> -    if ( is_thumb )
> -        ctxt->user_regs.cpsr |= PSR_THUMB;
>      ctxt->flags = VGCF_online;
>  
>      domain_lock(d);
> 

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