Re: [Qemu-devel] [qemu PATCH] hw/i386/pc_sysfw: support more than one flash drive

[Markus Armbruster]

Laszlo Ersek <address@hidden> writes:

> On 11/22/13 13:21, Markus Armbruster wrote:
>> Laszlo Ersek <address@hidden> writes:
>> 
>>> This patch allows the user to usefully specify
>>>
>>>   -drive file=img_1,if=pflash,format=raw,readonly \
>>>   -drive file=img_2,if=pflash,format=raw
>>>
>>> on the command line. The flash images will be mapped under 4G in their
>>> reverse unit order -- that is, with their base addresses progressing
>>> downwards, in increasing unit order.
>>>
>>> (The unit number increases with command line order if not explicitly
>>> specified.)
>>>
>>> This accommodates the following use case: suppose that OVMF is split in
>>> two parts, a writeable host file for non-volatile variable storage, and a
>>> read-only part for bootstrap and decompressible executable code.
>>>
>>> The binary code part would be read-only, centrally managed on the host
>>> system, and passed in as unit 0. The variable store would be writeable,
>>> VM-specific, and passed in as unit 1.
>>>
>>>   00000000ffe00000-00000000ffe1ffff (prio 0, R-): system.flash1
>>>   00000000ffe20000-00000000ffffffff (prio 0, R-): system.flash0
>>>
>>> (If the guest tries to write to the flash range that is backed by the
>>> read-only drive, bdrv_write() in pflash_update() will correctly deny the
>>> write with -EACCES, and pflash_update() won't care, which suits us well.)
>> 
>> Before this patch:
>> 
>> You can define as many if=pflash drives as you want.  Any with non-zero
>> index are silently ignored.
>> 
>> If you don't specify one with index=0, you get a ROM at the top of the
>> 32 bit address space, contents taken from -bios (default "bios.bin").
>> Up to its last 128KiB are aliased at the top of the ISA address space.
>> 
>> If you do specify one with index=0, you get a pflash device at the top
>> of the 32 bit address space, with contents from the drive, and -bios is
>> silently ignored.  Up to its last 128KiB are copied to a ROM at the top
>> of the (20 bit) ISA address space.
>> 
>> After this patch (please correct misunderstandings):
>> 
>> Now the drives after the first unused index are silently ignored.
>> 
>> If you don't specify one with index=0, no change.
>> 
>> If you do, you now get N pflash devices, where N is the first unused
>> index.  Each pflash's contents is taken from the respective drive.  The
>> flashes are mapped at the top of the 32 bit address space in reverse
>> index order.  -bios is silently ignored, as before.  Up to the last
>> 128KiB of the index=0 flash are copied to a ROM at the top of the ISA
>> address space.
>> 
>> Thus, no change for index=0.  For index=1..N, we now get additional
>> flash devices.
>> 
>> Correct?
>
> Yes.

Thanks.

1. Multiple pflash devices

Is there any way for a guest to see the N separate pflash devices, or do
they look exactly like a single pflash device?

2. More board code device configuration magic

Our long term goal is to configure machines by configuration files
(i.e. data) rather than code.

Your patch extends the PC board code dealing with if=pflash for multiple
drives.

Reminder: -drive if=X (where X != none) creates a backend, and leaves it
in a place where board code can find it.  Board code may elect to create
a frontend using that backend.

It's desirable that any new board code creating a frontend for -drive
if=X,... is sufficiently simple so that users can get the same result
with some -drive if=none,... -device ... incantation.  The second form
provides full control over device properties.  See section "Block
Devices" in docs/qdev-device-use.txt for examples of such mappings.

This is the case for if=ide, if=scsi, if=floppy and if=virtio (see
docs/qdev-device-use.txt).  It's not yet the case for if=pflash, because
the qdev used with it (cfi.pflash01) is a sysbus device, and these
aren't available with -device, yet.

When that gets fixed, I'd expect support for putting the flash device at
a specific address.  An equivalent if=none incantation (free of board
code magic) should be possible.

Thus, the board magic your patch adds should be of the harmless kind.