Re: [Qemu-devel] [PATCH v2] vexpress: Add support for the -bios flag to provide firmware

[Peter Crosthwaite]

On Fri, May 30, 2014 at 12:46 AM, Laszlo Ersek <address@hidden> 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
>> <address@hidden> wrote:
>>> From: Grant Likely <address@hidden>
>>>
>>> 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 <address@hidden>
>>> Tested-by: Roy Franz <address@hidden>
>>> [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 <address@hidden>
>>> ---
>>> 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 <address@hidden>
>>
>> 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