Re: [Qemu-devel] [PATCH 4/5] integratorcp: convert integratorcm to VMState

[Avi Kivity <avi@redhat.com>]

On 11/08/2011 03:50 PM, Anthony Liguori wrote:
>> We agree, the only difference is in what "core" refers to.  I don't want
>> memory.c do become intermingled with everything else.  It should be in a
>> separate layer.  Devices would then talk to this layer, and not to the
>> gluing by themselves as they have to now.
>>
>> Or maybe memory.c will be layered on top of QOM, and then it can take
>> care of everything.  I really don't know QOM well enough to say.
>> Copying Anthony for more insight.
>
>
> QOM fixes all problems, but I don't think this has anything to do with
> QOM :-)
>
> We fundamentally should do save/restore using a rigorous,
> automatically generated mechanism where every field is save/restored[1].  

"fundamentally", "rigrous", "automatically", "generated", "mechanism",
"every", "field", and even "a" are all words that I associate with QOM. 
Am I misunderstanding anything?

> That means we should have a VMSTATE_MEMORY_REGION().
>
> VMSTATE_MEMORY_REGION should save off the state of the memory region,
> and restore it appropriately.  VMSTATE_MEMORY_REGION's implementation
> does not need to live in memory.c.  It can certainly live in savevm.c
> or somewhere else more appropriate.

What state is that?  Some devices have fixed size, offset, parent, and
enable/disable state (is there a word for that?), so there is no state
that needs to be transferred.  For other devices this is all dynamic.

The way I see it, we create a link between some device state (a
register) and a memory API field (like the offset).  This way, when one
changes, so does the other.  In complicated devices we'll have to write
a callback.

>
> Where the device is mapped within the address space is no longer a
> property of the device, so restoring the mapping really should happen
> at whatever owns the address space (in this case sysbus).
>
> In this case, integratorcp is the one mapping things into its own
> address space so flash_mapped ought to be saved by integratorcp.

flash_mapped always reflects a bit in a real register.  We shouldn't
duplicate state.

> [1] Deciding that a field doesn't need to be saved should be an
> exception.

It should indicate that the field doesn't need to exist.

-- 
error compiling committee.c: too many arguments to function