[Xen-devel,4/7] xen/arm: vsmc: Implement SMCCC 1.1

Message ID 20180205132011.27996-5-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update
Related show

Commit Message

Julien Grall Feb. 5, 2018, 1:20 p.m.
The new SMC Calling Convention (v1.1) allows for a reduced overhead when
calling into the firmware, and provides a new feature discovery
mechanism. See ARM DEN 00070A.

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

Comments

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

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

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

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

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

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

Cheers,
Volodymyr Babchuk Feb. 7, 2018, 1:39 p.m. | #3
Hi Julien,

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

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

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

Patch

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