[v2] vexpress: Add support for the -bios flag to provide firmware

From: Grant Likely <grant.likely@linaro.org>

Right now to run firmware inside the QEMU VExpress model requires
padding out the firmware image to the size of the virtual flash and
passing it in via the -pflash argument. If the firmware image is passed
without padding, then QEMU will fail. Also, when passed as a -pflash
argument, QEMU treats the file as persistent storage and will modify the
file.

The -bios flag provides the semantics that we want for providing a
firmware image. This patch maps the contents of the -bios file into the
address space at the boot flash location.

Tested with the vexpress-a15 model and the Tianocore port.

Signed-off-by: Grant Likely <grant.likely@linaro.org>
Tested-by: Roy Franz <roy.franz@linaro.org>
[PMM: folded long line, removed stray \n from error message,
 use correct variable for printing image name, exit(1) rather than 0]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Grant submitted v1 of this a few months back; it was pretty nearly
correct, so I've just tidied up the loose ends so we can get it into
QEMU 2.1. Tested by booting the UEFI blob from
https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU

 hw/arm/vexpress.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

On Thu, May 29, 2014 at 10:24 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> From: Grant Likely <grant.likely@linaro.org>
>
> Right now to run firmware inside the QEMU VExpress model requires
> padding out the firmware image to the size of the virtual flash and
> passing it in via the -pflash argument. If the firmware image is passed
> without padding, then QEMU will fail. Also, when passed as a -pflash
> argument, QEMU treats the file as persistent storage and will modify the
> file.
>

A little out of scope, but no support for read-only backing images is
somewhat annoying. Is it feasible to patch the block layer (or perhaps
we need to do something to pflash?) to handle a read-only -pflash file
as init-only data (and then use the normal volatile ram-based storage
once the file is loaded)?

Then the semantics of "don't change my ROM file even if the real
hardware can do it in real life" can just be handled with file perms.

> The -bios flag provides the semantics that we want for providing a
> firmware image. This patch maps the contents of the -bios file into the
> address space at the boot flash location.
>
> Tested with the vexpress-a15 model and the Tianocore port.
>
> Signed-off-by: Grant Likely <grant.likely@linaro.org>
> Tested-by: Roy Franz <roy.franz@linaro.org>
> [PMM: folded long line, removed stray \n from error message,
>  use correct variable for printing image name, exit(1) rather than 0]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Grant submitted v1 of this a few months back; it was pretty nearly
> correct, so I've just tidied up the loose ends so we can get it into
> QEMU 2.1. Tested by booting the UEFI blob from
> https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU
>
>  hw/arm/vexpress.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index 33ff422..0f8f175 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -28,6 +28,7 @@
>  #include "net/net.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/boards.h"
> +#include "hw/loader.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/blockdev.h"
>  #include "hw/block/flash.h"
> @@ -528,6 +529,18 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
>      daughterboard->init(daughterboard, machine->ram_size, machine->cpu_model,
>                          pic);
>
> +    /*
> +     * If a bios file was provided, attempt to map it into memory
> +     */
> +    if (bios_name) {
> +        const char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +        if (!fn || load_image_targphys(fn, map[VE_NORFLASH0],
> +                                       VEXPRESS_FLASH_SIZE) < 0) {
> +            error_report("Could not load rom image '%s'", bios_name);

"ROM"

Otherwise

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Regards,
Peter

> +            exit(1);
> +        }
> +    }
> +
>      /* Motherboard peripherals: the wiring is the same but the
>       * addresses vary between the legacy and A-Series memory maps.
>       */
> --
> 1.9.2
>
>

CC'ing Drew, Eric and Michal.

