From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 2/2] xen/arm: If the DOM0 zImage as a DTB appended replace it Date: Thu, 30 May 2013 15:47:30 +0100 Message-ID: <51A76682.8090309@linaro.org> References: <1369922720-10015-1-git-send-email-julien.grall@linaro.org> <1369922720-10015-3-git-send-email-julien.grall@linaro.org> <1369923870.18727.9.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1369923870.18727.9.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: patches@linaro.org, Stefano.Stabellini@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 05/30/2013 03:24 PM, Ian Campbell wrote: > 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. Shall we load the DTB if there is one appended? If yes, I think the user will be confused because Linux won't use the right device tree. It's hard for the user to find the issue because there is no log. > 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. I can add a warning and also fix the check the line: if ( addr + end - start + sizeof(dtb_hdr) <= size ) must be replace by: if ( end - start + sizeof(dtb_hdr) <= size ) >> 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 >> --- >> 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; >> } >> > >