Re: [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj

[Markus Armbruster]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> + qemu-block experts.
>
> On 7/14/20 11:16 AM, Markus Armbruster wrote:
>> Havard Skinnemoen <hskinnemoen@google.com> writes:
>> 
>>> On Mon, Jul 13, 2020 at 7:57 AM Cédric Le Goater <clg@kaod.org> wrote:
>>>>
>>>> On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
>>>>> This allows these NPCM7xx-based boards to boot from a flash image, e.g.
>>>>> one built with OpenBMC. For example like this:
>>>>>
>>>>> IMAGE=${OPENBMC}/build/tmp/deploy/images/gsj/image-bmc
>>>>> qemu-system-arm -machine quanta-gsj -nographic \
>>>>>       -bios ~/qemu/bootrom/npcm7xx_bootrom.bin \
>>>>>       -drive file=${IMAGE},if=mtd,bus=0,unit=0,format=raw,snapshot=on
>>>>>
>>>>> Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
>>>>> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
>>>>
>>>> May be we don't need to create the flash object if dinfo is NULL.
>>>
>>> It's soldered on the board, so you can't really boot the board without
>>> it. But if you think it's better to remove it altogether if we don't
>>> have an image to load into it, I can do that.
>> 
>> If a device is a fixed part of the physical board, it should be a fixed
>> part of the virtual board, too.
>
> We agree so far but ... how to do it?
>
> I never used this API, does that makes sense?
>
>     if (!dinfo) {
>         QemuOpts *opts;
>
>         opts = qemu_opts_create(NULL, "spi-flash", 1, &error_abort);
>         qdict_put_str(opts, "format", "null-co");
>         qdict_put_int(opts, BLOCK_OPT_SIZE, 64 * MiB);
>         qdict_put_bool(opts, NULL_OPT_ZEROES, false); // XXX
>
>         dinfo = drive_new(opts, IF_MTD, &error_abort);
>         qemu_opts_del(opts);
>     }

I believe existing code special-cases "no backend" instead of making one
up.

Example: pflash_cfi0?.c

If ->blk is non-null, we read its contents into the memory buffer and
write updates back, else we leave it blank and don't write updates back.

Making one up could be more elegant.  To find out, you have to try.

We make up a few default drives (i.e. drives the user doesn't specify):
floppy, CD-ROM and SD card.  Ancient part of the user interface, uses
DriveInfo.  I doubt we should create more of them.

I believe block backends we make up for internal use should stay away
from DriveInfo.  Kevin, what do you think?  How would you make up a
null-co block backend for a device's internal use?

> We should probably add a public helper for that.

If we decide we want to make up backends, then yes, we should do that in
a helper, not in each device.

> 'XXX' because NOR flashes erase content is when hardware bit
> is set, so it would be more useful to return -1/0xff... rather
> than zeroes.