From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2] xen: arm: introduce uImage probe function for Dom0 Date: Wed, 20 Aug 2014 10:59:47 -0500 Message-ID: <53F4C5F3.2090301@linaro.org> References: <1408529074-20013-1-git-send-email-oleksandr.dmytryshyn@globallogic.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1408529074-20013-1-git-send-email-oleksandr.dmytryshyn@globallogic.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: Oleksandr Dmytryshyn , Ian Campbell , Stefano Stabellini , Tim Deegan , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org Hi Oleksandr, On 20/08/14 05:04, 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. Can you add a link to the uImage format in the commit message and/or patch? > Signed-off-by: Oleksandr Dmytryshyn > --- > Changed since v1: > * fixed commit message > * added uimage structure definition > * removed checking of the append device tree > > xen/arch/arm/kernel.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 63 insertions(+) > > diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c > index d635a7e..20e6884 100644 > --- a/xen/arch/arm/kernel.c > +++ b/xen/arch/arm/kernel.c > @@ -16,6 +16,9 @@ > > #include "kernel.h" > > +#define UIMAGE32_MAGIC 0x27051956 > +#define UIMAGE32_NMLEN 32 > + Why did you add "32" in the name? The uimage header is architecture independent. [..] > + start = be32_to_cpu(uimage.load); > + len = be32_to_cpu(uimage.size); > + > + if ( len > size ) > + return -EINVAL; > + > + /* > + * If start is sizeof(zimage) zImage is position independent -- load it > + * at 32k from start of RAM. The comment doesn't seems relevant. Why are you talking about zimage? > + */ > + if ( start == 0 ) > + info->zimage.start = info->mem.bank[0].start + 0x8000; > + else > + info->zimage.start = start; I know we have this check on zImage. As the kernel should be position independent, otherwise it will unlikely boot as DOM0, I think it would be better to print a warning if start != 0. Or maybe we should just ignore it so the same uImage would be able to work on both bare-metal and Xen. > + > + info->zimage.kernel_addr = addr + sizeof(uimage); > + info->zimage.len = len; > + info->entry = info->zimage.start; > + info->load = kernel_zimage_load; > + > +#ifdef CONFIG_ARM_64 > + info->type = DOMAIN_32BIT; > +#endif As said a bit earlier, the uImage is architecture independent. You have to check the arch field to know the kernel is 32 or 64 bits. Regards, -- Julien Grall