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
>
