Re: [PATCH v2 1/3] xen/riscv: read/save hart_id and dtb_base passed by bootloader

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/03/2023 2:53 pm, Oleksii wrote:
> On Thu, 2023-03-02 at 14:02 +0000, Andrew Cooper wrote:
>> On 02/03/2023 1:23 pm, Oleksii Kurochko wrote:
>>> +
>>> +        /*
>>> +         * 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.
> I think that I'll use s2 and s3 to save bootcpu_id.
>
> But I am unsure I understand why the asm code shouldn't be changed.

Because nothing in the asm code (right now) touches any of the a registers.

Therefore the parameters that OpenSBI prepared for start() (i.e. a0 and
a1 here) are still correct for start_xen().

If, and only if, we need to modify a* for other reasons in start() do we
need to preserve their values somehow.

> If I understand you correctly I can write in a comment ahead of
> ENTRY(start) that a0, and a1 are reserved for hart_id and dtb_base
> which are passed from a bootloader but it will work only if start_xen
> will be only C function called from head.S.

Not quite.  You want a comment explaining what the OpenSBI -> start()
ABI is.  So people know what a0/etc is at ENTRY(start).

Here is an example from a different project:
https://github.com/TrenchBoot/secure-kernel-loader/blob/master/head.S#L52-L68


There is no need to do unnecessary work (i.e. preserving them right
now), until you find a reason to need to spill them.  Right now, there's
not need, and this isn't obviously going to change in the short term.

~Andrew