linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
@ 2016-06-13 13:25 Paolo Bonzini
  2016-06-13 13:25 ` [PATCH 1/3] KVM: introduce kvm->created_vcpus Paolo Bonzini
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-06-13 13:25 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Cornelia Huck, Christian Borntraeger

kvm->online_vcpus is only updated at the very end of KVM_CREATE_VCPU.
Thus, it can be racy to use it as a check for "has any VCPU been created
already?".  While x86 detects the race through kvm_vcpu_compatible,
it is kinda hackish and s390 does not do anything similar.  Provide a
better fix for both.

Paolo Bonzini (3):
  KVM: introduce created_vcpus
  KVM: remove kvm_vcpu_compatible
  KVM: s390: use created_vcpus

 arch/s390/kvm/kvm-s390.c | 10 +++++-----
 arch/x86/kvm/Kconfig     |  1 -
 arch/x86/kvm/x86.c       | 11 +++--------
 include/linux/kvm_host.h | 14 ++++++++------
 virt/kvm/Kconfig         |  3 ---
 virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
 6 files changed, 33 insertions(+), 33 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] KVM: introduce kvm->created_vcpus
  2016-06-13 13:25 [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Paolo Bonzini
@ 2016-06-13 13:25 ` Paolo Bonzini
  2016-06-13 14:43   ` Cornelia Huck
  2016-06-13 13:25 ` [PATCH 2/3] KVM: remove kvm_vcpu_compatible Paolo Bonzini
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2016-06-13 13:25 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Cornelia Huck, Christian Borntraeger

The race between creating the irqchip and the first VCPU is
currently fixed by checking the presence of an irqchip before
updating kvm->online_vcpus, and undoing the whole VCPU creation
if someone created the irqchip in the meanwhile.

Instead, introduce a new field in struct kvm that will count VCPUs
under a mutex, without the atomic access and memory ordering that we
need elsewhere to protect the vcpus array.  This also plugs the race
and is more easily applicable in all similar circumstances.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/linux/kvm_host.h |  8 ++++++++
 virt/kvm/kvm_main.c      | 23 +++++++++++++++++------
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1c9c973a7dd9..63c6ab30bc81 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -371,7 +371,15 @@ struct kvm {
 	struct srcu_struct srcu;
 	struct srcu_struct irq_srcu;
 	struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
+
+	/*
+	 * created_vcpus is protected by kvm->lock, and is incremented
+	 * at the beginning of KVM_CREATE_VCPU.  online_vcpus is only
+	 * incremented after storing the kvm_vcpu pointer in vcpus,
+	 * and is accessed atomically.
+	 */
 	atomic_t online_vcpus;
+	int created_vcpus;
 	int last_boosted_vcpu;
 	struct list_head vm_list;
 	struct mutex lock;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 02e98f3131bd..15b757ae64e1 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2346,9 +2346,20 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 	if (id >= KVM_MAX_VCPU_ID)
 		return -EINVAL;
 
+	mutex_lock(&kvm->lock);
+	if (kvm->created_vcpus == KVM_MAX_VCPUS) {
+		mutex_unlock(&kvm->lock);
+		return -EINVAL;
+	}
+
+	kvm->created_vcpus++;
+	mutex_unlock(&kvm->lock);
+
 	vcpu = kvm_arch_vcpu_create(kvm, id);
-	if (IS_ERR(vcpu))
-		return PTR_ERR(vcpu);
+	if (IS_ERR(vcpu)) {
+		r = PTR_ERR(vcpu);
+		goto vcpu_decrement;
+	}
 
 	preempt_notifier_init(&vcpu->preempt_notifier, &kvm_preempt_ops);
 
@@ -2361,10 +2372,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 		r = -EINVAL;
 		goto unlock_vcpu_destroy;
 	}
