Re: [PATCH v4 1/2] xen/riscv: introduce function for physical offset calculation

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

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.

Jan