On 02.03.2023 14:23, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -13,6 +13,15 @@ ENTRY(start)
>          lla     a6, _dtb_base
>          REG_S   a1, (a6)
>  
> +        la      a3, __bss_start
> +        la      a4, __bss_end
> +        ble     a4, a3, clear_bss_done

While it may be that .bss is indeed empty right now, even short term
it won't be, and never will. I'd drop this conditional (and in
particular the label), inserting a transient item into .bss for the
time being. As soon as your patch introducing page tables has landed,
there will be multiple pages worth of .bss.

Also are this and ...

> +clear_bss:
> +        REG_S   zero, (a3)
> +        add     a3, a3, RISCV_SZPTR
> +        blt     a3, a4, clear_bss

... this branch actually the correct ones? I'd expect the unsigned
flavors to be used when comparing addresses. It may not matter here
and/or right now, but it'll set a bad precedent unless you expect
to only ever work on addresses which have the sign bit clear.

Jan

> +clear_bss_done:
> +
>          la      sp, cpu0_boot_stack
>          li      t0, STACK_SIZE
>          add     sp, sp, t0


Reply via email to