On 05/29/14 14:45, Peter Crosthwaite wrote:
> On Thu, May 29, 2014 at 10:24 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> From: Grant Likely <grant.likely@linaro.org>
>>
>> Right now to run firmware inside the QEMU VExpress model requires
>> padding out the firmware image to the size of the virtual flash and
>> passing it in via the -pflash argument. If the firmware image is passed
>> without padding, then QEMU will fail. Also, when passed as a -pflash
>> argument, QEMU treats the file as persistent storage and will modify the
>> file.
>>
> 
> A little out of scope, but no support for read-only backing images is
> somewhat annoying. Is it feasible to patch the block layer (or perhaps
> we need to do something to pflash?) to handle a read-only -pflash file
> as init-only data (and then use the normal volatile ram-based storage
> once the file is loaded)?
> 
> Then the semantics of "don't change my ROM file even if the real
> hardware can do it in real life" can just be handled with file perms.
> 
>> The -bios flag provides the semantics that we want for providing a
>> firmware image. This patch maps the contents of the -bios file into the
>> address space at the boot flash location.
>>
>> Tested with the vexpress-a15 model and the Tianocore port.
>>
>> Signed-off-by: Grant Likely <grant.likely@linaro.org>
>> Tested-by: Roy Franz <roy.franz@linaro.org>
>> [PMM: folded long line, removed stray \n from error message,
>>  use correct variable for printing image name, exit(1) rather than 0]
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> Grant submitted v1 of this a few months back; it was pretty nearly
>> correct, so I've just tidied up the loose ends so we can get it into
>> QEMU 2.1. Tested by booting the UEFI blob from
>> https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU
>>
>>  hw/arm/vexpress.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
>> index 33ff422..0f8f175 100644
>> --- a/hw/arm/vexpress.c
>> +++ b/hw/arm/vexpress.c
>> @@ -28,6 +28,7 @@
>>  #include "net/net.h"
>>  #include "sysemu/sysemu.h"
>>  #include "hw/boards.h"
>> +#include "hw/loader.h"
>>  #include "exec/address-spaces.h"
>>  #include "sysemu/blockdev.h"
>>  #include "hw/block/flash.h"
>> @@ -528,6 +529,18 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
>>      daughterboard->init(daughterboard, machine->ram_size, machine->cpu_model,
>>                          pic);
>>
>> +    /*
>> +     * If a bios file was provided, attempt to map it into memory
>> +     */
>> +    if (bios_name) {
>> +        const char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> +        if (!fn || load_image_targphys(fn, map[VE_NORFLASH0],
>> +                                       VEXPRESS_FLASH_SIZE) < 0) {
>> +            error_report("Could not load rom image '%s'", bios_name);
> 
> "ROM"
> 
> Otherwise
> 
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> Regards,
> Peter
> 
>> +            exit(1);
>> +        }
>> +    }
>> +
>>      /* Motherboard peripherals: the wiring is the same but the
>>       * addresses vary between the legacy and A-Series memory maps.
>>       */
>> --
>> 1.9.2
>>
>>
> 

OK, big chaos in my mind now, because I don't know enough about ARM, so
I'll just shoot out some thoughts and questions.

(1) The pflash interface perfectly well supports read-only backing files
(or read-write backing files opened in read-only mode), and the flash
programming interface does communicate that to the guest. Please search
"hw/block/pflash_cfi01.c" for "bdrv_is_read_only()", and follow the code
from there.

One just needs to use the

  -drive file=img,if=pflash,format=raw,readonly

option rather than the -pflash shorthand. Please refer to qemu commit
637a5acb ("hw/i386/pc_sysfw: support two flash drives").

(2) Can anyone post / link a concise, *up-to-date* command line for
upstream qemu, in connection with *upstream* edk2 build instructions, so
that I can test aarch64 UEFI stuff on an x86_64 host (with TCG)?

What DSC platform is built in edk2? What qemu machine type is used? If
the needed edk2 changes are downstream only, what branch of what repo
should I look at?

The last info I have on this is that ArmPlatformPkg "intended" to grow a
dedicated platform (a DSC) for qemu's -M virt, because Olivier didn't
like the vexpress DSCs being reused for virtual machines.

(Side note: I had posted a simple virtio-mmio transport enumeration
patchset (suiting -M virt) to edk2-devel, for ArmFvpDxe. The goal was to
auto-detect all virtio (block, net, scsi) devices of qemu's -M virt.
Olivier seemed to approve, but didn't apply the relevant part of the set
because of this "pending split" of platforms (DSCs). Did that go anywhere?)

(3) I'm interested in the qemu command line because I'd like to see how
the UEFI variable store is differentiated from the flash device that
contains the executable (potentially post-decompression executable)
firmware code.

In *upstream* OVMF the variable store and the main fw image are built
into the same FD file. I have a downstream patch that splits them apart,
thereby allowing the variable store to be VM-private, and the fw image
to be host-central.

Qemu commit 637a5acb that I mentioned above targets this use case with
the following suggested command line:

      -drive file=firmware_image.img,if=pflash,format=raw,readonly \
      -drive file=variable_store.img,if=pflash,format=raw

In other words, for x86_64 guests, for UEFI support we're actually
moving *away* from -bios, and toward -pflash.

Any difference here with Aarch64 guests is interesting for two reasons:

- Maybe the Aarch64 target machtypes have completely different device
  models (from that of pc-i440fx-*) for storing the non-volatile
  variables (I vaguely recall something about NOR flash).

- If libvirt wants to accommodate such guests (ie. firmware image
  separate from nvvarstore) -- and I do want libvirt to handle the
  x86_64 case down the road :) -- then differences between the qemu
  command lines that the same (future) domain XML fragments will map to,
  dependent on target arch, should be considered consciously and
  probably early enough.

Thanks,
Laszlo

