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

[David Gibson]

On Wed, Sep 26, 2012 at 03:03:10AM +0200, Alexander Graf wrote:
> 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:
[snip]
> >>> 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"]);

Ok.  That's what I do now.

> spapr-nvram:
> 
>   if (!drive || checksum_is_bad(drive))
>     autogenerate_nvram_contents();

Actually, I'm planning for the initialization of the content to be
done from the guest firmware.


> Then we can later add in vl.c:
> 
>   case OPTION_nvram:
>     create_drive("nvram", option);
>     machine_opts["nvram"] = drive["nvram"];

Ok, that all works for me.

Blue, does that seem reasonable to you?

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson