Hi Vijaya,
On 27/01/2017 08:17, Vijay Kilari wrote:
> 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?
I understand this is not needed since The GITS_TYPER Physical Bit is
RES1 indicating the ITS always supports physical LPIs.
Thanks
Eric
>
>> + 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
>>
