On Sun, Jan 27, 2013 at 4:38 PM, David Woodhouse <dw...@infradead.org> wrote: > 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.
That problem could be easily solved by allowing a combination of two images with different RO/RW settings, for example -bios bios.bin[,offset=0,ro] -bios flash.img, offset=0x8000,rw. > 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. So the duplication already exists. :-( > > -- > dwmw2 >
