Re: [Xen-devel] [PATCH 3/7] xen/arm: Rework psr_mode_is_32bit()

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 29/07/2019 22:52, Stefano Stabellini wrote:
> On Fri, 26 Jul 2019, Volodymyr Babchuk wrote:
> > Julien Grall writes:
> >
> > > Hi,
> > >
> > > On 26/07/2019 13:31, Volodymyr Babchuk wrote:
> > > > Julien Grall writes:
> > > >
> > > > > psr_mode_is_32bit() prototype does not match the rest of the helpers for
> > > > > the process state. Looking at the callers, most of them will access
> > > > > struct cpu_user_regs just for calling psr_mode_is_32bit().
> > > > >
> > > > > The macro is now reworked to take a struct cpu_user_regs in parameter.
> > > > > At the same time take the opportunity to switch to a static inline
> > > > > helper.
> > > > I'm a bit concerned about naming now. As psr_mode_is_32bit() is now have
> > > > no psr parameter, and ARM ARM uses term "state" instead of "mode", maybe
> > > > it is worth to rename this helper to something like "is_32bit_state"?
> > > It really depends how you see it. The bit is part of the "mode" field,
> > > so technically we are checking whether the mode corresponds to a
> > > 32-bit one or not. This is also inline with the rest of the helpers
> > > within this header.
> > >
> > > I would be willing to consider renaming the helper to regs_mode_is_32bit().
> > I'm fine with this name.
> The patch is fine by me, as is, or with the name changed to
> regs_mode_is_32bit. (I didn't commit it.)
I am thinking to get the renaming separately. So I can also rework the other 
helpers.
> Either way:
>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
So I will commit as is and add in my todo list renaming.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel