[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] xen/arm: add warning if memory modules overlap
Hi,
On 10/11/19 5:43 PM, Brian Woods wrote:
On Thu, Oct 10, 2019 at 04:39:07PM +0100, Julien Grall wrote:
Hi Brian,
Thank you for the patch.
On 10/9/19 8:47 PM, Brian Woods wrote:
It's possible for a misconfigured device tree to cause Xen to crash when
there are overlapping addresses in the memory modules. Add a warning
when printing the addresses to let the user know there's a possible
issue when DEBUG is enabled.
Signed-off-by: Brian Woods <brian.woods@xxxxxxxxxx>
---
sample output:
...
(XEN) MODULE[0]: 0000000001400000 - 000000000153b8f1 Xen
(XEN) MODULE[1]: 00000000076d2000 - 00000000076dc080 Device Tree
(XEN) MODULE[2]: 00000000076df000 - 0000000007fff364 Ramdisk
(XEN) MODULE[3]: 0000000000080000 - 0000000003180000 Kernel
(XEN) RESVD[0]: 00000000076d2000 - 00000000076dc000
(XEN) RESVD[1]: 00000000076df000 - 0000000007fff364
(XEN)
(XEN) WARNING: modules Xen and Kernel overlap
(XEN)
(XEN) Command line: console=dtuart dtuart=serial0 dom0_mem=1G bootscrub=0
maxcpus=1 timer_slop=0
...
xen/arch/arm/bootfdt.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 08fb59f..3cb0c43 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -387,6 +387,23 @@ static void __init early_print_info(void)
mem_resv->bank[j].start + mem_resv->bank[j].size - 1);
}
printk("\n");
+
+#ifndef NDEBUG
+ /*
+ * Assuming all combinations are checked, only the starting address
+ * has to be checked if it's in another memory module's range.
+ */
+ for ( i = 0 ; i < mods->nr_mods; i++ )
+ for ( j = 0 ; j < mods->nr_mods; j++ )
+ if ( (i != j) &&
+ (mods->module[i].start >= mods->module[j].start) &&
+ (mods->module[i].start <
+ mods->module[j].start + mods->module[j].size) )
+ printk("WARNING: modules %-12s and %-12s overlap\n",
+ boot_module_kind_as_string(mods->module[i].kind),
+ boot_module_kind_as_string(mods->module[j].kind));
I am not entirely happy with the double for-loop here.
As we already go through all the modules in add_boot_module(). Could you
look whether this check could be part of it?
This should also allow to have this check for non-debug build as well.
Cheers,
--
Julien Grall
Please at least remove the signature in the e-mail you reply to. The
best would be to trim the e-mail and answer right below the specific
paragraph.
To make sure the module is going to get added, you'd need to do the
check after the for loop. This means there's going to be multiple for
loops just spread over the course of adding the boot modules rather than
one place.
I don't think you need to do the check after the loop. The only way to
go out of the loop in add_boot_module() is when i reached mods->nr_mods.
I had this before but decided against it but after changing it to both
starts rather than the stand and end (ends look much uglier), it looks
cleaner.
for ( i = 0 ; i < mods->nr_mods-1; i++ )
for ( j = i+1 ; j < mods->nr_mods; j++ )
if ( ((mods->module[i].start >= mods->module[j].start) &&
(mods->module[i].start <=
mods->module[j].start + mods->module[j].size)) ||
((mods->module[j].start >= mods->module[i].start) &&
(mods->module[j].start <=
mods->module[i].start + mods->module[i].size)) )
printk("WARNING: modules %-12s and %-12s overlap\n",
boot_module_kind_as_string(mods->module[i].kind),
boot_module_kind_as_string(mods->module[j].kind));
That's also a possibility.
I just don't see a way around it, computationally. You can split where
the loops are executed but in the end the same amount of checks/total
iterations have to be run.
I was talking to someone and he suggested you could just check Xen at
early boot and then check other modules later.
What's wrong with:
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 705a917abf..ecd09ec698 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -254,6 +254,10 @@ struct bootmodule __init
*add_boot_module(bootmodule_kind kind,
mod->domU = false;
return mod;
}
+
+ if ((mod->start >= start) &&
+ (mod->start < (start + size)))
+ printk("WARNING: modules...\n");
}
mod = &mods->module[mods->nr_mods++];
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
