Re: [Qemu-devel] [Qemu-ppc] RFC: NVRAM for pseries machine

[Alexander Graf]

On 26.09.2012, at 02:27, David Gibson wrote:

> On Mon, Sep 24, 2012 at 12:38:59PM +0200, Alexander Graf wrote:
>> 
>> On 24.09.2012, at 02:31, David Gibson wrote:
>> 
>>> On Sat, Sep 22, 2012 at 01:31:08PM +0000, Blue Swirl wrote:
>>>> On Fri, Sep 21, 2012 at 3:08 AM, David Gibson
>>>> <address@hidden> wrote:
>>>>> Below is a patch which implements the (PAPR mandated) NVRAM for the
>>>>> pseries machine.  It raises a couple of generic questions.
>>>>> 
>>>>> First, this adds a new "nvram" machine option which is used to give a
>>>>> block device id to back the NVRAM so it is persistent.  Since some
>>>>> sort of NVRAM is quite common, it seems this might be useful on other
>>>>> machines one day, although obviously nothing else implements it yet.
>>>> 
>>>> Yes, there have been discussions earlier since loading NVRAM contents
>>>> from a file would be useful for many architectures too.
>>>> 
>>>>> 
>>>>> Second, if a block device is not specified, it simply allocates a
>>>>> block of memory to make a non-persistent NVRAM.  Obviously that isn't
>>>>> really "NV", but it's enough to make many guests happy most of the
>>>>> time, and doesn't require setting up an image file and drive.  It does
>>>>> mean a different set of code paths in the driver though, and it will
>>>>> need special case handling for savevm (not implemented yet).  Is this
>>>>> the right approach, or should I be creating a dummy block device for a
>>>>> one-run NVRAM of this kind?  I couldn't see an obvious way to do that,
>>>>> but maybe I'm missing something.
>>>> 
>>>> That was the problem earlier too, it looks like a generic way for all
>>>> NVRAM/flash devices should be obvious but so far nobody has been able
>>>> to propose something.
>>>> 
>>>> What if there are two devices which could use this, for example CMOS
>>>> and flash on x86?
>>>> 
>>>> This should be extending  -device syntax rather than adding another
>>>> top level option. Something like
>>>> -drive foo,file=nvram.qcow2,format=qcow2,id=main_nvram -device
>>>> spapr-nvram,drive_id=main_nvram
>>> 
>>> So, if you look at the patch there is actually a -device form within
>>> there, the machine option is a wrapper around it.  Without the machine
>>> option, I don't see how to get the desired properties for the
>>> configuration that is:
>>> * NVRAM is always instantiated by default (even if it's
>>> non-persistent)
>>> * It's easy to set the drive for that always-present NVRAM
>> 
>> I suppose the idea is that when creating a machine from a qtree
>> dump, we can still recreate it. Or maybe when using -nodefaults? Not
>> sure. But the way you do it right now is very close to how we want
>> to model USB too, so I do like it. It's consistent.
> 
> I really don't follow what point you're making here.
> 
> The problem with -device syntax for my purpose is that with *no* extra
> command line arguments we should always have some sort of NVRAM - it's
> mandated by the platform spec, and should always be there, just like
> the PCI bridge and VIO bridge.  That means instantiating the device
> from the machine setup code.  But then, using -device will create a
> second instance of the device, which is no good, because only one can
> actually be used.

What I'm trying to say is that the machine file should create a device. Always 
in the case of PAPR. But I suppose pseudo-code is easier to read:

spapr.c:

  create_device("spapr-nvram", drive=machine_opts["nvram"]);

spapr-nvram:

  if (!drive || checksum_is_bad(drive))
    autogenerate_nvram_contents();

Then we can later add in vl.c:

  case OPTION_nvram:
    create_drive("nvram", option);
    machine_opts["nvram"] = drive["nvram"];


Alex