| Message ID | b253e61bebbc029c94b89389d81643f9587200b7.1673278109.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Basic early_printk and smoke test implementation | expand |
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@gmail.com> It looks like the comments from Andrew were missed. > --- > 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>? In any case, please order the include alphabetically. I haven't look at the rest of the series. But please go through the series and check that generic comments (like this one) are addressed everywhere. Cheers,
On 09.01.2023 16:46, Oleksii Kurochko wrote: > --- 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) Btw, nit: No need for parentheses here. Jan
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@gmail.com> > > 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
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> + +/* Xen stack for bringing up the first CPU. */ +unsigned char __initdata cpu0_boot_stack[STACK_SIZE] + __aligned(STACK_SIZE);
The patch introduces and sets up a stack in order to go to C environment Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- 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