-	if (atomic_read(&kvm->online_vcpus) == KVM_MAX_VCPUS) {
-		r = -EINVAL;
-		goto unlock_vcpu_destroy;
-	}
 	if (kvm_get_vcpu_by_id(kvm, id)) {
 		r = -EEXIST;
 		goto unlock_vcpu_destroy;
@@ -2397,6 +2404,10 @@ unlock_vcpu_destroy:
 	mutex_unlock(&kvm->lock);
 vcpu_destroy:
 	kvm_arch_vcpu_destroy(vcpu);
+vcpu_decrement:
+	mutex_lock(&kvm->lock);
+	kvm->created_vcpus--;
+	mutex_unlock(&kvm->lock);
 	return r;
 }
 
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/3] KVM: remove kvm_vcpu_compatible
  2016-06-13 13:25 [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Paolo Bonzini
  2016-06-13 13:25 ` [PATCH 1/3] KVM: introduce kvm->created_vcpus Paolo Bonzini
@ 2016-06-13 13:25 ` Paolo Bonzini
  2016-06-13 13:25 ` [PATCH 3/3] KVM: s390: use kvm->created_vcpus Paolo Bonzini
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-06-13 13:25 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Cornelia Huck, Christian Borntraeger

The new created_vcpus field makes it possible to avoid the race between
irqchip and VCPU creation in a much nicer way; just check under kvm->lock
whether a VCPU has already been created.

We can then remove KVM_APIC_ARCHITECTURE too, because at this point the
symbol is only governing the default definition of kvm_vcpu_compatible.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/Kconfig     |  1 -
 arch/x86/kvm/x86.c       | 11 +++--------
 include/linux/kvm_host.h |  6 ------
 virt/kvm/Kconfig         |  3 ---
 virt/kvm/kvm_main.c      |  4 ----
 5 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 639a6e34500c..ab8e32f7b9a8 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -32,7 +32,6 @@ config KVM
 	select HAVE_KVM_IRQ_BYPASS
 	select HAVE_KVM_IRQ_ROUTING
 	select HAVE_KVM_EVENTFD
-	select KVM_APIC_ARCHITECTURE
 	select KVM_ASYNC_PF
 	select USER_RETURN_NOTIFIER
 	select KVM_MMIO
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 902d9da12392..d52039b3985d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3773,7 +3773,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		r = -EEXIST;
 		if (irqchip_in_kernel(kvm))
 			goto split_irqchip_unlock;
-		if (atomic_read(&kvm->online_vcpus))
+		if (kvm->created_vcpus)
 			goto split_irqchip_unlock;
 		r = kvm_setup_empty_irq_routing(kvm);
 		if (r)
@@ -3838,7 +3838,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		if (kvm->arch.vpic)
 			goto create_irqchip_unlock;
 		r = -EINVAL;
-		if (atomic_read(&kvm->online_vcpus))
+		if (kvm->created_vcpus)
 			goto create_irqchip_unlock;
 		r = -ENOMEM;
 		vpic = kvm_create_pic(kvm);
@@ -3994,7 +3994,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	case KVM_SET_BOOT_CPU_ID:
 		r = 0;
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus) != 0)
+		if (kvm->created_vcpus)
 			r = -EBUSY;
 		else
 			kvm->arch.bsp_vcpu_id = arg;
@@ -7625,11 +7625,6 @@ bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu)
 	return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0;
 }
 
-bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu)
-{
-	return irqchip_in_kernel(vcpu->kvm) == lapic_in_kernel(vcpu);
-}
-
 struct static_key kvm_no_apic_vcpu __read_mostly;
 EXPORT_SYMBOL_GPL(kvm_no_apic_vcpu);
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 63c6ab30bc81..0640ee92b978 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1105,12 +1105,6 @@ static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
 
 #endif /* CONFIG_HAVE_KVM_EVENTFD */
 
-#ifdef CONFIG_KVM_APIC_ARCHITECTURE
-bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu);
-#else
-static inline bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu) { return true; }
-#endif
-
 static inline void kvm_make_request(int req, struct kvm_vcpu *vcpu)
 {
 	/*
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index e5d6108f5e85..b0cc1a34db27 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -16,9 +16,6 @@ config HAVE_KVM_EVENTFD
        bool
        select EVENTFD
 
-config KVM_APIC_ARCHITECTURE
-       bool
-
 config KVM_MMIO
        bool
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 15b757ae64e1..ef54b4c31792 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2368,10 +2368,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 		goto vcpu_destroy;
 
 	mutex_lock(&kvm->lock);
-	if (!kvm_vcpu_compatible(vcpu)) {
-		r = -EINVAL;
-		goto unlock_vcpu_destroy;
-	}
 	if (kvm_get_vcpu_by_id(kvm, id)) {
 		r = -EEXIST;
 		goto unlock_vcpu_destroy;
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] KVM: s390: use kvm->created_vcpus
  2016-06-13 13:25 [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Paolo Bonzini
  2016-06-13 13:25 ` [PATCH 1/3] KVM: introduce kvm->created_vcpus Paolo Bonzini
  2016-06-13 13:25 ` [PATCH 2/3] KVM: remove kvm_vcpu_compatible Paolo Bonzini
@ 2016-06-13 13:25 ` Paolo Bonzini
  2016-06-13 14:36   ` Cornelia Huck
  2016-06-13 13:33 ` [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Christian Borntraeger
  2016-06-13 14:44 ` Cornelia Huck
  4 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2016-06-13 13:25 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: Cornelia Huck, Christian Borntraeger

The new created_vcpus field avoids possible races between enabling
capabilities and creating VCPUs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/s390/kvm/kvm-s390.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 43f2a2b80490..a3f845f25eff 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -355,7 +355,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 		break;
 	case KVM_CAP_S390_VECTOR_REGISTERS:
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus)) {
+		if (kvm->created_vcpus) {
 			r = -EBUSY;
 		} else if (MACHINE_HAS_VX) {
 			set_kvm_facility(kvm->arch.model.fac_mask, 129);
@@ -370,7 +370,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 	case KVM_CAP_S390_RI:
 		r = -EINVAL;
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus)) {
+		if (kvm->created_vcpus) {
 			r = -EBUSY;
 		} else if (test_facility(64)) {
 			set_kvm_facility(kvm->arch.model.fac_mask, 64);
@@ -426,7 +426,7 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
 		ret = -EBUSY;
 		VM_EVENT(kvm, 3, "%s", "ENABLE: CMMA support");
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus) == 0) {
+		if (kvm->created_vcpus) {
 			kvm->arch.use_cmma = 1;
 			ret = 0;
 		}
@@ -467,7 +467,7 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
 
 		ret = -EBUSY;
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus) == 0) {
+		if (kvm->created_vcpus) {
 			/* gmap_alloc will round the limit up */
 			struct gmap *new = gmap_alloc(current->mm, new_limit);
 
@@ -644,7 +644,7 @@ static int kvm_s390_set_processor(struct kvm *kvm, struct kvm_device_attr *attr)
 	int ret = 0;
 
 	mutex_lock(&kvm->lock);
-	if (atomic_read(&kvm->online_vcpus)) {
+	if (kvm->created_vcpus) {
 		ret = -EBUSY;
 		goto out;
 	}
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
  2016-06-13 13:25 [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Paolo Bonzini
                   ` (2 preceding siblings ...)
  2016-06-13 13:25 ` [PATCH 3/3] KVM: s390: use kvm->created_vcpus Paolo Bonzini
@ 2016-06-13 13:33 ` Christian Borntraeger
  2016-06-13 14:44 ` Cornelia Huck
  4 siblings, 0 replies; 13+ messages in thread
From: Christian Borntraeger @ 2016-06-13 13:33 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm; +Cc: Cornelia Huck, Christian Borntraeger

On 06/13/2016 03:25 PM, Paolo Bonzini wrote:
> kvm->online_vcpus is only updated at the very end of KVM_CREATE_VCPU.
> Thus, it can be racy to use it as a check for "has any VCPU been created
> already?".  While x86 detects the race through kvm_vcpu_compatible,
> it is kinda hackish and s390 does not do anything similar.  Provide a
> better fix for both.
> 
> Paolo Bonzini (3):
>   KVM: introduce created_vcpus
>   KVM: remove kvm_vcpu_compatible
>   KVM: s390: use created_vcpus
> 
>  arch/s390/kvm/kvm-s390.c | 10 +++++-----
>  arch/x86/kvm/Kconfig     |  1 -
>  arch/x86/kvm/x86.c       | 11 +++--------
>  include/linux/kvm_host.h | 14 ++++++++------
>  virt/kvm/Kconfig         |  3 ---
>  virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
>  6 files changed, 33 insertions(+), 33 deletions(-)
> 

>From a quick look this looks "the right thing". Will do a proper review
later this day or tomorrow.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] KVM: s390: use kvm->created_vcpus
  2016-06-13 13:25 ` [PATCH 3/3] KVM: s390: use kvm->created_vcpus Paolo Bonzini
@ 2016-06-13 14:36   ` Cornelia Huck
  0 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-06-13 14:36 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, Christian Borntraeger

On Mon, 13 Jun 2016 15:25:32 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The new created_vcpus field avoids possible races between enabling
> capabilities and creating VCPUs.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

> @@ -426,7 +426,7 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
>  		ret = -EBUSY;
>  		VM_EVENT(kvm, 3, "%s", "ENABLE: CMMA support");
>  		mutex_lock(&kvm->lock);
> -		if (atomic_read(&kvm->online_vcpus) == 0) {
> +		if (kvm->created_vcpus) {

if (!kvm->created_vcpus) {

>  			kvm->arch.use_cmma = 1;
>  			ret = 0;
>  		}
> @@ -467,7 +467,7 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
> 
>  		ret = -EBUSY;
>  		mutex_lock(&kvm->lock);
> -		if (atomic_read(&kvm->online_vcpus) == 0) {
> +		if (kvm->created_vcpus) {

dito

>  			/* gmap_alloc will round the limit up */
>  			struct gmap *new = gmap_alloc(current->mm, new_limit);
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] KVM: introduce kvm->created_vcpus
  2016-06-13 13:25 ` [PATCH 1/3] KVM: introduce kvm->created_vcpus Paolo Bonzini
@ 2016-06-13 14:43   ` Cornelia Huck
  0 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2016-06-13 14:43 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, Christian Borntraeger

On Mon, 13 Jun 2016 15:25:30 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The race between creating the irqchip and the first VCPU is
> currently fixed by checking the presence of an irqchip before
> updating kvm->online_vcpus, and undoing the whole VCPU creation
> if someone created the irqchip in the meanwhile.
> 
> Instead, introduce a new field in struct kvm that will count VCPUs
> under a mutex, without the atomic access and memory ordering that we
> need elsewhere to protect the vcpus array.  This also plugs the race
> and is more easily applicable in all similar circumstances.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/linux/kvm_host.h |  8 ++++++++
>  virt/kvm/kvm_main.c      | 23 +++++++++++++++++------
>  2 files changed, 25 insertions(+), 6 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
  2016-06-13 13:25 [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Paolo Bonzini
                   ` (3 preceding siblings ...)
  2016-06-13 13:33 ` [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Christian Borntraeger
@ 2016-06-13 14:44 ` Cornelia Huck
  2016-06-15 22:00   ` Paolo Bonzini
  4 siblings, 1 reply; 13+ messages in thread
From: Cornelia Huck @ 2016-06-13 14:44 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, Christian Borntraeger

On Mon, 13 Jun 2016 15:25:29 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> kvm->online_vcpus is only updated at the very end of KVM_CREATE_VCPU.
> Thus, it can be racy to use it as a check for "has any VCPU been created
> already?".  While x86 detects the race through kvm_vcpu_compatible,
> it is kinda hackish and s390 does not do anything similar.  Provide a
> better fix for both.
> 
> Paolo Bonzini (3):
>   KVM: introduce created_vcpus
>   KVM: remove kvm_vcpu_compatible
>   KVM: s390: use created_vcpus
> 
>  arch/s390/kvm/kvm-s390.c | 10 +++++-----
>  arch/x86/kvm/Kconfig     |  1 -
>  arch/x86/kvm/x86.c       | 11 +++--------
>  include/linux/kvm_host.h | 14 ++++++++------
>  virt/kvm/Kconfig         |  3 ---
>  virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
>  6 files changed, 33 insertions(+), 33 deletions(-)
> 

Looks like a sane approach, only two inversions in the s390 patch :)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
  2016-06-13 14:44 ` Cornelia Huck
@ 2016-06-15 22:00   ` Paolo Bonzini
  2016-06-16  7:30     ` Christian Borntraeger
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2016-06-15 22:00 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: linux-kernel, kvm, Christian Borntraeger



On 13/06/2016 16:44, Cornelia Huck wrote:
>> >  arch/s390/kvm/kvm-s390.c | 10 +++++-----
>> >  arch/x86/kvm/Kconfig     |  1 -
>> >  arch/x86/kvm/x86.c       | 11 +++--------
>> >  include/linux/kvm_host.h | 14 ++++++++------
>> >  virt/kvm/Kconfig         |  3 ---
>> >  virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
>> >  6 files changed, 33 insertions(+), 33 deletions(-)
>> > 
> Looks like a sane approach, only two inversions in the s390 patch :)

So it's okay to push patch 3 to kvm/next?

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
  2016-06-15 22:00   ` Paolo Bonzini
@ 2016-06-16  7:30     ` Christian Borntraeger
  2016-06-16  7:59       ` Cornelia Huck
  2016-06-16  8:07       ` Paolo Bonzini
  0 siblings, 2 replies; 13+ messages in thread
From: Christian Borntraeger @ 2016-06-16  7:30 UTC (permalink / raw)
  To: Paolo Bonzini, Cornelia Huck; +Cc: linux-kernel, kvm, Christian Borntraeger

On 06/16/2016 12:00 AM, Paolo Bonzini wrote:
> 
> 
> On 13/06/2016 16:44, Cornelia Huck wrote:
>>>>  arch/s390/kvm/kvm-s390.c | 10 +++++-----
>>>>  arch/x86/kvm/Kconfig     |  1 -
>>>>  arch/x86/kvm/x86.c       | 11 +++--------
>>>>  include/linux/kvm_host.h | 14 ++++++++------
>>>>  virt/kvm/Kconfig         |  3 ---
>>>>  virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
>>>>  6 files changed, 33 insertions(+), 33 deletions(-)
>>>>
>> Looks like a sane approach, only two inversions in the s390 patch :)
> 
> So it's okay to push patch 3 to kvm/next?


