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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.)

Either way:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


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