Re: [question] Shall we flush ITS tables into guest RAM when shutdown the VM?

[Eric Auger]

Hi Kunkun,

On 6/29/21 11:33 AM, Kunkun Jiang wrote:
> Hi all,
>
> Accroding to the patch cddafd8f353d2d251b1a5c6c948a577a85838582,
> our original intention is to flush the ITS tables into guest RAM at
> the point
> RUN_STATE_FINISH_MIGRATE, but sometimes the VM gets stopped before
> migration launch so let's simply flush the tables each time the VM gets
> stopped.
>
> But I encountered an error when I shut down the virtual machine.
>
>> qemu-system-aarch64: KVM_SET_DEVICE_ATTR failed: Group 4 attr
>> 0x0000000000000001: Permission denied
>
> Shall we need to flush ITS tables into guest RAM when 'shutdown' the VM?
> Or do you think this error is normal?
yes we determined in the past this was the right moment do save the tables

"with a live migration the guest is still running after
the RAM has been first saved, and so the tables may still change
during the iterative RAM save. You would actually need to do this
at just the point we stop the VM before the final RAM save; that *might*
be possible by using a notifier hook a vm run state change to 
RUN_STATE_FINISH_MIGRATE
- if you can do the changes just as the migration flips into that mode
it *should* work. " said David. 

But sometimes as the commit msg says, the VM is stopped before the migration 
launch - I do not remember the exact scenario tbh -.
So each time the VM is stopped we flush the caches into guest RAM.



>
> This error occurs in the following scenario:
> Kunpeng 920 、enable GICv4、passthrough a accelerator Hisilicon SEC to
> the VM.
>
> The flow is as follows:
>
> QEMU:
> vm_shutdown
>     do_vm_stop(RUN_STATE_SHUTDOWN)
>         vm_state_notify
>             ...
>             vm_change_state_handler (hw/intc/arm_gicv3_its_kvm.c)
>                 kvm_device_access
>
> Kernel:
>     vgic_its_save_tables_v0
>         vgic_its_save_device_tables
>             vgic_its_save_itt
>
> There is such a code in vgic_its_save_itt():
>> /*
>>  * If an LPI carries the HW bit, this means that this
>>  * interrupt is controlled by GICv4, and we do not
>>  * have direct access to that state without GICv4.1.
>>  * Let's simply fail the save operation...
>>  */
>> if (ite->irq->hw && !kvm_vgic_global_state.has_gicv4_1)
>>           return -EACCES;
As far as I understand you need a v4.1 to migrate,
following Shenming's series
[PATCH v5 0/6] KVM: arm64: Add VLPI migration support on GICv4.1
Maybe sync with him?

Thanks

Eric


>
> Looking forward to your reply.
>
> Thanks,
> Kunkun Jiang
>
>
>
>
>
>
>