With the 2 fixes that Conny requested, yes.

With that fixed up, 
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
  2016-06-16  7:30     ` Christian Borntraeger
@ 2016-06-16  7:59       ` Cornelia Huck
  2016-06-16  8:23         ` Paolo Bonzini
  2016-06-16  8:07       ` Paolo Bonzini
  1 sibling, 1 reply; 13+ messages in thread
From: Cornelia Huck @ 2016-06-16  7:59 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Christian Borntraeger, linux-kernel, kvm, Christian Borntraeger

On Thu, 16 Jun 2016 09:30:34 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 06/16/2016 12:00 AM, Paolo Bonzini wrote:
> > 
> > 
> > On 13/06/2016 16:44, Cornelia Huck wrote:
> >>>>  arch/s390/kvm/kvm-s390.c | 10 +++++-----
> >>>>  arch/x86/kvm/Kconfig     |  1 -
> >>>>  arch/x86/kvm/x86.c       | 11 +++--------
> >>>>  include/linux/kvm_host.h | 14 ++++++++------
> >>>>  virt/kvm/Kconfig         |  3 ---
> >>>>  virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
> >>>>  6 files changed, 33 insertions(+), 33 deletions(-)
> >>>>
> >> Looks like a sane approach, only two inversions in the s390 patch :)
> > 
> > So it's okay to push patch 3 to kvm/next?
> 
> 
> With the 2 fixes that Conny requested, yes.

