| Message ID | 1410346790-31743-5-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 10 September 2014 11:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Add support for loading DTB images when booting ELF images using > -kernel. If there are no conflicts with the placement of the ELF > segments, the DTB image is loaded at the base of RAM. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > hw/arm/boot.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index 1f73614d8843..3878cbd97aad 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -464,7 +464,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > int kernel_size; > int initrd_size; > int is_linux = 0; > - uint64_t elf_entry; > + uint64_t elf_entry, elf_low_addr; > int elf_machine; > hwaddr entry, kernel_load_offset; > int big_endian; > @@ -531,7 +531,19 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > > /* Assume that raw images are linux kernels, and ELF images are not. */ > kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry, > - NULL, NULL, big_endian, elf_machine, 1); > + &elf_low_addr, NULL, big_endian, elf_machine, 1); > + if (kernel_size > 0 && have_dtb(info)) { > + /* If there is still some room left between the base of RAM and the > + * low end of the ELF image we just loaded, try and put the DTB at the > + * base of RAM like we do for bootloaders. Just ignore the potential 0 > + * return value of load_dtb() which indicates that the dtb didn't fit, > + * in that case we just proceed without it. > + */ > + if (elf_low_addr > info->loader_start && > + load_dtb(info->loader_start, info, elf_low_addr) < 0) { > + exit(1); > + } > + } The conditional means we won't try to load the DTB even if the ELF file fit into the address space entirely below loader_start, which doesn't look right. thanks -- PMM
On 10 September 2014 13:21, Peter Maydell <peter.maydell@linaro.org> wrote: > On 10 September 2014 11:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> Add support for loading DTB images when booting ELF images using >> -kernel. If there are no conflicts with the placement of the ELF >> segments, the DTB image is loaded at the base of RAM. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> hw/arm/boot.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c >> index 1f73614d8843..3878cbd97aad 100644 >> --- a/hw/arm/boot.c >> +++ b/hw/arm/boot.c >> @@ -464,7 +464,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) >> int kernel_size; >> int initrd_size; >> int is_linux = 0; >> - uint64_t elf_entry; >> + uint64_t elf_entry, elf_low_addr; >> int elf_machine; >> hwaddr entry, kernel_load_offset; >> int big_endian; >> @@ -531,7 +531,19 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) >> >> /* Assume that raw images are linux kernels, and ELF images are not. */ >> kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry, >> - NULL, NULL, big_endian, elf_machine, 1); >> + &elf_low_addr, NULL, big_endian, elf_machine, 1); >> + if (kernel_size > 0 && have_dtb(info)) { >> + /* If there is still some room left between the base of RAM and the >> + * low end of the ELF image we just loaded, try and put the DTB at the >> + * base of RAM like we do for bootloaders. Just ignore the potential 0 >> + * return value of load_dtb() which indicates that the dtb didn't fit, >> + * in that case we just proceed without it. >> + */ >> + if (elf_low_addr > info->loader_start && >> + load_dtb(info->loader_start, info, elf_low_addr) < 0) { >> + exit(1); >> + } >> + } > > The conditional means we won't try to load the DTB even if the > ELF file fit into the address space entirely below loader_start, > which doesn't look right. > Ah right, I though loader_start was the start of usable RAM What about assigning elf_high_addr as well, and changing the test to if ((elf_low_addr > info->loader_start || elf_high_addr < info_loader_start) && load_dtb(info->loader_start, info, (elf_high_addr < info_loader_start) ? 0 : elf_low_addr) < 0) {
On 10 September 2014 12:28, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 10 September 2014 13:21, Peter Maydell <peter.maydell@linaro.org> wrote: >> The conditional means we won't try to load the DTB even if the >> ELF file fit into the address space entirely below loader_start, >> which doesn't look right. >> > > Ah right, I though loader_start was the start of usable RAM It is, but the ELF file doesn't necessarily have to be in RAM: it could be linked so as to load at an address in the flash... > What about assigning elf_high_addr as well, and changing the test to > > if ((elf_low_addr > info->loader_start > || elf_high_addr < info_loader_start) > && load_dtb(info->loader_start, info, > (elf_high_addr < info_loader_start) ? 0 : elf_low_addr) < 0) { I think that's right, though maybe there's a less confusing way of phrasing it. -- PMM
diff --git a/hw/arm/boot.c b/hw/arm/boot.c index 1f73614d8843..3878cbd97aad 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -464,7 +464,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) int kernel_size; int initrd_size; int is_linux = 0; - uint64_t elf_entry; + uint64_t elf_entry, elf_low_addr; int elf_machine; hwaddr entry, kernel_load_offset; int big_endian; @@ -531,7 +531,19 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) /* Assume that raw images are linux kernels, and ELF images are not. */ kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry, - NULL, NULL, big_endian, elf_machine, 1); + &elf_low_addr, NULL, big_endian, elf_machine, 1); + if (kernel_size > 0 && have_dtb(info)) { + /* If there is still some room left between the base of RAM and the + * low end of the ELF image we just loaded, try and put the DTB at the + * base of RAM like we do for bootloaders. Just ignore the potential 0 + * return value of load_dtb() which indicates that the dtb didn't fit, + * in that case we just proceed without it. + */ + if (elf_low_addr > info->loader_start && + load_dtb(info->loader_start, info, elf_low_addr) < 0) { + exit(1); + } + } entry = elf_entry; if (kernel_size < 0) { kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
Add support for loading DTB images when booting ELF images using -kernel. If there are no conflicts with the placement of the ELF segments, the DTB image is loaded at the base of RAM. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- hw/arm/boot.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)