| Message ID | c01a0c54edcf5ca46a7b2069740d7a81eb6f2330.1686080337.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/riscv: introduce identity mapping | expand |
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 > >
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? 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 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
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 +
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/riscv64/head.S | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)