| Message ID | 1409930126-28449-7-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
> On 5 sep. 2014, at 17:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Add support for loading DTB images when booting ELF images via -kernel. > The DTB image is located at the next 4 KB boundary above the highest address > covered by the loaded ELF image. > Apologies, this commit message is out of date: the DTB is put at base of RAM, bur only if it does not conflict with any of the ELF segments, otherwise we abort > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > hw/arm/boot.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index 8d8653978dfe..60c4f6af7884 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -458,7 +458,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; > @@ -529,7 +529,21 @@ 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 we have a DTB in combination with an ELF executable image, > + * put the DTB at the base of RAM like we do for bootloaders. > + */ > + uint32_t dtb_size; > + > + if (load_dtb(info->loader_start, info, &dtb_size)) { > + exit(1); > + } > + if (info->loader_start + dtb_size > elf_low_addr) { > + fprintf(stderr, "Image %s overlaps DTB\n", info->kernel_filename); > + exit(1); > + } > + } > entry = elf_entry; > if (kernel_size < 0) { > kernel_size = load_uimage(info->kernel_filename, &entry, NULL, > -- > 1.8.3.2 >
On 5 September 2014 16:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Add support for loading DTB images when booting ELF images via -kernel. > The DTB image is located at the next 4 KB boundary above the highest address > covered by the loaded ELF image. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > hw/arm/boot.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index 8d8653978dfe..60c4f6af7884 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -458,7 +458,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; > @@ -529,7 +529,21 @@ 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 we have a DTB in combination with an ELF executable image, > + * put the DTB at the base of RAM like we do for bootloaders. > + */ > + uint32_t dtb_size; > + > + if (load_dtb(info->loader_start, info, &dtb_size)) { > + exit(1); > + } > + if (info->loader_start + dtb_size > elf_low_addr) { > + fprintf(stderr, "Image %s overlaps DTB\n", info->kernel_filename); > + exit(1); > + } > + } This shouldn't abort. The reason we need to manually check for overlap is because the default rom-blob behaviour of "overlap means don't start QEMU" isn't what we want. In particular, boards like virt which autogenerate DTBs will always have_dtb(), but we don't want to prevent non-DTB-aware ELF blobs from loading anywhere they like. The behaviour we need is "if blobs don't overlap then load both, otherwise load the ELF file and ignore the DTB". (Thinking about it, that implies we either need a rom_del_blob() or we need to tell load_dtb() about areas of address space it needs to check for overlap with before it registers the rom blob for the dtb.) thanks -- PMM
On 9 September 2014 20:08, Peter Maydell <peter.maydell@linaro.org> wrote: > On 5 September 2014 16:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> Add support for loading DTB images when booting ELF images via -kernel. >> The DTB image is located at the next 4 KB boundary above the highest address >> covered by the loaded ELF image. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> hw/arm/boot.c | 18 ++++++++++++++++-- >> 1 file changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c >> index 8d8653978dfe..60c4f6af7884 100644 >> --- a/hw/arm/boot.c >> +++ b/hw/arm/boot.c >> @@ -458,7 +458,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; >> @@ -529,7 +529,21 @@ 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 we have a DTB in combination with an ELF executable image, >> + * put the DTB at the base of RAM like we do for bootloaders. >> + */ >> + uint32_t dtb_size; >> + >> + if (load_dtb(info->loader_start, info, &dtb_size)) { >> + exit(1); >> + } >> + if (info->loader_start + dtb_size > elf_low_addr) { >> + fprintf(stderr, "Image %s overlaps DTB\n", info->kernel_filename); >> + exit(1); >> + } >> + } > > This shouldn't abort. The reason we need to manually check > for overlap is because the default rom-blob behaviour of > "overlap means don't start QEMU" isn't what we want. > In particular, boards like virt which autogenerate DTBs > will always have_dtb(), but we don't want to prevent > non-DTB-aware ELF blobs from loading anywhere they like. > The behaviour we need is "if blobs don't overlap then > load both, otherwise load the ELF file and ignore the > DTB". > OK > (Thinking about it, that implies we either need a > rom_del_blob() or we need to tell load_dtb() about > areas of address space it needs to check for overlap > with before it registers the rom blob for the dtb.) > ... or we just call load_elf() again
On 9 September 2014 19:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 9 September 2014 20:08, Peter Maydell <peter.maydell@linaro.org> wrote: >> (Thinking about it, that implies we either need a >> rom_del_blob() or we need to tell load_dtb() about >> areas of address space it needs to check for overlap >> with before it registers the rom blob for the dtb.) >> > > ... or we just call load_elf() again That won't work, because we'll still trip the overlap check in rom_load_all(), won't we? -- PMM
On 9 September 2014 20:17, Peter Maydell <peter.maydell@linaro.org> wrote: > On 9 September 2014 19:15, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> On 9 September 2014 20:08, Peter Maydell <peter.maydell@linaro.org> wrote: >>> (Thinking about it, that implies we either need a >>> rom_del_blob() or we need to tell load_dtb() about >>> areas of address space it needs to check for overlap >>> with before it registers the rom blob for the dtb.) >>> >> >> ... or we just call load_elf() again > > That won't work, because we'll still trip the overlap > check in rom_load_all(), won't we? > Yeah, you're right. My fingers are moving faster than my brain again I will go ahead and rework load_dtb() to take a max_size parameter, and load the dtb only if its size doesn't exceed max_size. This should be sufficient to (a) implement the ELF case, and (b) not complicate the other call sites too much
diff --git a/hw/arm/boot.c b/hw/arm/boot.c index 8d8653978dfe..60c4f6af7884 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -458,7 +458,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; @@ -529,7 +529,21 @@ 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 we have a DTB in combination with an ELF executable image, + * put the DTB at the base of RAM like we do for bootloaders. + */ + uint32_t dtb_size; + + if (load_dtb(info->loader_start, info, &dtb_size)) { + exit(1); + } + if (info->loader_start + dtb_size > elf_low_addr) { + fprintf(stderr, "Image %s overlaps DTB\n", info->kernel_filename); + 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 via -kernel. The DTB image is located at the next 4 KB boundary above the highest address covered by the loaded ELF image. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- hw/arm/boot.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)