| Message ID | d5971bce174c7bbae61c7e16337ef95c7f3d1f62.1686080337.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/riscv: introduce identity mapping | expand |
On 06.06.2023 21:55, Oleksii Kurochko wrote: > The function was introduced to not calculate and save physical > offset before MMU is enabled because access to start() is > PC-relative and in case of linker_addr != load_addr it will result > in incorrect value in phys_offset. "... to _not_ calculate ..."? > --- a/xen/arch/riscv/mm.c > +++ b/xen/arch/riscv/mm.c > @@ -19,9 +19,11 @@ struct mmu_desc { > > extern unsigned char cpu0_boot_stack[STACK_SIZE]; > > -#define PHYS_OFFSET ((unsigned long)_start - XEN_VIRT_START) > -#define LOAD_TO_LINK(addr) ((addr) - PHYS_OFFSET) > -#define LINK_TO_LOAD(addr) ((addr) + PHYS_OFFSET) > +static unsigned long phys_offset; __ro_after_init? > +#define LOAD_TO_LINK(addr) ((unsigned long)(addr) - phys_offset) > +#define LINK_TO_LOAD(addr) ((unsigned long)(addr) + phys_offset) > + > Nit: No double blank lines please. > @@ -273,3 +275,13 @@ void __init noreturn noinline enable_mmu() > switch_stack_and_jump((unsigned long)cpu0_boot_stack + STACK_SIZE, > cont_after_mmu_is_enabled); > } > + > +/* > + * calc_phys_offset() should be used before MMU is enabled because access to > + * start() is PC-relative and in case when load_addr != linker_addr phys_offset > + * will have an incorrect value > + */ > +void calc_phys_offset(void) __init? And nit: No double blanks please. Jan
On Mon, 2023-06-12 at 09:15 +0200, Jan Beulich wrote: > On 06.06.2023 21:55, Oleksii Kurochko wrote: > > The function was introduced to not calculate and save physical > > offset before MMU is enabled because access to start() is > > PC-relative and in case of linker_addr != load_addr it will result > > in incorrect value in phys_offset. > > "... to _not_ calculate ..."? _not_ should be dropped. Thanks. > > > --- a/xen/arch/riscv/mm.c > > +++ b/xen/arch/riscv/mm.c > > @@ -19,9 +19,11 @@ struct mmu_desc { > > > > extern unsigned char cpu0_boot_stack[STACK_SIZE]; > > > > -#define PHYS_OFFSET ((unsigned long)_start - XEN_VIRT_START) > > -#define LOAD_TO_LINK(addr) ((addr) - PHYS_OFFSET) > > -#define LINK_TO_LOAD(addr) ((addr) + PHYS_OFFSET) > > +static unsigned long phys_offset; > > __ro_after_init? It makes sense to mark variable as __ro_after_init. I'll take into account in new version of patch. > > > +#define LOAD_TO_LINK(addr) ((unsigned long)(addr) - phys_offset) > > +#define LINK_TO_LOAD(addr) ((unsigned long)(addr) + phys_offset) > > + > > > > Nit: No double blank lines please. Sorry. Missed that. > > > @@ -273,3 +275,13 @@ void __init noreturn noinline enable_mmu() > > switch_stack_and_jump((unsigned long)cpu0_boot_stack + > > STACK_SIZE, > > cont_after_mmu_is_enabled); > > } > > + > > +/* > > + * calc_phys_offset() should be used before MMU is enabled because > > access to > > + * start() is PC-relative and in case when load_addr != > > linker_addr phys_offset > > + * will have an incorrect value > > + */ > > +void calc_phys_offset(void) > > __init? And nit: No double blanks please. Thanks. It should be __init. I'll remove double blanks in new patch version. ~ Oleksii
diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h index 64293eacee..996041ce81 100644 --- a/xen/arch/riscv/include/asm/mm.h +++ b/xen/arch/riscv/include/asm/mm.h @@ -11,4 +11,6 @@ void setup_initial_pagetables(void); void enable_mmu(void); void cont_after_mmu_is_enabled(void); +void calc_phys_offset(void); + #endif /* _ASM_RISCV_MM_H */ diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c index 8ceed445cf..c092897f9a 100644 --- a/xen/arch/riscv/mm.c +++ b/xen/arch/riscv/mm.c @@ -19,9 +19,11 @@ struct mmu_desc { extern unsigned char cpu0_boot_stack[STACK_SIZE]; -#define PHYS_OFFSET ((unsigned long)_start - XEN_VIRT_START) -#define LOAD_TO_LINK(addr) ((addr) - PHYS_OFFSET) -#define LINK_TO_LOAD(addr) ((addr) + PHYS_OFFSET) +static unsigned long phys_offset; + +#define LOAD_TO_LINK(addr) ((unsigned long)(addr) - phys_offset) +#define LINK_TO_LOAD(addr) ((unsigned long)(addr) + phys_offset) + /* * It is expected that Xen won't be more then 2 MB. @@ -273,3 +275,13 @@ void __init noreturn noinline enable_mmu() switch_stack_and_jump((unsigned long)cpu0_boot_stack + STACK_SIZE, cont_after_mmu_is_enabled); } + +/* + * calc_phys_offset() should be used before MMU is enabled because access to + * start() is PC-relative and in case when load_addr != linker_addr phys_offset + * will have an incorrect value + */ +void calc_phys_offset(void) +{ + phys_offset = (unsigned long)start - XEN_VIRT_START; +} diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S index 6fb7dd80fd..69f3a24987 100644 --- a/xen/arch/riscv/riscv64/head.S +++ b/xen/arch/riscv/riscv64/head.S @@ -29,6 +29,8 @@ ENTRY(start) jal reset_stack + jal calc_phys_offset + tail start_xen ENTRY(reset_stack)
The function was introduced to not calculate and save physical offset before MMU is enabled because access to start() is PC-relative and in case of linker_addr != load_addr it will result in incorrect value in phys_offset. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/include/asm/mm.h | 2 ++ xen/arch/riscv/mm.c | 18 +++++++++++++++--- xen/arch/riscv/riscv64/head.S | 2 ++ 3 files changed, 19 insertions(+), 3 deletions(-)