Re: [libvirt] [Qemu-devel] Configuring pflash devices for OVMF firmware

Hi Markus, On 02/07/19 10:30, Markus Armbruster wrote: > The thread got long, let me try to summarize, and elaborate a few > points. > > * The problem at hand is configuring firmware residing in flash memory > (OVMF requires this) without legacy -drive. > > * The wider problem is configuring onboard devices. Our general device > configuration interface doesn't cover them. Instead, we have a zoo of > ad hoc interfaces that are much more limited. Some of them we'd > rather deprecate (-drive, -net), but can't until we have a suitable > replacements. > > I think a board should be a composite object that exposes properties > of its own and its parts, just like other composite devices, so that > "create, set properties, realize" just works. That would extend our > common device configuration mechanism naturally to onboard devices. > > A PC board's flash memory device would be just another part. It could > be something like /machine/q35/cfi.pflash01/ in the QOM tree. To > configure it, you'd set its properties, such as > /machine/q35/cfi.pflash01/drive. > > Note that this requires a way to set an existing device's properties. > Perhaps qom-set already works. > > * While I do believe we should tackle the wider problem, I'd rather not > sit on the narrow problem until we crack it. So, what can we do about > it? > > - Paolo proposed to add block backend properties to the PC machine, > settable like -machine pflash0=BLOCK-BACKEND. > > Possible drawback: if we add /machine/q35/pflash0 to the QOM tree > now, and later replace it by /machine/q35/cfi.pflash01/drive, we'll > have to deal with yet another machine type variation. We'll live. > > - I proposed to sidestep our onboard device configuration problem by > adding the cfi.pflash01 devices with our existing general device > configuration interface: -device. Possible since the onboard > cfi.pflash01 devices are optional. Requires a small extension to > the firmware descriptors, and a bit of extra work in libvirt to > process that extension. I think it's workable, but Paolo's idea is > simpler. > > I can give Paolo's idea a try. Objections? > > * A flash device supporting multiple regions is desirable, because it's > what physical hardware has. We currently use multiple flash devices > instead. We'll be stuck with them for existing machine types due to > guest ABI and migration compatibility. > > * cfi.pflash01 currently requires users to opt out of "bad, do not use". > It should require opt in, to guard against accidental new uses of > "bad". > > > PS: Big thanks to László, whose patient guidance helped me map this part > of the jungle. > I've read the above carefully. At the QEMU design level, I don't have any opinion or preference; there I simply don't know enough -- and don't suffer from bad decisions enough -- to make sensible comments. Regarding the choice betwen "-machine pflash0=BLOCK-BACKEND" and "-device pflash": I don't object to exploring the former first. I'd just like to note that "-machine pflash0=BLOCK-BACKEND" will also require changes to the firmware descriptor schema. Not to the types that the schema defines -- and therefore concrete descriptor *documents* that already conform to the schema wouldn't be affected --, but to the documentation that the schema directs at management applications. The schema is supposed to specify (in the documentation) QEMU command line options for management applications. If we add "-machine pflash0=BLOCK-BACKEND", then even if the types in the schema stay the same, some mappings to the QEMU cmdline will have to be re-documented.

Of course, that's still easier / less intrusive than changing the types! ... Which does make me prefer "-machine pflash0=BLOCK-BACKEND", if I'm being honest. (I hope my followup isn't totally useless. I certainly didn't want to ignore your summary.)