| Message ID | 1494084283-12723-17-git-send-email-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, May 06 2017 at 4:24:35 pm BST, Eric Auger <eric.auger@redhat.com> wrote: > Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group: > - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM > - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal > structures. > > We hold the vcpus lock during the save and restore to make > sure no vcpu is running. > > At this stage the functionality is not yet implemented. Only > the skeleton is put in place. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > > --- > v6 -> v7: > - also hold the its_lock on save and restore > > v5 -> v6: > - remove the pending table sync from the ITS table restore > > v4 -> v5: > - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES > - rename *flush* into *save* > - call its_sync_lpi_pending_table at the end of restore > - use abi framework > > v3 -> v4: > - pass kvm struct handle to vgic_its_flush/restore_pending_tables > - take the kvm lock and vcpu locks > - ABI revision check > - check attr->attr is null > > v1 -> v2: > - remove useless kvm parameter > --- > arch/arm/include/uapi/asm/kvm.h | 4 +- > arch/arm64/include/uapi/asm/kvm.h | 4 +- > virt/kvm/arm/vgic/vgic-its.c | 107 ++++++++++++++++++++++++++++++++++++-- > 3 files changed, 109 insertions(+), 6 deletions(-) > [...] > @@ -1718,7 +1777,37 @@ static int vgic_its_save_tables_v0(struct vgic_its *its) > */ > static int vgic_its_restore_tables_v0(struct vgic_its *its) > { > - return -ENXIO; > + struct kvm *kvm = its->dev->kvm; > + int ret; > + > + mutex_lock(&kvm->lock); > + mutex_lock(&its->its_lock); > + > + if (!lock_all_vcpus(kvm)) { > + mutex_unlock(&its->its_lock); > + mutex_unlock(&kvm->lock); > + return -EBUSY; > + } > + > + ret = vgic_its_restore_collection_table(its); > + if (ret) > + goto out; > + > + ret = vgic_its_restore_device_tables(its); > + > +out: > + unlock_all_vcpus(kvm); > + mutex_unlock(&its->its_lock); > + mutex_unlock(&kvm->lock); > + > + if (ret) > + return ret; > + > + /* > + * On restore path, MSI injections can happen before the > + * first VCPU run so let's complete the GIC init here. > + */ > + return kvm_vgic_map_resources(its->dev->kvm); This one is still a rather sore spot, but I've lost track of what we can do about it. Until now, this would only happen on first run. But it looks like QEMU and KVM have different views of what "runable" is. I'm not sure there is a good way to solve this, unfortunately. From a device PoV, everything is ready and the fact that nothing is clocking the CPUs is very much irrelevant. I'm almost tempted to say that the map_resource() call in kvm_vcpu_first_run_init shouldn't be there, and that we're missing a synchronization point with userspace that would say "system is entierely configured", triggering the iodev registration. Oh well. At that point, and short of having something better to offer: Acked-by: Marc Zyngier <marc.zyngier@arm.com> M.
On Sun, May 07 2017 at 2:00:30 pm BST, Marc Zyngier <marc.zyngier@arm.com> wrote: > On Sat, May 06 2017 at 4:24:35 pm BST, Eric Auger <eric.auger@redhat.com> wrote: >> Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group: >> - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM >> - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal >> structures. >> >> We hold the vcpus lock during the save and restore to make >> sure no vcpu is running. >> >> At this stage the functionality is not yet implemented. Only >> the skeleton is put in place. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> >> --- >> v6 -> v7: >> - also hold the its_lock on save and restore >> >> v5 -> v6: >> - remove the pending table sync from the ITS table restore >> >> v4 -> v5: >> - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES >> - rename *flush* into *save* >> - call its_sync_lpi_pending_table at the end of restore >> - use abi framework >> >> v3 -> v4: >> - pass kvm struct handle to vgic_its_flush/restore_pending_tables >> - take the kvm lock and vcpu locks >> - ABI revision check >> - check attr->attr is null >> >> v1 -> v2: >> - remove useless kvm parameter >> --- >> arch/arm/include/uapi/asm/kvm.h | 4 +- >> arch/arm64/include/uapi/asm/kvm.h | 4 +- >> virt/kvm/arm/vgic/vgic-its.c | 107 ++++++++++++++++++++++++++++++++++++-- >> 3 files changed, 109 insertions(+), 6 deletions(-) >> > > [...] > >> @@ -1718,7 +1777,37 @@ static int vgic_its_save_tables_v0(struct vgic_its *its) >> */ >> static int vgic_its_restore_tables_v0(struct vgic_its *its) >> { >> - return -ENXIO; >> + struct kvm *kvm = its->dev->kvm; >> + int ret; >> + >> + mutex_lock(&kvm->lock); >> + mutex_lock(&its->its_lock); >> + >> + if (!lock_all_vcpus(kvm)) { >> + mutex_unlock(&its->its_lock); >> + mutex_unlock(&kvm->lock); >> + return -EBUSY; >> + } >> + >> + ret = vgic_its_restore_collection_table(its); >> + if (ret) >> + goto out; >> + >> + ret = vgic_its_restore_device_tables(its); >> + >> +out: >> + unlock_all_vcpus(kvm); >> + mutex_unlock(&its->its_lock); >> + mutex_unlock(&kvm->lock); >> + >> + if (ret) >> + return ret; >> + >> + /* >> + * On restore path, MSI injections can happen before the >> + * first VCPU run so let's complete the GIC init here. >> + */ >> + return kvm_vgic_map_resources(its->dev->kvm); > > This one is still a rather sore spot, but I've lost track of what we can > do about it. Until now, this would only happen on first run. But it > looks like QEMU and KVM have different views of what "runable" is. > > I'm not sure there is a good way to solve this, unfortunately. From a > device PoV, everything is ready and the fact that nothing is clocking > the CPUs is very much irrelevant. I'm almost tempted to say that the > map_resource() call in kvm_vcpu_first_run_init shouldn't be there, and > that we're missing a synchronization point with userspace that would say > "system is entierely configured", triggering the iodev registration. > > Oh well. At that point, and short of having something better to offer: > > Acked-by: Marc Zyngier <marc.zyngier@arm.com> > > M. Actually, there is a rather big problem here. This function is called on a per-ITS basis. But once we have run map_resources *once*, any other call becomes ineffective (vgic_ready() returns true). What happens to the other ITSs? Can they still be successfully restored? Don't they end-up with the same "blind spot" you've tried to close? Thanks, M.
On Sun, May 07, 2017 at 02:51:37PM +0100, Marc Zyngier wrote: > On Sun, May 07 2017 at 2:00:30 pm BST, Marc Zyngier <marc.zyngier@arm.com> wrote: > > On Sat, May 06 2017 at 4:24:35 pm BST, Eric Auger <eric.auger@redhat.com> wrote: > >> Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group: > >> - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM > >> - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal > >> structures. > >> > >> We hold the vcpus lock during the save and restore to make > >> sure no vcpu is running. > >> > >> At this stage the functionality is not yet implemented. Only > >> the skeleton is put in place. > >> > >> Signed-off-by: Eric Auger <eric.auger@redhat.com> > >> > >> --- > >> v6 -> v7: > >> - also hold the its_lock on save and restore > >> > >> v5 -> v6: > >> - remove the pending table sync from the ITS table restore > >> > >> v4 -> v5: > >> - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES > >> - rename *flush* into *save* > >> - call its_sync_lpi_pending_table at the end of restore > >> - use abi framework > >> > >> v3 -> v4: > >> - pass kvm struct handle to vgic_its_flush/restore_pending_tables > >> - take the kvm lock and vcpu locks > >> - ABI revision check > >> - check attr->attr is null > >> > >> v1 -> v2: > >> - remove useless kvm parameter > >> --- > >> arch/arm/include/uapi/asm/kvm.h | 4 +- > >> arch/arm64/include/uapi/asm/kvm.h | 4 +- > >> virt/kvm/arm/vgic/vgic-its.c | 107 ++++++++++++++++++++++++++++++++++++-- > >> 3 files changed, 109 insertions(+), 6 deletions(-) > >> > > > > [...] > > > >> @@ -1718,7 +1777,37 @@ static int vgic_its_save_tables_v0(struct vgic_its *its) > >> */ > >> static int vgic_its_restore_tables_v0(struct vgic_its *its) > >> { > >> - return -ENXIO; > >> + struct kvm *kvm = its->dev->kvm; > >> + int ret; > >> + > >> + mutex_lock(&kvm->lock); > >> + mutex_lock(&its->its_lock); > >> + > >> + if (!lock_all_vcpus(kvm)) { > >> + mutex_unlock(&its->its_lock); > >> + mutex_unlock(&kvm->lock); > >> + return -EBUSY; > >> + } > >> + > >> + ret = vgic_its_restore_collection_table(its); > >> + if (ret) > >> + goto out; > >> + > >> + ret = vgic_its_restore_device_tables(its); > >> + > >> +out: > >> + unlock_all_vcpus(kvm); > >> + mutex_unlock(&its->its_lock); > >> + mutex_unlock(&kvm->lock); > >> + > >> + if (ret) > >> + return ret; > >> + > >> + /* > >> + * On restore path, MSI injections can happen before the > >> + * first VCPU run so let's complete the GIC init here. > >> + */ > >> + return kvm_vgic_map_resources(its->dev->kvm); > > > > This one is still a rather sore spot, but I've lost track of what we can > > do about it. Until now, this would only happen on first run. But it > > looks like QEMU and KVM have different views of what "runable" is. > > > > I'm not sure there is a good way to solve this, unfortunately. From a > > device PoV, everything is ready and the fact that nothing is clocking > > the CPUs is very much irrelevant. I'm almost tempted to say that the > > map_resource() call in kvm_vcpu_first_run_init shouldn't be there, and > > that we're missing a synchronization point with userspace that would say > > "system is entierely configured", triggering the iodev registration. > > > > Oh well. At that point, and short of having something better to offer: > > > > Acked-by: Marc Zyngier <marc.zyngier@arm.com> > > > > M. > > Actually, there is a rather big problem here. This function is called on > a per-ITS basis. But once we have run map_resources *once*, any other > call becomes ineffective (vgic_ready() returns true). What happens to > the other ITSs? Can they still be successfully restored? Don't they > end-up with the same "blind spot" you've tried to close? > I really think the only proper solution to this is to have map_resources do nothing at all on GICv3, and register the iodevs when the base addresses are set, and all that map_resources end up doing is to actually map stuff for GICv2 in the guest address space (the virtual CPU interface). That way the initialization is tied to the data that makes the initialization possible, we don't need another sync point, it becomes ITS-specific, and migration should work. Take two? :) -Christoffer
Hi, On 07/05/2017 15:51, Marc Zyngier wrote: > On Sun, May 07 2017 at 2:00:30 pm BST, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On Sat, May 06 2017 at 4:24:35 pm BST, Eric Auger <eric.auger@redhat.com> wrote: >>> Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group: >>> - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM >>> - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal >>> structures. >>> >>> We hold the vcpus lock during the save and restore to make >>> sure no vcpu is running. >>> >>> At this stage the functionality is not yet implemented. Only >>> the skeleton is put in place. >>> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> >>> --- >>> v6 -> v7: >>> - also hold the its_lock on save and restore >>> >>> v5 -> v6: >>> - remove the pending table sync from the ITS table restore >>> >>> v4 -> v5: >>> - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES >>> - rename *flush* into *save* >>> - call its_sync_lpi_pending_table at the end of restore >>> - use abi framework >>> >>> v3 -> v4: >>> - pass kvm struct handle to vgic_its_flush/restore_pending_tables >>> - take the kvm lock and vcpu locks >>> - ABI revision check >>> - check attr->attr is null >>> >>> v1 -> v2: >>> - remove useless kvm parameter >>> --- >>> arch/arm/include/uapi/asm/kvm.h | 4 +- >>> arch/arm64/include/uapi/asm/kvm.h | 4 +- >>> virt/kvm/arm/vgic/vgic-its.c | 107 ++++++++++++++++++++++++++++++++++++-- >>> 3 files changed, 109 insertions(+), 6 deletions(-) >>> >> >> [...] >> >>> @@ -1718,7 +1777,37 @@ static int vgic_its_save_tables_v0(struct vgic_its *its) >>> */ >>> static int vgic_its_restore_tables_v0(struct vgic_its *its) >>> { >>> - return -ENXIO; >>> + struct kvm *kvm = its->dev->kvm; >>> + int ret; >>> + >>> + mutex_lock(&kvm->lock); >>> + mutex_lock(&its->its_lock); >>> + >>> + if (!lock_all_vcpus(kvm)) { >>> + mutex_unlock(&its->its_lock); >>> + mutex_unlock(&kvm->lock); >>> + return -EBUSY; >>> + } >>> + >>> + ret = vgic_its_restore_collection_table(its); >>> + if (ret) >>> + goto out; >>> + >>> + ret = vgic_its_restore_device_tables(its); >>> + >>> +out: >>> + unlock_all_vcpus(kvm); >>> + mutex_unlock(&its->its_lock); >>> + mutex_unlock(&kvm->lock); >>> + >>> + if (ret) >>> + return ret; >>> + >>> + /* >>> + * On restore path, MSI injections can happen before the >>> + * first VCPU run so let's complete the GIC init here. >>> + */ >>> + return kvm_vgic_map_resources(its->dev->kvm); >> >> This one is still a rather sore spot, but I've lost track of what we can >> do about it. Until now, this would only happen on first run. But it >> looks like QEMU and KVM have different views of what "runable" is. >> >> I'm not sure there is a good way to solve this, unfortunately. From a >> device PoV, everything is ready and the fact that nothing is clocking >> the CPUs is very much irrelevant. I'm almost tempted to say that the >> map_resource() call in kvm_vcpu_first_run_init shouldn't be there, and >> that we're missing a synchronization point with userspace that would say >> "system is entierely configured", triggering the iodev registration. >> >> Oh well. At that point, and short of having something better to offer: >> >> Acked-by: Marc Zyngier <marc.zyngier@arm.com> >> >> M. > > Actually, there is a rather big problem here. This function is called on > a per-ITS basis. But once we have run map_resources *once*, any other > call becomes ineffective (vgic_ready() returns true). What happens to > the other ITSs? Can they still be successfully restored? Don't they > end-up with the same "blind spot" you've tried to close? The 1st call to map_resources() maps GICV3 regs and also maps all ITSes. So I don't think the problem comes from the fact subsequent calls are inefficient (and return 0) but that for this 1st call to succeeds all ITS base addresses must have been provided by the userspace. Otherwise this restoration of this ITS will fail. So this is not explicitly documented that way in the ABI doc. Thanks Eric > > Thanks, > > M. >
On Sat, May 06, 2017 at 05:24:35PM +0200, Eric Auger wrote: > Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group: > - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM > - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal > structures. > > We hold the vcpus lock during the save and restore to make > sure no vcpu is running. > > At this stage the functionality is not yet implemented. Only > the skeleton is put in place. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> [with the expectation that the map_resources thing is fixed later] Reviewed-by: Christoffer Dall <cdall@linaro.org> > > --- > v6 -> v7: > - also hold the its_lock on save and restore > > v5 -> v6: > - remove the pending table sync from the ITS table restore > > v4 -> v5: > - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES > - rename *flush* into *save* > - call its_sync_lpi_pending_table at the end of restore > - use abi framework > > v3 -> v4: > - pass kvm struct handle to vgic_its_flush/restore_pending_tables > - take the kvm lock and vcpu locks > - ABI revision check > - check attr->attr is null > > v1 -> v2: > - remove useless kvm parameter > --- > arch/arm/include/uapi/asm/kvm.h | 4 +- > arch/arm64/include/uapi/asm/kvm.h | 4 +- > virt/kvm/arm/vgic/vgic-its.c | 107 ++++++++++++++++++++++++++++++++++++-- > 3 files changed, 109 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h > index 4beb83b..8e6563c 100644 > --- a/arch/arm/include/uapi/asm/kvm.h > +++ b/arch/arm/include/uapi/asm/kvm.h > @@ -199,7 +199,9 @@ struct kvm_arch_memory_slot { > #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff > #define VGIC_LEVEL_INFO_LINE_LEVEL 0 > > -#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 > +#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 > +#define KVM_DEV_ARM_ITS_SAVE_TABLES 1 > +#define KVM_DEV_ARM_ITS_RESTORE_TABLES 2 > > /* KVM_IRQ_LINE irq field index values */ > #define KVM_ARM_IRQ_TYPE_SHIFT 24 > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index 7e8dd69..1e35115 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -219,7 +219,9 @@ struct kvm_arch_memory_slot { > #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff > #define VGIC_LEVEL_INFO_LINE_LEVEL 0 > > -#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 > +#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 > +#define KVM_DEV_ARM_ITS_SAVE_TABLES 1 > +#define KVM_DEV_ARM_ITS_RESTORE_TABLES 2 > > /* Device Control API on vcpu fd */ > #define KVM_ARM_VCPU_PMU_V3_CTRL 0 > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index ffd0a80..cb7ae4c 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -1703,12 +1703,71 @@ int vgic_its_attr_regs_access(struct kvm_device *dev, > } > > /** > + * vgic_its_save_device_tables - Save the device table and all ITT > + * into guest RAM > + */ > +static int vgic_its_save_device_tables(struct vgic_its *its) > +{ > + return -ENXIO; > +} > + > +/** > + * vgic_its_restore_device_tables - Restore the device table and all ITT > + * from guest RAM to internal data structs > + */ > +static int vgic_its_restore_device_tables(struct vgic_its *its) > +{ > + return -ENXIO; > +} > + > +/** > + * vgic_its_save_collection_table - Save the collection table into > + * guest RAM > + */ > +static int vgic_its_save_collection_table(struct vgic_its *its) > +{ > + return -ENXIO; > +} > + > +/** > + * vgic_its_restore_collection_table - reads the collection table > + * in guest memory and restores the ITS internal state. Requires the > + * BASER registers to be restored before. > + */ > +static int vgic_its_restore_collection_table(struct vgic_its *its) > +{ > + return -ENXIO; > +} > + > +/** > * vgic_its_save_tables_v0 - Save the ITS tables into guest ARM > * according to v0 ABI > */ > static int vgic_its_save_tables_v0(struct vgic_its *its) > { > - return -ENXIO; > + struct kvm *kvm = its->dev->kvm; > + int ret; > + > + mutex_lock(&kvm->lock); > + mutex_lock(&its->its_lock); > + > + if (!lock_all_vcpus(kvm)) { > + mutex_unlock(&its->its_lock); > + mutex_unlock(&kvm->lock); > + return -EBUSY; > + } > + > + ret = vgic_its_save_device_tables(its); > + if (ret) > + goto out; > + > + ret = vgic_its_save_collection_table(its); > + > +out: > + unlock_all_vcpus(kvm); > + mutex_unlock(&its->its_lock); > + mutex_unlock(&kvm->lock); > + return ret; > } > > /** > @@ -1718,7 +1777,37 @@ static int vgic_its_save_tables_v0(struct vgic_its *its) > */ > static int vgic_its_restore_tables_v0(struct vgic_its *its) > { > - return -ENXIO; > + struct kvm *kvm = its->dev->kvm; > + int ret; > + > + mutex_lock(&kvm->lock); > + mutex_lock(&its->its_lock); > + > + if (!lock_all_vcpus(kvm)) { > + mutex_unlock(&its->its_lock); > + mutex_unlock(&kvm->lock); > + return -EBUSY; > + } > + > + ret = vgic_its_restore_collection_table(its); > + if (ret) > + goto out; > + > + ret = vgic_its_restore_device_tables(its); > + > +out: > + unlock_all_vcpus(kvm); > + mutex_unlock(&its->its_lock); > + mutex_unlock(&kvm->lock); > + > + if (ret) > + return ret; > + > + /* > + * On restore path, MSI injections can happen before the > + * first VCPU run so let's complete the GIC init here. > + */ > + return kvm_vgic_map_resources(its->dev->kvm); > } > > static int vgic_its_commit_v0(struct vgic_its *its) > @@ -1751,6 +1840,10 @@ static int vgic_its_has_attr(struct kvm_device *dev, > switch (attr->attr) { > case KVM_DEV_ARM_VGIC_CTRL_INIT: > return 0; > + case KVM_DEV_ARM_ITS_SAVE_TABLES: > + return 0; > + case KVM_DEV_ARM_ITS_RESTORE_TABLES: > + return 0; > } > break; > case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: > @@ -1786,14 +1879,20 @@ static int vgic_its_set_attr(struct kvm_device *dev, > > return 0; > } > - case KVM_DEV_ARM_VGIC_GRP_CTRL: > + case KVM_DEV_ARM_VGIC_GRP_CTRL: { > + const struct vgic_its_abi *abi = vgic_its_get_abi(its); > + > switch (attr->attr) { > case KVM_DEV_ARM_VGIC_CTRL_INIT: > its->initialized = true; > > return 0; > + case KVM_DEV_ARM_ITS_SAVE_TABLES: > + return abi->save_tables(its); > + case KVM_DEV_ARM_ITS_RESTORE_TABLES: > + return abi->restore_tables(its); > } > - break; > + } > case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { > u64 __user *uaddr = (u64 __user *)(long)attr->addr; > u64 reg; > -- > 2.5.5 >
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h index 4beb83b..8e6563c 100644 --- a/arch/arm/include/uapi/asm/kvm.h +++ b/arch/arm/include/uapi/asm/kvm.h @@ -199,7 +199,9 @@ struct kvm_arch_memory_slot { #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff #define VGIC_LEVEL_INFO_LINE_LEVEL 0 -#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 +#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 +#define KVM_DEV_ARM_ITS_SAVE_TABLES 1 +#define KVM_DEV_ARM_ITS_RESTORE_TABLES 2 /* KVM_IRQ_LINE irq field index values */ #define KVM_ARM_IRQ_TYPE_SHIFT 24 diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 7e8dd69..1e35115 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -219,7 +219,9 @@ struct kvm_arch_memory_slot { #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff #define VGIC_LEVEL_INFO_LINE_LEVEL 0 -#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 +#define KVM_DEV_ARM_VGIC_CTRL_INIT 0 +#define KVM_DEV_ARM_ITS_SAVE_TABLES 1 +#define KVM_DEV_ARM_ITS_RESTORE_TABLES 2 /* Device Control API on vcpu fd */ #define KVM_ARM_VCPU_PMU_V3_CTRL 0 diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c index ffd0a80..cb7ae4c 100644 --- a/virt/kvm/arm/vgic/vgic-its.c +++ b/virt/kvm/arm/vgic/vgic-its.c @@ -1703,12 +1703,71 @@ int vgic_its_attr_regs_access(struct kvm_device *dev, } /** + * vgic_its_save_device_tables - Save the device table and all ITT + * into guest RAM + */ +static int vgic_its_save_device_tables(struct vgic_its *its) +{ + return -ENXIO; +} + +/** + * vgic_its_restore_device_tables - Restore the device table and all ITT + * from guest RAM to internal data structs + */ +static int vgic_its_restore_device_tables(struct vgic_its *its) +{ + return -ENXIO; +} + +/** + * vgic_its_save_collection_table - Save the collection table into + * guest RAM + */ +static int vgic_its_save_collection_table(struct vgic_its *its) +{ + return -ENXIO; +} + +/** + * vgic_its_restore_collection_table - reads the collection table + * in guest memory and restores the ITS internal state. Requires the + * BASER registers to be restored before. + */ +static int vgic_its_restore_collection_table(struct vgic_its *its) +{ + return -ENXIO; +} + +/** * vgic_its_save_tables_v0 - Save the ITS tables into guest ARM * according to v0 ABI */ static int vgic_its_save_tables_v0(struct vgic_its *its) { - return -ENXIO; + struct kvm *kvm = its->dev->kvm; + int ret; + + mutex_lock(&kvm->lock); + mutex_lock(&its->its_lock); + + if (!lock_all_vcpus(kvm)) { + mutex_unlock(&its->its_lock); + mutex_unlock(&kvm->lock); + return -EBUSY; + } + + ret = vgic_its_save_device_tables(its); + if (ret) + goto out; + + ret = vgic_its_save_collection_table(its); + +out: + unlock_all_vcpus(kvm); + mutex_unlock(&its->its_lock); + mutex_unlock(&kvm->lock); + return ret; } /** @@ -1718,7 +1777,37 @@ static int vgic_its_save_tables_v0(struct vgic_its *its) */ static int vgic_its_restore_tables_v0(struct vgic_its *its) { - return -ENXIO; + struct kvm *kvm = its->dev->kvm; + int ret; + + mutex_lock(&kvm->lock); + mutex_lock(&its->its_lock); + + if (!lock_all_vcpus(kvm)) { + mutex_unlock(&its->its_lock); + mutex_unlock(&kvm->lock); + return -EBUSY; + } + + ret = vgic_its_restore_collection_table(its); + if (ret) + goto out; + + ret = vgic_its_restore_device_tables(its); + +out: + unlock_all_vcpus(kvm); + mutex_unlock(&its->its_lock); + mutex_unlock(&kvm->lock); + + if (ret) + return ret; + + /* + * On restore path, MSI injections can happen before the + * first VCPU run so let's complete the GIC init here. + */ + return kvm_vgic_map_resources(its->dev->kvm); } static int vgic_its_commit_v0(struct vgic_its *its) @@ -1751,6 +1840,10 @@ static int vgic_its_has_attr(struct kvm_device *dev, switch (attr->attr) { case KVM_DEV_ARM_VGIC_CTRL_INIT: return 0; + case KVM_DEV_ARM_ITS_SAVE_TABLES: + return 0; + case KVM_DEV_ARM_ITS_RESTORE_TABLES: + return 0; } break; case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: @@ -1786,14 +1879,20 @@ static int vgic_its_set_attr(struct kvm_device *dev, return 0; } - case KVM_DEV_ARM_VGIC_GRP_CTRL: + case KVM_DEV_ARM_VGIC_GRP_CTRL: { + const struct vgic_its_abi *abi = vgic_its_get_abi(its); + switch (attr->attr) { case KVM_DEV_ARM_VGIC_CTRL_INIT: its->initialized = true; return 0; + case KVM_DEV_ARM_ITS_SAVE_TABLES: + return abi->save_tables(its); + case KVM_DEV_ARM_ITS_RESTORE_TABLES: + return abi->restore_tables(its); } - break; + } case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { u64 __user *uaddr = (u64 __user *)(long)attr->addr; u64 reg;
Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group: - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal structures. We hold the vcpus lock during the save and restore to make sure no vcpu is running. At this stage the functionality is not yet implemented. Only the skeleton is put in place. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- v6 -> v7: - also hold the its_lock on save and restore v5 -> v6: - remove the pending table sync from the ITS table restore v4 -> v5: - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES - rename *flush* into *save* - call its_sync_lpi_pending_table at the end of restore - use abi framework v3 -> v4: - pass kvm struct handle to vgic_its_flush/restore_pending_tables - take the kvm lock and vcpu locks - ABI revision check - check attr->attr is null v1 -> v2: - remove useless kvm parameter --- arch/arm/include/uapi/asm/kvm.h | 4 +- arch/arm64/include/uapi/asm/kvm.h | 4 +- virt/kvm/arm/vgic/vgic-its.c | 107 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 109 insertions(+), 6 deletions(-)