I had been waiting for a v2 ;)

> 
> With that fixed up, 
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 

In that case, you can also add my

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>

to the s390 patch.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
  2016-06-16  7:30     ` Christian Borntraeger
  2016-06-16  7:59       ` Cornelia Huck
@ 2016-06-16  8:07       ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-06-16  8:07 UTC (permalink / raw)
  To: Christian Borntraeger, Cornelia Huck
  Cc: linux-kernel, kvm, Christian Borntraeger



On 16/06/2016 09:30, Christian Borntraeger wrote:
> On 06/16/2016 12:00 AM, Paolo Bonzini wrote:
>>
>>
>> On 13/06/2016 16:44, Cornelia Huck wrote:
>>>>>  arch/s390/kvm/kvm-s390.c | 10 +++++-----
>>>>>  arch/x86/kvm/Kconfig     |  1 -
>>>>>  arch/x86/kvm/x86.c       | 11 +++--------
>>>>>  include/linux/kvm_host.h | 14 ++++++++------
>>>>>  virt/kvm/Kconfig         |  3 ---
>>>>>  virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
>>>>>  6 files changed, 33 insertions(+), 33 deletions(-)
>>>>>
>>> Looks like a sane approach, only two inversions in the s390 patch :)
>>
>> So it's okay to push patch 3 to kvm/next?
> 
> 
> With the 2 fixes that Conny requested, yes.
> 
> With that fixed up, 
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

