Hi Peter,
On 13/03/2017 19:03, Peter Maydell wrote:
> On 6 March 2017 at 12:48, Eric Auger <eric.au...@redhat.com> wrote:
>> We change the restoration priority of both the GICv3 and ITS. The
>> GICv3 must be restored before the ITS and the ITS needs to be restored
>> before PCIe devices since it translates their MSI transactions.
>
>> We typically observe the virtio-pci-net device sending MSI transactions
>> very early (even before the first vcpu run) which looks weird. It
>> appears that not servicing those transactions cause the virtio-pci-net
>> to stall.
>
> This does seem rather weird and worth closer investigation.
> A stopped VM ought IMHO to be completely stopped, not still
> doing things...
After further investigations, here is what I observe:
Between the "running" vm_change_state_handler and the 1st
KVM_RUN I observe virtio_notify_config that calls
msix_notify(). This eventually attempts to inject an MSI.
This looks valid as the VM is in its running state.
Unfortunately the VGIC gets its final init stage, map_resources(),
executed on the first KVM_RUN. Until that point
no MMIO access can be performed. This looks the bad part to me.
So at the moment I trigger a map_resources() at the end of the ITS table
flush and that works.
If I don't do that, the virtio-net based network is not functional
on restore path.
Hope this clarifies.
Thanks
Eric
>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - handle case where migrate_add_blocker fails
>> - add comments along with ITS and GICv3 migration priorities
>> ---
>> hw/intc/arm_gicv3_common.c | 1 +
>> hw/intc/arm_gicv3_its_common.c | 3 ++-
>> hw/intc/arm_gicv3_its_kvm.c | 23 +++++++++++------------
>> include/migration/vmstate.h | 2 ++
>> 4 files changed, 16 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index c6493d6..4228b7c 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -145,6 +145,7 @@ static const VMStateDescription vmstate_gicv3 = {
>> .minimum_version_id = 1,
>> .pre_save = gicv3_pre_save,
>> .post_load = gicv3_post_load,
>> + .priority = MIG_PRI_GICV3,
>> .fields = (VMStateField[]) {
>> VMSTATE_UINT32(gicd_ctlr, GICv3State),
>> VMSTATE_UINT32_ARRAY(gicd_statusr, GICv3State, 2),
>> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
>> index 75b9f04..854709f 100644
>> --- a/hw/intc/arm_gicv3_its_common.c
>> +++ b/hw/intc/arm_gicv3_its_common.c
>> @@ -48,7 +48,8 @@ static const VMStateDescription vmstate_its = {
>> .name = "arm_gicv3_its",
>> .pre_save = gicv3_its_pre_save,
>> .post_load = gicv3_its_post_load,
>> - .unmigratable = true,
>> + .unmigratable = false,
>
> unmigratable = false is the default, so we can just delete the line.
>
>> + .priority = MIG_PRI_GICV3_ITS,
>> .fields = (VMStateField[]) {
>> VMSTATE_UINT32(ctlr, GICv3ITSState),
>> VMSTATE_UINT64(cbaser, GICv3ITSState),
>> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
>> index 45e57d6..3bd2873 100644
>> --- a/hw/intc/arm_gicv3_its_kvm.c
>> +++ b/hw/intc/arm_gicv3_its_kvm.c
>> @@ -76,18 +76,6 @@ static void kvm_arm_its_realize(DeviceState *dev, Error
>> **errp)
>> GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
>> Error *local_err = NULL;
>>
>> - /*
>> - * Block migration of a KVM GICv3 ITS device: the API for saving and
>> - * restoring the state in the kernel is not yet available
>> - */
>> - error_setg(&s->migration_blocker, "vITS migration is not implemented");
>> - migrate_add_blocker(s->migration_blocker, &local_err);
>> - if (local_err) {
>> - error_propagate(errp, local_err);
>> - error_free(s->migration_blocker);
>> - return;
>> - }
>> -
>> s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_ITS,
>> false);
>> if (s->dev_fd < 0) {
>> error_setg_errno(errp, -s->dev_fd, "error creating in-kernel ITS");
>> @@ -104,6 +92,17 @@ static void kvm_arm_its_realize(DeviceState *dev, Error
>> **errp)
>>
>> gicv3_its_init_mmio(s, NULL);
>>
>> + if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
>> + GITS_CTLR)) {
>> + error_setg(&s->migration_blocker, "vITS migration is not
>> implemented");
>
> I think we should specifically say that it's the host kernel
> that doesn't support ITS migration (ie not QEMU that's missing
> support).
>
> The GICv3 uses the message
> "This operating system kernel does not support vGICv3 migration"
> which is slightly odd phrasing but I guess we should follow that.
>
>> + migrate_add_blocker(s->migration_blocker, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + error_free(s->migration_blocker);
>> + return;
>> + }
>> + }
>> +
>> kvm_msi_use_devid = true;
>> kvm_gsi_direct_mapping = false;
>> kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index f2dbf84..8dab9c7 100644
>> --- a/include/migration/vmstate.h
>> +++ b/include/migration/vmstate.h
>> @@ -198,6 +198,8 @@ enum VMStateFlags {
>> typedef enum {
>> MIG_PRI_DEFAULT = 0,
>> MIG_PRI_IOMMU, /* Must happen before PCI devices */
>> + MIG_PRI_GICV3_ITS, /* Must happen before PCI devices */
>> + MIG_PRI_GICV3, /* Must happen before the ITS */
>> MIG_PRI_MAX,
>> } MigrationPriority;
>>
>> --
>> 2.5.5
>>
>
> thanks
> -- PMM
>
