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

[Alexander Graf]

On 24.09.2012, at 14:25, David Gibson wrote:

> On Mon, Sep 24, 2012 at 12:36:33PM +0200, Alexander Graf wrote:
>> On 21.09.2012, at 05:08, David Gibson wrote:
> [snip]
>>> +void spapr_create_nvram(sPAPREnvironment *spapr)
>>> +{
>>> +    QemuOpts *machine_opts;
>>> +    DeviceState *dev;
>>> +
>>> +    dev = qdev_create(&spapr->vio_bus->bus, "spapr-nvram");
>>> +
>>> +    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>>> +    if (machine_opts) {
>>> +        const char *drivename;
>>> +
>>> +        drivename = qemu_opt_get(machine_opts, "nvram");
>>> +        if (drivename) {
>>> +            BlockDriverState *bs;
>>> +
>>> +            bs = bdrv_find(drivename);
>>> +            if (!bs) {
>>> +                fprintf(stderr, "No such block device \"%s\" for nvram\n",
>>> +                        drivename);
>>> +                exit(1);
>> 
>> I thought you wanted to autogenerate contents in RAM if no drive is 
>> available?
> 
> We do.  This is the case where the user *has* specified a drive id,
> but a drive of that name does not exist.

Ah :).

> 
>>> +            }
>>> +            qdev_prop_set_drive_nofail(dev, "drive", bs);
>>> +        }
>>> +    }
>>> +
>>> +    qdev_init_nofail(dev);
>>> +
>>> +    spapr->nvram = (sPAPRNVRAM *)dev;
>>> +    spapr_rtas_register("nvram-fetch", rtas_nvram_fetch);
>>> +    spapr_rtas_register("nvram-store", rtas_nvram_store);
>>> +}
>>> diff --git a/qemu-config.c b/qemu-config.c
>>> index 12eafbb..1cd9a1b 100644
>>> --- a/qemu-config.c
>>> +++ b/qemu-config.c
>>> @@ -619,6 +619,10 @@ static QemuOptsList qemu_machine_opts = {
>>>            .name = "mem-merge",
>>>            .type = QEMU_OPT_BOOL,
>>>            .help = "enable/disable memory merge support",
>>> +        }, {
>>> +            .name = "nvram",
>>> +            .type = QEMU_OPT_STRING,
>>> +            .help = "Drive backing persistent NVRAM",
>> 
>> I like the idea of a machine implemented NVRAM. Maybe we should add
>> an -nvram option that does an automatic -drive and -machine
>> ...,machine=drive_id for the user :).
> 
> Uh, sure, whatever.  I don't really care what syntactic sugar is
> added, as long as the basic thing is working.

It certainly works for me :).


Alex