Re: [Xen-devel] [PATCH] xen/arm: add warning if memory modules overlap

From: Julien Grall <julien.grall@arm.com>
To: Brian Woods <brian.woods@xilinx.com>, xen-devel@lists.xenproject.org
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>
Subject: Re: [Xen-devel] [PATCH] xen/arm: add warning if memory modules overlap
Date: Thu, 10 Oct 2019 16:39:07 +0100	[thread overview]
Message-ID: <4da9c328-3968-5d46-6000-73e824b26962@arm.com> (raw)
In-Reply-To: <1570650447-3225-1-git-send-email-brian.woods@xilinx.com>

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@xilinx.com>
> ---
> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel