Re: [Xen-devel] [PATCH v3] xen: arm: introduce uImage probe function for Dom0

[Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>]

On Tue, Aug 26, 2014 at 11:44 PM, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> 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@xxxxxxxxxxxxxxx>
>
> 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.
I'll fix this in the next version of the patch.

>> +    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?)
len doesn't include the header. I'll fix this in the next version of the patch.

>> +    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.
I'll fix this in the next version of the patch.

>
> Ian.
>

-- 
Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

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