Yes, of course.  Pushed, thanks!

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?"
  2016-06-16  7:59       ` Cornelia Huck
@ 2016-06-16  8:23         ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2016-06-16  8:23 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Christian Borntraeger, linux-kernel, kvm, Christian Borntraeger



On 16/06/2016 09:59, Cornelia Huck wrote:
> On Thu, 16 Jun 2016 09:30:34 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 06/16/2016 12:00 AM, Paolo Bonzini wrote:
>>>
>>>
>>> On 13/06/2016 16:44, Cornelia Huck wrote:
>>>>>>  arch/s390/kvm/kvm-s390.c | 10 +++++-----
>>>>>>  arch/x86/kvm/Kconfig     |  1 -
>>>>>>  arch/x86/kvm/x86.c       | 11 +++--------
>>>>>>  include/linux/kvm_host.h | 14 ++++++++------
>>>>>>  virt/kvm/Kconfig         |  3 ---
>>>>>>  virt/kvm/kvm_main.c      | 27 +++++++++++++++++----------
>>>>>>  6 files changed, 33 insertions(+), 33 deletions(-)
>>>>>>
>>>> Looks like a sane approach, only two inversions in the s390 patch :)
>>>
>>> So it's okay to push patch 3 to kvm/next?
>>
>>
>> With the 2 fixes that Conny requested, yes.
> 
> I had been waiting for a v2 ;)

This is the patch I've pushed:

-------------------- 8< ---------------------
>From a03825bbd0c39feeba605912cdbc28e79e4e01e1 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 13 Jun 2016 14:50:04 +0200
Subject: [PATCH] KVM: s390: use kvm->created_vcpus

The new created_vcpus field avoids possible races between enabling
capabilities and creating VCPUs.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 49c60393a15c..0dcf9b8fc12c 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -422,7 +422,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 		break;
 	case KVM_CAP_S390_VECTOR_REGISTERS:
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus)) {
+		if (kvm->created_vcpus) {
 			r = -EBUSY;
 		} else if (MACHINE_HAS_VX) {
 			set_kvm_facility(kvm->arch.model.fac_mask, 129);
@@ -437,7 +437,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 	case KVM_CAP_S390_RI:
 		r = -EINVAL;
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus)) {
+		if (kvm->created_vcpus) {
 			r = -EBUSY;
 		} else if (test_facility(64)) {
 			set_kvm_facility(kvm->arch.model.fac_mask, 64);
@@ -492,7 +492,7 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
 		ret = -EBUSY;
 		VM_EVENT(kvm, 3, "%s", "ENABLE: CMMA support");
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus) == 0) {
+		if (!kvm->created_vcpus) {
 			kvm->arch.use_cmma = 1;
 			ret = 0;
 		}
@@ -536,7 +536,7 @@ static int kvm_s390_set_mem_control(struct kvm *kvm, struct kvm_device_attr *att
 
 		ret = -EBUSY;
 		mutex_lock(&kvm->lock);
-		if (atomic_read(&kvm->online_vcpus) == 0) {
+		if (!kvm->created_vcpus) {
 			/* gmap_alloc will round the limit up */
 			struct gmap *new = gmap_alloc(current->mm, new_limit);
 
@@ -713,7 +713,7 @@ static int kvm_s390_set_processor(struct kvm *kvm, struct kvm_device_attr *attr)
 	int ret = 0;
 
 	mutex_lock(&kvm->lock);
-	if (atomic_read(&kvm->online_vcpus)) {
+	if (kvm->created_vcpus) {
 		ret = -EBUSY;
 		goto out;
 	}

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-06-16  8:23 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13 13:25 [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Paolo Bonzini
2016-06-13 13:25 ` [PATCH 1/3] KVM: introduce kvm->created_vcpus Paolo Bonzini
2016-06-13 14:43   ` Cornelia Huck
2016-06-13 13:25 ` [PATCH 2/3] KVM: remove kvm_vcpu_compatible Paolo Bonzini
2016-06-13 13:25 ` [PATCH 3/3] KVM: s390: use kvm->created_vcpus Paolo Bonzini
2016-06-13 14:36   ` Cornelia Huck
2016-06-13 13:33 ` [PATCH 0/3] KVM: do not use kvm->online_vcpus to check "has one VCPU been created?" Christian Borntraeger
2016-06-13 14:44 ` Cornelia Huck
2016-06-15 22:00   ` Paolo Bonzini
2016-06-16  7:30     ` Christian Borntraeger
2016-06-16  7:59       ` Cornelia Huck
2016-06-16  8:23         ` Paolo Bonzini
2016-06-16  8:07       ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).