Re: [PATCH v1 3/8] xen/riscv: introduce stack stuff

[Oleksii <oleksii.kurochko@gmail.com>]

Hi,

On Fri, 2023-01-06 at 13:54 +0000, Julien Grall wrote:
> Hi,
> 
> On 06/01/2023 13:14, Oleksii Kurochko wrote:
> > The patch introduces and sets up a stack in order to go to C
> > environment
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> >   xen/arch/riscv/Makefile       | 1 +
> >   xen/arch/riscv/riscv64/head.S | 8 +++++++-
> >   xen/arch/riscv/setup.c        | 6 ++++++
> >   3 files changed, 14 insertions(+), 1 deletion(-)
> >   create mode 100644 xen/arch/riscv/setup.c
> > 
> > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> > index 248f2cbb3e..5a67a3f493 100644
> > --- a/xen/arch/riscv/Makefile
> > +++ b/xen/arch/riscv/Makefile
> > @@ -1,4 +1,5 @@
> >   obj-$(CONFIG_RISCV_64) += riscv64/
> > +obj-y += setup.o
> >   
> >   $(TARGET): $(TARGET)-syms
> >         $(OBJCOPY) -O binary -S $< $@
> > diff --git a/xen/arch/riscv/riscv64/head.S
> > b/xen/arch/riscv/riscv64/head.S
> > index 990edb70a0..ddc7104701 100644
> > --- a/xen/arch/riscv/riscv64/head.S
> > +++ b/xen/arch/riscv/riscv64/head.S
> > @@ -1,4 +1,10 @@
> >           .section .text.header, "ax", %progbits
> >   
> >   ENTRY(start)
> > -        j  start
> > +        la      sp, cpu0_boot_stack
> > +        li      t0, PAGE_SIZE
> 
> I would recommend to a define STACK_SIZE. So you don't make
> assumption 
> on the size in the code and it is easier to update.
> 
Thanks. I decied to not define STACK_SIZE because it is used now only
once and the STACK_SIZE that was introduced in Bobby's patch series was
too big at least for current purpose.

Any way it probably makes sensne to intrdouce STACK_SIZE from the start
so will take it into account during a work at new patch series.
> > +        add     sp, sp, t0
> > +
> > +_start_hang:
> > +        wfi
> > +        j  _start_hang
> > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
> > new file mode 100644
> > index 0000000000..2c7dca1daa
> > --- /dev/null
> > +++ b/xen/arch/riscv/setup.c
> > @@ -0,0 +1,6 @@
> > +#include <xen/init.h>
> > +#include <xen/compile.h>
> > +
> > +/* Xen stack for bringing up the first CPU. */
> > +unsigned char __initdata cpu0_boot_stack[PAGE_SIZE]
> > +    __aligned(PAGE_SIZE);
> 
> Cheers,
>