Eric Auger <eric.au...@redhat.com> wrote:
> Hi Dave,
>
> On 7/6/21 4:19 PM, Dr. David Alan Gilbert wrote:
>> * Eric Auger (eric.au...@redhat.com) wrote:
...
>>>>>> Well, I initially wanted to know more about this scenario to determine
>>>>>> whether
>>>>>> a normal shutdown would fall into it.😂
>>>>> I think it was for save/restore use case. In that case you need to flush
>>>>> the KVM cache in memory on VM shutdown.
>>>> Sorry for late reply.
>>>>
>>>> Can we distinguish from the 'RunState'?
>>>> When we stop the VM, the RunState will be set. There are many types of
>>>> RunState, such as RUN_STATE_FINISH_MIGRATE/RUN_STATE_SAVE_VM/
>>>> RUN_STATE_SHUTDOWN/RUN_STATE_GUEST_PANICKED, etc.
>>>>
>>>> Maybe RUN_STATE_SHUTDOWN doesn't belong to save/restore use case,
>>>> right?
>>> Adding Dave, Juan and Peter in the loop for migration expertise.
>>>
>>> At the moment we save the ARM ITS MSI controller tables whenever the VM
>>> gets stopped. Saving the tables from KVM caches into the guest RAM is
>>> needed for migration and save/restore use cases.
>>> However with GICv4 this fails at KVM level because some MSIs are
>>> forwarded and saving their state is not supported with GICv4.
>>>
>>> While GICv4 migration is not supported we would like the VM to work
>>> properly, ie. being stoppable without taking care of table saving.
>>>
>>> So could we be more precise and identifiy the save/restore and migration
>>> use cases instead of saving the tables on each VM shutdown.
>> During the precopy migration (not sure about others), we do:
>>
>> static void migration_completion(MigrationState *s)
>> {
>> ....
>> ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
>> ...
>> ret = qemu_savevm_state_complete_precopy(s->to_dst_file,
>> false,
>> inactivate);
>>
>> so I think we do have that state there to hook off.
>
> That's consistent with what you suggested in the past ans what is logged
> in the commit message of
>
> cddafd8f353d2d251b1a5c6c948a577a85838582 ("hw/intc/arm_gicv3_its:
> Implement state save/restore").
Hi
Ouch, it is really a mess. Why do we need to save it to RAM instead of
saving it to anywhere else?
I guess that the answer is that we don't want to know what the state is,
so we are mgrating a opaque blob.
> However does the save/restore enters that state. If I remember
> correctly that's why I decided to do the save on each VM stop instead.
>>> The tables are saved into guest RAM so when need the CPUs and devices to
>>> be stopped but we need the guest RAM to be saved after the ITS save
>>> operation.
Saving this data into RAM dirties the bitmaps, right?
>> Yeh so what should happen is that you:
>> a) Iterate RAM a lot
>> b) You stop everything
>> -> Flushes remaining changes into RAM
>> c) Transmit device state and last bits of RAM changes.
>>
>> so that flush should happen at (b).
> That's correct.
/* does a state transition even if the VM is already stopped,
current state is forgotten forever */
int vm_stop_force_state(RunState state)
{
if (runstate_is_running()) {
return vm_stop(state);
} else {
int ret;
runstate_set(state);
bdrv_drain_all();
/* Make sure to return an error if the flush in a previous vm_stop()
* failed. */
ret = bdrv_flush_all();
trace_vm_stop_flush_all(ret);
return ret;
}
}
You really want to hook here, like the block layer.
But as far as I can see, there is no generic way to put a hook there.
And the path is different if the machine is running or not.
Thinking about how to put a hook there.
Welcome if you have a good name for the hook.
Later, Juan.
