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

[Anthony Liguori <anthony@codemonkey.ws>]

On 11/08/2011 06:38 AM, Avi Kivity wrote:
> On 11/08/2011 02:30 PM, Peter Maydell wrote:
>> On 8 November 2011 12:21, Avi Kivity<avi@redhat.com>  wrote:
>>> On 11/08/2011 02:15 PM, Peter Maydell wrote:
>>>> This needs to be in the MemoryRegion core implementation, please.
>>>
>>> Right; see the memory/mutators branch.  I plan to push this as soon as
>>> everything is converted.
>>
>> OK, then this save/restore patch should wait until that has landed.
>
> Please don't add interdependencies, especially as I can't promise a firm
> date as to when everything is converted (however I'll take this
> opportunity to remind you that patches, especially for omap, are more
> welcome than I can possibly articulate).
>
>>>> Ideally memory.c should just ensure that the memory hierarchy
>>>> is saved and restored without devices having to do anything.
>>>
>>> That's a bit far-fetched - I don't want the memory core involved in
>>> save/restore.  But if/when we integrate the memory core into QOM, then
>>> yes, that layer can take care of it.  If we have an observable attribute
>>> (that fires off a callback when changed), we can link memory API
>>> properties into that attribute.
>>
>> The memory hierarchy is a visible part of the state which can
>> change as the guest does things -- it has to get saved and
>> restored somehow. I think it would be better done in the core
>> where it will always be done right, rather than relying on
>> each device to handle it correctly (especially since it's not
>> always obvious if it's been missed out from the device.)
>
> 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].  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.

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.

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

Regards,

Anthony Liguori

>