diff mbox series

[v2,2/6] xen/riscv: introduce reset_stack() function

Message ID 6024617719467cd2da8ae03b81ddc899f2ba4311.1687178053.git.oleksii.kurochko@gmail.com (mailing list archive)
State Superseded
Headers show
Series xen/riscv: introduce identity mapping | expand

Commit Message

Oleksii Kurochko June 19, 2023, 1:34 p.m. UTC 
The reason for reset_stack() introduction is that stack should be
reset twice:
1. Before jumping to C world at the start of _start() function.
2. After jumping from 1:1 mapping world.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
Changes in V2:
  - update the commit message.
  - move out reset_stack() from .text.header to .text.
  - add Reviewed-by: Alistair Francis <alistair.francis@wdc.com>.
---
 xen/arch/riscv/riscv64/head.S | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Jan Beulich July 6, 2023, 11:17 a.m. UTC | #1 
On 19.06.2023 15:34, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/riscv64/head.S
> +++ b/xen/arch/riscv/riscv64/head.S
> @@ -27,8 +27,16 @@ ENTRY(start)
>          add     t3, t3, __SIZEOF_POINTER__
>          bltu    t3, t4, .L_clear_bss
>  
> +        jal     reset_stack
> +
> +        tail    start_xen
> +
> +        .section .text, "ax", %progbits
> +
> +ENTRY(reset_stack)
>          la      sp, cpu0_boot_stack
>          li      t0, STACK_SIZE
>          add     sp, sp, t0
>  
> -        tail    start_xen
> +        ret
> +

Looking at patch 4 you will want to add a comment here to emphasize
that a0 and a1 have to remain unclobbered.

Jan
Oleksii Kurochko July 7, 2023, 9:08 a.m. UTC | #2 
On Thu, 2023-07-06 at 13:17 +0200, Jan Beulich wrote:
> On 19.06.2023 15:34, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/riscv64/head.S
> > +++ b/xen/arch/riscv/riscv64/head.S
> > @@ -27,8 +27,16 @@ ENTRY(start)
> >          add     t3, t3, __SIZEOF_POINTER__
> >          bltu    t3, t4, .L_clear_bss
> >  
> > +        jal     reset_stack
> > +
> > +        tail    start_xen
> > +
> > +        .section .text, "ax", %progbits
> > +
> > +ENTRY(reset_stack)
> >          la      sp, cpu0_boot_stack
> >          li      t0, STACK_SIZE
> >          add     sp, sp, t0
> >  
> > -        tail    start_xen
> > +        ret
> > +
> 
> Looking at patch 4 you will want to add a comment here to emphasize
> that a0 and a1 have to remain unclobbered.
Thanks for a note. I'll add it in the new patch version

~ Oleksii
Jan Beulich July 7, 2023, 9:33 a.m. UTC | #3 
On 07.07.2023 11:08, Oleksii wrote:
> On Thu, 2023-07-06 at 13:17 +0200, Jan Beulich wrote:
>> On 19.06.2023 15:34, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/riscv64/head.S
>>> +++ b/xen/arch/riscv/riscv64/head.S
>>> @@ -27,8 +27,16 @@ ENTRY(start)
>>>          add     t3, t3, __SIZEOF_POINTER__
>>>          bltu    t3, t4, .L_clear_bss
>>>  
>>> +        jal     reset_stack
>>> +
>>> +        tail    start_xen
>>> +
>>> +        .section .text, "ax", %progbits
>>> +
>>> +ENTRY(reset_stack)
>>>          la      sp, cpu0_boot_stack
>>>          li      t0, STACK_SIZE
>>>          add     sp, sp, t0
>>>  
>>> -        tail    start_xen
>>> +        ret
>>> +
>>
>> Looking at patch 4 you will want to add a comment here to emphasize
>> that a0 and a1 have to remain unclobbered.
> Thanks for a note. I'll add it in the new patch version

Having seen how things end up by the end of the series, there's an
alternative: You could save a0 and a1 ahead of the 1st call to
reset_stack, rather than immediately afterwards.

Jan
Oleksii Kurochko July 10, 2023, 8:09 a.m. UTC | #4 
On Fri, 2023-07-07 at 11:33 +0200, Jan Beulich wrote:
> On 07.07.2023 11:08, Oleksii wrote:
> > On Thu, 2023-07-06 at 13:17 +0200, Jan Beulich wrote:
> > > On 19.06.2023 15:34, Oleksii Kurochko wrote:
> > > > --- a/xen/arch/riscv/riscv64/head.S
> > > > +++ b/xen/arch/riscv/riscv64/head.S
> > > > @@ -27,8 +27,16 @@ ENTRY(start)
> > > >          add     t3, t3, __SIZEOF_POINTER__
> > > >          bltu    t3, t4, .L_clear_bss
> > > >  
> > > > +        jal     reset_stack
> > > > +
> > > > +        tail    start_xen
> > > > +
> > > > +        .section .text, "ax", %progbits
> > > > +
> > > > +ENTRY(reset_stack)
> > > >          la      sp, cpu0_boot_stack
> > > >          li      t0, STACK_SIZE
> > > >          add     sp, sp, t0
> > > >  
> > > > -        tail    start_xen
> > > > +        ret
> > > > +
> > > 
> > > Looking at patch 4 you will want to add a comment here to
> > > emphasize
> > > that a0 and a1 have to remain unclobbered.
> > Thanks for a note. I'll add it in the new patch version
> 
> Having seen how things end up by the end of the series, there's an
> alternative: You could save a0 and a1 ahead of the 1st call to
> reset_stack, rather than immediately afterwards.
It makes sense. So lets stick to saving of a0 and a1 before 1st call of
reset_stack().

~ Oleksii
diff mbox series

Patch

diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
index 8887f0cbd4..2c0304646a 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -27,8 +27,16 @@  ENTRY(start)
         add     t3, t3, __SIZEOF_POINTER__
         bltu    t3, t4, .L_clear_bss
 
+        jal     reset_stack
+
+        tail    start_xen
+
+        .section .text, "ax", %progbits
+
+ENTRY(reset_stack)
         la      sp, cpu0_boot_stack
         li      t0, STACK_SIZE
         add     sp, sp, t0
 
-        tail    start_xen
+        ret
+