Re: [PATCH v4 17/30] xen/riscv: introduce regs.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

Hi Julien,

On Sun, 2024-02-18 at 18:22 +0000, Julien Grall wrote:
> Hi,
> 
> On 05/02/2024 15:32, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ------
> > Changes in V4:
> >   - add Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> >   - s/BUG()/BUG_ON("unimplemented")
> > ---
> > Changes in V3:
> >   - update the commit message
> >   - add Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> >   - remove "include <asm/current.h>" and use a forward declaration
> > instead.
> > ---
> > Changes in V2:
> >   - change xen/lib.h to xen/bug.h
> >   - remove unnecessary empty line
> > ---
> > xen/arch/riscv/include/asm/regs.h | 29
> > +++++++++++++++++++++++++++++
> >   1 file changed, 29 insertions(+)
> >   create mode 100644 xen/arch/riscv/include/asm/regs.h
> > 
> > diff --git a/xen/arch/riscv/include/asm/regs.h
> > b/xen/arch/riscv/include/asm/regs.h
> > new file mode 100644
> > index 0000000000..c70ea2aa0c
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/regs.h
> > @@ -0,0 +1,29 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ARM_RISCV_REGS_H__
> > +#define __ARM_RISCV_REGS_H__
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <xen/bug.h>
> > +
> > +#define hyp_mode(r)     (0)
> 
> I don't understand where here you return 0 (which should really be 
> false) but ...
> 
> > +
> > +struct cpu_user_regs;
> > +
> > +static inline bool guest_mode(const struct cpu_user_regs *r)
> > +{
> > +    BUG_ON("unimplemented");
> > +}
> 
> ... here you return BUG_ON(). But I couldn't find any user of both 
> guest_mode() and hyp_mode(). So isn't it a bit prematurate to
> introduce 
> the helpers?

I agree regarding hyp_mode() it can be dropped , but gest_mode() is
used by common/keyhandler.c:142.

~ Oleksii