| Message ID | 20190409141347.3029-1-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Tue, Apr 09, 2019 at 04:13:46PM +0200, Cédric Le Goater wrote: > On P9 sPAPR guests, the interrupt mode (XICS legacy or XIVE native) is > determine at CAS time and the chosen mode is activated after a machine > reset. To be able to switch from one mode to another, subsequent > patches will introduce the capability to destroy the KVM device > without destroying the VM. > > This is not considered as a safe operation as the vCPUs are still > running and could be referencing the KVM device through their > presenters. > > To protect the system from any breakage, the kvmppc_xive objects > representing both KVM devices are now stored in an array under the > VM. Allocation is performed on first usage and memory is freed only > when the VM exits. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Although... > --- > arch/powerpc/include/asm/kvm_host.h | 1 + > arch/powerpc/kvm/book3s_xive.h | 1 + > arch/powerpc/kvm/book3s_xive.c | 23 +++++++++++++++++++++-- > arch/powerpc/kvm/book3s_xive_native.c | 9 +++++++-- > arch/powerpc/kvm/powerpc.c | 6 ++++++ > 5 files changed, 36 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 9cc6abdce1b9..ed059c95e56a 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -314,6 +314,7 @@ struct kvm_arch { > #ifdef CONFIG_KVM_XICS > struct kvmppc_xics *xics; > struct kvmppc_xive *xive; > + struct kvmppc_xive *xive_devices[2]; ... as noted on the previous revision, I feel this use of a bool-indexed array instead of separate fields makes things more confusing than necessary, for a negligible reduction in code size. > struct kvmppc_passthru_irqmap *pimap; > #endif > struct kvmppc_ops *kvm_ops; > diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h > index e011622dc038..426146332984 100644 > --- a/arch/powerpc/kvm/book3s_xive.h > +++ b/arch/powerpc/kvm/book3s_xive.h > @@ -283,6 +283,7 @@ void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb); > int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio); > int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio, > bool single_escalation); > +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type); > > #endif /* CONFIG_KVM_XICS */ > #endif /* _KVM_PPC_BOOK3S_XICS_H */ > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c > index 480a3fc6b9fd..4d4e1730de84 100644 > --- a/arch/powerpc/kvm/book3s_xive.c > +++ b/arch/powerpc/kvm/book3s_xive.c > @@ -1846,11 +1846,30 @@ static void kvmppc_xive_free(struct kvm_device *dev) > if (xive->vp_base != XIVE_INVALID_VP) > xive_native_free_vp_block(xive->vp_base); > > + /* > + * A reference of the kvmppc_xive pointer is now kept under > + * the xive_devices[] array of the machine for reuse. It is > + * freed when the VM is destroyed. > + */ > > - kfree(xive); > kfree(dev); > } > > +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type) > +{ > + struct kvmppc_xive *xive; > + bool xive_native_index = type == KVM_DEV_TYPE_XIVE; > + > + xive = kvm->arch.xive_devices[xive_native_index]; > + > + if (!xive) { > + xive = kzalloc(sizeof(*xive), GFP_KERNEL); > + kvm->arch.xive_devices[xive_native_index] = xive; > + } > + > + return xive; > +} > + > static int kvmppc_xive_create(struct kvm_device *dev, u32 type) > { > struct kvmppc_xive *xive; > @@ -1859,7 +1878,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type) > > pr_devel("Creating xive for partition\n"); > > - xive = kzalloc(sizeof(*xive), GFP_KERNEL); > + xive = kvmppc_xive_get_device(kvm, type); > if (!xive) > return -ENOMEM; > > diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c > index 62648f833adf..092db0efe628 100644 > --- a/arch/powerpc/kvm/book3s_xive_native.c > +++ b/arch/powerpc/kvm/book3s_xive_native.c > @@ -987,7 +987,12 @@ static void kvmppc_xive_native_free(struct kvm_device *dev) > if (xive->vp_base != XIVE_INVALID_VP) > xive_native_free_vp_block(xive->vp_base); > > - kfree(xive); > + /* > + * A reference of the kvmppc_xive pointer is now kept under > + * the xive_devices[] array of the machine for reuse. It is > + * freed when the VM is destroyed. > + */ > + > kfree(dev); > } > > @@ -1002,7 +1007,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type) > if (kvm->arch.xive) > return -EEXIST; > > - xive = kzalloc(sizeof(*xive), GFP_KERNEL); > + xive = kvmppc_xive_get_device(kvm, type); > if (!xive) > return -ENOMEM; > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index f54926c78320..d0914316ddc7 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -501,6 +501,12 @@ void kvm_arch_destroy_vm(struct kvm *kvm) > > mutex_unlock(&kvm->lock); > > + for (i = 0; i < ARRAY_SIZE(kvm->arch.xive_devices); i++) { > + struct kvmppc_xive *xive = kvm->arch.xive_devices[i]; > + if (xive) > + kfree(xive); > + } > + > /* drop the module reference */ > module_put(kvm->arch.kvm_ops->owner); > }
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 9cc6abdce1b9..ed059c95e56a 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -314,6 +314,7 @@ struct kvm_arch { #ifdef CONFIG_KVM_XICS struct kvmppc_xics *xics; struct kvmppc_xive *xive; + struct kvmppc_xive *xive_devices[2]; struct kvmppc_passthru_irqmap *pimap; #endif struct kvmppc_ops *kvm_ops; diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h index e011622dc038..426146332984 100644 --- a/arch/powerpc/kvm/book3s_xive.h +++ b/arch/powerpc/kvm/book3s_xive.h @@ -283,6 +283,7 @@ void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb); int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio); int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio, bool single_escalation); +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type); #endif /* CONFIG_KVM_XICS */ #endif /* _KVM_PPC_BOOK3S_XICS_H */ diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c index 480a3fc6b9fd..4d4e1730de84 100644 --- a/arch/powerpc/kvm/book3s_xive.c +++ b/arch/powerpc/kvm/book3s_xive.c @@ -1846,11 +1846,30 @@ static void kvmppc_xive_free(struct kvm_device *dev) if (xive->vp_base != XIVE_INVALID_VP) xive_native_free_vp_block(xive->vp_base); + /* + * A reference of the kvmppc_xive pointer is now kept under + * the xive_devices[] array of the machine for reuse. It is + * freed when the VM is destroyed. + */ - kfree(xive); kfree(dev); } +struct kvmppc_xive *kvmppc_xive_get_device(struct kvm *kvm, u32 type) +{ + struct kvmppc_xive *xive; + bool xive_native_index = type == KVM_DEV_TYPE_XIVE; + + xive = kvm->arch.xive_devices[xive_native_index]; + + if (!xive) { + xive = kzalloc(sizeof(*xive), GFP_KERNEL); + kvm->arch.xive_devices[xive_native_index] = xive; + } + + return xive; +} + static int kvmppc_xive_create(struct kvm_device *dev, u32 type) { struct kvmppc_xive *xive; @@ -1859,7 +1878,7 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type) pr_devel("Creating xive for partition\n"); - xive = kzalloc(sizeof(*xive), GFP_KERNEL); + xive = kvmppc_xive_get_device(kvm, type); if (!xive) return -ENOMEM; diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c index 62648f833adf..092db0efe628 100644 --- a/arch/powerpc/kvm/book3s_xive_native.c +++ b/arch/powerpc/kvm/book3s_xive_native.c @@ -987,7 +987,12 @@ static void kvmppc_xive_native_free(struct kvm_device *dev) if (xive->vp_base != XIVE_INVALID_VP) xive_native_free_vp_block(xive->vp_base); - kfree(xive); + /* + * A reference of the kvmppc_xive pointer is now kept under + * the xive_devices[] array of the machine for reuse. It is + * freed when the VM is destroyed. + */ + kfree(dev); } @@ -1002,7 +1007,7 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type) if (kvm->arch.xive) return -EEXIST; - xive = kzalloc(sizeof(*xive), GFP_KERNEL); + xive = kvmppc_xive_get_device(kvm, type); if (!xive) return -ENOMEM; diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index f54926c78320..d0914316ddc7 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -501,6 +501,12 @@ void kvm_arch_destroy_vm(struct kvm *kvm) mutex_unlock(&kvm->lock); + for (i = 0; i < ARRAY_SIZE(kvm->arch.xive_devices); i++) { + struct kvmppc_xive *xive = kvm->arch.xive_devices[i]; + if (xive) + kfree(xive); + } + /* drop the module reference */ module_put(kvm->arch.kvm_ops->owner); }
On P9 sPAPR guests, the interrupt mode (XICS legacy or XIVE native) is determine at CAS time and the chosen mode is activated after a machine reset. To be able to switch from one mode to another, subsequent patches will introduce the capability to destroy the KVM device without destroying the VM. This is not considered as a safe operation as the vCPUs are still running and could be referencing the KVM device through their presenters. To protect the system from any breakage, the kvmppc_xive objects representing both KVM devices are now stored in an array under the VM. Allocation is performed on first usage and memory is freed only when the VM exits. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- arch/powerpc/include/asm/kvm_host.h | 1 + arch/powerpc/kvm/book3s_xive.h | 1 + arch/powerpc/kvm/book3s_xive.c | 23 +++++++++++++++++++++-- arch/powerpc/kvm/book3s_xive_native.c | 9 +++++++-- arch/powerpc/kvm/powerpc.c | 6 ++++++ 5 files changed, 36 insertions(+), 4 deletions(-)