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

[Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>]

Hi Andrew

On 08.08.17 23: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.
Yes, I know this. But Julien asked me to keep this ([1])

> 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.
Yep, will do. Thanks.

> ~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
> >   
[1] https://www.mail-archive.com/xen-devel@xxxxxxxxxxxxx/msg113198.html

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