Re: [PATCH v1 01/14] xen/riscv: add _zicsr to CFLAGS

[Alistair Francis <alistair23@xxxxxxxxx>]

On Mon, Jan 23, 2023 at 8:43 PM Oleksii <oleksii.kurochko@xxxxxxxxx> wrote:
>
> On Fri, 2023-01-20 at 15:29 +0000, Andrew Cooper wrote:
> > On 20/01/2023 2:59 pm, Oleksii Kurochko wrote:
> > > Work with some registers requires csr command which is part of
> > > Zicsr.
> > >
> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > > ---
> > >  xen/arch/riscv/arch.mk | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
> > > index 012dc677c3..95b41d9f3e 100644
> > > --- a/xen/arch/riscv/arch.mk
> > > +++ b/xen/arch/riscv/arch.mk
> > > @@ -10,7 +10,7 @@ riscv-march-$(CONFIG_RISCV_ISA_C)       :=
> > > $(riscv-march-y)c
> > >  # into the upper half _or_ the lower half of the address space.
> > >  # -mcmodel=medlow would force Xen into the lower half.
> > >
> > > -CFLAGS += -march=$(riscv-march-y) -mstrict-align -mcmodel=medany
> > > +CFLAGS += -march=$(riscv-march-y)_zicsr -mstrict-align -
> > > mcmodel=medany
> >
> > Should we just go straight for G, rather than bumping it along every
> > time we make a tweak?
> >
> I didn't go straight for G as it represents the “IMAFDZicsr Zifencei”
> base and extensions thereby it will be needed to add support for FPU
> (at least it requires {save,restore}_fp_state) but I am not sure that
> we need it in general.

That seems fair enough. I don't see a reason to restrict ourselves if
we aren't using something. Although we probably will hit a requirement
on G at some point anyway.

Alistair

>
> Another one reason is that Linux kernel introduces _zicsr extenstion
> separately (but I am not sure that it can be considered as a serious
> argument):
> https://elixir.bootlin.com/linux/latest/source/arch/riscv/Makefile#L58
> https://lore.kernel.org/all/20221024113000.891820486@xxxxxxxxxxxxxxxxxxx/
>
> > ~Andrew
> ~Oleksii
>
>