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

[Julien Grall <julien.grall@arm.com>]

On 08/08/17 21:37, Andrew Cooper wrote:
> 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.

I want to avoid the implicit cast from 64-bit register to 32-bit that 
Volodymyr introduced in his series.

uint32_t pstate = get_user_reg(regs, 1);

IHMO this is a call to mistake. Another solution is to provide 3 helpers
	- get_user_reg32(...)
	- get_user_reg64(...)
	- get_user_reg(...) -> Return the full register (32-bit on ARM32, 
64-bit on ARM64).

This would at least document the return value of get_user_reg*.

Cheers,

-- 
Julien Grall

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