On Sun, 2013-01-27 at 16:02 +0000, Blue Swirl wrote: > On Sun, Jan 27, 2013 at 3:50 PM, David Woodhouse <dw...@infradead.org> wrote: > > On Sun, 2013-01-27 at 15:14 +0000, Blue Swirl wrote: > >> It looks like this duplicates rom_add_file() and fw_cfg_add_file(), so > >> I don't see the point. > > > > Both of those are read-only, surely? The firmware inside the guest can't > > use them for non-volatile storage. > > Right. But why do we need PV non-volatile storage, instead of normal flash?
People have looked at adding normal flash to the PC target, but it has a number of problems. Firstly, it doesn't even *work* with KVM enabled. But even if we were to somehow fix that, the simple approach of emulating a BIOS flash is also very suboptimal; it puts the whole of the firmware *into* the flash and lets it store its non-volatile variables in the same emulated flash chip. So if you want to update the firmware (which would normally be just a case of updating the bios.bin file), you end up having to do each guest system individually *and* the update is painful because you have to preserve the non-volatile storage while updating the other parts. It's much better to have a separate storage for the non-volatile variables. Yes, we *could* teach it to use a dedicated flash chip for that rather than using flash for the firmware *and* storage as we would on real hardware. But if we're going to do something different for the virtualised case, a writeable fw_cfg file seems a whole lot easier and saner. > > It doesn't duplicate fw_cfg_add_file(); it extends it to allow a > > writeable mode too. fw_cfg_add_file() becomes a simple wrapper around > > fw_cfg_add_file_writeable(), which actually contains most of the > > contents of the original function. > > OK, however most of the loading logic is now in loader.c and that > should not be duplicated either. I actually got file_get_size() from pc.c, and had marked it with a horrid C99 comment so we couldn't miss the duplication. I'll sort out that kind of detail later, if we can reach consensus on the basic approach. -- dwmw2
smime.p7s
Description: S/MIME cryptographic signature