Re: [Xen-devel] [PATCH 1/7] arm: traps: psci: use generic register accessors

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/08/2017 21:08, Volodymyr Babchuk wrote:
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 6cf9ee7..ed78b36 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1449,13 +1449,12 @@ static void do_debug_trap(struct cpu_user_regs *regs, 
> unsigned int code)
>  }
>  #endif
>  
> +#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val)
> +#define PSCI_ARG(reg,n) get_user_reg(reg, n)
> +
>  #ifdef CONFIG_ARM_64
> -#define PSCI_RESULT_REG(reg) (reg)->x0
> -#define PSCI_ARG(reg,n) (reg)->x##n
> -#define PSCI_ARG32(reg,n) (uint32_t)( (reg)->x##n & 0x00000000FFFFFFFF )
> +#define PSCI_ARG32(reg,n) (uint32_t)(get_user_reg(reg, n) & 
> 0x00000000FFFFFFFF)

There is no need for the mask as well as the explicit (uint32_t) cast. 
I'd recommend dropping the mask entirely.

If you insist on keeping the mask, then it should be 0xffffffffu or
0x00000000ffffffffull to be compliant with the C standard (A pedantic
compiler will complain that the literal is out of range of int).

Also as you are changing all of these macros, it would be nice to apply
correct style to them, by inserting spaces after all the commas.

~Andrew

>  #else
> -#define PSCI_RESULT_REG(reg) (reg)->r0
> -#define PSCI_ARG(reg,n) (reg)->r##n
>  #define PSCI_ARG32(reg,n) PSCI_ARG(reg,n)
>  #endif
>  
>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel