diff mbox

[7/7] arm: vsmc: remove 64 bit mode check in psci handler

Message ID 1502222922-25821-8-git-send-email-volodymyr_babchuk@epam.com (mailing list archive)
State New, archived
Headers show

Commit Message

Volodymyr Babchuk Aug. 8, 2017, 8:08 p.m. UTC 
PSCI handling code had helper routine that checked calling convention.
It does not needed anymore, because:

 - Generic handler checks that 64 bit calls can be made only by
   64 bit guests.

 - SMCCC requires that 64-bit handler should support both 32 and 64 bit
   calls even if they originate from 64 bit caller.

This patch removes that extra check.

Also, as there are no more natural scope ( if { } ) to hold local variables,
paramters to do_psci_*() are taken right from SMC arguments, without storing
in intermediate local variables.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
Now this patch removes { } blocks completely. Not sure if I had to
split it into two separate patches.

---
 xen/arch/arm/vsmc.c | 45 ++++++++++-----------------------------------
 1 file changed, 10 insertions(+), 35 deletions(-)

Comments

Julien Grall Aug. 9, 2017, 11:38 a.m. UTC | #1 
Hi Volodymyr,

On 08/08/17 21:08, Volodymyr Babchuk wrote:
> PSCI handling code had helper routine that checked calling convention.
> It does not needed anymore, because:
>
>  - Generic handler checks that 64 bit calls can be made only by
>    64 bit guests.
>
>  - SMCCC requires that 64-bit handler should support both 32 and 64 bit
>    calls even if they originate from 64 bit caller.
>
> This patch removes that extra check.
>
> Also, as there are no more natural scope ( if { } ) to hold local variables,
> paramters to do_psci_*() are taken right from SMC arguments, without storing
> in intermediate local variables.

You could do:

case ....:
{
    uint64_t ...;
    uint32_t ...;

    ....
}

Now you have a scope and keep the code readable.

>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> Now this patch removes { } blocks completely. Not sure if I had to
> split it into two separate patches.
>
> ---
>  xen/arch/arm/vsmc.c | 45 ++++++++++-----------------------------------
>  1 file changed, 10 insertions(+), 35 deletions(-)
>
> diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
> index ea86eea..0fd4f5a 100644
> --- a/xen/arch/arm/vsmc.c
> +++ b/xen/arch/arm/vsmc.c
> @@ -84,14 +84,6 @@ static bool handle_arch(struct cpu_user_regs *regs)
>      return false;
>  }
>
> -/* helper function for checking arm mode 32/64 bit */
> -static inline int psci_mode_check(struct domain *d, register_t fid)
> -{
> -    return !( is_64bit_domain(d)^
> -              ((fid & (ARM_SMCCC_SMC_64 << ARM_SMCCC_CALL_CONV_SHIFT)) >>
> -               ARM_SMCCC_CALL_CONV_SHIFT) );
> -}
> -
>  /* PSCI 2.0 interface */
>  static bool handle_ssc(struct cpu_user_regs *regs)
>  {
> @@ -113,8 +105,7 @@ static bool handle_ssc(struct cpu_user_regs *regs)
>          return true;
>      case PSCI_0_2_FUNC_MIGRATE_INFO_UP_CPU:
>          perfc_incr(vpsci_migrate_info_up_cpu);
> -        if ( psci_mode_check(current->domain, fid) )
> -            PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
> +        PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
>          return true;
>      case PSCI_0_2_FUNC_SYSTEM_OFF:
>          perfc_incr(vpsci_system_off);
> @@ -128,40 +119,24 @@ static bool handle_ssc(struct cpu_user_regs *regs)
>          return true;
>      case PSCI_0_2_FUNC_CPU_ON:
>          perfc_incr(vpsci_cpu_on);
> -        if ( psci_mode_check(current->domain, fid) )
> -        {
> -            register_t vcpuid = PSCI_ARG(regs,1);
> -            register_t epoint = PSCI_ARG(regs,2);
> -            register_t cid = PSCI_ARG(regs,3);
> -            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
> -        }
> +        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(PSCI_ARG(regs, 1),
> +                                                 PSCI_ARG(regs, 2),
> +                                                 PSCI_ARG(regs, 3)));
>          return true;
>      case PSCI_0_2_FUNC_CPU_SUSPEND:
>          perfc_incr(vpsci_cpu_suspend);
> -        if ( psci_mode_check(current->domain, fid) )
> -        {
> -            uint32_t pstate = PSCI_ARG32(regs,1);
> -            register_t epoint = PSCI_ARG(regs,2);
> -            register_t cid = PSCI_ARG(regs,3);
> -            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
> -        }
> +        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(PSCI_ARG32(regs, 1),
> +                                                      PSCI_ARG(regs, 2),
> +                                                      PSCI_ARG(regs, 3)));
>          return true;
>      case PSCI_0_2_FUNC_AFFINITY_INFO:
>          perfc_incr(vpsci_cpu_affinity_info);
> -        if ( psci_mode_check(current->domain, fid) )
> -        {
> -            register_t taff = PSCI_ARG(regs,1);
> -            uint32_t laff = PSCI_ARG32(regs,2);
> -            PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
> -        }
> +        PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(PSCI_ARG(regs, 1),
> +                                                        PSCI_ARG32(regs,2)));
>          return true;
>      case PSCI_0_2_FUNC_MIGRATE:
>          perfc_incr(vpsci_cpu_migrate);
> -        if ( psci_mode_check(current->domain, fid) )
> -        {
> -            uint32_t tcpu = PSCI_ARG32(regs,1);
> -            PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu));
> -        }
> +        PSCI_SET_RESULT(regs, do_psci_0_2_migrate(PSCI_ARG32(regs, 1)));
>          return true;
>      case ARM_SMCCC_FUNC_CALL_COUNT:
>          set_user_reg(regs, 0, SSC_SMCCC_FUNCTION_COUNT);
>

