Re: [Xen-devel] [PATCH v2 2/2] xen/arm: If the DOM0 zImage as a DTB appended replace it

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-05-30 at 15:05 +0100, Julien Grall wrote:
> If a DTB is appended to the DOM0 zImage, it's probably means that the linux
> decompression stage will replace the DBT register value (r2 on arm32)
> by the address of the appended DTB.

I don't think we should do this, it certainly violates the principal of
least surprise, since no other "bootloader" does anything like this
AFAIK. Yes that means you occasionally trip over your DTB changes
appearing not to take affect, but that's true on native and is one of
the known and understood bad things about appended DTB.

So I think this is a case of "don't do that then". At most we could
issue a warning, I suppose. But really we shouldn't be making any
assumptions (good or bad) about what happens to live in the memory just
past the end of the kernel, it may or may not be an appended DTB.

> In this case, to avoid issue with Linux, Xen needs to load the new device tree
> just after the kernel.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/arm/kernel.c |   26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index f8c8850..1d6c927 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -123,20 +123,6 @@ static int kernel_try_zimage_prepare(struct kernel_info 
> *info,
>      if ( (end - start) > size )
>          return -EINVAL;
>  
> -    /*
> -     * Check for an appended DTB.
> -     */
> -    if ( addr + end - start + sizeof(dtb_hdr) <= size )
> -    {
> -        copy_from_paddr(&dtb_hdr, addr + end - start,
> -                        sizeof(dtb_hdr), DEV_SHARED);
> -        if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC) {
> -            end += be32_to_cpu(dtb_hdr.total_size);
> -
> -            if ( end > addr + size )
> -                return -EINVAL;
> -        }
> -    }
>  
>      info->zimage.kernel_addr = addr;
>  
> @@ -154,6 +140,18 @@ static int kernel_try_zimage_prepare(struct kernel_info 
> *info,
>      info->load = kernel_zimage_load;
>      info->type = KERNEL_ZIMAGE;
>  
> +    /*
> +     * If there is an appended DTB, ask XEN to replace the DTB
> +     * by the generate one.
> +     */
> +    if ( info->zimage.len + sizeof(dtb_hdr) <= size )
> +    {
> +        copy_from_paddr(&dtb_hdr, addr + end - start,
> +                        sizeof(dtb_hdr), DEV_SHARED);
> +        if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC)
> +            info->dtb_paddr = info->zimage.load_addr + info->zimage.len;
> +    }
> +
>      return 0;
>  }
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel