On 02/03/2023 1:23 pm, Oleksii Kurochko wrote: > diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S > index ffd95f9f89..851b4691a5 100644 > --- a/xen/arch/riscv/riscv64/head.S > +++ b/xen/arch/riscv/riscv64/head.S > @@ -6,8 +7,31 @@ ENTRY(start) > /* Mask all interrupts */ > csrw CSR_SIE, zero > > + /* Save HART ID and DTB base */ > + lla a6, _bootcpu_id > + REG_S a0, (a6) > + lla a6, _dtb_base > + REG_S a1, (a6) > + > la sp, cpu0_boot_stack > li t0, STACK_SIZE > add sp, sp, t0 > > + lla a6, _bootcpu_id > + REG_L a0, (a6) > + lla a6, _dtb_base > + REG_L a1, (a6)
This is overkill. Put a comment at start identifying which parameters are in which registers, and just make sure not to clobber them - RISCV has plenty of registers. Right now, we don't touch the a registers at all, which is why your v1 patch worked. (a0 and a1 still have the same value when we get into C). If we do need to start calling more complex things here (and I'm not sure that we do), we could either store the parameters in s2-11, or spill them onto the stack; both of which are preferable to spilling to global variables like this. > + > tail start_xen > + > + /* > + * Boot cpu id is passed by a bootloader > + */ > +_bootcpu_id: > + RISCV_PTR 0x0 Just a note (as you want to delete this anyway), this isn't a PTR, it's a LONG. > + > + /* > + * DTB base is passed by a bootloader > + */ > +_dtb_base: > + RISCV_PTR 0x0 > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c > index 1c87899e8e..d9723fe1c0 100644 > --- a/xen/arch/riscv/setup.c > +++ b/xen/arch/riscv/setup.c > @@ -7,7 +7,8 @@ > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > __aligned(STACK_SIZE); > > -void __init noreturn start_xen(void) > +void __init noreturn start_xen(unsigned long bootcpu_id, > + unsigned long dtb_base) To be clear, this change should be this hunk exactly as it is, and a comment immediately ahead of ENTRY(start) describing the entry ABI. There is no need currently to change any of the asm code. ~Andrew
