Hi, Peter I updated the code: 1. make it a common code: is_uboot_image_format(); 2. in case of a match in initrd, move the start pointer. no memory move. Could you please have a review? Thanks.
Attachment:
0001-semi_loader-Add-support-to-initrd-of-u-boot-image-fo.patch
Description: Binary data
Best regards, Guodong Xu 在 2012-8-24，下午9:42， Peter Maydell 写道： > On 24 August 2012 09:21, Guodong Xu <guodong.xu@xxxxxxxxxx> wrote: >> Hi, Peter and Marc >> >> This is Guodong Xu from Linaro. And I am new to join the development of >> Linaro ARM landing team. >> >> As requested by Ryan, I made an update to boot-wrapper so it can handle both >> initrd and uInitrd, by checking u-boot image's header magic. Attached is my >> patch. Could you please have a review, and suggest whether and how it can be >> pushed to boot wrapper.git? >> >> The modification is based on >> http://linux-arm.org/git?p=boot-wrapper.git;a=summary > > Thanks for this patch. Some code review comments follow: > >> Subject: [PATCH] Add support to load uInitrd > > This patch could use a slightly more detailed commit > message, and definitely needs a Signed-off-by: line. > >> >> --- >> semi_loader.c | 41 +++++++++++++++++++++++++++++++++++++++-- >> 1 files changed, 39 insertions(+), 2 deletions(-) >> >> diff --git a/semi_loader.c b/semi_loader.c >> index 2a30c5a..8995b99 100644 >> --- a/semi_loader.c >> +++ b/semi_loader.c >> @@ -340,6 +340,36 @@ static void correct_kernel_location(struct loader_info *info) >> info->kernel_size - UIMAGE_HEADER_SIZE); >> } >> >> +/* move initrd to correct place, if necessary */ >> +static void correct_initrd_location(struct loader_info *info) >> +{ >> + /* >> + * UIMAGE_HEADER_SIZE and uImage_magic are actually header and magic >> + * for all U-Boot images, including uInitrd. >> + * Ref to: http://www.denx.de/wiki/view/DULG/UBootImages >> + */ >> + char *const uInitrd_payload = >> + (char *)info->initrd_start + UIMAGE_HEADER_SIZE; >> + >> + if(info->initrd_size <= UIMAGE_HEADER_SIZE) >> + return; >> + >> + if(memcmp((char *)info->initrd_start, uImage_magic, >> + sizeof uImage_magic)) >> + return; >> + >> + warn("Ignoring uInitrd meta-data\n"); > > It would be nice to pull out the "recognise a uimage/uinitrd header" > common code from here and correct_kernel_location() into its own function. > >> + >> + /* >> + * Move the uInitrd payload to replace the u-boot image header, and >> + * leave the entry point unmodified. initrd_size is updated. >> + */ >> + info->initrd_size -= UIMAGE_HEADER_SIZE; >> + memmove((char *)info->initrd_start, uInitrd_payload, info->initrd_size); > > Do we really need to move the whole initrd rather than just changing > our idea of where its start address is? > > > >> + >> + return; >> +} >> + >> static char semi_cmdline[SEMI_CMDLINE_MAX]; >> >> static char *kernel_arg = (void *)0; >> @@ -496,8 +526,15 @@ args_done: >> "Failed to load initrd image"); >> info("Loaded initrd: ", initrd_arg, "\n"); >> >> - info->initrd_start = atag.initrd.start = start; >> - info->initrd_size = atag.initrd.size = (unsigned)phys - start; >> + info->initrd_start = start; >> + info->initrd_size = (unsigned)phys - start; >> + >> + /* Move initrd to correct place, if necessary */ >> + correct_initrd_location(info); >> + >> + atag.initrd.start = info->initrd_start; >> + atag.initrd.size = info->initrd_size; >> + >> } else if(info->initrd_size) { >> if(noinitrd_arg) { >> info->initrd_size = 0; >> -- >> 1.7.4.1 > > thanks > -- PMM
_______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
