Re: [PATCH v5 1/7] xen/riscv: introduce boot information structure

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/03/2023 2:39 pm, Oleksii Kurochko wrote:
> The structure holds information about:
> 1. linker start/end address
> 2. load start/end address
>
> Also the patch introduces offsets for boot information structure
> members to access them in assembly code.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes in V5:
>  * the patch was introduced in the current patch series (V5)
> ---
>  xen/arch/riscv/include/asm/boot-info.h | 15 +++++++++++++++
>  xen/arch/riscv/riscv64/asm-offsets.c   |  3 +++
>  2 files changed, 18 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/boot-info.h
>
> diff --git a/xen/arch/riscv/include/asm/boot-info.h 
> b/xen/arch/riscv/include/asm/boot-info.h
> new file mode 100644
> index 0000000000..cda3d278f5
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/boot-info.h
> @@ -0,0 +1,15 @@
> +#ifndef _ASM_BOOT_INFO_H
> +#define _ASM_BOOT_INFO_H
> +
> +extern struct boot_info {
> +    unsigned long linker_start;
> +    unsigned long linker_end;
> +    unsigned long load_start;
> +    unsigned long load_end;
> +} boot_info;
> +
> +/* LINK_TO_LOAD() and LOAD_TO_LINK() works only when MMU isn't enabled. */
> +#define LINK_TO_LOAD(addr) ((addr) - boot_info.linker_start + 
> boot_info.load_start)
> +#define LOAD_TO_LINK(addr) ((addr) - boot_info.load_start + 
> boot_info.linker_start)
> +
> +#endif
> \ No newline at end of file

As a minor point, you should have newlines at the end of each file.

However, I'm not sure boot info like this is a clever move.  You're
creating a 3rd different way of doing something which should be entirely
common.  Some details are in
https://lore.kernel.org/xen-devel/115c178b-f0a7-cf6e-3e33-e6aa49b17baf@xxxxxxxx/
and note how many errors I already found in x86 and ARM.

Perhaps its time to dust that plan off again.  As Jan says, there's
_start and _end (or future variations therefore), and xen_phys_start
which is all that ought to exist in order to build the common functionality.

~Andrew