| Message ID | e8f65c43d20ebdaba61738200360b14152531321.1673009740.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Basic early_printk and smoke test implementation | expand |
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. > + 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,
On 06.01.2023 14:14, Oleksii Kurochko wrote: > --- 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 > + add sp, sp, t0 > + > +_start_hang: > + wfi > + j _start_hang Nit: I think it would be nice if in a new port assembly code used consistent padding between insn mnemonic and operand(s). Jan
On 06/01/2023 1:14 pm, Oleksii Kurochko wrote: > 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 > + 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); Ah, I didn't spot his when looking at the unified delta of the series. You want most of patch 7 merged into this, so we end up with +void __init noreturn start_xen(void) +{ + for ( ;; ) + asm volatile ("wfi"); + + unreachable(); +} in setup.c too. That means we don't transiently add _start_hang just to delete it 3 patches later. (And it will fix Jan's observation about the misaligned operand.) Adding the early_printk("Hello from C env\n"); line wants merging into patch 6. ~Andrew
On Fri, 2023-01-06 at 15:15 +0100, Jan Beulich wrote: > On 06.01.2023 14:14, Oleksii Kurochko wrote: > > --- 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 > > + add sp, sp, t0 > > + > > +_start_hang: > > + wfi > > + j _start_hang > > Nit: I think it would be nice if in a new port assembly code used > consistent padding between insn mnemonic and operand(s). > Missed it. Actually it will be "fixed" in the later patches of the patch series but i'll fix this while working on a new version of the patch series. ~Oleksii > Jan
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, >
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 + 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);
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