Cheers,
diff mbox

Patch

diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index ea86eea..0fd4f5a 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -84,14 +84,6 @@  static bool handle_arch(struct cpu_user_regs *regs)
     return false;
 }
 
-/* helper function for checking arm mode 32/64 bit */
-static inline int psci_mode_check(struct domain *d, register_t fid)
-{
-    return !( is_64bit_domain(d)^
-              ((fid & (ARM_SMCCC_SMC_64 << ARM_SMCCC_CALL_CONV_SHIFT)) >>
-               ARM_SMCCC_CALL_CONV_SHIFT) );
-}
-
 /* PSCI 2.0 interface */
 static bool handle_ssc(struct cpu_user_regs *regs)
 {
@@ -113,8 +105,7 @@  static bool handle_ssc(struct cpu_user_regs *regs)
         return true;
     case PSCI_0_2_FUNC_MIGRATE_INFO_UP_CPU:
         perfc_incr(vpsci_migrate_info_up_cpu);
-        if ( psci_mode_check(current->domain, fid) )
-            PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
+        PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu());
         return true;
     case PSCI_0_2_FUNC_SYSTEM_OFF:
         perfc_incr(vpsci_system_off);
@@ -128,40 +119,24 @@  static bool handle_ssc(struct cpu_user_regs *regs)
         return true;
     case PSCI_0_2_FUNC_CPU_ON:
         perfc_incr(vpsci_cpu_on);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            register_t vcpuid = PSCI_ARG(regs,1);
-            register_t epoint = PSCI_ARG(regs,2);
-            register_t cid = PSCI_ARG(regs,3);
-            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid));
-        }
+        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(PSCI_ARG(regs, 1),
+                                                 PSCI_ARG(regs, 2),
+                                                 PSCI_ARG(regs, 3)));
         return true;
     case PSCI_0_2_FUNC_CPU_SUSPEND:
         perfc_incr(vpsci_cpu_suspend);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            uint32_t pstate = PSCI_ARG32(regs,1);
-            register_t epoint = PSCI_ARG(regs,2);
-            register_t cid = PSCI_ARG(regs,3);
-            PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid));
-        }
+        PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(PSCI_ARG32(regs, 1),
+                                                      PSCI_ARG(regs, 2),
+                                                      PSCI_ARG(regs, 3)));
         return true;
     case PSCI_0_2_FUNC_AFFINITY_INFO:
         perfc_incr(vpsci_cpu_affinity_info);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            register_t taff = PSCI_ARG(regs,1);
-            uint32_t laff = PSCI_ARG32(regs,2);
-            PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff));
-        }
+        PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(PSCI_ARG(regs, 1),
+                                                        PSCI_ARG32(regs,2)));
         return true;
     case PSCI_0_2_FUNC_MIGRATE:
         perfc_incr(vpsci_cpu_migrate);
-        if ( psci_mode_check(current->domain, fid) )
-        {
-            uint32_t tcpu = PSCI_ARG32(regs,1);
-            PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu));
-        }
+        PSCI_SET_RESULT(regs, do_psci_0_2_migrate(PSCI_ARG32(regs, 1)));
         return true;
     case ARM_SMCCC_FUNC_CALL_COUNT:
         set_user_reg(regs, 0, SSC_SMCCC_FUNCTION_COUNT);