Hi Eric,
On Thu, Jan 26, 2017 at 2:49 PM, Eric Auger <eric.au...@redhat.com> wrote:
> We need to handle both registers and ITS tables. While
> register handling is standard, ITS table handling is more
> challenging since the kernel API is devised so that the
> tables are flushed into guest RAM and not in vmstate buffers.
>
> Flushing the ITS tables on device pre_save() is too late
> since the guest RAM had already been saved at this point.
>
> Table flushing needs to happen when we are sure the vcpus
> are stopped and before the last dirty page saving. The
> right point is 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.
>
> For regular ITS registers we just can use vmstate pre_save
> and post_load callbacks.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>
> ---
> ---
> hw/intc/arm_gicv3_its_common.c | 8 ++++
> hw/intc/arm_gicv3_its_kvm.c | 86
> ++++++++++++++++++++++++++++++++++
> include/hw/intc/arm_gicv3_its_common.h | 6 +++
> 3 files changed, 100 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
> index 9d67c5c..75b9f04 100644
> --- a/hw/intc/arm_gicv3_its_common.c
> +++ b/hw/intc/arm_gicv3_its_common.c
> @@ -49,6 +49,14 @@ static const VMStateDescription vmstate_its = {
> .pre_save = gicv3_its_pre_save,
> .post_load = gicv3_its_post_load,
> .unmigratable = true,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(ctlr, GICv3ITSState),
> + VMSTATE_UINT64(cbaser, GICv3ITSState),
> + VMSTATE_UINT64(cwriter, GICv3ITSState),
> + VMSTATE_UINT64(creadr, GICv3ITSState),
> + VMSTATE_UINT64_ARRAY(baser, GICv3ITSState, 8),
> + VMSTATE_END_OF_LIST()
> + },
> };
>
> static MemTxResult gicv3_its_trans_read(void *opaque, hwaddr offset,
> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index fc246e0..3f8017d 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -53,6 +53,24 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t
> value, uint16_t devid)
> return kvm_vm_ioctl(kvm_state, KVM_SIGNAL_MSI, &msi);
> }
>
> +/**
> + * vm_change_state_handler - VM change state callback aiming at flushing
> + * ITS tables into guest RAM
> + *
> + * The tables get flushed to guest RAM whenever the VM gets stopped.
> + */
> +static void vm_change_state_handler(void *opaque, int running,
> + RunState state)
> +{
> + GICv3ITSState *s = (GICv3ITSState *)opaque;
> +
> + if (running) {
> + return;
> + }
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_TABLES,
> + 0, NULL, false);
> +}
> +
> static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
> {
> GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
> @@ -83,6 +101,8 @@ static void kvm_arm_its_realize(DeviceState *dev, Error
> **errp)
> kvm_msi_use_devid = true;
> kvm_gsi_direct_mapping = false;
> kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
> +
> + qemu_add_vm_change_state_handler(vm_change_state_handler, s);
> }
>
> static void kvm_arm_its_init(Object *obj)
> @@ -96,6 +116,70 @@ static void kvm_arm_its_init(Object *obj)
> &error_abort);
> }
>
> +/**
> + * kvm_arm_its_get - handles the saving of ITS registers.
> + * ITS tables, being flushed into guest RAM needs to be saved before
> + * the pre_save() callback, hence the migration state change notifiers
> + */
> +static void kvm_arm_its_get(GICv3ITSState *s)
> +{
> + uint64_t reg;
> + int i;
> +
Don't we need to check for LPI support before save/restore?.
I mean, reading GITS_TYPER and check for LPI support?
> + for (i = 0; i < 8; i++) {
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_BASER + i * 8, &s->baser[i], false);
> + }
> +
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CTLR, ®, false);
> + s->ctlr = extract64(reg, 0, 32);
> +
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CBASER, &s->cbaser, false);
> +
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CREADR, &s->creadr, false);
> +
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CWRITER, &s->cwriter, false);
> +}
> +
> +/**
> + * kvm_arm_its_put - Restore both the ITS registers and guest RAM tables
> + * ITS tables, being flushed into guest RAM needs to be saved before
> + * the pre_save() callback. The restoration order matters since there
> + * are dependencies between register settings, as specified by the
> + * architecture specification
> + */
> +static void kvm_arm_its_put(GICv3ITSState *s)
> +{
> + uint64_t reg;
> + int i;
> +
> + /* must be written before GITS_CREADR since it resets this latter*/
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CBASER, &s->cbaser, true);
> +
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CREADR, &s->creadr, true);
> +
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CWRITER, &s->cwriter, true);
> +
> + for (i = 0; i < 8; i++) {
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_BASER + i * 8, &s->baser[i], true);
> + }
> +
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_TABLES,
> + 0, NULL, true);
> +
> + reg = s->ctlr;
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,
> + GITS_CTLR, ®, true);
> +}
> +
> static void kvm_arm_its_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -103,6 +187,8 @@ static void kvm_arm_its_class_init(ObjectClass *klass,
> void *data)
>
> dc->realize = kvm_arm_its_realize;
> icc->send_msi = kvm_its_send_msi;
> + icc->pre_save = kvm_arm_its_get;
> + icc->post_load = kvm_arm_its_put;
> }
>
> static const TypeInfo kvm_arm_its_info = {
> diff --git a/include/hw/intc/arm_gicv3_its_common.h
> b/include/hw/intc/arm_gicv3_its_common.h
> index 1ba1894..ed5d6df 100644
> --- a/include/hw/intc/arm_gicv3_its_common.h
> +++ b/include/hw/intc/arm_gicv3_its_common.h
> @@ -28,6 +28,12 @@
> #define ITS_TRANS_SIZE 0x10000
> #define ITS_SIZE (ITS_CONTROL_SIZE + ITS_TRANS_SIZE)
>
> +#define GITS_CTLR 0x0
> +#define GITS_CBASER 0x80
> +#define GITS_CWRITER 0x88
> +#define GITS_CREADR 0x90
> +#define GITS_BASER 0x100
> +
> struct GICv3ITSState {
> SysBusDevice parent_obj;
>
> --
> 2.5.5
>
