Hi Michal,
On 07/12/2023 12:39, Michal Orzel wrote:
On 07/12/2023 13:20, Julien Grall wrote:
Hi Michal,
On 07/12/2023 10:14, Michal Orzel wrote:
As a result of not checking the return code of device_tree_for_each_node()
in boot_fdt_info(), any error occured during early FDT parsing does not
stop Xen from booting. This can result in an unwanted behavior in later
boot stages. Fix it by checking the return code and panicing on an error.
Fixes: 9cf4a9a46717 ("device tree: add device_tree_for_each_node()")
Signed-off-by: Michal Orzel <michal.or...@amd.com>
With one remark below:
Acked-by: Julien Grall <jgr...@amazon.com>
---
I've lost count how many times I had to fix missing rc check. However, I have
not yet found any checker for this (-Wunused-result is pretty useless).
---
xen/arch/arm/bootfdt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index b1f03eb2fcdd..f496a8cf9494 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -541,7 +541,9 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr)
add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
- device_tree_for_each_node((void *)fdt, 0, early_scan_node, NULL);
+ ret = device_tree_for_each_node((void *)fdt, 0, early_scan_node, NULL);
+ if ( ret )
+ panic("Early FDT parsing failed (%d)\n", ret);
AFAIU, the parsing is done before the console is setup. This means a
user would not be able to get some logs unless they are re-compiling Xen
with earlyprintk.
I understand this is not a new issue, but I am getting concerned of the
amount of check we add before the console is up and running.
What is the impact if we don't check the return here? Is it missing regions?
There are many things that can go wrong.
Quite recently, I faced an issue where I specified 2 dom0less domUs in
configuration
and due to the error while parsing the last node of domU1, domU2 node was
skipped and
Xen booted only domU1 without giving any errors.
Issues with shared memory led to either Xen continue to run with improper
configuration,
silencing overlap conditions, errors at later boot stages that were impossible
to deduct
from the logs.
All in all, early boot code parsing assume the error to result in a failure and
the parsing
for domain creation makes this assumption as well (checks are more relaxed to
avoid duplication).
For now, we can't do anything better than panicing early if we want to avoid
unwanted behavior.
I wonder whether we can either enable the console earlier, or make
earlyprintk more dynamic (similar to what Linux is doing with
earlyprintk=...).
The most imporatant part is early fdt parsing. The main console init cannot be
moved that early.
I think we need to understand a bit more why because on x86
consoel_init_preirq() is called very early. So we ought to be able to do
the same on Arm.
I will add this in my list after I get the earlyprintk working while
switching the MMU.
Cheers,
--
Julien Grall
