Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/riscv64/head.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
index 8887f0cbd4..6fb7dd80fd 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -27,8 +27,14 @@ ENTRY(start)
         add     t3, t3, __SIZEOF_POINTER__
         bltu    t3, t4, .L_clear_bss
 
+        jal     reset_stack
+
+        tail    start_xen
+
+ENTRY(reset_stack)
         la      sp, cpu0_boot_stack
         li      t0, STACK_SIZE
         add     sp, sp, t0
 
-        tail    start_xen
+        ret
+
-- 
2.40.1

On 06.06.2023 21:55, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

This wants addressing the "Why?" aspect in the description. Is the present
code wrong in some perhaps subtle way? Are you meaning to re-use the code?
If so, in which way (which is relevant to determine whether the new
function may actually continue to live in .text.header)?

Jan

> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -27,8 +27,14 @@ ENTRY(start)
>          add     t3, t3, __SIZEOF_POINTER__
>          bltu    t3, t4, .L_clear_bss
>  
> +        jal     reset_stack
> +
> +        tail    start_xen
> +
> +ENTRY(reset_stack)
>          la      sp, cpu0_boot_stack
>          li      t0, STACK_SIZE
>          add     sp, sp, t0
>  
> -        tail    start_xen
> +        ret
> +

On Mon, 2023-06-12 at 09:12 +0200, Jan Beulich wrote:
> On 06.06.2023 21:55, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> This wants addressing the "Why?" aspect in the description. Is the
> present
> code wrong in some perhaps subtle way? Are you meaning to re-use the
> code?
> If so, in which way (which is relevant to determine whether the new
> function may actually continue to live in .text.header)?
> 
As I mentioned in previous e-mail there is no such requirement for
reset_stack() function to live in .text.header.

I think such requirement exists only for _start() function as we would
like to see it at the start of image as a bootloader jumps to it and it
is expected to be the first instructions.

Even though I don't see any issue for reset_stack() to live in
.text.header section, should it be moved to .text section? If yes, I
would appreciate hearing why it would be better.

~ Oleksii

On 14.06.2023 14:19, Oleksii wrote:
> On Mon, 2023-06-12 at 09:12 +0200, Jan Beulich wrote:
>> On 06.06.2023 21:55, Oleksii Kurochko wrote:
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>
>> This wants addressing the "Why?" aspect in the description. Is the
>> present
>> code wrong in some perhaps subtle way? Are you meaning to re-use the
>> code?
>> If so, in which way (which is relevant to determine whether the new
>> function may actually continue to live in .text.header)?
>>
> As I mentioned in previous e-mail there is no such requirement for
> reset_stack() function to live in .text.header.
> 
> I think such requirement exists only for _start() function as we would
> like to see it at the start of image as a bootloader jumps to it and it
> is expected to be the first instructions.
> 
> Even though I don't see any issue for reset_stack() to live in
> .text.header section, should it be moved to .text section? If yes, I
> would appreciate hearing why it would be better.

Because of the simple principle of special sections better only holding
what really needs to be there.

Jan

On Mon, 2023-06-12 at 09:12 +0200, Jan Beulich wrote:
> On 06.06.2023 21:55, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> This wants addressing the "Why?" aspect in the description. Is the
> present
> code wrong in some perhaps subtle way? Are you meaning to re-use the
> code?
I am re-using it after switching from 1:1 world to reset a stack.

> If so, in which way (which is relevant to determine whether the new
> function may actually continue to live in .text.header)?
Actually there is no such requirement for reset_stack to live in
.text.header.

> > --- a/xen/arch/riscv/riscv64/head.S
> > +++ b/xen/arch/riscv/riscv64/head.S
> > @@ -27,8 +27,14 @@ ENTRY(start)
> >          add     t3, t3, __SIZEOF_POINTER__
> >          bltu    t3, t4, .L_clear_bss
> >  
> > +        jal     reset_stack
> > +
> > +        tail    start_xen
> > +
> > +ENTRY(reset_stack)
> >          la      sp, cpu0_boot_stack
> >          li      t0, STACK_SIZE
> >          add     sp, sp, t0
> >  
> > -        tail    start_xen
> > +        ret
> > +
> 

~ Oleksii

On Wed, Jun 7, 2023 at 5:55 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  xen/arch/riscv/riscv64/head.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
> index 8887f0cbd4..6fb7dd80fd 100644
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -27,8 +27,14 @@ ENTRY(start)
>          add     t3, t3, __SIZEOF_POINTER__
>          bltu    t3, t4, .L_clear_bss
>
> +        jal     reset_stack
> +
> +        tail    start_xen
> +
> +ENTRY(reset_stack)
>          la      sp, cpu0_boot_stack
>          li      t0, STACK_SIZE
>          add     sp, sp, t0
>
> -        tail    start_xen
> +        ret
> +
> --
> 2.40.1
>
>