On Mon, 2023-07-24 at 15:40 +0200, Jan Beulich wrote: > On 24.07.2023 11:42, Oleksii Kurochko wrote: > > @@ -19,9 +20,10 @@ struct mmu_desc { > > pte_t *pgtbl_base; > > }; > > > > -#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) > > +unsigned long __ro_after_init phys_offset; > > While Misra compliance is distant future for RISC-V, there's a > problem here with there not being any declaration for this global > variable. Adding a declaration isn't the only solution though: > Patch 2 also only uses the variable in assembly code. Therefore > the variable here could be made static, with ... > > > @@ -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 __init calc_phys_offset(void) > > +{ > > + phys_offset = (unsigned long)start - XEN_VIRT_START; > > +} > > ... this function (being invoked by the same assembly code > function) returning the value alongside storing it. Thanks for explanation about MISRA compliance.
> > FTAOD I wouldn't insist on this being taken care of right away, > so if you get a maintainer ack this way, I'd be happy to commit as > is. If I don't get an ACK, I'll update the code with the next patch version. ~ Oleksii