* [PATCH for 4.20 v1 0/5] RISCV device tree mapping
@ 2024-07-03 10:42 Oleksii Kurochko
2024-07-03 10:42 ` [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common Oleksii Kurochko
` (5 more replies)
0 siblings, 6 replies; 28+ messages in thread
From: Oleksii Kurochko @ 2024-07-03 10:42 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Andrew Cooper, Jan Beulich, Julien Grall,
Stefano Stabellini, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Daniel P. Smith, Alistair Francis,
Bob Eshleman, Connor Davis
Current patch series introduces device tree mapping for RISC-V.
Also, it introduces common stuff for working with fdt which is
based on the patches from [1]:
[PATCH v4 2/6] xen/device-tree: Move Arm's setup.c bootinfo functions to common
[PATCH v4 3/6] xen/common: Move Arm's bootfdt.c
All changes which were done on top of Shawn's patches please find in "Changes" section
of each patch.
[1] https://lore.kernel.org/xen-devel/cover.1712893887.git.sanastasio@raptorengineering.com/
Oleksii Kurochko (3):
xen/riscv: enable CONFIG_HAS_DEVICE_TREE
xen/riscv: introduce device tree maping function
xen/riscv: map FDT
Shawn Anastasio (2):
xen/device-tree: Move Arm's setup.c bootinfo functions to common
xen/common: Move Arm's bootfdt.c to common
MAINTAINERS | 2 +
xen/arch/arm/Makefile | 1 -
xen/arch/arm/bootfdt.c | 622 ---------------------------
xen/arch/arm/include/asm/setup.h | 200 +--------
xen/arch/arm/setup.c | 432 -------------------
xen/arch/riscv/Kconfig | 1 +
xen/arch/riscv/include/asm/config.h | 6 +
xen/arch/riscv/include/asm/mm.h | 2 +
xen/arch/riscv/mm.c | 37 +-
xen/arch/riscv/riscv64/head.S | 3 +
xen/arch/riscv/setup.c | 21 +
xen/common/Makefile | 1 +
xen/common/device-tree/Makefile | 2 +
xen/common/device-tree/bootfdt.c | 635 ++++++++++++++++++++++++++++
xen/common/device-tree/bootinfo.c | 459 ++++++++++++++++++++
xen/include/xen/bootfdt.h | 210 +++++++++
16 files changed, 1375 insertions(+), 1259 deletions(-)
delete mode 100644 xen/arch/arm/bootfdt.c
create mode 100644 xen/common/device-tree/Makefile
create mode 100644 xen/common/device-tree/bootfdt.c
create mode 100644 xen/common/device-tree/bootinfo.c
create mode 100644 xen/include/xen/bootfdt.h
--
2.45.2
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common 2024-07-03 10:42 [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii Kurochko @ 2024-07-03 10:42 ` Oleksii Kurochko 2024-07-10 10:23 ` Jan Beulich 2024-07-03 10:42 ` [PATCH v1 2/5] xen/common: Move Arm's bootfdt.c " Oleksii Kurochko ` (4 subsequent siblings) 5 siblings, 1 reply; 28+ messages in thread From: Oleksii Kurochko @ 2024-07-03 10:42 UTC (permalink / raw) To: xen-devel Cc: Shawn Anastasio, Andrew Cooper, Jan Beulich, Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, Oleksii Kurochko From: Shawn Anastasio <sanastasio@raptorengineering.com> Arm's setup.c contains a collection of functions for parsing memory map and other boot information from a device tree. Since these routines are generally useful on any architecture that supports device tree booting, move them into xen/common/device-tree. Suggested-by: Julien Grall <julien@xen.org> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V5: - add xen/include/xen/bootfdt.h to MAINTAINERS file. - drop message "Early device tree parsing and". - After rebase on top of the current staging the following changes were done: - init bootinfo variable in <common/device-tree/bootinfo.c> with BOOTINFO_INIT; - update the code of dt_unreserved_regions(): CONFIG_STATIC_SHM related changes and getting of reserved_mem bootinfo_get_shmem() ?? - update the code of meminfo_overlap_check(): add check ( INVALID_PADDR == bank_start ) to if case. - update the code of check_reserved_regions_overlap(): CONFIG_STATIC_SHM related changes. - struct bootinfo was updated ( CONFIG_STATIC_SHM changes ) - add shared_meminfo ( because of CONFIG_STATIC_SHM ) - struct struct membanks was update with __struct group so <xen/kernel> is neeeded to be included in bootfdt.h - move BOOTINFO_ACPI_INIT, BOOTINFO_SHMEM_INIT, BOOTINFO_INIT to generic bootfdt.h - bootinfo_get_reserved_mem(), bootinfo_get_mem(), bootinfo_get_acpi(), bootinfo_get_shmem() and bootinfo_get_shmem_extra() were moved to xen/bootfdt.h - s/arm32/CONFIG_SEPARATE_XENHEAP/ - add inclusion of <xen/macros.h> because there are function in <xen/bootfdt.h> which are using container_of(). --- Changes in v4: - create new xen/include/bootinfo.h rather than relying on arch's asm/setup.h to provide required definitions for bootinfo.c - build bootinfo.c as .init.o - clean up and sort bootinfo.c's #includes - use CONFIG_SEPARATE_XENHEAP rather than CONFIG_ARM_32 to guard xenheap-specific behavior of populate_boot_allocator - (MAINTAINERS) include all of common/device-tree rather than just bootinfo.c --- MAINTAINERS | 2 + xen/arch/arm/include/asm/setup.h | 187 +----------- xen/arch/arm/setup.c | 432 ---------------------------- xen/common/Makefile | 1 + xen/common/device-tree/Makefile | 1 + xen/common/device-tree/bootinfo.c | 459 ++++++++++++++++++++++++++++++ xen/include/xen/bootfdt.h | 196 +++++++++++++ 7 files changed, 660 insertions(+), 618 deletions(-) create mode 100644 xen/common/device-tree/Makefile create mode 100644 xen/common/device-tree/bootinfo.c create mode 100644 xen/include/xen/bootfdt.h diff --git a/MAINTAINERS b/MAINTAINERS index 2b0c894527..505915b6b6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -295,9 +295,11 @@ M: Stefano Stabellini <sstabellini@kernel.org> M: Julien Grall <julien@xen.org> S: Supported F: xen/common/libfdt/ +F: xen/common/device-tree/ F: xen/common/device_tree.c F: xen/common/dt-overlay.c F: xen/include/xen/libfdt/ +F: xen/include/xen/bootfdt.h F: xen/include/xen/device_tree.h F: xen/drivers/passthrough/device_tree.c diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h index c34179da93..051e796716 100644 --- a/xen/arch/arm/include/asm/setup.h +++ b/xen/arch/arm/include/asm/setup.h @@ -3,159 +3,9 @@ #include <public/version.h> #include <asm/p2m.h> +#include <xen/bootfdt.h> #include <xen/device_tree.h> -#define MIN_FDT_ALIGN 8 -#define MAX_FDT_SIZE SZ_2M - -#define NR_MEM_BANKS 256 -#define NR_SHMEM_BANKS 32 - -#define MAX_MODULES 32 /* Current maximum useful modules */ - -typedef enum { - BOOTMOD_XEN, - BOOTMOD_FDT, - BOOTMOD_KERNEL, - BOOTMOD_RAMDISK, - BOOTMOD_XSM, - BOOTMOD_GUEST_DTB, - BOOTMOD_UNKNOWN -} bootmodule_kind; - -enum membank_type { - /* - * The MEMBANK_DEFAULT type refers to either reserved memory for the - * device/firmware (when the bank is in 'reserved_mem') or any RAM (when - * the bank is in 'mem'). - */ - MEMBANK_DEFAULT, - /* - * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory - * bank is bound to a static Xen domain. It is only valid when the bank - * is in reserved_mem. - */ - MEMBANK_STATIC_DOMAIN, - /* - * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory - * bank is reserved as static heap. It is only valid when the bank is - * in reserved_mem. - */ - MEMBANK_STATIC_HEAP, - /* - * The MEMBANK_FDT_RESVMEM type is used to indicate whether the memory - * bank is from the FDT reserve map. - */ - MEMBANK_FDT_RESVMEM, -}; - -/* Indicates the maximum number of characters(\0 included) for shm_id */ -#define MAX_SHM_ID_LENGTH 16 - -struct shmem_membank_extra { - char shm_id[MAX_SHM_ID_LENGTH]; - unsigned int nr_shm_borrowers; -}; - -struct membank { - paddr_t start; - paddr_t size; - union { - enum membank_type type; -#ifdef CONFIG_STATIC_SHM - struct shmem_membank_extra *shmem_extra; -#endif - }; -}; - -struct membanks { - __struct_group(membanks_hdr, common, , - unsigned int nr_banks; - unsigned int max_banks; - ); - struct membank bank[]; -}; - -struct meminfo { - struct membanks_hdr common; - struct membank bank[NR_MEM_BANKS]; -}; - -struct shared_meminfo { - struct membanks_hdr common; - struct membank bank[NR_SHMEM_BANKS]; - struct shmem_membank_extra extra[NR_SHMEM_BANKS]; -}; - -/* - * The domU flag is set for kernels and ramdisks of "xen,domain" nodes. - * The purpose of the domU flag is to avoid getting confused in - * kernel_probe, where we try to guess which is the dom0 kernel and - * initrd to be compatible with all versions of the multiboot spec. - */ -#define BOOTMOD_MAX_CMDLINE 1024 -struct bootmodule { - bootmodule_kind kind; - bool domU; - paddr_t start; - paddr_t size; -}; - -/* DT_MAX_NAME is the node name max length according the DT spec */ -#define DT_MAX_NAME 41 -struct bootcmdline { - bootmodule_kind kind; - bool domU; - paddr_t start; - char dt_name[DT_MAX_NAME]; - char cmdline[BOOTMOD_MAX_CMDLINE]; -}; - -struct bootmodules { - int nr_mods; - struct bootmodule module[MAX_MODULES]; -}; - -struct bootcmdlines { - unsigned int nr_mods; - struct bootcmdline cmdline[MAX_MODULES]; -}; - -struct bootinfo { - struct meminfo mem; - /* The reserved regions are only used when booting using Device-Tree */ - struct meminfo reserved_mem; - struct bootmodules modules; - struct bootcmdlines cmdlines; -#ifdef CONFIG_ACPI - struct meminfo acpi; -#endif -#ifdef CONFIG_STATIC_SHM - struct shared_meminfo shmem; -#endif - bool static_heap; -}; - -#ifdef CONFIG_ACPI -#define BOOTINFO_ACPI_INIT .acpi.common.max_banks = NR_MEM_BANKS, -#else -#define BOOTINFO_ACPI_INIT -#endif - -#ifdef CONFIG_STATIC_SHM -#define BOOTINFO_SHMEM_INIT .shmem.common.max_banks = NR_SHMEM_BANKS, -#else -#define BOOTINFO_SHMEM_INIT -#endif - -#define BOOTINFO_INIT \ -{ \ - .mem.common.max_banks = NR_MEM_BANKS, \ - .reserved_mem.common.max_banks = NR_MEM_BANKS, \ - BOOTINFO_ACPI_INIT \ - BOOTINFO_SHMEM_INIT \ -} - struct map_range_data { struct domain *d; @@ -167,39 +17,8 @@ struct map_range_data struct rangeset *irq_ranges; }; -extern struct bootinfo bootinfo; - extern domid_t max_init_domid; -static inline struct membanks *bootinfo_get_mem(void) -{ - return container_of(&bootinfo.mem.common, struct membanks, common); -} - -static inline struct membanks *bootinfo_get_reserved_mem(void) -{ - return container_of(&bootinfo.reserved_mem.common, struct membanks, common); -} - -#ifdef CONFIG_ACPI -static inline struct membanks *bootinfo_get_acpi(void) -{ - return container_of(&bootinfo.acpi.common, struct membanks, common); -} -#endif - -#ifdef CONFIG_STATIC_SHM -static inline struct membanks *bootinfo_get_shmem(void) -{ - return container_of(&bootinfo.shmem.common, struct membanks, common); -} - -static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void) -{ - return bootinfo.shmem.extra; -} -#endif - void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len); size_t estimate_efi_size(unsigned int mem_nr_banks); @@ -220,9 +39,6 @@ void fw_unreserved_regions(paddr_t s, paddr_t e, void (*cb)(paddr_t ps, paddr_t pe), unsigned int first); -size_t boot_fdt_info(const void *fdt, paddr_t paddr); -const char *boot_fdt_cmdline(const void *fdt); - bool check_reserved_regions_overlap(paddr_t region_start, paddr_t region_size); struct bootmodule *add_boot_module(bootmodule_kind kind, @@ -237,7 +53,6 @@ struct bootcmdline * boot_cmdline_find_by_name(const char *name); const char *boot_module_kind_as_string(bootmodule_kind kind); void init_pdx(void); -void populate_boot_allocator(void); void setup_mm(void); extern uint32_t hyp_traps_vector[]; diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 0c2fdaceaf..cb2c0a16b8 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -48,8 +48,6 @@ #include <xsm/xsm.h> #include <asm/acpi.h> -struct bootinfo __initdata bootinfo = BOOTINFO_INIT; - /* * Sanitized version of cpuinfo containing only features available on all * cores (only on arm64 as there is no sanitization support on arm32). @@ -203,321 +201,6 @@ static void __init processor_id(void) processor_setup(); } -static void __init dt_unreserved_regions(paddr_t s, paddr_t e, - void (*cb)(paddr_t ps, paddr_t pe), - unsigned int first) -{ - const struct membanks *reserved_mem = bootinfo_get_reserved_mem(); -#ifdef CONFIG_STATIC_SHM - const struct membanks *shmem = bootinfo_get_shmem(); - unsigned int offset; -#endif - unsigned int i; - - /* - * i is the current bootmodule we are evaluating across all possible - * kinds. - */ - for ( i = first; i < reserved_mem->nr_banks; i++ ) - { - paddr_t r_s = reserved_mem->bank[i].start; - paddr_t r_e = r_s + reserved_mem->bank[i].size; - - if ( s < r_e && r_s < e ) - { - dt_unreserved_regions(r_e, e, cb, i + 1); - dt_unreserved_regions(s, r_s, cb, i + 1); - return; - } - } - -#ifdef CONFIG_STATIC_SHM - /* - * When retrieving the corresponding shared memory addresses - * below, we need to index the shmem->bank starting from 0, hence - * we need to use i - reserved_mem->nr_banks. - */ - offset = reserved_mem->nr_banks; - for ( ; i - offset < shmem->nr_banks; i++ ) - { - paddr_t r_s, r_e; - - r_s = shmem->bank[i - offset].start; - - /* Shared memory banks can contain INVALID_PADDR as start */ - if ( INVALID_PADDR == r_s ) - continue; - - r_e = r_s + shmem->bank[i - offset].size; - - if ( s < r_e && r_s < e ) - { - dt_unreserved_regions(r_e, e, cb, i + 1); - dt_unreserved_regions(s, r_s, cb, i + 1); - return; - } - } -#endif - - cb(s, e); -} - -/* - * TODO: '*_end' could be 0 if the bank/region is at the end of the physical - * address space. This is for now not handled as it requires more rework. - */ -static bool __init meminfo_overlap_check(const struct membanks *mem, - paddr_t region_start, - paddr_t region_size) -{ - paddr_t bank_start = INVALID_PADDR, bank_end = 0; - paddr_t region_end = region_start + region_size; - unsigned int i, bank_num = mem->nr_banks; - - for ( i = 0; i < bank_num; i++ ) - { - bank_start = mem->bank[i].start; - bank_end = bank_start + mem->bank[i].size; - - if ( INVALID_PADDR == bank_start || region_end <= bank_start || - region_start >= bank_end ) - continue; - else - { - printk("Region: [%#"PRIpaddr", %#"PRIpaddr") overlapping with bank[%u]: [%#"PRIpaddr", %#"PRIpaddr")\n", - region_start, region_end, i, bank_start, bank_end); - return true; - } - } - - return false; -} - -/* - * TODO: '*_end' could be 0 if the module/region is at the end of the physical - * address space. This is for now not handled as it requires more rework. - */ -static bool __init bootmodules_overlap_check(struct bootmodules *bootmodules, - paddr_t region_start, - paddr_t region_size) -{ - paddr_t mod_start = INVALID_PADDR, mod_end = 0; - paddr_t region_end = region_start + region_size; - unsigned int i, mod_num = bootmodules->nr_mods; - - for ( i = 0; i < mod_num; i++ ) - { - mod_start = bootmodules->module[i].start; - mod_end = mod_start + bootmodules->module[i].size; - - if ( region_end <= mod_start || region_start >= mod_end ) - continue; - else - { - printk("Region: [%#"PRIpaddr", %#"PRIpaddr") overlapping with mod[%u]: [%#"PRIpaddr", %#"PRIpaddr")\n", - region_start, region_end, i, mod_start, mod_end); - return true; - } - } - - return false; -} - -void __init fw_unreserved_regions(paddr_t s, paddr_t e, - void (*cb)(paddr_t ps, paddr_t pe), - unsigned int first) -{ - if ( acpi_disabled ) - dt_unreserved_regions(s, e, cb, first); - else - cb(s, e); -} - -/* - * Given an input physical address range, check if this range is overlapping - * with the existing reserved memory regions defined in bootinfo. - * Return true if the input physical address range is overlapping with any - * existing reserved memory regions, otherwise false. - */ -bool __init check_reserved_regions_overlap(paddr_t region_start, - paddr_t region_size) -{ - const struct membanks *mem_banks[] = { - bootinfo_get_reserved_mem(), -#ifdef CONFIG_ACPI - bootinfo_get_acpi(), -#endif -#ifdef CONFIG_STATIC_SHM - bootinfo_get_shmem(), -#endif - }; - unsigned int i; - - /* - * Check if input region is overlapping with reserved memory banks or - * ACPI EfiACPIReclaimMemory (when ACPI feature is enabled) or static - * shared memory banks (when static shared memory feature is enabled) - */ - for ( i = 0; i < ARRAY_SIZE(mem_banks); i++ ) - if ( meminfo_overlap_check(mem_banks[i], region_start, region_size) ) - return true; - - /* Check if input region is overlapping with bootmodules */ - if ( bootmodules_overlap_check(&bootinfo.modules, - region_start, region_size) ) - return true; - - return false; -} - -struct bootmodule __init *add_boot_module(bootmodule_kind kind, - paddr_t start, paddr_t size, - bool domU) -{ - struct bootmodules *mods = &bootinfo.modules; - struct bootmodule *mod; - unsigned int i; - - if ( mods->nr_mods == MAX_MODULES ) - { - printk("Ignoring %s boot module at %"PRIpaddr"-%"PRIpaddr" (too many)\n", - boot_module_kind_as_string(kind), start, start + size); - return NULL; - } - - if ( check_reserved_regions_overlap(start, size) ) - return NULL; - - for ( i = 0 ; i < mods->nr_mods ; i++ ) - { - mod = &mods->module[i]; - if ( mod->kind == kind && mod->start == start ) - { - if ( !domU ) - mod->domU = false; - return mod; - } - } - - mod = &mods->module[mods->nr_mods++]; - mod->kind = kind; - mod->start = start; - mod->size = size; - mod->domU = domU; - - return mod; -} - -/* - * boot_module_find_by_kind can only be used to return Xen modules (e.g - * XSM, DTB) or Dom0 modules. This is not suitable for looking up guest - * modules. - */ -struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind) -{ - struct bootmodules *mods = &bootinfo.modules; - struct bootmodule *mod; - int i; - for (i = 0 ; i < mods->nr_mods ; i++ ) - { - mod = &mods->module[i]; - if ( mod->kind == kind && !mod->domU ) - return mod; - } - return NULL; -} - -void __init add_boot_cmdline(const char *name, const char *cmdline, - bootmodule_kind kind, paddr_t start, bool domU) -{ - struct bootcmdlines *cmds = &bootinfo.cmdlines; - struct bootcmdline *cmd; - - if ( cmds->nr_mods == MAX_MODULES ) - { - printk("Ignoring %s cmdline (too many)\n", name); - return; - } - - cmd = &cmds->cmdline[cmds->nr_mods++]; - cmd->kind = kind; - cmd->domU = domU; - cmd->start = start; - - ASSERT(strlen(name) <= DT_MAX_NAME); - safe_strcpy(cmd->dt_name, name); - - if ( strlen(cmdline) > BOOTMOD_MAX_CMDLINE ) - panic("module %s command line too long\n", name); - safe_strcpy(cmd->cmdline, cmdline); -} - -/* - * boot_cmdline_find_by_kind can only be used to return Xen modules (e.g - * XSM, DTB) or Dom0 modules. This is not suitable for looking up guest - * modules. - */ -struct bootcmdline * __init boot_cmdline_find_by_kind(bootmodule_kind kind) -{ - struct bootcmdlines *cmds = &bootinfo.cmdlines; - struct bootcmdline *cmd; - int i; - - for ( i = 0 ; i < cmds->nr_mods ; i++ ) - { - cmd = &cmds->cmdline[i]; - if ( cmd->kind == kind && !cmd->domU ) - return cmd; - } - return NULL; -} - -struct bootcmdline * __init boot_cmdline_find_by_name(const char *name) -{ - struct bootcmdlines *mods = &bootinfo.cmdlines; - struct bootcmdline *mod; - unsigned int i; - - for (i = 0 ; i < mods->nr_mods ; i++ ) - { - mod = &mods->cmdline[i]; - if ( strcmp(mod->dt_name, name) == 0 ) - return mod; - } - return NULL; -} - -struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind kind, - paddr_t start) -{ - struct bootmodules *mods = &bootinfo.modules; - struct bootmodule *mod; - unsigned int i; - - for (i = 0 ; i < mods->nr_mods ; i++ ) - { - mod = &mods->module[i]; - if ( mod->kind == kind && mod->start == start ) - return mod; - } - return NULL; -} - -const char * __init boot_module_kind_as_string(bootmodule_kind kind) -{ - switch ( kind ) - { - case BOOTMOD_XEN: return "Xen"; - case BOOTMOD_FDT: return "Device Tree"; - case BOOTMOD_KERNEL: return "Kernel"; - case BOOTMOD_RAMDISK: return "Ramdisk"; - case BOOTMOD_XSM: return "XSM"; - case BOOTMOD_GUEST_DTB: return "DTB"; - case BOOTMOD_UNKNOWN: return "Unknown"; - default: BUG(); - } -} - void __init discard_initial_modules(void) { struct bootmodules *mi = &bootinfo.modules; @@ -556,40 +239,6 @@ static void * __init relocate_fdt(paddr_t dtb_paddr, size_t dtb_size) return fdt; } -/* - * Return the end of the non-module region starting at s. In other - * words return s the start of the next modules after s. - * - * On input *end is the end of the region which should be considered - * and it is updated to reflect the end of the module, clipped to the - * end of the region if it would run over. - */ -static paddr_t __init next_module(paddr_t s, paddr_t *end) -{ - struct bootmodules *mi = &bootinfo.modules; - paddr_t lowest = ~(paddr_t)0; - int i; - - for ( i = 0; i < mi->nr_mods; i++ ) - { - paddr_t mod_s = mi->module[i].start; - paddr_t mod_e = mod_s + mi->module[i].size; - - if ( !mi->module[i].size ) - continue; - - if ( mod_s < s ) - continue; - if ( mod_s > lowest ) - continue; - if ( mod_s > *end ) - continue; - lowest = mod_s; - *end = min(*end, mod_e); - } - return lowest; -} - void __init init_pdx(void) { const struct membanks *mem = bootinfo_get_mem(); @@ -635,87 +284,6 @@ void __init init_pdx(void) } } -/* - * Populate the boot allocator. - * If a static heap was not provided by the admin, all the RAM but the - * following regions will be added: - * - Modules (e.g., Xen, Kernel) - * - Reserved regions - * - Xenheap (arm32 only) - * If a static heap was provided by the admin, populate the boot - * allocator with the corresponding regions only, but with Xenheap excluded - * on arm32. - */ -void __init populate_boot_allocator(void) -{ - unsigned int i; - const struct membanks *banks = bootinfo_get_mem(); - const struct membanks *reserved_mem = bootinfo_get_reserved_mem(); - paddr_t s, e; - - if ( bootinfo.static_heap ) - { - for ( i = 0 ; i < reserved_mem->nr_banks; i++ ) - { - if ( reserved_mem->bank[i].type != MEMBANK_STATIC_HEAP ) - continue; - - s = reserved_mem->bank[i].start; - e = s + reserved_mem->bank[i].size; -#ifdef CONFIG_ARM_32 - /* Avoid the xenheap, note that the xenheap cannot across a bank */ - if ( s <= mfn_to_maddr(directmap_mfn_start) && - e >= mfn_to_maddr(directmap_mfn_end) ) - { - init_boot_pages(s, mfn_to_maddr(directmap_mfn_start)); - init_boot_pages(mfn_to_maddr(directmap_mfn_end), e); - } - else -#endif - init_boot_pages(s, e); - } - - return; - } - - for ( i = 0; i < banks->nr_banks; i++ ) - { - const struct membank *bank = &banks->bank[i]; - paddr_t bank_end = bank->start + bank->size; - - s = bank->start; - while ( s < bank_end ) - { - paddr_t n = bank_end; - - e = next_module(s, &n); - - if ( e == ~(paddr_t)0 ) - e = n = bank_end; - - /* - * Module in a RAM bank other than the one which we are - * not dealing with here. - */ - if ( e > bank_end ) - e = bank_end; - -#ifdef CONFIG_ARM_32 - /* Avoid the xenheap */ - if ( s < mfn_to_maddr(directmap_mfn_end) && - mfn_to_maddr(directmap_mfn_start) < e ) - { - e = mfn_to_maddr(directmap_mfn_start); - n = mfn_to_maddr(directmap_mfn_end); - } -#endif - - fw_unreserved_regions(s, e, init_boot_pages, 0); - s = n; - } - } -} - size_t __read_mostly dcache_line_bytes; /* C entry point for boot CPU */ diff --git a/xen/common/Makefile b/xen/common/Makefile index f12a474d40..21359bab02 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -77,6 +77,7 @@ obj-$(CONFIG_UBSAN) += ubsan/ obj-$(CONFIG_NEEDS_LIBELF) += libelf/ obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/ +obj-$(CONFIG_HAS_DEVICE_TREE) += device-tree/ CONF_FILE := $(if $(patsubst /%,,$(KCONFIG_CONFIG)),$(objtree)/)$(KCONFIG_CONFIG) $(obj)/config.gz: $(CONF_FILE) diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile new file mode 100644 index 0000000000..947bad979c --- /dev/null +++ b/xen/common/device-tree/Makefile @@ -0,0 +1 @@ +obj-y += bootinfo.init.o diff --git a/xen/common/device-tree/bootinfo.c b/xen/common/device-tree/bootinfo.c new file mode 100644 index 0000000000..dcac9a40a0 --- /dev/null +++ b/xen/common/device-tree/bootinfo.c @@ -0,0 +1,459 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Derived from $xen/arch/arm/setup.c. + * + * bookkeeping routines. + * + * Tim Deegan <tim@xen.org> + * Copyright (c) 2011 Citrix Systems. + * Copyright (c) 2024 Raptor Engineering LLC + */ + +#include <xen/acpi.h> +#include <xen/bootfdt.h> +#include <xen/bug.h> +#include <xen/device_tree.h> +#include <xen/init.h> +#include <xen/libfdt/libfdt-xen.h> +#include <xen/mm.h> + +struct bootinfo __initdata bootinfo = BOOTINFO_INIT; + +const char * __init boot_module_kind_as_string(bootmodule_kind kind) +{ + switch ( kind ) + { + case BOOTMOD_XEN: return "Xen"; + case BOOTMOD_FDT: return "Device Tree"; + case BOOTMOD_KERNEL: return "Kernel"; + case BOOTMOD_RAMDISK: return "Ramdisk"; + case BOOTMOD_XSM: return "XSM"; + case BOOTMOD_GUEST_DTB: return "DTB"; + case BOOTMOD_UNKNOWN: return "Unknown"; + default: BUG(); + } +} + +static void __init dt_unreserved_regions(paddr_t s, paddr_t e, + void (*cb)(paddr_t ps, paddr_t pe), + unsigned int first) +{ + const struct membanks *reserved_mem = bootinfo_get_reserved_mem(); +#ifdef CONFIG_STATIC_SHM + const struct membanks *shmem = bootinfo_get_shmem(); + unsigned int offset; +#endif + unsigned int i; + + /* + * i is the current bootmodule we are evaluating across all possible + * kinds. + */ + for ( i = first; i < reserved_mem->nr_banks; i++ ) + { + paddr_t r_s = reserved_mem->bank[i].start; + paddr_t r_e = r_s + reserved_mem->bank[i].size; + + if ( s < r_e && r_s < e ) + { + dt_unreserved_regions(r_e, e, cb, i + 1); + dt_unreserved_regions(s, r_s, cb, i + 1); + return; + } + } + +#ifdef CONFIG_STATIC_SHM + /* + * When retrieving the corresponding shared memory addresses + * below, we need to index the shmem->bank starting from 0, hence + * we need to use i - reserved_mem->nr_banks. + */ + offset = reserved_mem->nr_banks; + for ( ; i - offset < shmem->nr_banks; i++ ) + { + paddr_t r_s, r_e; + + r_s = shmem->bank[i - offset].start; + + /* Shared memory banks can contain INVALID_PADDR as start */ + if ( INVALID_PADDR == r_s ) + continue; + + r_e = r_s + shmem->bank[i - offset].size; + + if ( s < r_e && r_s < e ) + { + dt_unreserved_regions(r_e, e, cb, i + 1); + dt_unreserved_regions(s, r_s, cb, i + 1); + return; + } + } +#endif + + cb(s, e); +} + +/* + * TODO: '*_end' could be 0 if the bank/region is at the end of the physical + * address space. This is for now not handled as it requires more rework. + */ +static bool __init meminfo_overlap_check(const struct membanks *mem, + paddr_t region_start, + paddr_t region_size) +{ + paddr_t bank_start = INVALID_PADDR, bank_end = 0; + paddr_t region_end = region_start + region_size; + unsigned int i, bank_num = mem->nr_banks; + + for ( i = 0; i < bank_num; i++ ) + { + bank_start = mem->bank[i].start; + bank_end = bank_start + mem->bank[i].size; + + if ( INVALID_PADDR == bank_start || region_end <= bank_start || + region_start >= bank_end ) + continue; + else + { + printk("Region: [%#"PRIpaddr", %#"PRIpaddr") overlapping with bank[%u]: [%#"PRIpaddr", %#"PRIpaddr")\n", + region_start, region_end, i, bank_start, bank_end); + return true; + } + } + + return false; +} + +/* + * TODO: '*_end' could be 0 if the module/region is at the end of the physical + * address space. This is for now not handled as it requires more rework. + */ +static bool __init bootmodules_overlap_check(struct bootmodules *bootmodules, + paddr_t region_start, + paddr_t region_size) +{ + paddr_t mod_start = INVALID_PADDR, mod_end = 0; + paddr_t region_end = region_start + region_size; + unsigned int i, mod_num = bootmodules->nr_mods; + + for ( i = 0; i < mod_num; i++ ) + { + mod_start = bootmodules->module[i].start; + mod_end = mod_start + bootmodules->module[i].size; + + if ( region_end <= mod_start || region_start >= mod_end ) + continue; + else + { + printk("Region: [%#"PRIpaddr", %#"PRIpaddr") overlapping with mod[%u]: [%#"PRIpaddr", %#"PRIpaddr")\n", + region_start, region_end, i, mod_start, mod_end); + return true; + } + } + + return false; +} + +void __init fw_unreserved_regions(paddr_t s, paddr_t e, + void (*cb)(paddr_t ps, paddr_t pe), + unsigned int first) +{ + if ( acpi_disabled ) + dt_unreserved_regions(s, e, cb, first); + else + cb(s, e); +} + +/* + * Given an input physical address range, check if this range is overlapping + * with the existing reserved memory regions defined in bootinfo. + * Return true if the input physical address range is overlapping with any + * existing reserved memory regions, otherwise false. + */ +bool __init check_reserved_regions_overlap(paddr_t region_start, + paddr_t region_size) +{ + const struct membanks *mem_banks[] = { + bootinfo_get_reserved_mem(), +#ifdef CONFIG_ACPI + bootinfo_get_acpi(), +#endif +#ifdef CONFIG_STATIC_SHM + bootinfo_get_shmem(), +#endif + }; + unsigned int i; + + /* + * Check if input region is overlapping with reserved memory banks or + * ACPI EfiACPIReclaimMemory (when ACPI feature is enabled) or static + * shared memory banks (when static shared memory feature is enabled) + */ + for ( i = 0; i < ARRAY_SIZE(mem_banks); i++ ) + if ( meminfo_overlap_check(mem_banks[i], region_start, region_size) ) + return true; + + /* Check if input region is overlapping with bootmodules */ + if ( bootmodules_overlap_check(&bootinfo.modules, + region_start, region_size) ) + return true; + + return false; +} + +struct bootmodule __init *add_boot_module(bootmodule_kind kind, + paddr_t start, paddr_t size, + bool domU) +{ + struct bootmodules *mods = &bootinfo.modules; + struct bootmodule *mod; + unsigned int i; + + if ( mods->nr_mods == MAX_MODULES ) + { + printk("Ignoring %s boot module at %"PRIpaddr"-%"PRIpaddr" (too many)\n", + boot_module_kind_as_string(kind), start, start + size); + return NULL; + } + + if ( check_reserved_regions_overlap(start, size) ) + return NULL; + + for ( i = 0 ; i < mods->nr_mods ; i++ ) + { + mod = &mods->module[i]; + if ( mod->kind == kind && mod->start == start ) + { + if ( !domU ) + mod->domU = false; + return mod; + } + } + + mod = &mods->module[mods->nr_mods++]; + mod->kind = kind; + mod->start = start; + mod->size = size; + mod->domU = domU; + + return mod; +} + +/* + * boot_module_find_by_kind can only be used to return Xen modules (e.g + * XSM, DTB) or Dom0 modules. This is not suitable for looking up guest + * modules. + */ +struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind) +{ + struct bootmodules *mods = &bootinfo.modules; + struct bootmodule *mod; + int i; + for (i = 0 ; i < mods->nr_mods ; i++ ) + { + mod = &mods->module[i]; + if ( mod->kind == kind && !mod->domU ) + return mod; + } + return NULL; +} + +void __init add_boot_cmdline(const char *name, const char *cmdline, + bootmodule_kind kind, paddr_t start, bool domU) +{ + struct bootcmdlines *cmds = &bootinfo.cmdlines; + struct bootcmdline *cmd; + + if ( cmds->nr_mods == MAX_MODULES ) + { + printk("Ignoring %s cmdline (too many)\n", name); + return; + } + + cmd = &cmds->cmdline[cmds->nr_mods++]; + cmd->kind = kind; + cmd->domU = domU; + cmd->start = start; + + ASSERT(strlen(name) <= DT_MAX_NAME); + safe_strcpy(cmd->dt_name, name); + + if ( strlen(cmdline) > BOOTMOD_MAX_CMDLINE ) + panic("module %s command line too long\n", name); + safe_strcpy(cmd->cmdline, cmdline); +} + +/* + * boot_cmdline_find_by_kind can only be used to return Xen modules (e.g + * XSM, DTB) or Dom0 modules. This is not suitable for looking up guest + * modules. + */ +struct bootcmdline * __init boot_cmdline_find_by_kind(bootmodule_kind kind) +{ + struct bootcmdlines *cmds = &bootinfo.cmdlines; + struct bootcmdline *cmd; + int i; + + for ( i = 0 ; i < cmds->nr_mods ; i++ ) + { + cmd = &cmds->cmdline[i]; + if ( cmd->kind == kind && !cmd->domU ) + return cmd; + } + return NULL; +} + +struct bootcmdline * __init boot_cmdline_find_by_name(const char *name) +{ + struct bootcmdlines *mods = &bootinfo.cmdlines; + struct bootcmdline *mod; + unsigned int i; + + for (i = 0 ; i < mods->nr_mods ; i++ ) + { + mod = &mods->cmdline[i]; + if ( strcmp(mod->dt_name, name) == 0 ) + return mod; + } + return NULL; +} + +struct bootmodule * __init boot_module_find_by_addr_and_kind(bootmodule_kind kind, + paddr_t start) +{ + struct bootmodules *mods = &bootinfo.modules; + struct bootmodule *mod; + unsigned int i; + + for (i = 0 ; i < mods->nr_mods ; i++ ) + { + mod = &mods->module[i]; + if ( mod->kind == kind && mod->start == start ) + return mod; + } + return NULL; +} + +/* + * Return the end of the non-module region starting at s. In other + * words return s the start of the next modules after s. + * + * On input *end is the end of the region which should be considered + * and it is updated to reflect the end of the module, clipped to the + * end of the region if it would run over. + */ +static paddr_t __init next_module(paddr_t s, paddr_t *end) +{ + struct bootmodules *mi = &bootinfo.modules; + paddr_t lowest = ~(paddr_t)0; + int i; + + for ( i = 0; i < mi->nr_mods; i++ ) + { + paddr_t mod_s = mi->module[i].start; + paddr_t mod_e = mod_s + mi->module[i].size; + + if ( !mi->module[i].size ) + continue; + + if ( mod_s < s ) + continue; + if ( mod_s > lowest ) + continue; + if ( mod_s > *end ) + continue; + lowest = mod_s; + *end = min(*end, mod_e); + } + return lowest; +} + +/* + * Populate the boot allocator. + * If a static heap was not provided by the admin, all the RAM but the + * following regions will be added: + * - Modules (e.g., Xen, Kernel) + * - Reserved regions + * - Xenheap (CONFIG_SEPARATE_XENHEAP only) + * If a static heap was provided by the admin, populate the boot + * allocator with the corresponding regions only, but with Xenheap excluded + * on CONFIG_SEPARATE_XENHEAP. + */ +void __init populate_boot_allocator(void) +{ + unsigned int i; + const struct membanks *banks = bootinfo_get_mem(); + const struct membanks *reserved_mem = bootinfo_get_reserved_mem(); + paddr_t s, e; + + if ( bootinfo.static_heap ) + { + for ( i = 0 ; i < reserved_mem->nr_banks; i++ ) + { + if ( reserved_mem->bank[i].type != MEMBANK_STATIC_HEAP ) + continue; + + s = reserved_mem->bank[i].start; + e = s + reserved_mem->bank[i].size; +#ifdef CONFIG_SEPARATE_XENHEAP + /* Avoid the xenheap, note that the xenheap cannot across a bank */ + if ( s <= mfn_to_maddr(directmap_mfn_start) && + e >= mfn_to_maddr(directmap_mfn_end) ) + { + init_boot_pages(s, mfn_to_maddr(directmap_mfn_start)); + init_boot_pages(mfn_to_maddr(directmap_mfn_end), e); + } + else +#endif + init_boot_pages(s, e); + } + + return; + } + + for ( i = 0; i < banks->nr_banks; i++ ) + { + const struct membank *bank = &banks->bank[i]; + paddr_t bank_end = bank->start + bank->size; + + s = bank->start; + while ( s < bank_end ) + { + paddr_t n = bank_end; + + e = next_module(s, &n); + + if ( e == ~(paddr_t)0 ) + e = n = bank_end; + + /* + * Module in a RAM bank other than the one which we are + * not dealing with here. + */ + if ( e > bank_end ) + e = bank_end; + +#ifdef CONFIG_SEPARATE_XENHEAP + /* Avoid the xenheap */ + if ( s < mfn_to_maddr(directmap_mfn_end) && + mfn_to_maddr(directmap_mfn_start) < e ) + { + e = mfn_to_maddr(directmap_mfn_start); + n = mfn_to_maddr(directmap_mfn_end); + } +#endif + + fw_unreserved_regions(s, e, init_boot_pages, 0); + s = n; + } + } +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h new file mode 100644 index 0000000000..7cd45b3d4b --- /dev/null +++ b/xen/include/xen/bootfdt.h @@ -0,0 +1,196 @@ +#ifndef __XEN_BOOTFDT_H__ +#define __XEN_BOOTFDT_H__ + +#include <xen/types.h> +#include <xen/kernel.h> +#include <xen/macros.h> + +#define MIN_FDT_ALIGN 8 +#define MAX_FDT_SIZE SZ_2M + +#define NR_MEM_BANKS 256 +#define NR_SHMEM_BANKS 32 + +#define MAX_MODULES 32 /* Current maximum useful modules */ + +typedef enum { + BOOTMOD_XEN, + BOOTMOD_FDT, + BOOTMOD_KERNEL, + BOOTMOD_RAMDISK, + BOOTMOD_XSM, + BOOTMOD_GUEST_DTB, + BOOTMOD_UNKNOWN +} bootmodule_kind; + +enum membank_type { + /* + * The MEMBANK_DEFAULT type refers to either reserved memory for the + * device/firmware (when the bank is in 'reserved_mem') or any RAM (when + * the bank is in 'mem'). + */ + MEMBANK_DEFAULT, + /* + * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory + * bank is bound to a static Xen domain. It is only valid when the bank + * is in reserved_mem. + */ + MEMBANK_STATIC_DOMAIN, + /* + * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory + * bank is reserved as static heap. It is only valid when the bank is + * in reserved_mem. + */ + MEMBANK_STATIC_HEAP, + /* + * The MEMBANK_FDT_RESVMEM type is used to indicate whether the memory + * bank is from the FDT reserve map. + */ + MEMBANK_FDT_RESVMEM, +}; + +/* Indicates the maximum number of characters(\0 included) for shm_id */ +#define MAX_SHM_ID_LENGTH 16 + +struct shmem_membank_extra { + char shm_id[MAX_SHM_ID_LENGTH]; + unsigned int nr_shm_borrowers; +}; + +struct membank { + paddr_t start; + paddr_t size; + union { + enum membank_type type; +#ifdef CONFIG_STATIC_SHM + struct shmem_membank_extra *shmem_extra; +#endif + }; +}; + +struct membanks { + __struct_group(membanks_hdr, common, , + unsigned int nr_banks; + unsigned int max_banks; + ); + struct membank bank[]; +}; + +struct meminfo { + struct membanks_hdr common; + struct membank bank[NR_MEM_BANKS]; +}; + +struct shared_meminfo { + struct membanks_hdr common; + struct membank bank[NR_SHMEM_BANKS]; + struct shmem_membank_extra extra[NR_SHMEM_BANKS]; +}; + +/* + * The domU flag is set for kernels and ramdisks of "xen,domain" nodes. + * The purpose of the domU flag is to avoid getting confused in + * kernel_probe, where we try to guess which is the dom0 kernel and + * initrd to be compatible with all versions of the multiboot spec. + */ +#define BOOTMOD_MAX_CMDLINE 1024 +struct bootmodule { + bootmodule_kind kind; + bool domU; + paddr_t start; + paddr_t size; +}; + +/* DT_MAX_NAME is the node name max length according the DT spec */ +#define DT_MAX_NAME 41 +struct bootcmdline { + bootmodule_kind kind; + bool domU; + paddr_t start; + char dt_name[DT_MAX_NAME]; + char cmdline[BOOTMOD_MAX_CMDLINE]; +}; + +struct bootmodules { + int nr_mods; + struct bootmodule module[MAX_MODULES]; +}; + +struct bootcmdlines { + unsigned int nr_mods; + struct bootcmdline cmdline[MAX_MODULES]; +}; + +struct bootinfo { + struct meminfo mem; + /* The reserved regions are only used when booting using Device-Tree */ + struct meminfo reserved_mem; + struct bootmodules modules; + struct bootcmdlines cmdlines; +#ifdef CONFIG_ACPI + struct meminfo acpi; +#endif +#ifdef CONFIG_STATIC_SHM + struct shared_meminfo shmem; +#endif + bool static_heap; +}; + +#ifdef CONFIG_ACPI +#define BOOTINFO_ACPI_INIT .acpi.common.max_banks = NR_MEM_BANKS, +#else +#define BOOTINFO_ACPI_INIT +#endif + +#ifdef CONFIG_STATIC_SHM +#define BOOTINFO_SHMEM_INIT .shmem.common.max_banks = NR_SHMEM_BANKS, +#else +#define BOOTINFO_SHMEM_INIT +#endif + +#define BOOTINFO_INIT \ +{ \ + .mem.common.max_banks = NR_MEM_BANKS, \ + .reserved_mem.common.max_banks = NR_MEM_BANKS, \ + BOOTINFO_ACPI_INIT \ + BOOTINFO_SHMEM_INIT \ +} + +extern struct bootinfo bootinfo; + +void populate_boot_allocator(void); + +size_t boot_fdt_info(const void *fdt, paddr_t paddr); + +const char *boot_fdt_cmdline(const void *fdt); + +static inline struct membanks *bootinfo_get_reserved_mem(void) +{ + return container_of(&bootinfo.reserved_mem.common, struct membanks, common); +} + +static inline struct membanks *bootinfo_get_mem(void) +{ + return container_of(&bootinfo.mem.common, struct membanks, common); +} + +#ifdef CONFIG_ACPI +static inline struct membanks *bootinfo_get_acpi(void) +{ + return container_of(&bootinfo.acpi.common, struct membanks, common); +} +#endif + +#ifdef CONFIG_STATIC_SHM +static inline struct membanks *bootinfo_get_shmem(void) +{ + return container_of(&bootinfo.shmem.common, struct membanks, common); +} + +static inline struct shmem_membank_extra *bootinfo_get_shmem_extra(void) +{ + return bootinfo.shmem.extra; +} +#endif + +#endif /* __XEN_BOOTFDT_H__ */ -- 2.45.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common 2024-07-03 10:42 ` [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common Oleksii Kurochko @ 2024-07-10 10:23 ` Jan Beulich 2024-07-11 9:00 ` Oleksii 0 siblings, 1 reply; 28+ messages in thread From: Jan Beulich @ 2024-07-10 10:23 UTC (permalink / raw) To: Oleksii Kurochko Cc: Shawn Anastasio, Andrew Cooper, Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, xen-devel On 03.07.2024 12:42, Oleksii Kurochko wrote: > From: Shawn Anastasio <sanastasio@raptorengineering.com> > > Arm's setup.c contains a collection of functions for parsing memory map > and other boot information from a device tree. Since these routines are > generally useful on any architecture that supports device tree booting, > move them into xen/common/device-tree. > > Suggested-by: Julien Grall <julien@xen.org> > Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > Changes in V5: > - add xen/include/xen/bootfdt.h to MAINTAINERS file. > - drop message "Early device tree parsing and". > - After rebase on top of the current staging the following changes were done: > - init bootinfo variable in <common/device-tree/bootinfo.c> with BOOTINFO_INIT; > - update the code of dt_unreserved_regions(): > CONFIG_STATIC_SHM related changes and getting of reserved_mem > bootinfo_get_shmem() ?? > - update the code of meminfo_overlap_check(): > add check ( INVALID_PADDR == bank_start ) to if case. > - update the code of check_reserved_regions_overlap(): > CONFIG_STATIC_SHM related changes. > - struct bootinfo was updated ( CONFIG_STATIC_SHM changes ) > - add shared_meminfo ( because of CONFIG_STATIC_SHM ) > - struct struct membanks was update with __struct group so <xen/kernel> is > neeeded to be included in bootfdt.h > - move BOOTINFO_ACPI_INIT, BOOTINFO_SHMEM_INIT, BOOTINFO_INIT to generic bootfdt.h > - bootinfo_get_reserved_mem(), bootinfo_get_mem(), bootinfo_get_acpi(), > bootinfo_get_shmem() and bootinfo_get_shmem_extra() were moved to xen/bootfdt.h > - s/arm32/CONFIG_SEPARATE_XENHEAP/ > - add inclusion of <xen/macros.h> because there are function in <xen/bootfdt.h> which > are using container_of(). Just to mention it: This is confusing. The series is tagged "v1". I understand you took Shawn's work, which had already undergone revisions. But then imo you want to at least clarify how your v1 relates to his v4 or v5, i.e. then making clear to the reader whether all of the changes above were actually done by you on top of an earlier v4, or whether you took the earlier v5 verbatim. But anyway, this will need looking at by Arm folks. Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common 2024-07-10 10:23 ` Jan Beulich @ 2024-07-11 9:00 ` Oleksii 2024-07-11 9:21 ` Jan Beulich 0 siblings, 1 reply; 28+ messages in thread From: Oleksii @ 2024-07-11 9:00 UTC (permalink / raw) To: Jan Beulich Cc: Shawn Anastasio, Andrew Cooper, Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, xen-devel On Wed, 2024-07-10 at 12:23 +0200, Jan Beulich wrote: > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > From: Shawn Anastasio <sanastasio@raptorengineering.com> > > > > Arm's setup.c contains a collection of functions for parsing memory > > map > > and other boot information from a device tree. Since these routines > > are > > generally useful on any architecture that supports device tree > > booting, > > move them into xen/common/device-tree. > > > > Suggested-by: Julien Grall <julien@xen.org> > > Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > --- > > Changes in V5: > > - add xen/include/xen/bootfdt.h to MAINTAINERS file. > > - drop message "Early device tree parsing and". > > - After rebase on top of the current staging the following changes > > were done: > > - init bootinfo variable in <common/device-tree/bootinfo.c> with > > BOOTINFO_INIT; > > - update the code of dt_unreserved_regions(): > > CONFIG_STATIC_SHM related changes and getting of > > reserved_mem > > bootinfo_get_shmem() ?? > > - update the code of meminfo_overlap_check(): > > add check ( INVALID_PADDR == bank_start ) to if case. > > - update the code of check_reserved_regions_overlap(): > > CONFIG_STATIC_SHM related changes. > > - struct bootinfo was updated ( CONFIG_STATIC_SHM changes ) > > - add shared_meminfo ( because of CONFIG_STATIC_SHM ) > > - struct struct membanks was update with __struct group so > > <xen/kernel> is > > neeeded to be included in bootfdt.h > > - move BOOTINFO_ACPI_INIT, BOOTINFO_SHMEM_INIT, BOOTINFO_INIT to > > generic bootfdt.h > > - bootinfo_get_reserved_mem(), bootinfo_get_mem(), > > bootinfo_get_acpi(), > > bootinfo_get_shmem() and bootinfo_get_shmem_extra() were moved > > to xen/bootfdt.h > > - s/arm32/CONFIG_SEPARATE_XENHEAP/ > > - add inclusion of <xen/macros.h> because there are function in > > <xen/bootfdt.h> which > > are using container_of(). > > Just to mention it: This is confusing. The series is tagged "v1". I > understand > you took Shawn's work, which had already undergone revisions. But > then imo you > want to at least clarify how your v1 relates to his v4 or v5, i.e. > then making > clear to the reader whether all of the changes above were actually > done by you > on top of an earlier v4, or whether you took the earlier v5 verbatim. That is why I wrote "Changes in v5" to show that these changes were done on top of v4 version of Shawn's series, so what is mentioned in v5 here it is what was done by me. I'll reword that and probably I shouldn't drop "Changes in v4,3,2,1" from Shawn's patch. ~ Oleksii ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common 2024-07-11 9:00 ` Oleksii @ 2024-07-11 9:21 ` Jan Beulich 2024-07-11 9:41 ` Oleksii 0 siblings, 1 reply; 28+ messages in thread From: Jan Beulich @ 2024-07-11 9:21 UTC (permalink / raw) To: Oleksii Cc: Shawn Anastasio, Andrew Cooper, Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, xen-devel On 11.07.2024 11:00, Oleksii wrote: > On Wed, 2024-07-10 at 12:23 +0200, Jan Beulich wrote: >> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>> From: Shawn Anastasio <sanastasio@raptorengineering.com> >>> >>> Arm's setup.c contains a collection of functions for parsing memory >>> map >>> and other boot information from a device tree. Since these routines >>> are >>> generally useful on any architecture that supports device tree >>> booting, >>> move them into xen/common/device-tree. >>> >>> Suggested-by: Julien Grall <julien@xen.org> >>> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> >>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> >>> --- >>> Changes in V5: >>> - add xen/include/xen/bootfdt.h to MAINTAINERS file. >>> - drop message "Early device tree parsing and". >>> - After rebase on top of the current staging the following changes >>> were done: >>> - init bootinfo variable in <common/device-tree/bootinfo.c> with >>> BOOTINFO_INIT; >>> - update the code of dt_unreserved_regions(): >>> CONFIG_STATIC_SHM related changes and getting of >>> reserved_mem >>> bootinfo_get_shmem() ?? >>> - update the code of meminfo_overlap_check(): >>> add check ( INVALID_PADDR == bank_start ) to if case. >>> - update the code of check_reserved_regions_overlap(): >>> CONFIG_STATIC_SHM related changes. >>> - struct bootinfo was updated ( CONFIG_STATIC_SHM changes ) >>> - add shared_meminfo ( because of CONFIG_STATIC_SHM ) >>> - struct struct membanks was update with __struct group so >>> <xen/kernel> is >>> neeeded to be included in bootfdt.h >>> - move BOOTINFO_ACPI_INIT, BOOTINFO_SHMEM_INIT, BOOTINFO_INIT to >>> generic bootfdt.h >>> - bootinfo_get_reserved_mem(), bootinfo_get_mem(), >>> bootinfo_get_acpi(), >>> bootinfo_get_shmem() and bootinfo_get_shmem_extra() were moved >>> to xen/bootfdt.h >>> - s/arm32/CONFIG_SEPARATE_XENHEAP/ >>> - add inclusion of <xen/macros.h> because there are function in >>> <xen/bootfdt.h> which >>> are using container_of(). >> >> Just to mention it: This is confusing. The series is tagged "v1". I >> understand >> you took Shawn's work, which had already undergone revisions. But >> then imo you >> want to at least clarify how your v1 relates to his v4 or v5, i.e. >> then making >> clear to the reader whether all of the changes above were actually >> done by you >> on top of an earlier v4, or whether you took the earlier v5 verbatim. > That is why I wrote "Changes in v5" to show that these changes were > done on top of v4 version of Shawn's series, so what is mentioned in v5 > here it is what was done by me. Except that what you sent is v1, not v5. > I'll reword that and probably I shouldn't drop "Changes in v4,3,2,1" > from Shawn's patch. I don't think you should drop anything. You want to clarify relationship of the version of your series with that of Shawn's earlier one. Or you'd want to continue numbering from what the previous series had, yet that may then also end up confusing if Shawn resumed work there. Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common 2024-07-11 9:21 ` Jan Beulich @ 2024-07-11 9:41 ` Oleksii 0 siblings, 0 replies; 28+ messages in thread From: Oleksii @ 2024-07-11 9:41 UTC (permalink / raw) To: Jan Beulich Cc: Shawn Anastasio, Andrew Cooper, Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, xen-devel On Thu, 2024-07-11 at 11:21 +0200, Jan Beulich wrote: > On 11.07.2024 11:00, Oleksii wrote: > > On Wed, 2024-07-10 at 12:23 +0200, Jan Beulich wrote: > > > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > > > From: Shawn Anastasio <sanastasio@raptorengineering.com> > > > > > > > > Arm's setup.c contains a collection of functions for parsing > > > > memory > > > > map > > > > and other boot information from a device tree. Since these > > > > routines > > > > are > > > > generally useful on any architecture that supports device tree > > > > booting, > > > > move them into xen/common/device-tree. > > > > > > > > Suggested-by: Julien Grall <julien@xen.org> > > > > Signed-off-by: Shawn Anastasio > > > > <sanastasio@raptorengineering.com> > > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > > > --- > > > > Changes in V5: > > > > - add xen/include/xen/bootfdt.h to MAINTAINERS file. > > > > - drop message "Early device tree parsing and". > > > > - After rebase on top of the current staging the following > > > > changes > > > > were done: > > > > - init bootinfo variable in <common/device-tree/bootinfo.c> > > > > with > > > > BOOTINFO_INIT; > > > > - update the code of dt_unreserved_regions(): > > > > CONFIG_STATIC_SHM related changes and getting of > > > > reserved_mem > > > > bootinfo_get_shmem() ?? > > > > - update the code of meminfo_overlap_check(): > > > > add check ( INVALID_PADDR == bank_start ) to if case. > > > > - update the code of check_reserved_regions_overlap(): > > > > CONFIG_STATIC_SHM related changes. > > > > - struct bootinfo was updated ( CONFIG_STATIC_SHM changes ) > > > > - add shared_meminfo ( because of CONFIG_STATIC_SHM ) > > > > - struct struct membanks was update with __struct group so > > > > <xen/kernel> is > > > > neeeded to be included in bootfdt.h > > > > - move BOOTINFO_ACPI_INIT, BOOTINFO_SHMEM_INIT, > > > > BOOTINFO_INIT to > > > > generic bootfdt.h > > > > - bootinfo_get_reserved_mem(), bootinfo_get_mem(), > > > > bootinfo_get_acpi(), > > > > bootinfo_get_shmem() and bootinfo_get_shmem_extra() were > > > > moved > > > > to xen/bootfdt.h > > > > - s/arm32/CONFIG_SEPARATE_XENHEAP/ > > > > - add inclusion of <xen/macros.h> because there are function > > > > in > > > > <xen/bootfdt.h> which > > > > are using container_of(). > > > > > > Just to mention it: This is confusing. The series is tagged "v1". > > > I > > > understand > > > you took Shawn's work, which had already undergone revisions. But > > > then imo you > > > want to at least clarify how your v1 relates to his v4 or v5, > > > i.e. > > > then making > > > clear to the reader whether all of the changes above were > > > actually > > > done by you > > > on top of an earlier v4, or whether you took the earlier v5 > > > verbatim. > > That is why I wrote "Changes in v5" to show that these changes were > > done on top of v4 version of Shawn's series, so what is mentioned > > in v5 > > here it is what was done by me. > > Except that what you sent is v1, not v5. > > > I'll reword that and probably I shouldn't drop "Changes in > > v4,3,2,1" > > from Shawn's patch. > > I don't think you should drop anything. You want to clarify > relationship > of the version of your series with that of Shawn's earlier one. Or > you'd > want to continue numbering from what the previous series had, yet > that > may then also end up confusing if Shawn resumed work there. Oh, I understood now what you meant. The Patch should be with the name: [PATCH *v5* 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common I will update that. Thanks. ~ Oleksii ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 2/5] xen/common: Move Arm's bootfdt.c to common 2024-07-03 10:42 [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii Kurochko 2024-07-03 10:42 ` [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common Oleksii Kurochko @ 2024-07-03 10:42 ` Oleksii Kurochko 2024-07-03 10:42 ` [PATCH v1 3/5] xen/riscv: enable CONFIG_HAS_DEVICE_TREE Oleksii Kurochko ` (3 subsequent siblings) 5 siblings, 0 replies; 28+ messages in thread From: Oleksii Kurochko @ 2024-07-03 10:42 UTC (permalink / raw) To: xen-devel Cc: Shawn Anastasio, Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, Oleksii Kurochko From: Shawn Anastasio <sanastasio@raptorengineering.com> Move Arm's bootfdt.c to xen/common so that it can be used by other device tree architectures like PPC and RISCV. Suggested-by: Julien Grall <julien@xen.org> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> Acked-by: Julien Grall <julien@xen.org> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in v5: - add guard #ifdef CONFIG_STATIC_SHM around inclusion of <asm/static-shmem.h> in common/device-tree/bootfdt.c. - add stub for process_shm_node() in case CONFIG_STATIC_SHM isn't enabled. - add guard around #ifdef CONFIG_STATIC_SHM aroud early_print_info_shmem() in early_print_info(). --- Changes in v4: - move function prototypes to patch 2's xen/include/bootfdt.h - clean up #includes --- xen/arch/arm/Makefile | 1 - xen/arch/arm/bootfdt.c | 622 ------------------------------ xen/arch/arm/include/asm/setup.h | 13 - xen/common/device-tree/Makefile | 1 + xen/common/device-tree/bootfdt.c | 635 +++++++++++++++++++++++++++++++ xen/include/xen/bootfdt.h | 14 + 6 files changed, 650 insertions(+), 636 deletions(-) delete mode 100644 xen/arch/arm/bootfdt.c create mode 100644 xen/common/device-tree/bootfdt.c diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 45dc29ea53..da9c979dc4 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -10,7 +10,6 @@ obj-$(CONFIG_TEE) += tee/ obj-$(CONFIG_HAS_VPCI) += vpci.o obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o -obj-y += bootfdt.init.o obj-y += cpuerrata.o obj-y += cpufeature.o obj-y += decode.o diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c deleted file mode 100644 index 6e060111d9..0000000000 --- a/xen/arch/arm/bootfdt.c +++ /dev/null @@ -1,622 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* - * Early Device Tree - * - * Copyright (C) 2012-2014 Citrix Systems, Inc. - */ -#include <xen/types.h> -#include <xen/lib.h> -#include <xen/kernel.h> -#include <xen/init.h> -#include <xen/efi.h> -#include <xen/device_tree.h> -#include <xen/lib.h> -#include <xen/libfdt/libfdt-xen.h> -#include <xen/sort.h> -#include <xsm/xsm.h> -#include <asm/setup.h> -#include <asm/static-shmem.h> - -static void __init __maybe_unused build_assertions(void) -{ - /* - * Check that no padding is between struct membanks "bank" flexible array - * member and struct meminfo "bank" member - */ - BUILD_BUG_ON((offsetof(struct membanks, bank) != - offsetof(struct meminfo, bank))); - /* Ensure "struct membanks" is 8-byte aligned */ - BUILD_BUG_ON(alignof(struct membanks) != 8); -} - -static bool __init device_tree_node_is_available(const void *fdt, int node) -{ - const char *status; - int len; - - status = fdt_getprop(fdt, node, "status", &len); - if ( !status ) - return true; - - if ( len > 0 ) - { - if ( !strcmp(status, "ok") || !strcmp(status, "okay") ) - return true; - } - - return false; -} - -static bool __init device_tree_node_matches(const void *fdt, int node, - const char *match) -{ - const char *name; - size_t match_len; - - name = fdt_get_name(fdt, node, NULL); - match_len = strlen(match); - - /* Match both "match" and "match@..." patterns but not - "match-foo". */ - return strncmp(name, match, match_len) == 0 - && (name[match_len] == '@' || name[match_len] == '\0'); -} - -static bool __init device_tree_node_compatible(const void *fdt, int node, - const char *match) -{ - int len, l; - const void *prop; - - prop = fdt_getprop(fdt, node, "compatible", &len); - if ( prop == NULL ) - return false; - - while ( len > 0 ) { - if ( !dt_compat_cmp(prop, match) ) - return true; - l = strlen(prop) + 1; - prop += l; - len -= l; - } - - return false; -} - -void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells, - uint32_t size_cells, paddr_t *start, - paddr_t *size) -{ - uint64_t dt_start, dt_size; - - /* - * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit. - * Thus, there is an implicit cast from uint64_t to paddr_t. - */ - dt_start = dt_next_cell(address_cells, cell); - dt_size = dt_next_cell(size_cells, cell); - - if ( dt_start != (paddr_t)dt_start ) - { - printk("Physical address greater than max width supported\n"); - WARN(); - } - - if ( dt_size != (paddr_t)dt_size ) - { - printk("Physical size greater than max width supported\n"); - WARN(); - } - - /* - * Xen will truncate the address/size if it is greater than the maximum - * supported width and it will give an appropriate warning. - */ - *start = dt_start; - *size = dt_size; -} - -static int __init device_tree_get_meminfo(const void *fdt, int node, - const char *prop_name, - u32 address_cells, u32 size_cells, - struct membanks *mem, - enum membank_type type) -{ - const struct fdt_property *prop; - unsigned int i, banks; - const __be32 *cell; - u32 reg_cells = address_cells + size_cells; - paddr_t start, size; - - if ( !device_tree_node_is_available(fdt, node) ) - return 0; - - if ( address_cells < 1 || size_cells < 1 ) - { - printk("fdt: property `%s': invalid #address-cells or #size-cells", - prop_name); - return -EINVAL; - } - - prop = fdt_get_property(fdt, node, prop_name, NULL); - if ( !prop ) - return -ENOENT; - - cell = (const __be32 *)prop->data; - banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32)); - - for ( i = 0; i < banks && mem->nr_banks < mem->max_banks; i++ ) - { - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); - if ( mem == bootinfo_get_reserved_mem() && - check_reserved_regions_overlap(start, size) ) - return -EINVAL; - /* Some DT may describe empty bank, ignore them */ - if ( !size ) - continue; - mem->bank[mem->nr_banks].start = start; - mem->bank[mem->nr_banks].size = size; - mem->bank[mem->nr_banks].type = type; - mem->nr_banks++; - } - - if ( i < banks ) - { - printk("Warning: Max number of supported memory regions reached.\n"); - return -ENOSPC; - } - - return 0; -} - -u32 __init device_tree_get_u32(const void *fdt, int node, - const char *prop_name, u32 dflt) -{ - const struct fdt_property *prop; - - prop = fdt_get_property(fdt, node, prop_name, NULL); - if ( !prop || prop->len < sizeof(u32) ) - return dflt; - - return fdt32_to_cpu(*(uint32_t*)prop->data); -} - -/** - * device_tree_for_each_node - iterate over all device tree sub-nodes - * @fdt: flat device tree. - * @node: parent node to start the search from - * @func: function to call for each sub-node. - * @data: data to pass to @func. - * - * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored. - * - * Returns 0 if all nodes were iterated over successfully. If @func - * returns a value different from 0, that value is returned immediately. - */ -int __init device_tree_for_each_node(const void *fdt, int node, - device_tree_node_func func, - void *data) -{ - /* - * We only care about relative depth increments, assume depth of - * node is 0 for simplicity. - */ - int depth = 0; - const int first_node = node; - u32 address_cells[DEVICE_TREE_MAX_DEPTH]; - u32 size_cells[DEVICE_TREE_MAX_DEPTH]; - int ret; - - do { - const char *name = fdt_get_name(fdt, node, NULL); - u32 as, ss; - - if ( depth >= DEVICE_TREE_MAX_DEPTH ) - { - printk("Warning: device tree node `%s' is nested too deep\n", - name); - continue; - } - - as = depth > 0 ? address_cells[depth-1] : DT_ROOT_NODE_ADDR_CELLS_DEFAULT; - ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT; - - address_cells[depth] = device_tree_get_u32(fdt, node, - "#address-cells", as); - size_cells[depth] = device_tree_get_u32(fdt, node, - "#size-cells", ss); - - /* skip the first node */ - if ( node != first_node ) - { - ret = func(fdt, node, name, depth, as, ss, data); - if ( ret != 0 ) - return ret; - } - - node = fdt_next_node(fdt, node, &depth); - } while ( node >= 0 && depth > 0 ); - - return 0; -} - -static int __init process_memory_node(const void *fdt, int node, - const char *name, int depth, - u32 address_cells, u32 size_cells, - struct membanks *mem) -{ - return device_tree_get_meminfo(fdt, node, "reg", address_cells, size_cells, - mem, MEMBANK_DEFAULT); -} - -static int __init process_reserved_memory_node(const void *fdt, int node, - const char *name, int depth, - u32 address_cells, - u32 size_cells, - void *data) -{ - int rc = process_memory_node(fdt, node, name, depth, address_cells, - size_cells, data); - - if ( rc == -ENOSPC ) - panic("Max number of supported reserved-memory regions reached.\n"); - else if ( rc != -ENOENT ) - return rc; - return 0; -} - -static int __init process_reserved_memory(const void *fdt, int node, - const char *name, int depth, - u32 address_cells, u32 size_cells) -{ - return device_tree_for_each_node(fdt, node, - process_reserved_memory_node, - bootinfo_get_reserved_mem()); -} - -static void __init process_multiboot_node(const void *fdt, int node, - const char *name, - u32 address_cells, u32 size_cells) -{ - static int __initdata kind_guess = 0; - const struct fdt_property *prop; - const __be32 *cell; - bootmodule_kind kind; - paddr_t start, size; - int len; - /* sizeof("/chosen/") + DT_MAX_NAME + '/' + DT_MAX_NAME + '/0' => 92 */ - char path[92]; - int parent_node, ret; - bool domU; - - parent_node = fdt_parent_offset(fdt, node); - ASSERT(parent_node >= 0); - - /* Check that the node is under "/chosen" (first 7 chars of path) */ - ret = fdt_get_path(fdt, node, path, sizeof (path)); - if ( ret != 0 || strncmp(path, "/chosen", 7) ) - return; - - prop = fdt_get_property(fdt, node, "reg", &len); - if ( !prop ) - panic("node %s missing `reg' property\n", name); - - if ( len < dt_cells_to_size(address_cells + size_cells) ) - panic("fdt: node `%s': `reg` property length is too short\n", - name); - - cell = (const __be32 *)prop->data; - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); - - if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 || - fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 ) - kind = BOOTMOD_KERNEL; - else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 || - fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 ) - kind = BOOTMOD_RAMDISK; - else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 ) - kind = BOOTMOD_XSM; - else if ( fdt_node_check_compatible(fdt, node, "multiboot,device-tree") == 0 ) - kind = BOOTMOD_GUEST_DTB; - else - kind = BOOTMOD_UNKNOWN; - - /** - * Guess the kind of these first two unknowns respectively: - * (1) The first unknown must be kernel. - * (2) Detect the XSM Magic from the 2nd unknown: - * a. If it's XSM, set the kind as XSM, and that also means we - * won't load ramdisk; - * b. if it's not XSM, set the kind as ramdisk. - * So if user want to load ramdisk, it must be the 2nd unknown. - * We also detect the XSM Magic for the following unknowns, - * then set its kind according to the return value of has_xsm_magic. - */ - if ( kind == BOOTMOD_UNKNOWN ) - { - switch ( kind_guess++ ) - { - case 0: kind = BOOTMOD_KERNEL; break; - case 1: kind = BOOTMOD_RAMDISK; break; - default: break; - } - if ( kind_guess > 1 && has_xsm_magic(start) ) - kind = BOOTMOD_XSM; - } - - domU = fdt_node_check_compatible(fdt, parent_node, "xen,domain") == 0; - add_boot_module(kind, start, size, domU); - - prop = fdt_get_property(fdt, node, "bootargs", &len); - if ( !prop ) - return; - add_boot_cmdline(fdt_get_name(fdt, parent_node, &len), prop->data, - kind, start, domU); -} - -static int __init process_chosen_node(const void *fdt, int node, - const char *name, - u32 address_cells, u32 size_cells) -{ - const struct fdt_property *prop; - paddr_t start, end; - int len; - - if ( fdt_get_property(fdt, node, "xen,static-heap", NULL) ) - { - int rc; - - printk("Checking for static heap in /chosen\n"); - - rc = device_tree_get_meminfo(fdt, node, "xen,static-heap", - address_cells, size_cells, - bootinfo_get_reserved_mem(), - MEMBANK_STATIC_HEAP); - if ( rc ) - return rc; - - bootinfo.static_heap = true; - } - - printk("Checking for initrd in /chosen\n"); - - prop = fdt_get_property(fdt, node, "linux,initrd-start", &len); - if ( !prop ) - /* No initrd present. */ - return 0; - if ( len != sizeof(u32) && len != sizeof(u64) ) - { - printk("linux,initrd-start property has invalid length %d\n", len); - return -EINVAL; - } - start = dt_read_paddr((const void *)&prop->data, dt_size_to_cells(len)); - - prop = fdt_get_property(fdt, node, "linux,initrd-end", &len); - if ( !prop ) - { - printk("linux,initrd-end not present but -start was\n"); - return -EINVAL; - } - if ( len != sizeof(u32) && len != sizeof(u64) ) - { - printk("linux,initrd-end property has invalid length %d\n", len); - return -EINVAL; - } - end = dt_read_paddr((const void *)&prop->data, dt_size_to_cells(len)); - - if ( start >= end ) - { - printk("linux,initrd limits invalid: %"PRIpaddr" >= %"PRIpaddr"\n", - start, end); - return -EINVAL; - } - - printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end); - - add_boot_module(BOOTMOD_RAMDISK, start, end-start, false); - - return 0; -} - -static int __init process_domain_node(const void *fdt, int node, - const char *name, - u32 address_cells, u32 size_cells) -{ - const struct fdt_property *prop; - - printk("Checking for \"xen,static-mem\" in domain node\n"); - - prop = fdt_get_property(fdt, node, "xen,static-mem", NULL); - if ( !prop ) - /* No "xen,static-mem" present. */ - return 0; - - return device_tree_get_meminfo(fdt, node, "xen,static-mem", address_cells, - size_cells, bootinfo_get_reserved_mem(), - MEMBANK_STATIC_DOMAIN); -} - -static int __init early_scan_node(const void *fdt, - int node, const char *name, int depth, - u32 address_cells, u32 size_cells, - void *data) -{ - int rc = 0; - - /* - * If Xen has been booted via UEFI, the memory banks are - * populated. So we should skip the parsing. - */ - if ( !efi_enabled(EFI_BOOT) && - device_tree_node_matches(fdt, node, "memory") ) - rc = process_memory_node(fdt, node, name, depth, - address_cells, size_cells, bootinfo_get_mem()); - else if ( depth == 1 && !dt_node_cmp(name, "reserved-memory") ) - rc = process_reserved_memory(fdt, node, name, depth, - address_cells, size_cells); - else if ( depth <= 3 && (device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) || - device_tree_node_compatible(fdt, node, "multiboot,module" ))) - process_multiboot_node(fdt, node, name, address_cells, size_cells); - else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") ) - rc = process_chosen_node(fdt, node, name, address_cells, size_cells); - else if ( depth == 2 && device_tree_node_compatible(fdt, node, "xen,domain") ) - rc = process_domain_node(fdt, node, name, address_cells, size_cells); - else if ( depth <= 3 && device_tree_node_compatible(fdt, node, "xen,domain-shared-memory-v1") ) - rc = process_shm_node(fdt, node, address_cells, size_cells); - - if ( rc < 0 ) - printk("fdt: node `%s': parsing failed\n", name); - return rc; -} - -static void __init early_print_info(void) -{ - const struct membanks *mi = bootinfo_get_mem(); - const struct membanks *mem_resv = bootinfo_get_reserved_mem(); - struct bootmodules *mods = &bootinfo.modules; - struct bootcmdlines *cmds = &bootinfo.cmdlines; - unsigned int i; - - for ( i = 0; i < mi->nr_banks; i++ ) - printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", - mi->bank[i].start, - mi->bank[i].start + mi->bank[i].size - 1); - printk("\n"); - for ( i = 0 ; i < mods->nr_mods; i++ ) - printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s\n", - i, - mods->module[i].start, - mods->module[i].start + mods->module[i].size, - boot_module_kind_as_string(mods->module[i].kind)); - - for ( i = 0; i < mem_resv->nr_banks; i++ ) - { - printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, - mem_resv->bank[i].start, - mem_resv->bank[i].start + mem_resv->bank[i].size - 1); - } - early_print_info_shmem(); - printk("\n"); - for ( i = 0 ; i < cmds->nr_mods; i++ ) - printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start, - cmds->cmdline[i].dt_name, - &cmds->cmdline[i].cmdline[0]); - printk("\n"); -} - -/* This function assumes that memory regions are not overlapped */ -static int __init cmp_memory_node(const void *key, const void *elem) -{ - const struct membank *handler0 = key; - const struct membank *handler1 = elem; - - if ( handler0->start < handler1->start ) - return -1; - - if ( handler0->start >= (handler1->start + handler1->size) ) - return 1; - - return 0; -} - -static void __init swap_memory_node(void *_a, void *_b, size_t size) -{ - struct membank *a = _a, *b = _b; - - SWAP(*a, *b); -} - -/** - * boot_fdt_info - initialize bootinfo from a DTB - * @fdt: flattened device tree binary - * - * Returns the size of the DTB. - */ -size_t __init boot_fdt_info(const void *fdt, paddr_t paddr) -{ - struct membanks *reserved_mem = bootinfo_get_reserved_mem(); - struct membanks *mem = bootinfo_get_mem(); - unsigned int i; - int nr_rsvd; - int ret; - - ret = fdt_check_header(fdt); - if ( ret < 0 ) - panic("No valid device tree\n"); - - add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false); - - nr_rsvd = fdt_num_mem_rsv(fdt); - if ( nr_rsvd < 0 ) - panic("Parsing FDT memory reserve map failed (%d)\n", nr_rsvd); - - for ( i = 0; i < nr_rsvd; i++ ) - { - struct membank *bank; - paddr_t s, sz; - - if ( fdt_get_mem_rsv_paddr(device_tree_flattened, i, &s, &sz) < 0 ) - continue; - - if ( reserved_mem->nr_banks < reserved_mem->max_banks ) - { - bank = &reserved_mem->bank[reserved_mem->nr_banks]; - bank->start = s; - bank->size = sz; - bank->type = MEMBANK_FDT_RESVMEM; - reserved_mem->nr_banks++; - } - else - panic("Cannot allocate reserved memory bank\n"); - } - - ret = device_tree_for_each_node(fdt, 0, early_scan_node, NULL); - if ( ret ) - panic("Early FDT parsing failed (%d)\n", ret); - - /* - * On Arm64 setup_directmap_mappings() expects to be called with the lowest - * bank in memory first. There is no requirement that the DT will provide - * the banks sorted in ascending order. So sort them through. - */ - sort(mem->bank, mem->nr_banks, sizeof(struct membank), - cmp_memory_node, swap_memory_node); - - early_print_info(); - - return fdt_totalsize(fdt); -} - -const __init char *boot_fdt_cmdline(const void *fdt) -{ - int node; - const struct fdt_property *prop; - - node = fdt_path_offset(fdt, "/chosen"); - if ( node < 0 ) - return NULL; - - prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL); - if ( prop == NULL ) - { - struct bootcmdline *dom0_cmdline = - boot_cmdline_find_by_kind(BOOTMOD_KERNEL); - - if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) || - ( dom0_cmdline && dom0_cmdline->cmdline[0] ) ) - prop = fdt_get_property(fdt, node, "bootargs", NULL); - } - if ( prop == NULL ) - return NULL; - - return prop->data; -} - -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h index 051e796716..5ee690aeb2 100644 --- a/xen/arch/arm/include/asm/setup.h +++ b/xen/arch/arm/include/asm/setup.h @@ -39,19 +39,6 @@ void fw_unreserved_regions(paddr_t s, paddr_t e, void (*cb)(paddr_t ps, paddr_t pe), unsigned int first); -bool check_reserved_regions_overlap(paddr_t region_start, paddr_t region_size); - -struct bootmodule *add_boot_module(bootmodule_kind kind, - paddr_t start, paddr_t size, bool domU); -struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind); -struct bootmodule * boot_module_find_by_addr_and_kind(bootmodule_kind kind, - paddr_t start); -void add_boot_cmdline(const char *name, const char *cmdline, - bootmodule_kind kind, paddr_t start, bool domU); -struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind); -struct bootcmdline * boot_cmdline_find_by_name(const char *name); -const char *boot_module_kind_as_string(bootmodule_kind kind); - void init_pdx(void); void setup_mm(void); diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile index 947bad979c..ff2de71c96 100644 --- a/xen/common/device-tree/Makefile +++ b/xen/common/device-tree/Makefile @@ -1 +1,2 @@ +obj-y += bootfdt.init.o obj-y += bootinfo.init.o diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c new file mode 100644 index 0000000000..748b5f7c69 --- /dev/null +++ b/xen/common/device-tree/bootfdt.c @@ -0,0 +1,635 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Early Device Tree + * + * Copyright (C) 2012-2014 Citrix Systems, Inc. + */ + +#include <xen/bootfdt.h> +#include <xen/device_tree.h> +#include <xen/efi.h> +#include <xen/init.h> +#include <xen/kernel.h> +#include <xen/libfdt/libfdt-xen.h> +#include <xen/sort.h> +#include <xsm/xsm.h> +#include <asm/setup.h> +#ifdef CONFIG_STATIC_SHM +#include <asm/static-shmem.h> +#endif + +static void __init __maybe_unused build_assertions(void) +{ + /* + * Check that no padding is between struct membanks "bank" flexible array + * member and struct meminfo "bank" member + */ + BUILD_BUG_ON((offsetof(struct membanks, bank) != + offsetof(struct meminfo, bank))); + /* Ensure "struct membanks" is 8-byte aligned */ + BUILD_BUG_ON(alignof(struct membanks) != 8); +} + +static bool __init device_tree_node_is_available(const void *fdt, int node) +{ + const char *status; + int len; + + status = fdt_getprop(fdt, node, "status", &len); + if ( !status ) + return true; + + if ( len > 0 ) + { + if ( !strcmp(status, "ok") || !strcmp(status, "okay") ) + return true; + } + + return false; +} + +static bool __init device_tree_node_matches(const void *fdt, int node, + const char *match) +{ + const char *name; + size_t match_len; + + name = fdt_get_name(fdt, node, NULL); + match_len = strlen(match); + + /* Match both "match" and "match@..." patterns but not + "match-foo". */ + return strncmp(name, match, match_len) == 0 + && (name[match_len] == '@' || name[match_len] == '\0'); +} + +static bool __init device_tree_node_compatible(const void *fdt, int node, + const char *match) +{ + int len, l; + const void *prop; + + prop = fdt_getprop(fdt, node, "compatible", &len); + if ( prop == NULL ) + return false; + + while ( len > 0 ) { + if ( !dt_compat_cmp(prop, match) ) + return true; + l = strlen(prop) + 1; + prop += l; + len -= l; + } + + return false; +} + +void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells, + uint32_t size_cells, paddr_t *start, + paddr_t *size) +{ + uint64_t dt_start, dt_size; + + /* + * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit. + * Thus, there is an implicit cast from uint64_t to paddr_t. + */ + dt_start = dt_next_cell(address_cells, cell); + dt_size = dt_next_cell(size_cells, cell); + + if ( dt_start != (paddr_t)dt_start ) + { + printk("Physical address greater than max width supported\n"); + WARN(); + } + + if ( dt_size != (paddr_t)dt_size ) + { + printk("Physical size greater than max width supported\n"); + WARN(); + } + + /* + * Xen will truncate the address/size if it is greater than the maximum + * supported width and it will give an appropriate warning. + */ + *start = dt_start; + *size = dt_size; +} + +static int __init device_tree_get_meminfo(const void *fdt, int node, + const char *prop_name, + u32 address_cells, u32 size_cells, + struct membanks *mem, + enum membank_type type) +{ + const struct fdt_property *prop; + unsigned int i, banks; + const __be32 *cell; + u32 reg_cells = address_cells + size_cells; + paddr_t start, size; + + if ( !device_tree_node_is_available(fdt, node) ) + return 0; + + if ( address_cells < 1 || size_cells < 1 ) + { + printk("fdt: property `%s': invalid #address-cells or #size-cells", + prop_name); + return -EINVAL; + } + + prop = fdt_get_property(fdt, node, prop_name, NULL); + if ( !prop ) + return -ENOENT; + + cell = (const __be32 *)prop->data; + banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32)); + + for ( i = 0; i < banks && mem->nr_banks < mem->max_banks; i++ ) + { + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); + if ( mem == bootinfo_get_reserved_mem() && + check_reserved_regions_overlap(start, size) ) + return -EINVAL; + /* Some DT may describe empty bank, ignore them */ + if ( !size ) + continue; + mem->bank[mem->nr_banks].start = start; + mem->bank[mem->nr_banks].size = size; + mem->bank[mem->nr_banks].type = type; + mem->nr_banks++; + } + + if ( i < banks ) + { + printk("Warning: Max number of supported memory regions reached.\n"); + return -ENOSPC; + } + + return 0; +} + +u32 __init device_tree_get_u32(const void *fdt, int node, + const char *prop_name, u32 dflt) +{ + const struct fdt_property *prop; + + prop = fdt_get_property(fdt, node, prop_name, NULL); + if ( !prop || prop->len < sizeof(u32) ) + return dflt; + + return fdt32_to_cpu(*(uint32_t*)prop->data); +} + +/** + * device_tree_for_each_node - iterate over all device tree sub-nodes + * @fdt: flat device tree. + * @node: parent node to start the search from + * @func: function to call for each sub-node. + * @data: data to pass to @func. + * + * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored. + * + * Returns 0 if all nodes were iterated over successfully. If @func + * returns a value different from 0, that value is returned immediately. + */ +int __init device_tree_for_each_node(const void *fdt, int node, + device_tree_node_func func, + void *data) +{ + /* + * We only care about relative depth increments, assume depth of + * node is 0 for simplicity. + */ + int depth = 0; + const int first_node = node; + u32 address_cells[DEVICE_TREE_MAX_DEPTH]; + u32 size_cells[DEVICE_TREE_MAX_DEPTH]; + int ret; + + do { + const char *name = fdt_get_name(fdt, node, NULL); + u32 as, ss; + + if ( depth >= DEVICE_TREE_MAX_DEPTH ) + { + printk("Warning: device tree node `%s' is nested too deep\n", + name); + continue; + } + + as = depth > 0 ? address_cells[depth-1] : DT_ROOT_NODE_ADDR_CELLS_DEFAULT; + ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT; + + address_cells[depth] = device_tree_get_u32(fdt, node, + "#address-cells", as); + size_cells[depth] = device_tree_get_u32(fdt, node, + "#size-cells", ss); + + /* skip the first node */ + if ( node != first_node ) + { + ret = func(fdt, node, name, depth, as, ss, data); + if ( ret != 0 ) + return ret; + } + + node = fdt_next_node(fdt, node, &depth); + } while ( node >= 0 && depth > 0 ); + + return 0; +} + +static int __init process_memory_node(const void *fdt, int node, + const char *name, int depth, + u32 address_cells, u32 size_cells, + struct membanks *mem) +{ + return device_tree_get_meminfo(fdt, node, "reg", address_cells, size_cells, + mem, MEMBANK_DEFAULT); +} + +static int __init process_reserved_memory_node(const void *fdt, int node, + const char *name, int depth, + u32 address_cells, + u32 size_cells, + void *data) +{ + int rc = process_memory_node(fdt, node, name, depth, address_cells, + size_cells, data); + + if ( rc == -ENOSPC ) + panic("Max number of supported reserved-memory regions reached.\n"); + else if ( rc != -ENOENT ) + return rc; + return 0; +} + +static int __init process_reserved_memory(const void *fdt, int node, + const char *name, int depth, + u32 address_cells, u32 size_cells) +{ + return device_tree_for_each_node(fdt, node, + process_reserved_memory_node, + bootinfo_get_reserved_mem()); +} + +static void __init process_multiboot_node(const void *fdt, int node, + const char *name, + u32 address_cells, u32 size_cells) +{ + static int __initdata kind_guess = 0; + const struct fdt_property *prop; + const __be32 *cell; + bootmodule_kind kind; + paddr_t start, size; + int len; + /* sizeof("/chosen/") + DT_MAX_NAME + '/' + DT_MAX_NAME + '/0' => 92 */ + char path[92]; + int parent_node, ret; + bool domU; + + parent_node = fdt_parent_offset(fdt, node); + ASSERT(parent_node >= 0); + + /* Check that the node is under "/chosen" (first 7 chars of path) */ + ret = fdt_get_path(fdt, node, path, sizeof (path)); + if ( ret != 0 || strncmp(path, "/chosen", 7) ) + return; + + prop = fdt_get_property(fdt, node, "reg", &len); + if ( !prop ) + panic("node %s missing `reg' property\n", name); + + if ( len < dt_cells_to_size(address_cells + size_cells) ) + panic("fdt: node `%s': `reg` property length is too short\n", + name); + + cell = (const __be32 *)prop->data; + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); + + if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 || + fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 ) + kind = BOOTMOD_KERNEL; + else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0 || + fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 ) + kind = BOOTMOD_RAMDISK; + else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 ) + kind = BOOTMOD_XSM; + else if ( fdt_node_check_compatible(fdt, node, "multiboot,device-tree") == 0 ) + kind = BOOTMOD_GUEST_DTB; + else + kind = BOOTMOD_UNKNOWN; + + /** + * Guess the kind of these first two unknowns respectively: + * (1) The first unknown must be kernel. + * (2) Detect the XSM Magic from the 2nd unknown: + * a. If it's XSM, set the kind as XSM, and that also means we + * won't load ramdisk; + * b. if it's not XSM, set the kind as ramdisk. + * So if user want to load ramdisk, it must be the 2nd unknown. + * We also detect the XSM Magic for the following unknowns, + * then set its kind according to the return value of has_xsm_magic. + */ + if ( kind == BOOTMOD_UNKNOWN ) + { + switch ( kind_guess++ ) + { + case 0: kind = BOOTMOD_KERNEL; break; + case 1: kind = BOOTMOD_RAMDISK; break; + default: break; + } + if ( kind_guess > 1 && has_xsm_magic(start) ) + kind = BOOTMOD_XSM; + } + + domU = fdt_node_check_compatible(fdt, parent_node, "xen,domain") == 0; + add_boot_module(kind, start, size, domU); + + prop = fdt_get_property(fdt, node, "bootargs", &len); + if ( !prop ) + return; + add_boot_cmdline(fdt_get_name(fdt, parent_node, &len), prop->data, + kind, start, domU); +} + +static int __init process_chosen_node(const void *fdt, int node, + const char *name, + u32 address_cells, u32 size_cells) +{ + const struct fdt_property *prop; + paddr_t start, end; + int len; + + if ( fdt_get_property(fdt, node, "xen,static-heap", NULL) ) + { + int rc; + + printk("Checking for static heap in /chosen\n"); + + rc = device_tree_get_meminfo(fdt, node, "xen,static-heap", + address_cells, size_cells, + bootinfo_get_reserved_mem(), + MEMBANK_STATIC_HEAP); + if ( rc ) + return rc; + + bootinfo.static_heap = true; + } + + printk("Checking for initrd in /chosen\n"); + + prop = fdt_get_property(fdt, node, "linux,initrd-start", &len); + if ( !prop ) + /* No initrd present. */ + return 0; + if ( len != sizeof(u32) && len != sizeof(u64) ) + { + printk("linux,initrd-start property has invalid length %d\n", len); + return -EINVAL; + } + start = dt_read_paddr((const void *)&prop->data, dt_size_to_cells(len)); + + prop = fdt_get_property(fdt, node, "linux,initrd-end", &len); + if ( !prop ) + { + printk("linux,initrd-end not present but -start was\n"); + return -EINVAL; + } + if ( len != sizeof(u32) && len != sizeof(u64) ) + { + printk("linux,initrd-end property has invalid length %d\n", len); + return -EINVAL; + } + end = dt_read_paddr((const void *)&prop->data, dt_size_to_cells(len)); + + if ( start >= end ) + { + printk("linux,initrd limits invalid: %"PRIpaddr" >= %"PRIpaddr"\n", + start, end); + return -EINVAL; + } + + printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end); + + add_boot_module(BOOTMOD_RAMDISK, start, end-start, false); + + return 0; +} + +static int __init process_domain_node(const void *fdt, int node, + const char *name, + u32 address_cells, u32 size_cells) +{ + const struct fdt_property *prop; + + printk("Checking for \"xen,static-mem\" in domain node\n"); + + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL); + if ( !prop ) + /* No "xen,static-mem" present. */ + return 0; + + return device_tree_get_meminfo(fdt, node, "xen,static-mem", address_cells, + size_cells, bootinfo_get_reserved_mem(), + MEMBANK_STATIC_DOMAIN); +} + +#ifndef CONFIG_STATIC_SHM +static inline int process_shm_node(const void *fdt, int node, + uint32_t address_cells, uint32_t size_cells) +{ + printk("CONFIG_STATIC_SHM must be enabled for parsing static shared" + " memory nodes\n"); + return -EINVAL; +} +#endif + +static int __init early_scan_node(const void *fdt, + int node, const char *name, int depth, + u32 address_cells, u32 size_cells, + void *data) +{ + int rc = 0; + + /* + * If Xen has been booted via UEFI, the memory banks are + * populated. So we should skip the parsing. + */ + if ( !efi_enabled(EFI_BOOT) && + device_tree_node_matches(fdt, node, "memory") ) + rc = process_memory_node(fdt, node, name, depth, + address_cells, size_cells, bootinfo_get_mem()); + else if ( depth == 1 && !dt_node_cmp(name, "reserved-memory") ) + rc = process_reserved_memory(fdt, node, name, depth, + address_cells, size_cells); + else if ( depth <= 3 && (device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) || + device_tree_node_compatible(fdt, node, "multiboot,module" ))) + process_multiboot_node(fdt, node, name, address_cells, size_cells); + else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") ) + rc = process_chosen_node(fdt, node, name, address_cells, size_cells); + else if ( depth == 2 && device_tree_node_compatible(fdt, node, "xen,domain") ) + rc = process_domain_node(fdt, node, name, address_cells, size_cells); + else if ( depth <= 3 && device_tree_node_compatible(fdt, node, "xen,domain-shared-memory-v1") ) + rc = process_shm_node(fdt, node, address_cells, size_cells); + + if ( rc < 0 ) + printk("fdt: node `%s': parsing failed\n", name); + return rc; +} + +static void __init early_print_info(void) +{ + const struct membanks *mi = bootinfo_get_mem(); + const struct membanks *mem_resv = bootinfo_get_reserved_mem(); + struct bootmodules *mods = &bootinfo.modules; + struct bootcmdlines *cmds = &bootinfo.cmdlines; + unsigned int i; + + for ( i = 0; i < mi->nr_banks; i++ ) + printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", + mi->bank[i].start, + mi->bank[i].start + mi->bank[i].size - 1); + printk("\n"); + for ( i = 0 ; i < mods->nr_mods; i++ ) + printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s\n", + i, + mods->module[i].start, + mods->module[i].start + mods->module[i].size, + boot_module_kind_as_string(mods->module[i].kind)); + + for ( i = 0; i < mem_resv->nr_banks; i++ ) + { + printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, + mem_resv->bank[i].start, + mem_resv->bank[i].start + mem_resv->bank[i].size - 1); + } +#ifdef CONFIG_STATIC_SHM + early_print_info_shmem(); +#endif + printk("\n"); + for ( i = 0 ; i < cmds->nr_mods; i++ ) + printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start, + cmds->cmdline[i].dt_name, + &cmds->cmdline[i].cmdline[0]); + printk("\n"); +} + +/* This function assumes that memory regions are not overlapped */ +static int __init cmp_memory_node(const void *key, const void *elem) +{ + const struct membank *handler0 = key; + const struct membank *handler1 = elem; + + if ( handler0->start < handler1->start ) + return -1; + + if ( handler0->start >= (handler1->start + handler1->size) ) + return 1; + + return 0; +} + +static void __init swap_memory_node(void *_a, void *_b, size_t size) +{ + struct membank *a = _a, *b = _b; + + SWAP(*a, *b); +} + +/** + * boot_fdt_info - initialize bootinfo from a DTB + * @fdt: flattened device tree binary + * + * Returns the size of the DTB. + */ +size_t __init boot_fdt_info(const void *fdt, paddr_t paddr) +{ + struct membanks *reserved_mem = bootinfo_get_reserved_mem(); + struct membanks *mem = bootinfo_get_mem(); + unsigned int i; + int nr_rsvd; + int ret; + + ret = fdt_check_header(fdt); + if ( ret < 0 ) + panic("No valid device tree\n"); + + add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false); + + nr_rsvd = fdt_num_mem_rsv(fdt); + if ( nr_rsvd < 0 ) + panic("Parsing FDT memory reserve map failed (%d)\n", nr_rsvd); + + for ( i = 0; i < nr_rsvd; i++ ) + { + struct membank *bank; + paddr_t s, sz; + + if ( fdt_get_mem_rsv_paddr(device_tree_flattened, i, &s, &sz) < 0 ) + continue; + + if ( reserved_mem->nr_banks < reserved_mem->max_banks ) + { + bank = &reserved_mem->bank[reserved_mem->nr_banks]; + bank->start = s; + bank->size = sz; + bank->type = MEMBANK_FDT_RESVMEM; + reserved_mem->nr_banks++; + } + else + panic("Cannot allocate reserved memory bank\n"); + } + + ret = device_tree_for_each_node(fdt, 0, early_scan_node, NULL); + if ( ret ) + panic("Early FDT parsing failed (%d)\n", ret); + + /* + * On Arm64 setup_directmap_mappings() expects to be called with the lowest + * bank in memory first. There is no requirement that the DT will provide + * the banks sorted in ascending order. So sort them through. + */ + sort(mem->bank, mem->nr_banks, sizeof(struct membank), + cmp_memory_node, swap_memory_node); + + early_print_info(); + + return fdt_totalsize(fdt); +} + +const __init char *boot_fdt_cmdline(const void *fdt) +{ + int node; + const struct fdt_property *prop; + + node = fdt_path_offset(fdt, "/chosen"); + if ( node < 0 ) + return NULL; + + prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL); + if ( prop == NULL ) + { + struct bootcmdline *dom0_cmdline = + boot_cmdline_find_by_kind(BOOTMOD_KERNEL); + + if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) || + ( dom0_cmdline && dom0_cmdline->cmdline[0] ) ) + prop = fdt_get_property(fdt, node, "bootargs", NULL); + } + if ( prop == NULL ) + return NULL; + + return prop->data; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h index 7cd45b3d4b..5943bc5fe3 100644 --- a/xen/include/xen/bootfdt.h +++ b/xen/include/xen/bootfdt.h @@ -158,6 +158,20 @@ struct bootinfo { extern struct bootinfo bootinfo; +bool check_reserved_regions_overlap(paddr_t region_start, + paddr_t region_size); + +struct bootmodule *add_boot_module(bootmodule_kind kind, + paddr_t start, paddr_t size, bool domU); +struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind); +struct bootmodule * boot_module_find_by_addr_and_kind(bootmodule_kind kind, + paddr_t start); +void add_boot_cmdline(const char *name, const char *cmdline, + bootmodule_kind kind, paddr_t start, bool domU); +struct bootcmdline *boot_cmdline_find_by_kind(bootmodule_kind kind); +struct bootcmdline * boot_cmdline_find_by_name(const char *name); +const char *boot_module_kind_as_string(bootmodule_kind kind); + void populate_boot_allocator(void); size_t boot_fdt_info(const void *fdt, paddr_t paddr); -- 2.45.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v1 3/5] xen/riscv: enable CONFIG_HAS_DEVICE_TREE 2024-07-03 10:42 [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii Kurochko 2024-07-03 10:42 ` [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common Oleksii Kurochko 2024-07-03 10:42 ` [PATCH v1 2/5] xen/common: Move Arm's bootfdt.c " Oleksii Kurochko @ 2024-07-03 10:42 ` Oleksii Kurochko 2024-07-10 10:25 ` Jan Beulich 2024-07-03 10:42 ` [PATCH v1 4/5] xen/riscv: introduce device tree maping function Oleksii Kurochko ` (2 subsequent siblings) 5 siblings, 1 reply; 28+ messages in thread From: Oleksii Kurochko @ 2024-07-03 10:42 UTC (permalink / raw) To: xen-devel Cc: Oleksii Kurochko, Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Jan Beulich, Julien Grall, Stefano Stabellini Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig index 74ad019fe7..1863a5d117 100644 --- a/xen/arch/riscv/Kconfig +++ b/xen/arch/riscv/Kconfig @@ -5,6 +5,7 @@ config RISCV config RISCV_64 def_bool y select 64BIT + select HAS_DEVICE_TREE select GENERIC_BUG_FRAME config ARCH_DEFCONFIG -- 2.45.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v1 3/5] xen/riscv: enable CONFIG_HAS_DEVICE_TREE 2024-07-03 10:42 ` [PATCH v1 3/5] xen/riscv: enable CONFIG_HAS_DEVICE_TREE Oleksii Kurochko @ 2024-07-10 10:25 ` Jan Beulich 0 siblings, 0 replies; 28+ messages in thread From: Jan Beulich @ 2024-07-10 10:25 UTC (permalink / raw) To: Oleksii Kurochko Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On 03.07.2024 12:42, Oleksii Kurochko wrote: > --- a/xen/arch/riscv/Kconfig > +++ b/xen/arch/riscv/Kconfig > @@ -5,6 +5,7 @@ config RISCV > config RISCV_64 > def_bool y > select 64BIT > + select HAS_DEVICE_TREE > select GENERIC_BUG_FRAME > > config ARCH_DEFCONFIG Same question here as for a change in the earlier series: Why here and not for RISCV? Plus, nit: Please keep the list of selects sorted alphabetically. Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 4/5] xen/riscv: introduce device tree maping function 2024-07-03 10:42 [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii Kurochko ` (2 preceding siblings ...) 2024-07-03 10:42 ` [PATCH v1 3/5] xen/riscv: enable CONFIG_HAS_DEVICE_TREE Oleksii Kurochko @ 2024-07-03 10:42 ` Oleksii Kurochko 2024-07-10 10:38 ` Jan Beulich 2024-07-03 10:42 ` [PATCH v1 5/5] xen/riscv: map FDT Oleksii Kurochko 2024-07-03 10:52 ` [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii 5 siblings, 1 reply; 28+ messages in thread From: Oleksii Kurochko @ 2024-07-03 10:42 UTC (permalink / raw) To: xen-devel Cc: Oleksii Kurochko, Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Jan Beulich, Julien Grall, Stefano Stabellini Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/include/asm/config.h | 6 +++++ xen/arch/riscv/include/asm/mm.h | 2 ++ xen/arch/riscv/mm.c | 37 +++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/xen/arch/riscv/include/asm/config.h b/xen/arch/riscv/include/asm/config.h index 50583aafdc..2395bafb91 100644 --- a/xen/arch/riscv/include/asm/config.h +++ b/xen/arch/riscv/include/asm/config.h @@ -74,6 +74,9 @@ #error "unsupported RV_STAGE1_MODE" #endif +#define XEN_SIZE MB(2) +#define XEN_VIRT_END (XEN_VIRT_START + XEN_SIZE) + #define DIRECTMAP_SLOT_END 509 #define DIRECTMAP_SLOT_START 200 #define DIRECTMAP_VIRT_START SLOTN(DIRECTMAP_SLOT_START) @@ -99,6 +102,9 @@ #define VMAP_VIRT_START SLOTN(VMAP_SLOT_START) #define VMAP_VIRT_SIZE GB(1) +#define BOOT_FDT_VIRT_START XEN_VIRT_END +#define BOOT_FDT_VIRT_SIZE MB(4) + #else #error "RV32 isn't supported" #endif diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h index 25af9e1aaa..d1db7ce098 100644 --- a/xen/arch/riscv/include/asm/mm.h +++ b/xen/arch/riscv/include/asm/mm.h @@ -255,4 +255,6 @@ static inline unsigned int arch_get_dma_bitsize(void) return 32; /* TODO */ } +void* early_fdt_map(paddr_t fdt_paddr); + #endif /* _ASM_RISCV_MM_H */ diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c index 7d09e781bf..ccc91f9a01 100644 --- a/xen/arch/riscv/mm.c +++ b/xen/arch/riscv/mm.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <xen/bootfdt.h> #include <xen/bug.h> #include <xen/compiler.h> #include <xen/init.h> @@ -7,7 +8,9 @@ #include <xen/macros.h> #include <xen/mm.h> #include <xen/pfn.h> +#include <xen/libfdt/libfdt.h> #include <xen/sections.h> +#include <xen/sizes.h> #include <asm/early_printk.h> #include <asm/csr.h> @@ -20,7 +23,7 @@ struct mmu_desc { unsigned int pgtbl_count; pte_t *next_pgtbl; pte_t *pgtbl_base; -}; +} mmu_desc = { CONFIG_PAGING_LEVELS, 0, NULL, 0 }; static unsigned long __ro_after_init phys_offset; @@ -39,9 +42,11 @@ static unsigned long __ro_after_init phys_offset; * isn't 2 MB aligned. * * CONFIG_PAGING_LEVELS page tables are needed for the identity mapping, - * except that the root page table is shared with the initial mapping + * except that the root page table is shared with the initial mapping. + * + * CONFIG_PAGING_LEVELS page tables are needed for device tree mapping. */ -#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 2 + 1) +#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 3 + 1 + 1) pte_t __section(".bss.page_aligned") __aligned(PAGE_SIZE) stage1_pgtbl_root[PAGETABLE_ENTRIES]; @@ -207,8 +212,6 @@ static bool __init check_pgtbl_mode_support(struct mmu_desc *mmu_desc, */ void __init setup_initial_pagetables(void) { - struct mmu_desc mmu_desc = { CONFIG_PAGING_LEVELS, 0, NULL, NULL }; - /* * Access to _start, _end is always PC-relative thereby when access * them we will get load adresses of start and end of Xen. @@ -296,6 +299,30 @@ unsigned long __init calc_phys_offset(void) return phys_offset; } +void* __init early_fdt_map(paddr_t fdt_paddr) +{ + unsigned long dt_phys_base = fdt_paddr; + unsigned long dt_virt_base; + unsigned long dt_virt_size; + + BUILD_BUG_ON(MIN_FDT_ALIGN < 8); + if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN || fdt_paddr % SZ_2M || + fdt_totalsize(fdt_paddr) > BOOT_FDT_VIRT_SIZE ) + return NULL; + + BUILD_BUG_ON(BOOT_FDT_VIRT_START % SZ_2M); + + dt_virt_base = BOOT_FDT_VIRT_START; + dt_virt_size = BOOT_FDT_VIRT_SIZE; + + /* Map device tree */ + setup_initial_mapping(&mmu_desc, dt_virt_base, + dt_virt_base + dt_virt_size, + dt_phys_base); + + return (void *)dt_virt_base; +} + void put_page(struct page_info *page) { BUG_ON("unimplemented"); -- 2.45.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v1 4/5] xen/riscv: introduce device tree maping function 2024-07-03 10:42 ` [PATCH v1 4/5] xen/riscv: introduce device tree maping function Oleksii Kurochko @ 2024-07-10 10:38 ` Jan Beulich 2024-07-11 9:31 ` Oleksii 0 siblings, 1 reply; 28+ messages in thread From: Jan Beulich @ 2024-07-10 10:38 UTC (permalink / raw) To: Oleksii Kurochko Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On 03.07.2024 12:42, Oleksii Kurochko wrote: > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > xen/arch/riscv/include/asm/config.h | 6 +++++ > xen/arch/riscv/include/asm/mm.h | 2 ++ > xen/arch/riscv/mm.c | 37 +++++++++++++++++++++++++---- > 3 files changed, 40 insertions(+), 5 deletions(-) I don't think a change like this can come without any description. > --- a/xen/arch/riscv/include/asm/config.h > +++ b/xen/arch/riscv/include/asm/config.h > @@ -74,6 +74,9 @@ > #error "unsupported RV_STAGE1_MODE" > #endif > > +#define XEN_SIZE MB(2) > +#define XEN_VIRT_END (XEN_VIRT_START + XEN_SIZE) Probably wants accompanying by an assertion in the linker script. Or else how would one notice when Xen grows bigger than this? > @@ -99,6 +102,9 @@ > #define VMAP_VIRT_START SLOTN(VMAP_SLOT_START) > #define VMAP_VIRT_SIZE GB(1) > > +#define BOOT_FDT_VIRT_START XEN_VIRT_END > +#define BOOT_FDT_VIRT_SIZE MB(4) Is the 4 selected arbitrarily, or derived from something? Also maybe better to keep these #define-s sorted by address? (As to "keep": I didn't check whether they currently are.) > --- a/xen/arch/riscv/include/asm/mm.h > +++ b/xen/arch/riscv/include/asm/mm.h > @@ -255,4 +255,6 @@ static inline unsigned int arch_get_dma_bitsize(void) > return 32; /* TODO */ > } > > +void* early_fdt_map(paddr_t fdt_paddr); Nit: * and blank want to change places. > --- a/xen/arch/riscv/mm.c > +++ b/xen/arch/riscv/mm.c > @@ -1,5 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0-only */ > > +#include <xen/bootfdt.h> > #include <xen/bug.h> > #include <xen/compiler.h> > #include <xen/init.h> > @@ -7,7 +8,9 @@ > #include <xen/macros.h> > #include <xen/mm.h> > #include <xen/pfn.h> > +#include <xen/libfdt/libfdt.h> This wants to move up, to retain sorting. > #include <xen/sections.h> > +#include <xen/sizes.h> > > #include <asm/early_printk.h> > #include <asm/csr.h> > @@ -20,7 +23,7 @@ struct mmu_desc { > unsigned int pgtbl_count; > pte_t *next_pgtbl; > pte_t *pgtbl_base; > -}; > +} mmu_desc = { CONFIG_PAGING_LEVELS, 0, NULL, 0 }; __initdata and static? > @@ -39,9 +42,11 @@ static unsigned long __ro_after_init phys_offset; > * isn't 2 MB aligned. > * > * CONFIG_PAGING_LEVELS page tables are needed for the identity mapping, > - * except that the root page table is shared with the initial mapping > + * except that the root page table is shared with the initial mapping. > + * > + * CONFIG_PAGING_LEVELS page tables are needed for device tree mapping. > */ > -#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 2 + 1) > +#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 3 + 1 + 1) Considering what would happen if two or three more of such requirements were added, maybe better #define PGTBL_INITIAL_COUNT (CONFIG_PAGING_LEVELS * 3 - 1) ? However, why is it CONFIG_PAGING_LEVELS that's needed, and not CONFIG_PAGING_LEVELS - 1? The top level table is the same as the identity map's, isn't it? > @@ -296,6 +299,30 @@ unsigned long __init calc_phys_offset(void) > return phys_offset; > } > > +void* __init early_fdt_map(paddr_t fdt_paddr) See earlier remark regarding * placement. Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 4/5] xen/riscv: introduce device tree maping function 2024-07-10 10:38 ` Jan Beulich @ 2024-07-11 9:31 ` Oleksii 2024-07-11 9:50 ` Jan Beulich 0 siblings, 1 reply; 28+ messages in thread From: Oleksii @ 2024-07-11 9:31 UTC (permalink / raw) To: Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On Wed, 2024-07-10 at 12:38 +0200, Jan Beulich wrote: > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > --- > > xen/arch/riscv/include/asm/config.h | 6 +++++ > > xen/arch/riscv/include/asm/mm.h | 2 ++ > > xen/arch/riscv/mm.c | 37 > > +++++++++++++++++++++++++---- > > 3 files changed, 40 insertions(+), 5 deletions(-) > > I don't think a change like this can come without any description. > > > --- a/xen/arch/riscv/include/asm/config.h > > +++ b/xen/arch/riscv/include/asm/config.h > > @@ -74,6 +74,9 @@ > > #error "unsupported RV_STAGE1_MODE" > > #endif > > > > +#define XEN_SIZE MB(2) > > +#define XEN_VIRT_END (XEN_VIRT_START + XEN_SIZE) > > Probably wants accompanying by an assertion in the linker script. Or > else > how would one notice when Xen grows bigger than this? I use XEN_SIZE in the linker script here: ASSERT(_end - _start <= MB(2), "Xen too large for early-boot assumptions") > > > @@ -99,6 +102,9 @@ > > #define VMAP_VIRT_START SLOTN(VMAP_SLOT_START) > > #define VMAP_VIRT_SIZE GB(1) > > > > +#define BOOT_FDT_VIRT_START XEN_VIRT_END > > +#define BOOT_FDT_VIRT_SIZE MB(4) > > Is the 4 selected arbitrarily, or derived from something? Yes, it was chosen arbitrarily. I just checked that I don't have any DTBs larger than 2 MB, but decided to add a little extra space and doubled it to an additional 2 MB. > > Also maybe better to keep these #define-s sorted by address? (As to > "keep": > I didn't check whether they currently are.) > > > --- a/xen/arch/riscv/include/asm/mm.h > > +++ b/xen/arch/riscv/include/asm/mm.h > > @@ -255,4 +255,6 @@ static inline unsigned int > > arch_get_dma_bitsize(void) > > return 32; /* TODO */ > > } > > > > +void* early_fdt_map(paddr_t fdt_paddr); > > Nit: * and blank want to change places. > > > --- a/xen/arch/riscv/mm.c > > +++ b/xen/arch/riscv/mm.c > > @@ -1,5 +1,6 @@ > > /* SPDX-License-Identifier: GPL-2.0-only */ > > > > +#include <xen/bootfdt.h> > > #include <xen/bug.h> > > #include <xen/compiler.h> > > #include <xen/init.h> > > @@ -7,7 +8,9 @@ > > #include <xen/macros.h> > > #include <xen/mm.h> > > #include <xen/pfn.h> > > +#include <xen/libfdt/libfdt.h> > > This wants to move up, to retain sorting. > > > #include <xen/sections.h> > > +#include <xen/sizes.h> > > > > #include <asm/early_printk.h> > > #include <asm/csr.h> > > @@ -20,7 +23,7 @@ struct mmu_desc { > > unsigned int pgtbl_count; > > pte_t *next_pgtbl; > > pte_t *pgtbl_base; > > -}; > > +} mmu_desc = { CONFIG_PAGING_LEVELS, 0, NULL, 0 }; > > __initdata and static? > > > @@ -39,9 +42,11 @@ static unsigned long __ro_after_init > > phys_offset; > > * isn't 2 MB aligned. > > * > > * CONFIG_PAGING_LEVELS page tables are needed for the identity > > mapping, > > - * except that the root page table is shared with the initial > > mapping > > + * except that the root page table is shared with the initial > > mapping. > > + * > > + * CONFIG_PAGING_LEVELS page tables are needed for device tree > > mapping. > > */ > > -#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 2 + 1) > > +#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 3 + 1 + > > 1) > > Considering what would happen if two or three more of such > requirements > were added, maybe better > > #define PGTBL_INITIAL_COUNT (CONFIG_PAGING_LEVELS * 3 - 1) > > ? However, why is it CONFIG_PAGING_LEVELS that's needed, and not > CONFIG_PAGING_LEVELS - 1? The top level table is the same as the > identity map's, isn't it? The top level table is the same, but I just wanted to be sure that if DTB size is bigger then 2Mb then we need 2xL0 page tables. Am I missing something? ~ Oleksii > > > @@ -296,6 +299,30 @@ unsigned long __init calc_phys_offset(void) > > return phys_offset; > > } > > > > +void* __init early_fdt_map(paddr_t fdt_paddr) > > See earlier remark regarding * placement. > > Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 4/5] xen/riscv: introduce device tree maping function 2024-07-11 9:31 ` Oleksii @ 2024-07-11 9:50 ` Jan Beulich 0 siblings, 0 replies; 28+ messages in thread From: Jan Beulich @ 2024-07-11 9:50 UTC (permalink / raw) To: Oleksii Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On 11.07.2024 11:31, Oleksii wrote: > On Wed, 2024-07-10 at 12:38 +0200, Jan Beulich wrote: >> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>> --- a/xen/arch/riscv/include/asm/config.h >>> +++ b/xen/arch/riscv/include/asm/config.h >>> @@ -74,6 +74,9 @@ >>> #error "unsupported RV_STAGE1_MODE" >>> #endif >>> >>> +#define XEN_SIZE MB(2) >>> +#define XEN_VIRT_END (XEN_VIRT_START + XEN_SIZE) >> >> Probably wants accompanying by an assertion in the linker script. Or >> else >> how would one notice when Xen grows bigger than this? > I use XEN_SIZE in the linker script here: > ASSERT(_end - _start <= MB(2), "Xen too large for early-boot > assumptions") And that's the problem: You want to switch to using XEN_SIZE there. There should never be two separate constant that need updating at the same time. Keep such to a single place. >>> @@ -99,6 +102,9 @@ >>> #define VMAP_VIRT_START SLOTN(VMAP_SLOT_START) >>> #define VMAP_VIRT_SIZE GB(1) >>> >>> +#define BOOT_FDT_VIRT_START XEN_VIRT_END >>> +#define BOOT_FDT_VIRT_SIZE MB(4) >> >> Is the 4 selected arbitrarily, or derived from something? > Yes, it was chosen arbitrarily. I just checked that I don't have any > DTBs larger than 2 MB, but decided to add a little extra space and > doubled it to an additional 2 MB. Code comment then, please, or at the very least mention of this in the description. >>> @@ -39,9 +42,11 @@ static unsigned long __ro_after_init >>> phys_offset; >>> * isn't 2 MB aligned. >>> * >>> * CONFIG_PAGING_LEVELS page tables are needed for the identity >>> mapping, >>> - * except that the root page table is shared with the initial >>> mapping >>> + * except that the root page table is shared with the initial >>> mapping. >>> + * >>> + * CONFIG_PAGING_LEVELS page tables are needed for device tree >>> mapping. >>> */ >>> -#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 2 + 1) >>> +#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 3 + 1 + >>> 1) >> >> Considering what would happen if two or three more of such >> requirements >> were added, maybe better >> >> #define PGTBL_INITIAL_COUNT (CONFIG_PAGING_LEVELS * 3 - 1) >> >> ? However, why is it CONFIG_PAGING_LEVELS that's needed, and not >> CONFIG_PAGING_LEVELS - 1? The top level table is the same as the >> identity map's, isn't it? > The top level table is the same, but I just wanted to be sure that if > DTB size is bigger then 2Mb then we need 2xL0 page tables. Makes sense, but then needs expressing that way (by using BOOT_FDT_VIRT_SIZE). Otherwise (see also above) think of what will happen if BOOT_FDT_VIRT_SIZE is updated without touching this expression. Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 5/5] xen/riscv: map FDT 2024-07-03 10:42 [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii Kurochko ` (3 preceding siblings ...) 2024-07-03 10:42 ` [PATCH v1 4/5] xen/riscv: introduce device tree maping function Oleksii Kurochko @ 2024-07-03 10:42 ` Oleksii Kurochko 2024-07-10 12:38 ` Jan Beulich 2024-07-03 10:52 ` [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii 5 siblings, 1 reply; 28+ messages in thread From: Oleksii Kurochko @ 2024-07-03 10:42 UTC (permalink / raw) To: xen-devel Cc: Oleksii Kurochko, Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Jan Beulich, Julien Grall, Stefano Stabellini Except mapping of FDT, it is also printing command line passed by a DTB and initialize bootinfo from a DTB. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/riscv64/head.S | 3 +++ xen/arch/riscv/setup.c | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S index 3261e9fce8..22fb36a861 100644 --- a/xen/arch/riscv/riscv64/head.S +++ b/xen/arch/riscv/riscv64/head.S @@ -41,6 +41,9 @@ FUNC(start) jal setup_initial_pagetables + mv a0, s1 + jal fdt_map + /* Calculate proper VA after jump from 1:1 mapping */ la a0, .L_primary_switched sub a0, a0, s2 diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c index 4f06203b46..b346956e06 100644 --- a/xen/arch/riscv/setup.c +++ b/xen/arch/riscv/setup.c @@ -1,7 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <xen/bootfdt.h> #include <xen/bug.h> #include <xen/compile.h> +#include <xen/device_tree.h> #include <xen/init.h> #include <xen/mm.h> @@ -33,15 +35,34 @@ static void test_macros_from_bug_h(void) printk("WARN is most likely working\n"); } +void __init fdt_map(paddr_t dtb_addr) +{ + device_tree_flattened = early_fdt_map(dtb_addr); + if ( !device_tree_flattened ) + { + printk("wrong FDT\n"); + die(); + } +} + void __init noreturn start_xen(unsigned long bootcpu_id, paddr_t dtb_addr) { + size_t fdt_size; + const char *cmdline; + remove_identity_mapping(); trap_init(); test_macros_from_bug_h(); + fdt_size = boot_fdt_info(device_tree_flattened, dtb_addr); + + cmdline = boot_fdt_cmdline(device_tree_flattened); + printk("Command line: %s\n", cmdline); + cmdline_parse(cmdline); + printk("All set up\n"); for ( ;; ) -- 2.45.2 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-03 10:42 ` [PATCH v1 5/5] xen/riscv: map FDT Oleksii Kurochko @ 2024-07-10 12:38 ` Jan Beulich 2024-07-11 9:40 ` Oleksii 0 siblings, 1 reply; 28+ messages in thread From: Jan Beulich @ 2024-07-10 12:38 UTC (permalink / raw) To: Oleksii Kurochko Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On 03.07.2024 12:42, Oleksii Kurochko wrote: > Except mapping of FDT, it is also printing command line passed by > a DTB and initialize bootinfo from a DTB. I'm glad the description isn't empty here. However, ... > --- a/xen/arch/riscv/riscv64/head.S > +++ b/xen/arch/riscv/riscv64/head.S > @@ -41,6 +41,9 @@ FUNC(start) > > jal setup_initial_pagetables > > + mv a0, s1 > + jal fdt_map > + > /* Calculate proper VA after jump from 1:1 mapping */ > la a0, .L_primary_switched > sub a0, a0, s2 ... it could do with clarifying why this needs calling from assembly code. Mapping the FDT clearly looks like something that wants doing from start_xen(), i.e. from C code. > @@ -33,15 +35,34 @@ static void test_macros_from_bug_h(void) > printk("WARN is most likely working\n"); > } > > +void __init fdt_map(paddr_t dtb_addr) > +{ > + device_tree_flattened = early_fdt_map(dtb_addr); > + if ( !device_tree_flattened ) > + { > + printk("wrong FDT\n"); > + die(); > + } > +} > + > void __init noreturn start_xen(unsigned long bootcpu_id, > paddr_t dtb_addr) > { > + size_t fdt_size; > + const char *cmdline; > + > remove_identity_mapping(); > > trap_init(); > > test_macros_from_bug_h(); > > + fdt_size = boot_fdt_info(device_tree_flattened, dtb_addr); You don't use the return value anywhere below. What use is the local var then? Jan > + cmdline = boot_fdt_cmdline(device_tree_flattened); > + printk("Command line: %s\n", cmdline); > + cmdline_parse(cmdline); > + > printk("All set up\n"); > > for ( ;; ) ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-10 12:38 ` Jan Beulich @ 2024-07-11 9:40 ` Oleksii 2024-07-11 9:54 ` Jan Beulich 2024-07-11 10:50 ` Julien Grall 0 siblings, 2 replies; 28+ messages in thread From: Oleksii @ 2024-07-11 9:40 UTC (permalink / raw) To: Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > Except mapping of FDT, it is also printing command line passed by > > a DTB and initialize bootinfo from a DTB. > > I'm glad the description isn't empty here. However, ... > > > --- a/xen/arch/riscv/riscv64/head.S > > +++ b/xen/arch/riscv/riscv64/head.S > > @@ -41,6 +41,9 @@ FUNC(start) > > > > jal setup_initial_pagetables > > > > + mv a0, s1 > > + jal fdt_map > > + > > /* Calculate proper VA after jump from 1:1 mapping */ > > la a0, .L_primary_switched > > sub a0, a0, s2 > > ... it could do with clarifying why this needs calling from assembly > code. Mapping the FDT clearly looks like something that wants doing > from start_xen(), i.e. from C code. fdt_map() expected to work while MMU is off as it is using setup_initial_mapping() which is working with physical address. > > > @@ -33,15 +35,34 @@ static void test_macros_from_bug_h(void) > > printk("WARN is most likely working\n"); > > } > > > > +void __init fdt_map(paddr_t dtb_addr) > > +{ > > + device_tree_flattened = early_fdt_map(dtb_addr); > > + if ( !device_tree_flattened ) > > + { > > + printk("wrong FDT\n"); > > + die(); > > + } > > +} > > + > > void __init noreturn start_xen(unsigned long bootcpu_id, > > paddr_t dtb_addr) > > { > > + size_t fdt_size; > > + const char *cmdline; > > + > > remove_identity_mapping(); > > > > trap_init(); > > > > test_macros_from_bug_h(); > > > > + fdt_size = boot_fdt_info(device_tree_flattened, dtb_addr); > > You don't use the return value anywhere below. What use is the local > var > then? I returned just for debug ( to see what is the fdt size ), it can be dropped now. ~ Oleksii > > Jan > > > + cmdline = boot_fdt_cmdline(device_tree_flattened); > > + printk("Command line: %s\n", cmdline); > > + cmdline_parse(cmdline); > > + > > printk("All set up\n"); > > > > for ( ;; ) > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 9:40 ` Oleksii @ 2024-07-11 9:54 ` Jan Beulich 2024-07-11 10:26 ` Oleksii 2024-07-11 10:50 ` Julien Grall 1 sibling, 1 reply; 28+ messages in thread From: Jan Beulich @ 2024-07-11 9:54 UTC (permalink / raw) To: Oleksii Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On 11.07.2024 11:40, Oleksii wrote: > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: >> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>> Except mapping of FDT, it is also printing command line passed by >>> a DTB and initialize bootinfo from a DTB. >> >> I'm glad the description isn't empty here. However, ... >> >>> --- a/xen/arch/riscv/riscv64/head.S >>> +++ b/xen/arch/riscv/riscv64/head.S >>> @@ -41,6 +41,9 @@ FUNC(start) >>> >>> jal setup_initial_pagetables >>> >>> + mv a0, s1 >>> + jal fdt_map >>> + >>> /* Calculate proper VA after jump from 1:1 mapping */ >>> la a0, .L_primary_switched >>> sub a0, a0, s2 >> >> ... it could do with clarifying why this needs calling from assembly >> code. Mapping the FDT clearly looks like something that wants doing >> from start_xen(), i.e. from C code. > fdt_map() expected to work while MMU is off as it is using > setup_initial_mapping() which is working with physical address. Hmm, interesting. When the MMU is off, what does "map" mean? Yet then it feels I'm misunderstanding what you're meaning to tell me ... Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 9:54 ` Jan Beulich @ 2024-07-11 10:26 ` Oleksii 2024-07-11 10:50 ` Jan Beulich 0 siblings, 1 reply; 28+ messages in thread From: Oleksii @ 2024-07-11 10:26 UTC (permalink / raw) To: Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: > On 11.07.2024 11:40, Oleksii wrote: > > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: > > > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > > > Except mapping of FDT, it is also printing command line passed > > > > by > > > > a DTB and initialize bootinfo from a DTB. > > > > > > I'm glad the description isn't empty here. However, ... > > > > > > > --- a/xen/arch/riscv/riscv64/head.S > > > > +++ b/xen/arch/riscv/riscv64/head.S > > > > @@ -41,6 +41,9 @@ FUNC(start) > > > > > > > > jal setup_initial_pagetables > > > > > > > > + mv a0, s1 > > > > + jal fdt_map > > > > + > > > > /* Calculate proper VA after jump from 1:1 mapping */ > > > > la a0, .L_primary_switched > > > > sub a0, a0, s2 > > > > > > ... it could do with clarifying why this needs calling from > > > assembly > > > code. Mapping the FDT clearly looks like something that wants > > > doing > > > from start_xen(), i.e. from C code. > > fdt_map() expected to work while MMU is off as it is using > > setup_initial_mapping() which is working with physical address. > > Hmm, interesting. When the MMU is off, what does "map" mean? Yet then > it feels I'm misunderstanding what you're meaning to tell me ... Let's look at examples of the code: 1. The first thing issue will be here: void* __init early_fdt_map(paddr_t fdt_paddr) { unsigned long dt_phys_base = fdt_paddr; unsigned long dt_virt_base; unsigned long dt_virt_size; BUILD_BUG_ON(MIN_FDT_ALIGN < 8); if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN || fdt_paddr % SZ_2M || fdt_totalsize(fdt_paddr) > BOOT_FDT_VIRT_SIZE ) MMU doesn't now about virtual address of fdt_paddr as fdt_paddr wasn't mapped. 2. In setup_initial_mapping() we have HANDLE_PGTBL() where pgtbl is a pointer to physical address ( which also should be mapped in MMU if we want to access it after MMU is enabled ): #define HANDLE_PGTBL(curr_lvl_num) \ index = pt_index(curr_lvl_num, page_addr); \ if ( pte_is_valid(pgtbl[index]) ) \ { \ /* Find L{ 0-3 } table */ \ pgtbl = (pte_t *)pte_to_paddr(pgtbl[index]); \ } \ else \ { \ /* Allocate new L{0-3} page table */ \ if ( mmu_desc->pgtbl_count == PGTBL_INITIAL_COUNT ) \ { \ early_printk("(XEN) No initial table available\n"); \ /* panic(), BUG() or ASSERT() aren't ready now. */ \ die(); \ } \ mmu_desc->pgtbl_count++; \ pgtbl[index] = paddr_to_pte((unsigned long)mmu_desc- >next_pgtbl, \ PTE_VALID); \ pgtbl = mmu_desc->next_pgtbl; \ mmu_desc->next_pgtbl += PAGETABLE_ENTRIES; \ } So we can't use setup_initial_mapping() when MMU is enabled as there is a part of the code which uses physical address which are not mapped. We have only mapping for for liner_start <-> load_start and the small piece of code in text section ( _ident_start ): setup_initial_mapping(&mmu_desc, linker_start, linker_end, load_start); if ( linker_start == load_start ) return; ident_start = (unsigned long)turn_on_mmu &XEN_PT_LEVEL_MAP_MASK(0); ident_end = ident_start + PAGE_SIZE; setup_initial_mapping(&mmu_desc, ident_start, ident_end, ident_start); We can use setup_initial_mapping() when MMU is enabled only in case when linker_start is equal to load_start. As an option we can consider for as a candidate for identaty mapping also section .bss.page_aligned where root and nonroot page tables are located. Does it make sense now? ~ Oleksii ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 10:26 ` Oleksii @ 2024-07-11 10:50 ` Jan Beulich 2024-07-11 11:28 ` Oleksii 0 siblings, 1 reply; 28+ messages in thread From: Jan Beulich @ 2024-07-11 10:50 UTC (permalink / raw) To: Oleksii Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel On 11.07.2024 12:26, Oleksii wrote: > On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: >> On 11.07.2024 11:40, Oleksii wrote: >>> On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: >>>> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>>>> Except mapping of FDT, it is also printing command line passed >>>>> by >>>>> a DTB and initialize bootinfo from a DTB. >>>> >>>> I'm glad the description isn't empty here. However, ... >>>> >>>>> --- a/xen/arch/riscv/riscv64/head.S >>>>> +++ b/xen/arch/riscv/riscv64/head.S >>>>> @@ -41,6 +41,9 @@ FUNC(start) >>>>> >>>>> jal setup_initial_pagetables >>>>> >>>>> + mv a0, s1 >>>>> + jal fdt_map >>>>> + >>>>> /* Calculate proper VA after jump from 1:1 mapping */ >>>>> la a0, .L_primary_switched >>>>> sub a0, a0, s2 >>>> >>>> ... it could do with clarifying why this needs calling from >>>> assembly >>>> code. Mapping the FDT clearly looks like something that wants >>>> doing >>>> from start_xen(), i.e. from C code. >>> fdt_map() expected to work while MMU is off as it is using >>> setup_initial_mapping() which is working with physical address. >> >> Hmm, interesting. When the MMU is off, what does "map" mean? Yet then >> it feels I'm misunderstanding what you're meaning to tell me ... > Let's look at examples of the code: > 1. The first thing issue will be here: > void* __init early_fdt_map(paddr_t fdt_paddr) > { > unsigned long dt_phys_base = fdt_paddr; > unsigned long dt_virt_base; > unsigned long dt_virt_size; > > BUILD_BUG_ON(MIN_FDT_ALIGN < 8); > if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN || fdt_paddr % SZ_2M > || > fdt_totalsize(fdt_paddr) > BOOT_FDT_VIRT_SIZE ) > MMU doesn't now about virtual address of fdt_paddr as fdt_paddr wasn't > mapped. > > 2. In setup_initial_mapping() we have HANDLE_PGTBL() where pgtbl is a > pointer to physical address ( which also should be mapped in MMU if we > want to access it after MMU is enabled ): > #define HANDLE_PGTBL(curr_lvl_num) > \ > index = pt_index(curr_lvl_num, page_addr); > \ > if ( pte_is_valid(pgtbl[index]) ) > \ > { > \ > /* Find L{ 0-3 } table */ > \ > pgtbl = (pte_t *)pte_to_paddr(pgtbl[index]); > \ > } > \ > else > \ > { > \ > /* Allocate new L{0-3} page table */ > \ > if ( mmu_desc->pgtbl_count == PGTBL_INITIAL_COUNT ) > \ > { > \ > early_printk("(XEN) No initial table available\n"); > \ > /* panic(), BUG() or ASSERT() aren't ready now. */ > \ > die(); > \ > } > \ > mmu_desc->pgtbl_count++; > \ > pgtbl[index] = paddr_to_pte((unsigned long)mmu_desc- > >next_pgtbl, \ > PTE_VALID); > \ > pgtbl = mmu_desc->next_pgtbl; > \ > mmu_desc->next_pgtbl += PAGETABLE_ENTRIES; > \ > } > > So we can't use setup_initial_mapping() when MMU is enabled as there is > a part of the code which uses physical address which are not mapped. > > We have only mapping for for liner_start <-> load_start and the small > piece of code in text section ( _ident_start ): > setup_initial_mapping(&mmu_desc, > linker_start, > linker_end, > load_start); > > if ( linker_start == load_start ) > return; > > ident_start = (unsigned long)turn_on_mmu &XEN_PT_LEVEL_MAP_MASK(0); > ident_end = ident_start + PAGE_SIZE; > > setup_initial_mapping(&mmu_desc, > ident_start, > ident_end, > ident_start); > > We can use setup_initial_mapping() when MMU is enabled only in case > when linker_start is equal to load_start. > > As an option we can consider for as a candidate for identaty mapping > also section .bss.page_aligned where root and nonroot page tables are > located. > > Does it make sense now? I think so, yet at the same time it only changes the question: Why is it that you absolutely need to use setup_initial_mapping()? Surely down the road there are going to be more thing that need mapping relatively early, but after the MMU was enabled. Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 10:50 ` Jan Beulich @ 2024-07-11 11:28 ` Oleksii 2024-07-11 11:54 ` Julien Grall 2024-07-11 14:37 ` Jan Beulich 0 siblings, 2 replies; 28+ messages in thread From: Oleksii @ 2024-07-11 11:28 UTC (permalink / raw) To: Jan Beulich, Julien Grall Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel Add Julien as he asked basically the same question in another thread. On Thu, 2024-07-11 at 12:50 +0200, Jan Beulich wrote: > On 11.07.2024 12:26, Oleksii wrote: > > On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: > > > On 11.07.2024 11:40, Oleksii wrote: > > > > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: > > > > > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > > > > > Except mapping of FDT, it is also printing command line > > > > > > passed > > > > > > by > > > > > > a DTB and initialize bootinfo from a DTB. > > > > > > > > > > I'm glad the description isn't empty here. However, ... > > > > > > > > > > > --- a/xen/arch/riscv/riscv64/head.S > > > > > > +++ b/xen/arch/riscv/riscv64/head.S > > > > > > @@ -41,6 +41,9 @@ FUNC(start) > > > > > > > > > > > > jal setup_initial_pagetables > > > > > > > > > > > > + mv a0, s1 > > > > > > + jal fdt_map > > > > > > + > > > > > > /* Calculate proper VA after jump from 1:1 mapping > > > > > > */ > > > > > > la a0, .L_primary_switched > > > > > > sub a0, a0, s2 > > > > > > > > > > ... it could do with clarifying why this needs calling from > > > > > assembly > > > > > code. Mapping the FDT clearly looks like something that wants > > > > > doing > > > > > from start_xen(), i.e. from C code. > > > > fdt_map() expected to work while MMU is off as it is using > > > > setup_initial_mapping() which is working with physical address. > > > > > > Hmm, interesting. When the MMU is off, what does "map" mean? Yet > > > then > > > it feels I'm misunderstanding what you're meaning to tell me ... > > Let's look at examples of the code: > > 1. The first thing issue will be here: > > void* __init early_fdt_map(paddr_t fdt_paddr) > > { > > unsigned long dt_phys_base = fdt_paddr; > > unsigned long dt_virt_base; > > unsigned long dt_virt_size; > > > > BUILD_BUG_ON(MIN_FDT_ALIGN < 8); > > if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN || fdt_paddr % > > SZ_2M > > || > > fdt_totalsize(fdt_paddr) > BOOT_FDT_VIRT_SIZE ) > > MMU doesn't now about virtual address of fdt_paddr as fdt_paddr > > wasn't > > mapped. > > > > 2. In setup_initial_mapping() we have HANDLE_PGTBL() where pgtbl is > > a > > pointer to physical address ( which also should be mapped in MMU > > if we > > want to access it after MMU is enabled ): > > #define > > HANDLE_PGTBL(curr_lvl_num) > > \ > > index = pt_index(curr_lvl_num, > > page_addr); > > \ > > if ( pte_is_valid(pgtbl[index]) > > ) > > \ > > > > { > > \ > > /* Find L{ 0-3 } table > > */ > > \ > > pgtbl = (pte_t > > *)pte_to_paddr(pgtbl[index]); > > \ > > > > } > > \ > > > > else > > \ > > > > { > > \ > > /* Allocate new L{0-3} page table > > */ > > \ > > if ( mmu_desc->pgtbl_count == PGTBL_INITIAL_COUNT > > ) > > \ > > > > { > > \ > > early_printk("(XEN) No initial table > > available\n"); > > \ > > /* panic(), BUG() or ASSERT() aren't ready now. > > */ > > \ > > > > die(); > > \ > > > > } > > \ > > mmu_desc- > > >pgtbl_count++; > > \ > > pgtbl[index] = paddr_to_pte((unsigned long)mmu_desc- > > >next_pgtbl, \ > > > > PTE_VALID); > > \ > > pgtbl = mmu_desc- > > >next_pgtbl; > > \ > > mmu_desc->next_pgtbl += > > PAGETABLE_ENTRIES; > > \ > > } > > > > So we can't use setup_initial_mapping() when MMU is enabled as > > there is > > a part of the code which uses physical address which are not > > mapped. > > > > We have only mapping for for liner_start <-> load_start and the > > small > > piece of code in text section ( _ident_start ): > > setup_initial_mapping(&mmu_desc, > > linker_start, > > linker_end, > > load_start); > > > > if ( linker_start == load_start ) > > return; > > > > ident_start = (unsigned long)turn_on_mmu > > &XEN_PT_LEVEL_MAP_MASK(0); > > ident_end = ident_start + PAGE_SIZE; > > > > setup_initial_mapping(&mmu_desc, > > ident_start, > > ident_end, > > ident_start); > > > > We can use setup_initial_mapping() when MMU is enabled only in case > > when linker_start is equal to load_start. > > > > As an option we can consider for as a candidate for identaty > > mapping > > also section .bss.page_aligned where root and nonroot page tables > > are > > located. > > > > Does it make sense now? > > I think so, yet at the same time it only changes the question: Why is > it > that you absolutely need to use setup_initial_mapping()? There is no strict requirement to use setup_initial_mapping(). That function is available to me at the moment, and I haven't found a better option other than reusing what I currently have. If not to use setup_initial_mapping() then it is needed to introduce xen_{un}map_table(), create_xen_table(), xen_pt_next_level(), xen_pt_update{_entry}(), map_pages_to_xen(). What I did a little bit later here: https://gitlab.com/xen-project/people/olkur/xen/-/commit/a4619d0902e0a012ac2f709d31621a8d499bca97 Am I confusing something? Could you please recommend me how to better? > Surely down the > road there are going to be more thing that need mapping relatively > early, > but after the MMU was enabled. For sure, but for mapping other things it would be introduced setup_mm() and necessary functions to implementinitialization and handling of mm. ~ Oleksii ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 11:28 ` Oleksii @ 2024-07-11 11:54 ` Julien Grall 2024-07-11 12:29 ` Oleksii 2024-07-11 14:37 ` Jan Beulich 1 sibling, 1 reply; 28+ messages in thread From: Julien Grall @ 2024-07-11 11:54 UTC (permalink / raw) To: Oleksii, Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Stefano Stabellini, xen-devel Hi, On 11/07/2024 12:28, Oleksii wrote: > Add Julien as he asked basically the same question in another thread. Thanks! > > On Thu, 2024-07-11 at 12:50 +0200, Jan Beulich wrote: >> On 11.07.2024 12:26, Oleksii wrote: >>> On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: >>>> On 11.07.2024 11:40, Oleksii wrote: >>>>> On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: >>>>>> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>> Does it make sense now? >> >> I think so, yet at the same time it only changes the question: Why is >> it >> that you absolutely need to use setup_initial_mapping()? > There is no strict requirement to use setup_initial_mapping(). That > function is available to me at the moment, and I haven't found a better > option other than reusing what I currently have. I am not very familiar with the code base for RISC-V, but looking at the context in the patch, it seems you will still have the identity mapping mapped until start_xen(). I assume we don't exactly know where the loader will put Xen in memory. Which means that the region may clash with your defined runtime regions (such as the FDT). Did I misunderstand anything? That's one of the reason on Arm we are trying to enable the MMU very early. The only thing we setup is a mapping for Xen (and earlyprintk) all the rest will be mapped once the MMU is on. With that, the only thing you need to take care off the runtime Xen address overlapping with the identity mapping. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 11:54 ` Julien Grall @ 2024-07-11 12:29 ` Oleksii 2024-07-11 12:44 ` Julien Grall 0 siblings, 1 reply; 28+ messages in thread From: Oleksii @ 2024-07-11 12:29 UTC (permalink / raw) To: Julien Grall, Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Stefano Stabellini, xen-devel On Thu, 2024-07-11 at 12:54 +0100, Julien Grall wrote: > Hi, > > On 11/07/2024 12:28, Oleksii wrote: > > Add Julien as he asked basically the same question in another > > thread. > > Thanks! > > > > > On Thu, 2024-07-11 at 12:50 +0200, Jan Beulich wrote: > > > On 11.07.2024 12:26, Oleksii wrote: > > > > On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: > > > > > On 11.07.2024 11:40, Oleksii wrote: > > > > > > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: > > > > > > > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > > > Does it make sense now? > > > > > > I think so, yet at the same time it only changes the question: > > > Why is > > > it > > > that you absolutely need to use setup_initial_mapping()? > > There is no strict requirement to use setup_initial_mapping(). That > > function is available to me at the moment, and I haven't found a > > better > > option other than reusing what I currently have. > > I am not very familiar with the code base for RISC-V, but looking at > the > context in the patch, it seems you will still have the identity > mapping > mapped until start_xen(). We have identity mapping only for a small piece of .text section: . = ALIGN(IDENT_AREA_SIZE); _ident_start = .; *(.text.ident) _ident_end = .; All other will be identically mapped only in case of linker address is equal to load address. > > I assume we don't exactly know where the loader will put Xen in > memory. > Which means that the region may clash with your defined runtime > regions > (such as the FDT). Did I misunderstand anything? I am not really get what is the issue here. If we are speaking about physical regions then loader will guarantee that Xen and FDT regions don't overlap. If we are speaking about virtual regions then Xen will check that nothing is overlaped. And the virtual regions are mapped so high so I am not sure that loader will put something there. ( FDT in Xen is mapped to 0xffffffffc0200000 ). Could you please clarify what I am missing? > > That's one of the reason on Arm we are trying to enable the MMU very > early. The only thing we setup is a mapping for Xen (and earlyprintk) > all the rest will be mapped once the MMU is on. It makes sense. Then I have to introduce map_pages_to_xen() first and then early_fdt_map(). ~ Oleksii > > With that, the only thing you need to take care off the runtime Xen > address overlapping with the identity mapping. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 12:29 ` Oleksii @ 2024-07-11 12:44 ` Julien Grall 2024-07-11 13:14 ` Oleksii 0 siblings, 1 reply; 28+ messages in thread From: Julien Grall @ 2024-07-11 12:44 UTC (permalink / raw) To: Oleksii, Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Stefano Stabellini, xen-devel Hi Oleksii, On 11/07/2024 13:29, Oleksii wrote: > On Thu, 2024-07-11 at 12:54 +0100, Julien Grall wrote: >> Hi, >> >> On 11/07/2024 12:28, Oleksii wrote: >>> Add Julien as he asked basically the same question in another >>> thread. >> >> Thanks! >> >>> >>> On Thu, 2024-07-11 at 12:50 +0200, Jan Beulich wrote: >>>> On 11.07.2024 12:26, Oleksii wrote: >>>>> On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: >>>>>> On 11.07.2024 11:40, Oleksii wrote: >>>>>>> On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: >>>>>>>> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>>>> Does it make sense now? >>>> >>>> I think so, yet at the same time it only changes the question: >>>> Why is >>>> it >>>> that you absolutely need to use setup_initial_mapping()? >>> There is no strict requirement to use setup_initial_mapping(). That >>> function is available to me at the moment, and I haven't found a >>> better >>> option other than reusing what I currently have. >> >> I am not very familiar with the code base for RISC-V, but looking at >> the >> context in the patch, it seems you will still have the identity >> mapping >> mapped until start_xen(). > We have identity mapping only for a small piece of .text section: > . = ALIGN(IDENT_AREA_SIZE); > _ident_start = .; > *(.text.ident) > _ident_end = .; > > All other will be identically mapped only in case of linker address is > equal to load address. > >> >> I assume we don't exactly know where the loader will put Xen in >> memory. >> Which means that the region may clash with your defined runtime >> regions >> (such as the FDT). Did I misunderstand anything? > I am not really get what is the issue here. > > If we are speaking about physical regions then loader will guarantee > that Xen and FDT regions don't overlap. Sure. But I was referring to... > > If we are speaking about virtual regions then Xen will check that > nothing is overlaped. ... this part. The more regions you mapped with MMU off, the more work you have to do to ensure nothing will clash. > And the virtual regions are mapped so high so I > am not sure that loader will put something there. ( FDT in Xen is > mapped to 0xffffffffc0200000 ) Never say never :). On Arm, some 64-bit HW (such as ADLink AVA platform) has the RAM starting very high and load Xen around 8TB. For Arm, we still decided to put a limit (10TB) where Xen can be loaded but this is mainly done for convenience (otherwise it is a bit more complicated to get off the identity mapping). We still have a check in place to ensure that Xen is not loaded above 10TB. If you map the FDT within the same L1. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 12:44 ` Julien Grall @ 2024-07-11 13:14 ` Oleksii 0 siblings, 0 replies; 28+ messages in thread From: Oleksii @ 2024-07-11 13:14 UTC (permalink / raw) To: Julien Grall, Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Stefano Stabellini, xen-devel On Thu, 2024-07-11 at 13:44 +0100, Julien Grall wrote: > Hi Oleksii, Hi Julien, > > On 11/07/2024 13:29, Oleksii wrote: > > On Thu, 2024-07-11 at 12:54 +0100, Julien Grall wrote: > > > Hi, > > > > > > On 11/07/2024 12:28, Oleksii wrote: > > > > Add Julien as he asked basically the same question in another > > > > thread. > > > > > > Thanks! > > > > > > > > > > > On Thu, 2024-07-11 at 12:50 +0200, Jan Beulich wrote: > > > > > On 11.07.2024 12:26, Oleksii wrote: > > > > > > On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: > > > > > > > On 11.07.2024 11:40, Oleksii wrote: > > > > > > > > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: > > > > > > > > > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > > > > > Does it make sense now? > > > > > > > > > > I think so, yet at the same time it only changes the > > > > > question: > > > > > Why is > > > > > it > > > > > that you absolutely need to use setup_initial_mapping()? > > > > There is no strict requirement to use setup_initial_mapping(). > > > > That > > > > function is available to me at the moment, and I haven't found > > > > a > > > > better > > > > option other than reusing what I currently have. > > > > > > I am not very familiar with the code base for RISC-V, but looking > > > at > > > the > > > context in the patch, it seems you will still have the identity > > > mapping > > > mapped until start_xen(). > > We have identity mapping only for a small piece of .text section: > > . = ALIGN(IDENT_AREA_SIZE); > > _ident_start = .; > > *(.text.ident) > > _ident_end = .; > > > > All other will be identically mapped only in case of linker address > > is > > equal to load address. > > > > > > > > I assume we don't exactly know where the loader will put Xen in > > > memory. > > > Which means that the region may clash with your defined runtime > > > regions > > > (such as the FDT). Did I misunderstand anything? > > I am not really get what is the issue here. > > > > If we are speaking about physical regions then loader will > > guarantee > > that Xen and FDT regions don't overlap. > > Sure. But I was referring to... > > > > > If we are speaking about virtual regions then Xen will check that > > nothing is overlaped. > > ... this part. The more regions you mapped with MMU off, the more > work > you have to do to ensure nothing will clash. Yes, agree here. Then I have to look at what I need now to introduce map_pages_to_xen(). Thanks for clarifying. > > > And the virtual regions are mapped so high so I > > am not sure that loader will put something there. ( FDT in Xen is > > mapped to 0xffffffffc0200000 ) > Never say never :). On Arm, some 64-bit HW (such as ADLink AVA > platform) > has the RAM starting very high and load Xen around 8TB. For Arm, we > still decided to put a limit (10TB) where Xen can be loaded but this > is > mainly done for convenience (otherwise it is a bit more complicated > to > get off the identity mapping). Oh, it is very high. I couldn't even expect. ~ Oleksii > > We still have a check in place to ensure that Xen is not loaded above > 10TB. If you map the FDT within the same L1. > > Cheers, > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 11:28 ` Oleksii 2024-07-11 11:54 ` Julien Grall @ 2024-07-11 14:37 ` Jan Beulich 1 sibling, 0 replies; 28+ messages in thread From: Jan Beulich @ 2024-07-11 14:37 UTC (permalink / raw) To: Oleksii Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Stefano Stabellini, xen-devel, Julien Grall On 11.07.2024 13:28, Oleksii wrote: > On Thu, 2024-07-11 at 12:50 +0200, Jan Beulich wrote: >> On 11.07.2024 12:26, Oleksii wrote: >>> On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote: >>>> On 11.07.2024 11:40, Oleksii wrote: >>>>> On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: >>>>>> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>>>>>> Except mapping of FDT, it is also printing command line >>>>>>> passed >>>>>>> by >>>>>>> a DTB and initialize bootinfo from a DTB. >>>>>> >>>>>> I'm glad the description isn't empty here. However, ... >>>>>> >>>>>>> --- a/xen/arch/riscv/riscv64/head.S >>>>>>> +++ b/xen/arch/riscv/riscv64/head.S >>>>>>> @@ -41,6 +41,9 @@ FUNC(start) >>>>>>> >>>>>>> jal setup_initial_pagetables >>>>>>> >>>>>>> + mv a0, s1 >>>>>>> + jal fdt_map >>>>>>> + >>>>>>> /* Calculate proper VA after jump from 1:1 mapping >>>>>>> */ >>>>>>> la a0, .L_primary_switched >>>>>>> sub a0, a0, s2 >>>>>> >>>>>> ... it could do with clarifying why this needs calling from >>>>>> assembly >>>>>> code. Mapping the FDT clearly looks like something that wants >>>>>> doing >>>>>> from start_xen(), i.e. from C code. >>>>> fdt_map() expected to work while MMU is off as it is using >>>>> setup_initial_mapping() which is working with physical address. >>>> >>>> Hmm, interesting. When the MMU is off, what does "map" mean? Yet >>>> then >>>> it feels I'm misunderstanding what you're meaning to tell me ... >>> Let's look at examples of the code: >>> 1. The first thing issue will be here: >>> void* __init early_fdt_map(paddr_t fdt_paddr) >>> { >>> unsigned long dt_phys_base = fdt_paddr; >>> unsigned long dt_virt_base; >>> unsigned long dt_virt_size; >>> >>> BUILD_BUG_ON(MIN_FDT_ALIGN < 8); >>> if ( !fdt_paddr || fdt_paddr % MIN_FDT_ALIGN || fdt_paddr % >>> SZ_2M >>> || >>> fdt_totalsize(fdt_paddr) > BOOT_FDT_VIRT_SIZE ) >>> MMU doesn't now about virtual address of fdt_paddr as fdt_paddr >>> wasn't >>> mapped. >>> >>> 2. In setup_initial_mapping() we have HANDLE_PGTBL() where pgtbl is >>> a >>> pointer to physical address ( which also should be mapped in MMU >>> if we >>> want to access it after MMU is enabled ): >>> #define >>> HANDLE_PGTBL(curr_lvl_num) >>> \ >>> index = pt_index(curr_lvl_num, >>> page_addr); >>> \ >>> if ( pte_is_valid(pgtbl[index]) >>> ) >>> \ >>> >>> { >>> \ >>> /* Find L{ 0-3 } table >>> */ >>> \ >>> pgtbl = (pte_t >>> *)pte_to_paddr(pgtbl[index]); >>> \ >>> >>> } >>> \ >>> >>> else >>> \ >>> >>> { >>> \ >>> /* Allocate new L{0-3} page table >>> */ >>> \ >>> if ( mmu_desc->pgtbl_count == PGTBL_INITIAL_COUNT >>> ) >>> \ >>> >>> { >>> \ >>> early_printk("(XEN) No initial table >>> available\n"); >>> \ >>> /* panic(), BUG() or ASSERT() aren't ready now. >>> */ >>> \ >>> >>> die(); >>> \ >>> >>> } >>> \ >>> mmu_desc- >>>> pgtbl_count++; >>> \ >>> pgtbl[index] = paddr_to_pte((unsigned long)mmu_desc- >>> >next_pgtbl, \ >>> >>> PTE_VALID); >>> \ >>> pgtbl = mmu_desc- >>>> next_pgtbl; >>> \ >>> mmu_desc->next_pgtbl += >>> PAGETABLE_ENTRIES; >>> \ >>> } >>> >>> So we can't use setup_initial_mapping() when MMU is enabled as >>> there is >>> a part of the code which uses physical address which are not >>> mapped. >>> >>> We have only mapping for for liner_start <-> load_start and the >>> small >>> piece of code in text section ( _ident_start ): >>> setup_initial_mapping(&mmu_desc, >>> linker_start, >>> linker_end, >>> load_start); >>> >>> if ( linker_start == load_start ) >>> return; >>> >>> ident_start = (unsigned long)turn_on_mmu >>> &XEN_PT_LEVEL_MAP_MASK(0); >>> ident_end = ident_start + PAGE_SIZE; >>> >>> setup_initial_mapping(&mmu_desc, >>> ident_start, >>> ident_end, >>> ident_start); >>> >>> We can use setup_initial_mapping() when MMU is enabled only in case >>> when linker_start is equal to load_start. >>> >>> As an option we can consider for as a candidate for identaty >>> mapping >>> also section .bss.page_aligned where root and nonroot page tables >>> are >>> located. >>> >>> Does it make sense now? >> >> I think so, yet at the same time it only changes the question: Why is >> it >> that you absolutely need to use setup_initial_mapping()? > There is no strict requirement to use setup_initial_mapping(). That > function is available to me at the moment, and I haven't found a better > option other than reusing what I currently have. > > If not to use setup_initial_mapping() then it is needed to introduce > xen_{un}map_table(), create_xen_table(), xen_pt_next_level(), > xen_pt_update{_entry}(), map_pages_to_xen(). What I did a little bit > later here: > https://gitlab.com/xen-project/people/olkur/xen/-/commit/a4619d0902e0a012ac2f709d31621a8d499bca97 > Am I confusing something? > > Could you please recommend me how to better? I think you've sorted this for yourself already, judging from subsequent communication on this thread, where you indicate you'll introduce map_pages_to_xen() first. That's the way I'd have expected you to go. Jan ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 9:40 ` Oleksii 2024-07-11 9:54 ` Jan Beulich @ 2024-07-11 10:50 ` Julien Grall 2024-07-11 11:31 ` Oleksii 1 sibling, 1 reply; 28+ messages in thread From: Julien Grall @ 2024-07-11 10:50 UTC (permalink / raw) To: Oleksii, Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Stefano Stabellini, xen-devel Hi, On 11/07/2024 10:40, Oleksii wrote: > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: >> On 03.07.2024 12:42, Oleksii Kurochko wrote: >>> Except mapping of FDT, it is also printing command line passed by >>> a DTB and initialize bootinfo from a DTB. >> >> I'm glad the description isn't empty here. However, ... >> >>> --- a/xen/arch/riscv/riscv64/head.S >>> +++ b/xen/arch/riscv/riscv64/head.S >>> @@ -41,6 +41,9 @@ FUNC(start) >>> >>> jal setup_initial_pagetables >>> >>> + mv a0, s1 >>> + jal fdt_map >>> + >>> /* Calculate proper VA after jump from 1:1 mapping */ >>> la a0, .L_primary_switched >>> sub a0, a0, s2 >> >> ... it could do with clarifying why this needs calling from assembly >> code. Mapping the FDT clearly looks like something that wants doing >> from start_xen(), i.e. from C code. > fdt_map() expected to work while MMU is off as it is using > setup_initial_mapping() which is working with physical address. Somewhat related to what Jan is asking. You only seem to part the FDT in start_xen(). So why do you need to call fdt_map() that early? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 5/5] xen/riscv: map FDT 2024-07-11 10:50 ` Julien Grall @ 2024-07-11 11:31 ` Oleksii 0 siblings, 0 replies; 28+ messages in thread From: Oleksii @ 2024-07-11 11:31 UTC (permalink / raw) To: Julien Grall, Jan Beulich Cc: Alistair Francis, Bob Eshleman, Connor Davis, Andrew Cooper, Stefano Stabellini, xen-devel On Thu, 2024-07-11 at 11:50 +0100, Julien Grall wrote: > Hi, Hi Julien, > > On 11/07/2024 10:40, Oleksii wrote: > > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote: > > > On 03.07.2024 12:42, Oleksii Kurochko wrote: > > > > Except mapping of FDT, it is also printing command line passed > > > > by > > > > a DTB and initialize bootinfo from a DTB. > > > > > > I'm glad the description isn't empty here. However, ... > > > > > > > --- a/xen/arch/riscv/riscv64/head.S > > > > +++ b/xen/arch/riscv/riscv64/head.S > > > > @@ -41,6 +41,9 @@ FUNC(start) > > > > > > > > jal setup_initial_pagetables > > > > > > > > + mv a0, s1 > > > > + jal fdt_map > > > > + > > > > /* Calculate proper VA after jump from 1:1 mapping */ > > > > la a0, .L_primary_switched > > > > sub a0, a0, s2 > > > > > > ... it could do with clarifying why this needs calling from > > > assembly > > > code. Mapping the FDT clearly looks like something that wants > > > doing > > > from start_xen(), i.e. from C code. > > fdt_map() expected to work while MMU is off as it is using > > setup_initial_mapping() which is working with physical address. > > Somewhat related to what Jan is asking. You only seem to part the FDT > in > start_xen(). So why do you need to call fdt_map() that early? Let's continue our discussion in another thread ( I added you there ). But the answer on your question is here: https://lore.kernel.org/xen-devel/cover.1720002425.git.oleksii.kurochko@gmail.com/T/#m2890200a53c6ea2101e0f9e9ea77f589bd187e26 And here:https://lore.kernel.org/xen-devel/cover.1720002425.git.oleksii.kurochko@gmail.com/T/#m4e20792121b97465db7081cc4c1e27bdb15cdd51 Let me know if the link above answers your question. ~ Oleksii ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH for 4.20 v1 0/5] RISCV device tree mapping 2024-07-03 10:42 [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii Kurochko ` (4 preceding siblings ...) 2024-07-03 10:42 ` [PATCH v1 5/5] xen/riscv: map FDT Oleksii Kurochko @ 2024-07-03 10:52 ` Oleksii 5 siblings, 0 replies; 28+ messages in thread From: Oleksii @ 2024-07-03 10:52 UTC (permalink / raw) To: xen-devel Cc: Andrew Cooper, Jan Beulich, Julien Grall, Stefano Stabellini, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Daniel P. Smith, Alistair Francis, Bob Eshleman, Connor Davis Missed to write in the cover letter that this patch series is based on: [PATCH for 4.20 v9 0/5] RISCV basic exception handling implementation ~ Oleksii On Wed, 2024-07-03 at 12:42 +0200, Oleksii Kurochko wrote: > Current patch series introduces device tree mapping for RISC-V. > > Also, it introduces common stuff for working with fdt which is > based on the patches from [1]: > [PATCH v4 2/6] xen/device-tree: Move Arm's setup.c bootinfo > functions to common > [PATCH v4 3/6] xen/common: Move Arm's bootfdt.c > All changes which were done on top of Shawn's patches please find in > "Changes" section > of each patch. > > [1] > https://lore.kernel.org/xen-devel/cover.1712893887.git.sanastasio@raptorengineering.com/ > > Oleksii Kurochko (3): > xen/riscv: enable CONFIG_HAS_DEVICE_TREE > xen/riscv: introduce device tree maping function > xen/riscv: map FDT > > Shawn Anastasio (2): > xen/device-tree: Move Arm's setup.c bootinfo functions to common > xen/common: Move Arm's bootfdt.c to common > > MAINTAINERS | 2 + > xen/arch/arm/Makefile | 1 - > xen/arch/arm/bootfdt.c | 622 -------------------------- > - > xen/arch/arm/include/asm/setup.h | 200 +-------- > xen/arch/arm/setup.c | 432 ------------------- > xen/arch/riscv/Kconfig | 1 + > xen/arch/riscv/include/asm/config.h | 6 + > xen/arch/riscv/include/asm/mm.h | 2 + > xen/arch/riscv/mm.c | 37 +- > xen/arch/riscv/riscv64/head.S | 3 + > xen/arch/riscv/setup.c | 21 + > xen/common/Makefile | 1 + > xen/common/device-tree/Makefile | 2 + > xen/common/device-tree/bootfdt.c | 635 > ++++++++++++++++++++++++++++ > xen/common/device-tree/bootinfo.c | 459 ++++++++++++++++++++ > xen/include/xen/bootfdt.h | 210 +++++++++ > 16 files changed, 1375 insertions(+), 1259 deletions(-) > delete mode 100644 xen/arch/arm/bootfdt.c > create mode 100644 xen/common/device-tree/Makefile > create mode 100644 xen/common/device-tree/bootfdt.c > create mode 100644 xen/common/device-tree/bootinfo.c > create mode 100644 xen/include/xen/bootfdt.h > ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2024-07-11 14:37 UTC | newest] Thread overview: 28+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-03 10:42 [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii Kurochko 2024-07-03 10:42 ` [PATCH v1 1/5] xen/device-tree: Move Arm's setup.c bootinfo functions to common Oleksii Kurochko 2024-07-10 10:23 ` Jan Beulich 2024-07-11 9:00 ` Oleksii 2024-07-11 9:21 ` Jan Beulich 2024-07-11 9:41 ` Oleksii 2024-07-03 10:42 ` [PATCH v1 2/5] xen/common: Move Arm's bootfdt.c " Oleksii Kurochko 2024-07-03 10:42 ` [PATCH v1 3/5] xen/riscv: enable CONFIG_HAS_DEVICE_TREE Oleksii Kurochko 2024-07-10 10:25 ` Jan Beulich 2024-07-03 10:42 ` [PATCH v1 4/5] xen/riscv: introduce device tree maping function Oleksii Kurochko 2024-07-10 10:38 ` Jan Beulich 2024-07-11 9:31 ` Oleksii 2024-07-11 9:50 ` Jan Beulich 2024-07-03 10:42 ` [PATCH v1 5/5] xen/riscv: map FDT Oleksii Kurochko 2024-07-10 12:38 ` Jan Beulich 2024-07-11 9:40 ` Oleksii 2024-07-11 9:54 ` Jan Beulich 2024-07-11 10:26 ` Oleksii 2024-07-11 10:50 ` Jan Beulich 2024-07-11 11:28 ` Oleksii 2024-07-11 11:54 ` Julien Grall 2024-07-11 12:29 ` Oleksii 2024-07-11 12:44 ` Julien Grall 2024-07-11 13:14 ` Oleksii 2024-07-11 14:37 ` Jan Beulich 2024-07-11 10:50 ` Julien Grall 2024-07-11 11:31 ` Oleksii 2024-07-03 10:52 ` [PATCH for 4.20 v1 0/5] RISCV device tree mapping Oleksii
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.