Re: [PATCH v3 3/8] asm-generic: move parts of Arm's asm/kernel.h to common code

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 02/05/2025 18:22, Oleksii Kurochko wrote:
> Move the following parts to common with the following changes:
> - struct kernel_info:
>   - Create arch_kernel_info for arch specific kernel information.
>     At the moment, it contains domain_type for Arm.
>   - Rename vpl011 to vuart to have more generic name suitable for other archs.
Why do you want to make it common? At the moment it referres to vpl011 which is
Arm specific, so it would be better to move it to arch specific struct. Also,
there can be more than one emulated UART (especially if you want to make the
parsing of vuart common), in which case enum would be the best fit.

Also, one remark...

[...]

> diff --git a/xen/include/xen/fdt-kernel.h b/xen/include/xen/fdt-kernel.h
> new file mode 100644
> index 0000000000..c81e759423
> --- /dev/null
> +++ b/xen/include/xen/fdt-kernel.h
> @@ -0,0 +1,133 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * For Kernel image loading.
> + *
> + * Copyright (C) 2011 Citrix Systems, Inc.
> + */
> +#ifndef __XEN_FDT_KERNEL_H__
> +#define __XEN_FDT_KERNEL_H__
> +
> +#include <xen/bootfdt.h>
> +#include <xen/device_tree.h>
> +#include <xen/types.h>
> +
> +#if __has_include(<asm/kernel.h>)
> +#   include <asm/kernel.h>
> +#endif
> +
> +struct kernel_info {
> +    struct domain *d;
> +
> +    void *fdt; /* flat device tree */
> +    paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
> +    struct meminfo mem;
> +#ifdef CONFIG_STATIC_SHM
> +    struct shared_meminfo shm_mem;
> +#endif
> +
> +    /* kernel entry point */
> +    paddr_t entry;
> +
> +    /* grant table region */
> +    paddr_t gnttab_start;
> +    paddr_t gnttab_size;
> +
> +    /* boot blob load addresses */
> +    const struct bootmodule *kernel_bootmodule, *initrd_bootmodule, 
> *dtb_bootmodule;
> +    const char* cmdline;
> +    paddr_t dtb_paddr;
> +    paddr_t initrd_paddr;
> +
> +    /* Enable uart emulation */
> +    bool vuart;
> +
> +    /* Enable/Disable PV drivers interfaces */
> +    uint16_t dom0less_feature;
> +
> +    /* Interrupt controller phandle */
> +    uint32_t phandle_intc;
> +
> +    /* loader to use for this kernel */
> +    void (*load)(struct kernel_info *info);
> +
> +    /* loader specific state */
> +    union {
> +        struct {
> +            paddr_t kernel_addr;
> +            paddr_t len;
> +#if defined(CONFIG_ARM_64) || defined(CONFIG_RISCV_64)
> +            paddr_t text_offset; /* 64-bit Image only */
> +#endif
> +            paddr_t start; /* Must be 0 for 64-bit Image */
> +        } zimage;
> +    };
> +
> +#if __has_include(<asm/kernel.h>)
> +    struct arch_kernel_info arch;
> +#endif
> +};
> +
> +static inline struct membanks *kernel_info_get_mem(struct kernel_info *kinfo)
> +{
> +    return container_of(&kinfo->mem.common, struct membanks, common);
> +}
> +
> +static inline const struct membanks *
> +kernel_info_get_mem_const(const struct kernel_info *kinfo)
> +{
> +    return container_of(&kinfo->mem.common, const struct membanks, common);
> +}
> +
> +#ifndef KERNEL_INFO_SHM_MEM_INIT
> +
> +#ifdef CONFIG_STATIC_SHM
> +#define KERNEL_INFO_SHM_MEM_INIT .shm_mem.common.max_banks = NR_SHMEM_BANKS,
> +#else
> +#define KERNEL_INFO_SHM_MEM_INIT
> +#endif
> +
> +#endif /* KERNEL_INFO_SHM_MEM_INIT */
> +
> +#ifndef KERNEL_INFO_INIT
> +
> +#define KERNEL_INFO_INIT                        \
> +{                                               \
> +    .mem.common.max_banks = NR_MEM_BANKS,       \
> +    KERNEL_INFO_SHM_MEM_INIT                    \
> +}
> +
> +#endif /* KERNEL_INFO_INIT */
> +
> +/*
> + * Probe the kernel to detemine its type and select a loader.
> + *
> + * Sets in info:
> + *  ->type
Arm specific information in generic comment.

~Michal