On Fri, May 30, 2014 at 12:46 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> CC'ing Drew, Eric and Michal.
>
> On 05/29/14 14:45, Peter Crosthwaite wrote:
>> On Thu, May 29, 2014 at 10:24 PM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> From: Grant Likely <grant.likely@linaro.org>
>>>
>>> Right now to run firmware inside the QEMU VExpress model requires
>>> padding out the firmware image to the size of the virtual flash and
>>> passing it in via the -pflash argument. If the firmware image is passed
>>> without padding, then QEMU will fail. Also, when passed as a -pflash
>>> argument, QEMU treats the file as persistent storage and will modify the
>>> file.
>>>
>>
>> A little out of scope, but no support for read-only backing images is
>> somewhat annoying. Is it feasible to patch the block layer (or perhaps
>> we need to do something to pflash?) to handle a read-only -pflash file
>> as init-only data (and then use the normal volatile ram-based storage
>> once the file is loaded)?
>>
>> Then the semantics of "don't change my ROM file even if the real
>> hardware can do it in real life" can just be handled with file perms.
>>
>>> The -bios flag provides the semantics that we want for providing a
>>> firmware image. This patch maps the contents of the -bios file into the
>>> address space at the boot flash location.
>>>
>>> Tested with the vexpress-a15 model and the Tianocore port.
>>>
>>> Signed-off-by: Grant Likely <grant.likely@linaro.org>
>>> Tested-by: Roy Franz <roy.franz@linaro.org>
>>> [PMM: folded long line, removed stray \n from error message,
>>>  use correct variable for printing image name, exit(1) rather than 0]
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> Grant submitted v1 of this a few months back; it was pretty nearly
>>> correct, so I've just tidied up the loose ends so we can get it into
>>> QEMU 2.1. Tested by booting the UEFI blob from
>>> https://wiki.linaro.org/LEG/Engineering/Kernel/UEFI/VersatileExpress/QEMU
>>>
>>>  hw/arm/vexpress.c | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
>>> index 33ff422..0f8f175 100644
>>> --- a/hw/arm/vexpress.c
>>> +++ b/hw/arm/vexpress.c
>>> @@ -28,6 +28,7 @@
>>>  #include "net/net.h"
>>>  #include "sysemu/sysemu.h"
>>>  #include "hw/boards.h"
>>> +#include "hw/loader.h"
>>>  #include "exec/address-spaces.h"
>>>  #include "sysemu/blockdev.h"
>>>  #include "hw/block/flash.h"
>>> @@ -528,6 +529,18 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
>>>      daughterboard->init(daughterboard, machine->ram_size, machine->cpu_model,
>>>                          pic);
>>>
>>> +    /*
>>> +     * If a bios file was provided, attempt to map it into memory
>>> +     */
>>> +    if (bios_name) {
>>> +        const char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>> +        if (!fn || load_image_targphys(fn, map[VE_NORFLASH0],
>>> +                                       VEXPRESS_FLASH_SIZE) < 0) {
>>> +            error_report("Could not load rom image '%s'", bios_name);
>>
>> "ROM"
>>
>> Otherwise
>>
>> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> Regards,
>> Peter
>>
>>> +            exit(1);
>>> +        }
>>> +    }
>>> +
>>>      /* Motherboard peripherals: the wiring is the same but the
>>>       * addresses vary between the legacy and A-Series memory maps.
>>>       */
>>> --
>>> 1.9.2
>>>
>>>
>>
>
> OK, big chaos in my mind now, because I don't know enough about ARM, so
> I'll just shoot out some thoughts and questions.
>
> (1) The pflash interface perfectly well supports read-only backing files
> (or read-write backing files opened in read-only mode), and the flash
> programming interface does communicate that to the guest.

Well that would be a different semantic again:). Changing the guest
visible machine state based on backing store may actually be unwanted.
If read only mode is selected for the -drive then the pflash is
unwritable for the lifetime of the system. This is essentially saying
that the pflash solely exists for read-only firmware, which is not the
case in many embedded platforms (we have systems where the full image
stack as well as user disk storage is on one pflash).

A useful case, is to blob in some firmware at reset or machine
creation (from a read-only file) but the the actual device presents as
guest-writeable, backing onto RAM. This means you can blob in a image,
but then have a guest use the device as read/write storage.

Regards,
Peter

On 29 May 2014 15:46, Laszlo Ersek <lersek@redhat.com> wrote:
> (2) Can anyone post / link a concise, *up-to-date* command line for
> upstream qemu, in connection with *upstream* edk2 build instructions, so
> that I can test aarch64 UEFI stuff on an x86_64 host (with TCG)?

Note that AArch64 isn't relevant to this patch, because
this patch is for the vexpress models, which are AArch32 only.

AArch64 UEFI-on-QEMU stuff is very much work in progress as
I understand it and needs hacks to all of UEFI, QEMU and
the guest kernel. You could probably try starting with
https://wiki.linaro.org/LEG/UEFIforQEMU

> What DSC platform is built in edk2? What qemu machine type is used? If
> the needed edk2 changes are downstream only, what branch of what repo
> should I look at?
>
> The last info I have on this is that ArmPlatformPkg "intended" to grow a
> dedicated platform (a DSC) for qemu's -M virt, because Olivier didn't
> like the vexpress DSCs being reused for virtual machines.

This sort of question is probably better asked on a UEFI
mailing list...

thanks
-- PMM