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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-01-09 at 16:11 +0000, Julien Grall wrote:
> Hi,
> 
> On 09/01/2023 15:46, 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@xxxxxxxxx>
> 
> It looks like the comments from Andrew were missed.
> 
I will double check which one.
Thanks.
> > ---
> > Changes in V2:
> >      - introduce STACK_SIZE define.
> >      - use consistent padding between instruction mnemonic and
> > operand(s)
> > ---
> >   xen/arch/riscv/Makefile             | 1 +
> >   xen/arch/riscv/include/asm/config.h | 2 ++
> >   xen/arch/riscv/riscv64/head.S       | 8 +++++++-
> >   xen/arch/riscv/setup.c              | 6 ++++++
> >   4 files changed, 16 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/include/asm/config.h
> > b/xen/arch/riscv/include/asm/config.h
> > index 0370f865f3..bdd2237f01 100644
> > --- a/xen/arch/riscv/include/asm/config.h
> > +++ b/xen/arch/riscv/include/asm/config.h
> > @@ -43,6 +43,8 @@
> >   
> >   #define SMP_CACHE_BYTES (1 << 6)
> >   
> > +#define STACK_SIZE (PAGE_SIZE)
> > +
> >   #endif /* __RISCV_CONFIG_H__ */
> >   /*
> >    * Local variables:
> > diff --git a/xen/arch/riscv/riscv64/head.S
> > b/xen/arch/riscv/riscv64/head.S
> > index 990edb70a0..c1f33a1934 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, STACK_SIZE
> > +        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..41ef4912bd
> > --- /dev/null
> > +++ b/xen/arch/riscv/setup.c
> > @@ -0,0 +1,6 @@
> > +#include <xen/init.h>
> > +#include <xen/compile.h>
> 
> Why do you need to include <xen/compile.h>?
> 
It is needed to use __aligned define which looks better then
__attribute__((__aligned__(SOMETHING))) 
> In any case, please order the include alphabetically. I haven't look
> at 
Thanks. I didn't now that headers should be in alphabetic order too.
> the rest of the series. But please go through the series and check
> that 
> generic comments (like this one) are addressed everywhere.
> 
> Cheers,
> 
~Oleksii