From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v3] xen: arm: introduce uImage probe
	function for Dom0
Date: Tue, 26 Aug 2014 21:44:42 +0100
Message-ID: <1409085882.28009.41.camel@citrix.com>
References: <1408614502-17074-1-git-send-email-oleksandr.dmytryshyn@globallogic.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1408614502-17074-1-git-send-email-oleksandr.dmytryshyn@globallogic.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
Cc: Tim Deegan <tim@xen.org>, Stefano Stabellini <stefano.stabellini@citrix.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On Thu, 2014-08-21 at 12:48 +0300, Oleksandr Dmytryshyn wrote:
> Patch adds a possibility to boot dom0 kernel from uImage.
> This is needed to improve boot-time. Comparing to zImage,
> uImage is not packed, therefore we can save time needed
> to unpack.
> 
> uImage header format:
> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=include/image.h
> 
> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>

Mostly looks good, a few minor things.

> +/*
> + * Uimage CPU Architecture Codes
> + */

I think we can get away with only defining the two arm ones here.

> +    start = be32_to_cpu(uimage.load);
> +    len = be32_to_cpu(uimage.size);
> +
> +    if ( len > size )

Does len include the header? (IOW do you need to subtract sizeof(uimage)
from something?)

> +    info->entry = info->zimage.start;
> +    info->load = kernel_zimage_load;
> +
> +#ifdef CONFIG_ARM_64
> +    if ( uimage->arch == IH_ARCH_ARM )
> +        info->type = DOMAIN_32BIT;
> +    else if ( uimage->arch == IH_ARCH_ARM64 )
> +        info->type = DOMAIN_64BIT;
> +    else

Use switch() rather than a cascade of else if please.

Ian.