Hi Oleksii,
On 24/07/2024 16:31, Oleksii Kurochko wrote:
From: Shawn Anastasio <sanasta...@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 <jul...@xen.org>
Signed-off-by: Shawn Anastasio <sanasta...@raptorengineering.com>
Acked-by: Julien Grall <jul...@xen.org>
On Matrix you asked me to review this patch again. This wasn't obvious
given you kept my ack. If you think a review is needed, then please
either drop the ack or explain why you keep it and ask if it is fine.
Also, I tend to list in the changes where this was acked. In this case,
you said I acked v4.
Anyway, before confirming my ack, I would like to ask some clarification.
Signed-off-by: Oleksii Kurochko <oleksii.kuroc...@gmail.com>
---
Changes in V7:
- Nothing changed. Only rebase.
---
Changes in V6:
- update the version of the patch to v6.
---
Changes in V5:
- add xen/include/xen/bootfdt.h to MAINTAINERS file.
I don't see any change in MAINTAINERS within this patch. Did you happen
to copy/paste all the changes made in the series?
In fact the only change related to this patch doesn't seem to be listed.
[...]
+#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
I see you duplicated the stub from arch/arm/include/static-shmem.h. But
the one in static-shmem.h will now be unreachable. I think it needs to
be removed.
Also, I think this change deserve an explanation in the commit message.
[...]
+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
Similar remark here.
Cheers,
--
Julien Grall