Re: [Qemu-devel] [PATCH 1/5] Add TPM 1.2 device interface

[Stefan Berger]

On 02/18/2011 10:33 AM, Andreas Niederl wrote:
> +
> +static const VMStateDescription vmstate_loc = {
> +    .name       = "loc",
> +    .version_id = 0,
> +    .fields     = (VMStateField []) {
> +        VMSTATE_UINT32(state, TPMLocality),
> +        VMSTATE_UINT32(inte,  TPMLocality),
> +        VMSTATE_UINT32(ints,  TPMLocality),
> +        VMSTATE_UINT8(access, TPMLocality),
> +        VMSTATE_UINT8(sts,    TPMLocality),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_tpm = {
> +    .name       = "tpm",
> +    .version_id = 0,
> +    .minimum_version_id     = 0,
> +    .minimum_version_id_old = 0,
> +    .pre_save   = tpm_pre_save,
> +    .fields     = (VMStateField []) {
> +        VMSTATE_UINT32(irq,    TPMState),
> +        VMSTATE_UINT32(offset, TPMState),
> +        VMSTATE_BUFFER(buf,    TPMState),
> +        VMSTATE_UINT8(  active_locty, TPMState),
> +        VMSTATE_UINT8(aborting_locty, TPMState),
> +        VMSTATE_UINT8(    next_locty, TPMState),
> +        VMSTATE_STRUCT_ARRAY(loc, TPMState, NUM_LOCALITIES, 0,
> +                             vmstate_loc, TPMLocality),
Here you are pointing to the above vmstate_loc ...
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +/*

> +
> +    vmstate_register(&dev->qdev, 1,&vmstate_tpm, s);
> +    for (i = 0; i<  NUM_LOCALITIES; i++) {
> +        vmstate_register(&dev->qdev, 1,&vmstate_loc,&s->loc[i]);
... here you are (again) registering the localities ...
> +    }
> +
> +    isa_init_irq(dev,&s->pic, isairq);
> +    s->irq = isairq;
> +
> +    iomem = cpu_register_io_memory(tis_readfn, tis_writefn, s,
> +            DEVICE_LITTLE_ENDIAN);
> +    cpu_register_physical_memory(TIS_ADDR_BASE,
> +            0x1000 * NUM_LOCALITIES, iomem);
> +
> +    s->poll_timer = qemu_new_timer(vm_clock, tis_poll_timer, s);
> +
> +    return 0;
> +}
> +
> +static void qdev_tpm_reset(DeviceState *dev)
> +{
> +    TPMState *s = DO_UPCAST(TPMState, dev.qdev, dev);
> +    tpm_reset(s);
> +}
> +
> +static ISADeviceInfo tpm_info = {
> +    .init          = tpm_init,
> +    .qdev.name     = "tpm",
> +    .qdev.desc     = "TPM TIS Interface",
> +    .qdev.size     = sizeof (TPMState),
> +    .qdev.vmsd     =&vmstate_tpm,
... and here you're pointing to vmstate_tpm, which was also registered 
above using

+
+    vmstate_register(&dev->qdev, 1,&vmstate_tpm, s);


Are you sure this is necessary? In my implementation I only have the 
qdev.vmsd set and need not explicitly use vmstate_register(). Aren't you 
writing the state two times if you do that?

   Stefan