[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [question] Shall we flush ITS tables into guest RAM when shutdown th

From: Juan Quintela
Subject: Re: [question] Shall we flush ITS tables into guest RAM when shutdown the VM?
Date: Thu, 22 Jul 2021 15:19:03 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux)

Eric Auger <eric.auger@redhat.com> wrote:
> Hi Dave,
> On 7/6/21 4:19 PM, Dr. David Alan Gilbert wrote:
>> * Eric Auger (eric.auger@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
>>>> 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").


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;

        /* Make sure to return an error if the flush in a previous vm_stop()
         * failed. */
        ret = bdrv_flush_all();
        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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]