On 8 November 2011 06:33, Avi Kivity <a...@redhat.com> wrote: > On 11/08/2011 04:07 AM, Peter Maydell wrote: >> 2011/10/26 Peter Maydell <peter.mayd...@linaro.org>: >> > On 25 October 2011 12:09, Benoît Canet <benoit.ca...@gmail.com> wrote: >> >> +static const VMStateDescription vmstate_integratorcm = { >> >> + .name = "integratorcm", >> >> + .version_id = 1, >> >> + .minimum_version_id = 1, >> >> + .minimum_version_id_old = 1, >> >> + .fields = (VMStateField[]) { >> >> + VMSTATE_UINT32(memsz, integratorcm_state), >> >> + VMSTATE_BOOL(flash_mapped, integratorcm_state), >> > >> > This raises a question. flash_mapped here is a flag which just >> > tracks whether the associated MemoryRegion is currently mapped >> > or unmapped. Do we need to do anything special to ensure that >> > the MemoryRegion itself is reset to the correct mapped/unmapped >> > state on restore? >> > >> > I recall discussing this kind of thing with Avi on IRC but I >> > can't remember what the conclusion was. >> >> Avi, ping? I'm still interested in the answer to this question. > > Sorry, missed this. Yes, you need to ensure the memory region mapping > reflects flash_mapped.
This needs to be in the MemoryRegion core implementation, please. I don't want to have to redo it for every device that has a remappable region. In particular, at the moment memory.c's add/delete region functions will assert if the region was already added/deleted, which means the device has to keep track in a not-vm-saved state flag whether the memory was mapped so it can resync things on load (by comparing the non-saved flag and the saved-state). Ideally memory.c should just ensure that the memory hierarchy is saved and restored without devices having to do anything. > btw, is flash_mapped a real device state (corresponds to a real memory > bit on the device) or just an artefact? It's usually a bad idea to cast > implementation artefacts in vmstate concrete. It's an implementation artefact that you introduced when you did the memoryregion conversion :-) I have a half-a-patch to fix that lurking somewhere but it stalled because at the moment the non-saved flag is required in order to work around memory.c being unhelpful. -- PMM