On 12/12/14 14:20, Peter Maydell wrote: > On 9 December 2014 at 01:13, Laszlo Ersek <ler...@redhat.com> wrote: >> Introduce the new boolean field "arm_boot_info.firmware_loaded". When this >> field is set, it means that the portion of guest DRAM that the VCPU >> normally starts to execute, or the pflash chip that the VCPU normally >> starts to execute, has been populated by board-specific code with >> full-fledged guest firmware code, before the board calls >> arm_load_kernel(). >> >> Simultaneously, "arm_boot_info.firmware_loaded" guarantees that the board >> code has set up the global firmware config instance, for arm_load_kernel() >> to find with fw_cfg_find(). >> >> Guest kernel (-kernel) and guest firmware (-bios, -pflash) has always been >> possible to specify independently on the command line. The following cases >> should be considered: >> >> nr -bios -pflash -kernel description >> unit#0 >> -- ------- ------- ------- ------------------------------------------- >> 1 present present absent Board code rejects this case, -bios and >> present present present -pflash unit#0 are exclusive. Left intact >> by this patch. >> >> 2 absent absent present Traditional kernel loading, with qemu's >> minimal board firmware. Left intact by this >> patch. >> >> 3 absent present absent Preexistent case for booting guest firmware >> present absent absent loaded with -bios or -pflash. Left intact >> by this patch. >> >> 4 absent absent absent Preexistent case for not loading any >> firmware or kernel up-front. Left intact by >> this patch. >> >> 5 present absent present New case introduced by this patch: kernel >> absent present present image is passed to externally loaded >> firmware in unmodified form, using fw_cfg. >> >> An easy way to see that this patch doesn't interfere with existing cases >> is to realize that "info->firmware_loaded" is constant zero at this point. >> Which makes the "outer" condition unchanged, and the "inner" condition >> (with the fw_cfg-related code) dead. >> >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> --- >> >> Notes: >> v3: >> - unchanged >> >> include/hw/arm/arm.h | 5 +++ >> hw/arm/boot.c | 91 >> +++++++++++++++++++++++++++++++++++++++++++++++++--- >> 2 files changed, 91 insertions(+), 5 deletions(-) >> >> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h >> index cefc9e6..dd69d66 100644 >> --- a/include/hw/arm/arm.h >> +++ b/include/hw/arm/arm.h >> @@ -65,8 +65,13 @@ struct arm_boot_info { >> int is_linux; >> hwaddr initrd_start; >> hwaddr initrd_size; >> hwaddr entry; >> + >> + /* Boot firmware has been loaded, typically at address 0, with -bios or >> + * -pflash. It also implies that fw_cfg_find() will succeed. >> + */ >> + bool firmware_loaded; >> }; >> void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info); >> >> /* Multiplication factor to convert from system clock ticks to qemu timer >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c >> index 0014c34..01d6d3d 100644 >> --- a/hw/arm/boot.c >> +++ b/hw/arm/boot.c >> @@ -475,8 +475,62 @@ static void do_cpu_reset(void *opaque) >> } >> } >> } >> >> +/** >> + * load_image_to_fw_cfg() - Load an image file into an fw_cfg entry >> identified >> + * by key. >> + * @fw_cfg: The firmware config instance to store the data in. >> + * @size_key: The firmware config key to store the size of the loaded data >> + * under, with fw_cfg_add_i32(). >> + * @data_key: The firmware config key to store the loaded data under, with >> + * fw_cfg_add_bytes(). >> + * @image_name: The name of the image file to load. If it is NULL, the >> function >> + * returns without doing anything. >> + * @decompress: Whether the image should be decompressed (gunzipped) before >> + * adding it to fw_cfg. >> + * >> + * In case of failure, the function prints an error message to stderr and >> the >> + * process exits with status 1. >> + */ >> +static void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key, >> + uint16_t data_key, const char *image_name, >> + bool decompress) >> +{ >> + size_t size; >> + uint8_t *data; >> + >> + if (image_name == NULL) { >> + return; >> + } >> + >> + if (decompress) { >> + int bytes; >> + >> + bytes = load_image_gzipped_buffer (image_name, >> + LOAD_IMAGE_MAX_GUNZIP_BYTES, >> &data); > > Stray space again.
Will fix, thanks. > >> + if (bytes == -1) { >> + fprintf(stderr, "failed to load and decompress \"%s\"\n", >> + image_name); >> + exit(1); >> + } >> + size = bytes; >> + } else { >> + gchar *contents; >> + gsize length; >> + >> + if (!g_file_get_contents(image_name, &contents, &length, NULL)) { >> + fprintf(stderr, "failed to load \"%s\"\n", image_name); >> + exit(1); >> + } >> + size = length; >> + data = (uint8_t *)contents; >> + } >> + >> + fw_cfg_add_i32(fw_cfg, size_key, size); >> + fw_cfg_add_bytes(fw_cfg, data_key, data, size); > > We have no way to free these buffers later but x86 does the same, > so never mind. OK. > >> +} >> + >> void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) >> { >> CPUState *cs; >> int kernel_size; >> @@ -497,21 +551,48 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info >> *info) >> qemu_register_reset(do_cpu_reset, ARM_CPU(cs)); >> } >> >> /* Load the kernel. */ >> - if (!info->kernel_filename) { >> + if (!info->kernel_filename || info->firmware_loaded) { >> >> if (have_dtb(info)) { >> - /* If we have a device tree blob, but no kernel to supply it to, >> - * copy it to the base of RAM for a bootloader to pick up. >> + /* If we have a device tree blob, but no kernel to supply it to >> (or >> + * the kernel is supposed to be loaded by the bootloader), copy >> the >> + * DTB to the base of RAM for the bootloader to pick up. >> */ >> if (load_dtb(info->loader_start, info, 0) < 0) { >> exit(1); >> } >> } >> >> - /* If no kernel specified, do nothing; we will start from address 0 >> - * (typically a boot ROM image) in the same way as hardware. >> + if (info->kernel_filename) { >> + FWCfgState *fw_cfg; >> + bool decompress_kernel; >> + >> + fw_cfg = fw_cfg_find(); >> + decompress_kernel = arm_feature(&cpu->env, ARM_FEATURE_AARCH64); > > If we have a real bootloader in the UEFI firmware, why do we need > to do decompression for it? We only do this in the builtin bootloader > because QEMU is acting as the bootloader and has to support the > feature itself. I would have thought that the UEFI builtin kernel > booting support already supported decompressing the kernel... The "UEFI builtin kernel booting support" will qualify as "builtin" only after my 2500 line patch series is merged in edk2. http://thread.gmane.org/gmane.comp.bios.tianocore.devel/11651 Zlib decompression is not present in edk2 (it only has a TianoCore variant of LZMA), and I didn't want to impede (in the community sense) my edk2 patchset even more (ie. beyond its current size) by importing libz too. Thanks Laszlo > > Otherwise looks OK. > > -- PMM >