linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/1] KVM: s390: pv: make use of ultravisor AIV support
@ 2022-02-09 15:22 Michael Mueller
  2022-02-09 15:22 ` [PATCH v3 1/1] " Michael Mueller
  0 siblings, 1 reply; 8+ messages in thread
From: Michael Mueller @ 2022-02-09 15:22 UTC (permalink / raw)
  To: kvm
  Cc: cohuck, borntraeger, frankja, thuth, pasic, david, linux-s390,
	linux-kernel, Michael Mueller

This patch enables the ultravisor adapter interruption vitualization
support.

Changes in v3:
- cache and test GISA descriptor only once in kvm_s390_gisa_enable()
- modified some comments
- removed some whitespace issues

Changes in v2:
- moved GISA disable into "put CPUs in PV mode" routine
- moved GISA enable into "pull CPUs out of PV mode" routine 

[1] https://lore.kernel.org/lkml/ae7c65d8-f632-a7f4-926a-50b9660673a1@linux.ibm.com/T/#mcb67699bf458ba7482f6b7529afe589d1dbb5930
[2] https://lore.kernel.org/kvm/20220208165310.3905815-1-mimu@linux.ibm.com/T/#e0ecf74ad7fdccc5ed22007e783106d4ef7b849df 

Michael Mueller (1):
  KVM: s390: pv: make use of ultravisor AIV support

 arch/s390/include/asm/uv.h |  1 +
 arch/s390/kvm/interrupt.c  | 56 ++++++++++++++++++++++++++++++++++----
 arch/s390/kvm/kvm-s390.c   | 11 ++++++--
 arch/s390/kvm/kvm-s390.h   | 11 ++++++++
 4 files changed, 70 insertions(+), 9 deletions(-)

-- 
2.32.0


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

* [PATCH v3 1/1] KVM: s390: pv: make use of ultravisor AIV support
  2022-02-09 15:22 [PATCH v3 0/1] KVM: s390: pv: make use of ultravisor AIV support Michael Mueller
@ 2022-02-09 15:22 ` Michael Mueller
  2022-02-15 13:18   ` Janosch Frank
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Michael Mueller @ 2022-02-09 15:22 UTC (permalink / raw)
  To: kvm
  Cc: cohuck, borntraeger, frankja, thuth, pasic, david, linux-s390,
	linux-kernel, Michael Mueller

This patch enables the ultravisor adapter interruption vitualization
support indicated by UV feature BIT_UV_FEAT_AIV. This allows ISC
interruption injection directly into the GISA IPM for PV kvm guests.

Hardware that does not support this feature will continue to use the
UV interruption interception method to deliver ISC interruptions to
PV kvm guests. For this purpose, the ECA_AIV bit for all guest cpus
will be cleared and the GISA will be disabled during PV CPU setup.

In addition a check in __inject_io() has been removed. That reduces the
required instructions for interruption handling for PV and traditional
kvm guests.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/include/asm/uv.h |  1 +
 arch/s390/kvm/interrupt.c  | 54 +++++++++++++++++++++++++++++++++-----
 arch/s390/kvm/kvm-s390.c   | 11 +++++---
 arch/s390/kvm/kvm-s390.h   | 11 ++++++++
 4 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
index 86218382d29c..a2d376b8bce3 100644
--- a/arch/s390/include/asm/uv.h
+++ b/arch/s390/include/asm/uv.h
@@ -80,6 +80,7 @@ enum uv_cmds_inst {
 
 enum uv_feat_ind {
 	BIT_UV_FEAT_MISC = 0,
+	BIT_UV_FEAT_AIV = 1,
 };
 
 struct uv_cb_header {
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index db933c252dbc..9b30beac904d 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1901,13 +1901,12 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
 	isc = int_word_to_isc(inti->io.io_int_word);
 
 	/*
-	 * Do not make use of gisa in protected mode. We do not use the lock
-	 * checking variant as this is just a performance optimization and we
-	 * do not hold the lock here. This is ok as the code will pick
-	 * interrupts from both "lists" for delivery.
+	 * We do not use the lock checking variant as this is just a
+	 * performance optimization and we do not hold the lock here.
+	 * This is ok as the code will pick interrupts from both "lists"
+	 * for delivery.
 	 */
-	if (!kvm_s390_pv_get_handle(kvm) &&
-	    gi->origin && inti->type & KVM_S390_INT_IO_AI_MASK) {
+	if (gi->origin && inti->type & KVM_S390_INT_IO_AI_MASK) {
 		VM_EVENT(kvm, 4, "%s isc %1u", "inject: I/O (AI/gisa)", isc);
 		gisa_set_ipm_gisc(gi->origin, isc);
 		kfree(inti);
@@ -3171,9 +3170,33 @@ void kvm_s390_gisa_init(struct kvm *kvm)
 	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
 }
 
+void kvm_s390_gisa_enable(struct kvm *kvm)
+{
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+	struct kvm_vcpu *vcpu;
+	unsigned long i;
+	u32 gisa_desc;
+
+	if (gi->origin)
+		return;
+	kvm_s390_gisa_init(kvm);
+	gisa_desc = kvm_s390_get_gisa_desc(kvm);
+	if (!gisa_desc)
+		return;
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		mutex_lock(&vcpu->mutex);
+		vcpu->arch.sie_block->gd = gisa_desc;
+		vcpu->arch.sie_block->eca |= ECA_AIV;
+		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
+			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
+		mutex_unlock(&vcpu->mutex);
+	}
+}
+
 void kvm_s390_gisa_destroy(struct kvm *kvm)
 {
 	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+	struct kvm_s390_gisa *gisa = gi->origin;
 
 	if (!gi->origin)
 		return;
@@ -3184,6 +3207,25 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
 		cpu_relax();
 	hrtimer_cancel(&gi->timer);
 	gi->origin = NULL;
+	VM_EVENT(kvm, 3, "gisa 0x%pK destroyed", gisa);
+}
+
+void kvm_s390_gisa_disable(struct kvm *kvm)
+{
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+	struct kvm_vcpu *vcpu;
+	unsigned long i;
+
+	if (!gi->origin)
+		return;
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		mutex_lock(&vcpu->mutex);
+		vcpu->arch.sie_block->eca &= ~ECA_AIV;
+		vcpu->arch.sie_block->gd = 0U;
+		mutex_unlock(&vcpu->mutex);
+		VCPU_EVENT(vcpu, 3, "AIV disabled for cpu %03u", vcpu->vcpu_id);
+	}
+	kvm_s390_gisa_destroy(kvm);
 }
 
 /**
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 577f1ead6a51..c83330f98ff0 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2194,6 +2194,9 @@ static int kvm_s390_cpus_from_pv(struct kvm *kvm, u16 *rcp, u16 *rrcp)
 		}
 		mutex_unlock(&vcpu->mutex);
 	}
+	/* Ensure that we re-enable gisa if the non-PV guest used it but the PV guest did not. */
+	if (use_gisa)
+		kvm_s390_gisa_enable(kvm);
 	return ret;
 }
 
@@ -2205,6 +2208,10 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
 
 	struct kvm_vcpu *vcpu;
 
+	/* Disable the GISA if the ultravisor does not support AIV. */
+	if (!test_bit_inv(BIT_UV_FEAT_AIV, &uv_info.uv_feature_indications))
+		kvm_s390_gisa_disable(kvm);
+
 	kvm_for_each_vcpu(i, vcpu, kvm) {
 		mutex_lock(&vcpu->mutex);
 		r = kvm_s390_pv_create_cpu(vcpu, rc, rrc);
@@ -3263,9 +3270,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 
 	vcpu->arch.sie_block->icpua = vcpu->vcpu_id;
 	spin_lock_init(&vcpu->arch.local_int.lock);
-	vcpu->arch.sie_block->gd = (u32)(u64)vcpu->kvm->arch.gisa_int.origin;
-	if (vcpu->arch.sie_block->gd && sclp.has_gisaf)
-		vcpu->arch.sie_block->gd |= GISA_FORMAT1;
+	vcpu->arch.sie_block->gd = kvm_s390_get_gisa_desc(vcpu->kvm);
 	seqcount_init(&vcpu->arch.cputm_seqcount);
 
 	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 098831e815e6..4ba8fc30d87a 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -231,6 +231,15 @@ static inline unsigned long kvm_s390_get_gfn_end(struct kvm_memslots *slots)
 	return ms->base_gfn + ms->npages;
 }
 
+static inline u32 kvm_s390_get_gisa_desc(struct kvm *kvm)
+{
+	u32 gd = (u32)(u64)kvm->arch.gisa_int.origin;
+
+	if (gd && sclp.has_gisaf)
+		gd |= GISA_FORMAT1;
+	return gd;
+}
+
 /* implemented in pv.c */
 int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
 int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
@@ -450,6 +459,8 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu,
 void kvm_s390_gisa_init(struct kvm *kvm);
 void kvm_s390_gisa_clear(struct kvm *kvm);
 void kvm_s390_gisa_destroy(struct kvm *kvm);
+void kvm_s390_gisa_disable(struct kvm *kvm);
+void kvm_s390_gisa_enable(struct kvm *kvm);
 int kvm_s390_gib_init(u8 nisc);
 void kvm_s390_gib_destroy(void);
 
-- 
2.32.0


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

* Re: [PATCH v3 1/1] KVM: s390: pv: make use of ultravisor AIV support
  2022-02-09 15:22 ` [PATCH v3 1/1] " Michael Mueller
@ 2022-02-15 13:18   ` Janosch Frank
  2022-02-15 15:16   ` Christian Borntraeger
  2022-02-22  8:13   ` Christian Borntraeger
  2 siblings, 0 replies; 8+ messages in thread
From: Janosch Frank @ 2022-02-15 13:18 UTC (permalink / raw)
  To: Michael Mueller, kvm
  Cc: cohuck, borntraeger, thuth, pasic, david, linux-s390, linux-kernel

On 2/9/22 16:22, Michael Mueller wrote:
> This patch enables the ultravisor adapter interruption vitualization
> support indicated by UV feature BIT_UV_FEAT_AIV. This allows ISC
> interruption injection directly into the GISA IPM for PV kvm guests.
> 
> Hardware that does not support this feature will continue to use the
> UV interruption interception method to deliver ISC interruptions to
> PV kvm guests. For this purpose, the ECA_AIV bit for all guest cpus
> will be cleared and the GISA will be disabled during PV CPU setup.
> 
> In addition a check in __inject_io() has been removed. That reduces the
> required instructions for interruption handling for PV and traditional
> kvm guests.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>   arch/s390/include/asm/uv.h |  1 +
>   arch/s390/kvm/interrupt.c  | 54 +++++++++++++++++++++++++++++++++-----
>   arch/s390/kvm/kvm-s390.c   | 11 +++++---
>   arch/s390/kvm/kvm-s390.h   | 11 ++++++++
>   4 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 86218382d29c..a2d376b8bce3 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -80,6 +80,7 @@ enum uv_cmds_inst {
>   
>   enum uv_feat_ind {
>   	BIT_UV_FEAT_MISC = 0,
> +	BIT_UV_FEAT_AIV = 1,
>   };
>   
>   struct uv_cb_header {
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index db933c252dbc..9b30beac904d 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -1901,13 +1901,12 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
>   	isc = int_word_to_isc(inti->io.io_int_word);
>   
>   	/*
> -	 * Do not make use of gisa in protected mode. We do not use the lock
> -	 * checking variant as this is just a performance optimization and we
> -	 * do not hold the lock here. This is ok as the code will pick
> -	 * interrupts from both "lists" for delivery.
> +	 * We do not use the lock checking variant as this is just a
> +	 * performance optimization and we do not hold the lock here.
> +	 * This is ok as the code will pick interrupts from both "lists"
> +	 * for delivery.
>   	 */
> -	if (!kvm_s390_pv_get_handle(kvm) &&
> -	    gi->origin && inti->type & KVM_S390_INT_IO_AI_MASK) {
> +	if (gi->origin && inti->type & KVM_S390_INT_IO_AI_MASK) {
>   		VM_EVENT(kvm, 4, "%s isc %1u", "inject: I/O (AI/gisa)", isc);
>   		gisa_set_ipm_gisc(gi->origin, isc);
>   		kfree(inti);
> @@ -3171,9 +3170,33 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>   	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>   }
>   
> +void kvm_s390_gisa_enable(struct kvm *kvm)
> +{
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_vcpu *vcpu;
> +	unsigned long i;
> +	u32 gisa_desc;
> +
> +	if (gi->origin)
> +		return;
> +	kvm_s390_gisa_init(kvm);
> +	gisa_desc = kvm_s390_get_gisa_desc(kvm);
> +	if (!gisa_desc)
> +		return;
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		mutex_lock(&vcpu->mutex);
> +		vcpu->arch.sie_block->gd = gisa_desc;
> +		vcpu->arch.sie_block->eca |= ECA_AIV;
> +		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
> +			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
> +		mutex_unlock(&vcpu->mutex);
> +	}
> +}
> +
>   void kvm_s390_gisa_destroy(struct kvm *kvm)
>   {
>   	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_s390_gisa *gisa = gi->origin;
>   
>   	if (!gi->origin)
>   		return;
> @@ -3184,6 +3207,25 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
>   		cpu_relax();
>   	hrtimer_cancel(&gi->timer);
>   	gi->origin = NULL;
> +	VM_EVENT(kvm, 3, "gisa 0x%pK destroyed", gisa);
> +}
> +
> +void kvm_s390_gisa_disable(struct kvm *kvm)
> +{
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_vcpu *vcpu;
> +	unsigned long i;
> +
> +	if (!gi->origin)
> +		return;
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		mutex_lock(&vcpu->mutex);
> +		vcpu->arch.sie_block->eca &= ~ECA_AIV;
> +		vcpu->arch.sie_block->gd = 0U;
> +		mutex_unlock(&vcpu->mutex);
> +		VCPU_EVENT(vcpu, 3, "AIV disabled for cpu %03u", vcpu->vcpu_id);
> +	}
> +	kvm_s390_gisa_destroy(kvm);
>   }
>   
>   /**
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 577f1ead6a51..c83330f98ff0 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2194,6 +2194,9 @@ static int kvm_s390_cpus_from_pv(struct kvm *kvm, u16 *rcp, u16 *rrcp)
>   		}
>   		mutex_unlock(&vcpu->mutex);
>   	}
> +	/* Ensure that we re-enable gisa if the non-PV guest used it but the PV guest did not. */
> +	if (use_gisa)
> +		kvm_s390_gisa_enable(kvm);
>   	return ret;
>   }
>   
> @@ -2205,6 +2208,10 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>   
>   	struct kvm_vcpu *vcpu;
>   
> +	/* Disable the GISA if the ultravisor does not support AIV. */
> +	if (!test_bit_inv(BIT_UV_FEAT_AIV, &uv_info.uv_feature_indications))
> +		kvm_s390_gisa_disable(kvm);
> +
>   	kvm_for_each_vcpu(i, vcpu, kvm) {
>   		mutex_lock(&vcpu->mutex);
>   		r = kvm_s390_pv_create_cpu(vcpu, rc, rrc);
> @@ -3263,9 +3270,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>   
>   	vcpu->arch.sie_block->icpua = vcpu->vcpu_id;
>   	spin_lock_init(&vcpu->arch.local_int.lock);
> -	vcpu->arch.sie_block->gd = (u32)(u64)vcpu->kvm->arch.gisa_int.origin;
> -	if (vcpu->arch.sie_block->gd && sclp.has_gisaf)
> -		vcpu->arch.sie_block->gd |= GISA_FORMAT1;
> +	vcpu->arch.sie_block->gd = kvm_s390_get_gisa_desc(vcpu->kvm);
>   	seqcount_init(&vcpu->arch.cputm_seqcount);
>   
>   	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
> diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
> index 098831e815e6..4ba8fc30d87a 100644
> --- a/arch/s390/kvm/kvm-s390.h
> +++ b/arch/s390/kvm/kvm-s390.h
> @@ -231,6 +231,15 @@ static inline unsigned long kvm_s390_get_gfn_end(struct kvm_memslots *slots)
>   	return ms->base_gfn + ms->npages;
>   }
>   
> +static inline u32 kvm_s390_get_gisa_desc(struct kvm *kvm)
> +{
> +	u32 gd = (u32)(u64)kvm->arch.gisa_int.origin;
> +
> +	if (gd && sclp.has_gisaf)
> +		gd |= GISA_FORMAT1;
> +	return gd;
> +}
> +
>   /* implemented in pv.c */
>   int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
>   int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
> @@ -450,6 +459,8 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu,
>   void kvm_s390_gisa_init(struct kvm *kvm);
>   void kvm_s390_gisa_clear(struct kvm *kvm);
>   void kvm_s390_gisa_destroy(struct kvm *kvm);
> +void kvm_s390_gisa_disable(struct kvm *kvm);
> +void kvm_s390_gisa_enable(struct kvm *kvm);
>   int kvm_s390_gib_init(u8 nisc);
>   void kvm_s390_gib_destroy(void);
>   


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

* Re: [PATCH v3 1/1] KVM: s390: pv: make use of ultravisor AIV support
  2022-02-09 15:22 ` [PATCH v3 1/1] " Michael Mueller
  2022-02-15 13:18   ` Janosch Frank
@ 2022-02-15 15:16   ` Christian Borntraeger
  2022-02-22  8:13   ` Christian Borntraeger
  2 siblings, 0 replies; 8+ messages in thread
From: Christian Borntraeger @ 2022-02-15 15:16 UTC (permalink / raw)
  To: Michael Mueller, kvm
  Cc: cohuck, frankja, thuth, pasic, david, linux-s390, linux-kernel



Am 09.02.22 um 16:22 schrieb Michael Mueller:
> This patch enables the ultravisor adapter interruption vitualization
> support indicated by UV feature BIT_UV_FEAT_AIV. This allows ISC
> interruption injection directly into the GISA IPM for PV kvm guests.
> 
> Hardware that does not support this feature will continue to use the
> UV interruption interception method to deliver ISC interruptions to
> PV kvm guests. For this purpose, the ECA_AIV bit for all guest cpus
> will be cleared and the GISA will be disabled during PV CPU setup.
> 
> In addition a check in __inject_io() has been removed. That reduces the
> required instructions for interruption handling for PV and traditional
> kvm guests.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

We have to be careful that our turn off/turn on does not break anything, but
this will make the hot path __inject_io cheaper so it is probably a good thing.
I will apply/queue this so that we get some CI coverage. Patch looks good to
me.

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>   arch/s390/include/asm/uv.h |  1 +
>   arch/s390/kvm/interrupt.c  | 54 +++++++++++++++++++++++++++++++++-----
>   arch/s390/kvm/kvm-s390.c   | 11 +++++---
>   arch/s390/kvm/kvm-s390.h   | 11 ++++++++
>   4 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/s390/include/asm/uv.h b/arch/s390/include/asm/uv.h
> index 86218382d29c..a2d376b8bce3 100644
> --- a/arch/s390/include/asm/uv.h
> +++ b/arch/s390/include/asm/uv.h
> @@ -80,6 +80,7 @@ enum uv_cmds_inst {
>   
>   enum uv_feat_ind {
>   	BIT_UV_FEAT_MISC = 0,
> +	BIT_UV_FEAT_AIV = 1,
>   };
>   
>   struct uv_cb_header {
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index db933c252dbc..9b30beac904d 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -1901,13 +1901,12 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
>   	isc = int_word_to_isc(inti->io.io_int_word);
>   
>   	/*
> -	 * Do not make use of gisa in protected mode. We do not use the lock
> -	 * checking variant as this is just a performance optimization and we
> -	 * do not hold the lock here. This is ok as the code will pick
> -	 * interrupts from both "lists" for delivery.
> +	 * We do not use the lock checking variant as this is just a
> +	 * performance optimization and we do not hold the lock here.
> +	 * This is ok as the code will pick interrupts from both "lists"
> +	 * for delivery.
>   	 */
> -	if (!kvm_s390_pv_get_handle(kvm) &&
> -	    gi->origin && inti->type & KVM_S390_INT_IO_AI_MASK) {
> +	if (gi->origin && inti->type & KVM_S390_INT_IO_AI_MASK) {
>   		VM_EVENT(kvm, 4, "%s isc %1u", "inject: I/O (AI/gisa)", isc);
>   		gisa_set_ipm_gisc(gi->origin, isc);
>   		kfree(inti);
> @@ -3171,9 +3170,33 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>   	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>   }
>   
> +void kvm_s390_gisa_enable(struct kvm *kvm)
> +{
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_vcpu *vcpu;
> +	unsigned long i;
> +	u32 gisa_desc;
> +
> +	if (gi->origin)
> +		return;
> +	kvm_s390_gisa_init(kvm);
> +	gisa_desc = kvm_s390_get_gisa_desc(kvm);
> +	if (!gisa_desc)
> +		return;
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		mutex_lock(&vcpu->mutex);
> +		vcpu->arch.sie_block->gd = gisa_desc;
> +		vcpu->arch.sie_block->eca |= ECA_AIV;
> +		VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
> +			   vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
> +		mutex_unlock(&vcpu->mutex);
> +	}
> +}
> +
>   void kvm_s390_gisa_destroy(struct kvm *kvm)
>   {
>   	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_s390_gisa *gisa = gi->origin;
>   
>   	if (!gi->origin)
>   		return;
> @@ -3184,6 +3207,25 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
>   		cpu_relax();
>   	hrtimer_cancel(&gi->timer);
>   	gi->origin = NULL;
> +	VM_EVENT(kvm, 3, "gisa 0x%pK destroyed", gisa);
> +}
> +
> +void kvm_s390_gisa_disable(struct kvm *kvm)
> +{
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_vcpu *vcpu;
> +	unsigned long i;
> +
> +	if (!gi->origin)
> +		return;
> +	kvm_for_each_vcpu(i, vcpu, kvm) {
> +		mutex_lock(&vcpu->mutex);
> +		vcpu->arch.sie_block->eca &= ~ECA_AIV;
> +		vcpu->arch.sie_block->gd = 0U;
> +		mutex_unlock(&vcpu->mutex);
> +		VCPU_EVENT(vcpu, 3, "AIV disabled for cpu %03u", vcpu->vcpu_id);
> +	}
> +	kvm_s390_gisa_destroy(kvm);
>   }
>   
>   /**
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 577f1ead6a51..c83330f98ff0 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2194,6 +2194,9 @@ static int kvm_s390_cpus_from_pv(struct kvm *kvm, u16 *rcp, u16 *rrcp)
>   		}
>   		mutex_unlock(&vcpu->mutex);
>   	}
> +	/* Ensure that we re-enable gisa if the non-PV guest used it but the PV guest did not. */
> +	if (use_gisa)
> +		kvm_s390_gisa_enable(kvm);
>   	return ret;
>   }
>   
> @@ -2205,6 +2208,10 @@ static int kvm_s390_cpus_to_pv(struct kvm *kvm, u16 *rc, u16 *rrc)
>   
>   	struct kvm_vcpu *vcpu;
>   
> +	/* Disable the GISA if the ultravisor does not support AIV. */
> +	if (!test_bit_inv(BIT_UV_FEAT_AIV, &uv_info.uv_feature_indications))
> +		kvm_s390_gisa_disable(kvm);
> +
>   	kvm_for_each_vcpu(i, vcpu, kvm) {
>   		mutex_lock(&vcpu->mutex);
>   		r = kvm_s390_pv_create_cpu(vcpu, rc, rrc);
> @@ -3263,9 +3270,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>   
>   	vcpu->arch.sie_block->icpua = vcpu->vcpu_id;
>   	spin_lock_init(&vcpu->arch.local_int.lock);
> -	vcpu->arch.sie_block->gd = (u32)(u64)vcpu->kvm->arch.gisa_int.origin;
> -	if (vcpu->arch.sie_block->gd && sclp.has_gisaf)
> -		vcpu->arch.sie_block->gd |= GISA_FORMAT1;
> +	vcpu->arch.sie_block->gd = kvm_s390_get_gisa_desc(vcpu->kvm);
>   	seqcount_init(&vcpu->arch.cputm_seqcount);
>   
>   	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
> diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
> index 098831e815e6..4ba8fc30d87a 100644
> --- a/arch/s390/kvm/kvm-s390.h
> +++ b/arch/s390/kvm/kvm-s390.h
> @@ -231,6 +231,15 @@ static inline unsigned long kvm_s390_get_gfn_end(struct kvm_memslots *slots)
>   	return ms->base_gfn + ms->npages;
>   }
>   
> +static inline u32 kvm_s390_get_gisa_desc(struct kvm *kvm)
> +{
> +	u32 gd = (u32)(u64)kvm->arch.gisa_int.origin;
> +
> +	if (gd && sclp.has_gisaf)
> +		gd |= GISA_FORMAT1;
> +	return gd;
> +}
> +
>   /* implemented in pv.c */
>   int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
>   int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
> @@ -450,6 +459,8 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu,
>   void kvm_s390_gisa_init(struct kvm *kvm);
>   void kvm_s390_gisa_clear(struct kvm *kvm);
>   void kvm_s390_gisa_destroy(struct kvm *kvm);
> +void kvm_s390_gisa_disable(struct kvm *kvm);
> +void kvm_s390_gisa_enable(struct kvm *kvm);
>   int kvm_s390_gib_init(u8 nisc);
>   void kvm_s390_gib_destroy(void);
>   

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

* Re: [PATCH v3 1/1] KVM: s390: pv: make use of ultravisor AIV support
  2022-02-09 15:22 ` [PATCH v3 1/1] " Michael Mueller
  2022-02-15 13:18   ` Janosch Frank
  2022-02-15 15:16   ` Christian Borntraeger
@ 2022-02-22  8:13   ` Christian Borntraeger
  2022-02-22 17:27     ` Michael Mueller
  2022-02-24 15:47     ` Michael Mueller
  2 siblings, 2 replies; 8+ messages in thread
From: Christian Borntraeger @ 2022-02-22  8:13 UTC (permalink / raw)
  To: Michael Mueller, kvm
  Cc: cohuck, frankja, thuth, pasic, david, linux-s390, linux-kernel

Am 09.02.22 um 16:22 schrieb Michael Mueller:
> This patch enables the ultravisor adapter interruption vitualization
> support indicated by UV feature BIT_UV_FEAT_AIV. This allows ISC
> interruption injection directly into the GISA IPM for PV kvm guests.
> 
> Hardware that does not support this feature will continue to use the
> UV interruption interception method to deliver ISC interruptions to
> PV kvm guests. For this purpose, the ECA_AIV bit for all guest cpus
> will be cleared and the GISA will be disabled during PV CPU setup.
> 
> In addition a check in __inject_io() has been removed. That reduces the
> required instructions for interruption handling for PV and traditional
> kvm guests.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

The CI said the following with gisa_disable in the calltrace.
Will drop from next for now.

    LOCKDEP_CIRCULAR (suite: kvm-unit-tests-kvm, case: -)
                 WARNING: possible circular locking dependency detected
                 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1 Not tainted
                 ------------------------------------------------------
                 qemu-system-s39/161139 is trying to acquire lock:
                 0000000280dc0b98 (&kvm->lock){+.+.}-{3:3}, at: kvm_s390_set_tod_clock+0x36/0x220 [kvm]
                 but task is already holding lock:
                 0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
                 which lock already depends on the new lock.
                 the existing dependency chain (in reverse order) is:
                 -> #1 (&vcpu->mutex){+.+.}-{3:3}:
                        __lock_acquire+0x604/0xbd8
                        lock_acquire.part.0+0xe2/0x250
                        lock_acquire+0xb0/0x200
                        __mutex_lock+0x9e/0x8a0
                        mutex_lock_nested+0x32/0x40
                        kvm_s390_gisa_disable+0xa4/0x130 [kvm]
                        kvm_s390_handle_pv+0x718/0x778 [kvm]
                        kvm_arch_vm_ioctl+0x4ac/0x5f8 [kvm]
                        kvm_vm_ioctl+0x336/0x530 [kvm]
                        __s390x_sys_ioctl+0xbe/0x100
                        __do_syscall+0x1da/0x208
                        system_call+0x82/0xb0
                 -> #0 (&kvm->lock){+.+.}-{3:3}:
                        check_prev_add+0xe0/0xed8
                        validate_chain+0x736/0xb20
                        __lock_acquire+0x604/0xbd8
                        lock_acquire.part.0+0xe2/0x250
                        lock_acquire+0xb0/0x200
                        __mutex_lock+0x9e/0x8a0
                        mutex_lock_nested+0x32/0x40
                        kvm_s390_set_tod_clock+0x36/0x220 [kvm]
                        kvm_s390_handle_b2+0x378/0x728 [kvm]
                        kvm_handle_sie_intercept+0x13a/0x448 [kvm]
                        vcpu_post_run+0x28e/0x560 [kvm]
                        __vcpu_run+0x266/0x388 [kvm]
                        kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
                        kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
                        __s390x_sys_ioctl+0xbe/0x100
                        __do_syscall+0x1da/0x208
                        system_call+0x82/0xb0
                 other info that might help us debug this:
                  Possible unsafe locking scenario:
                        CPU0                    CPU1
                        ----                    ----
                   lock(&vcpu->mutex);
                                                lock(&kvm->lock);
                                                lock(&vcpu->mutex);
                   lock(&kvm->lock);
                  *** DEADLOCK ***
                 2 locks held by qemu-system-s39/161139:
                  #0: 0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
                  #1: 0000000280dc47c8 (&kvm->srcu){....}-{0:0}, at: __vcpu_run+0x1d4/0x388 [kvm]
                 stack backtrace:
                 CPU: 10 PID: 161139 Comm: qemu-system-s39 Not tainted 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1
                 Hardware name: IBM 8561 T01 701 (LPAR)
                 Call Trace:
                  [<00000001da4e89de>] dump_stack_lvl+0x8e/0xc8
                  [<00000001d9876c56>] check_noncircular+0x136/0x158
                  [<00000001d9877c70>] check_prev_add+0xe0/0xed8
                  [<00000001d987919e>] validate_chain+0x736/0xb20
                  [<00000001d987b23c>] __lock_acquire+0x604/0xbd8
                  [<00000001d987c432>] lock_acquire.part.0+0xe2/0x250
                  [<00000001d987c650>] lock_acquire+0xb0/0x200
                  [<00000001da4f72ae>] __mutex_lock+0x9e/0x8a0
                  [<00000001da4f7ae2>] mutex_lock_nested+0x32/0x40
                  [<000003ff8070cd6e>] kvm_s390_set_tod_clock+0x36/0x220 [kvm]
                  [<000003ff8071dd68>] kvm_s390_handle_b2+0x378/0x728 [kvm]
                  [<000003ff8071146a>] kvm_handle_sie_intercept+0x13a/0x448 [kvm]
                  [<000003ff8070dd46>] vcpu_post_run+0x28e/0x560 [kvm]
                  [<000003ff8070e27e>] __vcpu_run+0x266/0x388 [kvm]
                  [<000003ff8070eba2>] kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
                  [<000003ff806f4044>] kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
                  [<00000001d9b47ac6>] __s390x_sys_ioctl+0xbe/0x100
                  [<00000001da4ec152>] __do_syscall+0x1da/0x208
                  [<00000001da4fec42>] system_call+0x82/0xb0
                 INFO: lockdep is turned off.

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

* Re: [PATCH v3 1/1] KVM: s390: pv: make use of ultravisor AIV support
  2022-02-22  8:13   ` Christian Borntraeger
@ 2022-02-22 17:27     ` Michael Mueller
  2022-02-24 15:47     ` Michael Mueller
  1 sibling, 0 replies; 8+ messages in thread
From: Michael Mueller @ 2022-02-22 17:27 UTC (permalink / raw)
  To: Christian Borntraeger, kvm
  Cc: cohuck, frankja, thuth, pasic, david, linux-s390, linux-kernel



On 22.02.22 09:13, Christian Borntraeger wrote:
> Am 09.02.22 um 16:22 schrieb Michael Mueller:
>> This patch enables the ultravisor adapter interruption vitualization
>> support indicated by UV feature BIT_UV_FEAT_AIV. This allows ISC
>> interruption injection directly into the GISA IPM for PV kvm guests.
>>
>> Hardware that does not support this feature will continue to use the
>> UV interruption interception method to deliver ISC interruptions to
>> PV kvm guests. For this purpose, the ECA_AIV bit for all guest cpus
>> will be cleared and the GISA will be disabled during PV CPU setup.
>>
>> In addition a check in __inject_io() has been removed. That reduces the
>> required instructions for interruption handling for PV and traditional
>> kvm guests.
>>
>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> 
> The CI said the following with gisa_disable in the calltrace.
> Will drop from next for now.

The issue is reproducible with the GISA switched of:

echo > 0 /sys/modules/kvm/parameters/use_gisa

In that case the code for gisa_disable() is not touched.

The lock is taken in front of kvm_s390_pv_create_cpu()
in this case.

         kvm_for_each_vcpu(i, vcpu, kvm) {
                 mutex_lock(&vcpu->mutex);
                 r = kvm_s390_pv_create_cpu(vcpu, rc, rrc);
                 mutex_unlock(&vcpu->mutex);
                 if (r)
                         break;
         }

I have an idea how to prevent this and will send a patch for both
situations.

[  319.799638] ======================================================
[  319.799639] WARNING: possible circular locking dependency detected
[  319.799641] 5.17.0-rc5-08427-gfd14b6309198 #4661 Not tainted
[  319.799643] ------------------------------------------------------
[  319.799644] qemu-system-s39/14220 is trying to acquire lock:
[  319.799646] 00000000b30c0b50 (&kvm->lock){+.+.}-{3:3}, at: 
kvm_s390_set_tod_clock+0x36/0x250
[  319.799659]
                but task is already holding lock:
[  319.799660] 00000000b5beda60 (&vcpu->mutex){+.+.}-{3:3}, at: 
kvm_vcpu_ioctl+0x9a/0x958
[  319.799665]
                which lock already depends on the new lock.

[  319.799667]
                the existing dependency chain (in reverse order) is:
[  319.799668]
                -> #1 (&vcpu->mutex){+.+.}-{3:3}:
[  319.799671]        __mutex_lock+0x8a/0x798
[  319.799677]        mutex_lock_nested+0x32/0x40
[  319.799679]        kvm_arch_vm_ioctl+0x1902/0x2c58
[  319.799682]        kvm_vm_ioctl+0x5b0/0xa80
[  319.799685]        __s390x_sys_ioctl+0xbe/0x100
[  319.799688]        __do_syscall+0x1da/0x208
[  319.799689]        system_call+0x82/0xb0
[  319.799692]
                -> #0 (&kvm->lock){+.+.}-{3:3}:
[  319.799694]        __lock_acquire+0x1916/0x2e70
[  319.799699]        lock_acquire+0x164/0x388
[  319.799702]        __mutex_lock+0x8a/0x798
[  319.799757]        mutex_lock_nested+0x32/0x40
[  319.799759]        kvm_s390_set_tod_clock+0x36/0x250
[  319.799761]        kvm_s390_handle_b2+0x6cc/0x26f0
[  319.799764]        kvm_handle_sie_intercept+0x1fe/0xe98
[  319.799765]        kvm_arch_vcpu_ioctl_run+0xec8/0x1880
[  319.799768]        kvm_vcpu_ioctl+0x29e/0x958
[  319.799769]        __s390x_sys_ioctl+0xbe/0x100
[  319.799771]        __do_syscall+0x1da/0x208
[  319.799773]        system_call+0x82/0xb0
[  319.799774]
                other info that might help us debug this:

[  319.799776]  Possible unsafe locking scenario:

[  319.799777]        CPU0                    CPU1
[  319.799778]        ----                    ----
[  319.799779]   lock(&vcpu->mutex);
[  319.799780]                                lock(&kvm->lock);
[  319.799782]                                lock(&vcpu->mutex);
[  319.799783]   lock(&kvm->lock);
[  319.799784]
                 *** DEADLOCK ***

[  319.799785] 2 locks held by qemu-system-s39/14220:
[  319.799787]  #0: 00000000b5beda60 (&vcpu->mutex){+.+.}-{3:3}, at: 
kvm_vcpu_ioctl+0x9a/0x958
[  319.799791]  #1: 00000000b30c4588 (&kvm->srcu){....}-{0:0}, at: 
kvm_arch_vcpu_ioctl_run+0x6f2/0x1880
[  319.799796]
                stack backtrace:
[  319.799798] CPU: 5 PID: 14220 Comm: qemu-system-s39 Not tainted 
5.17.0-rc5-08427-gfd14b6309198 #4661
[  319.799801] Hardware name: IBM 8561 T01 701 (LPAR)
[  319.799802] Call Trace:
[  319.799803]  [<000000020d7410de>] dump_stack_lvl+0x76/0x98
[  319.799808]  [<000000020cbbd268>] check_noncircular+0x140/0x160
[  319.799811]  [<000000020cbc0efe>] __lock_acquire+0x1916/0x2e70
[  319.799813]  [<000000020cbc2dbc>] lock_acquire+0x164/0x388
[  319.799816]  [<000000020d75013a>] __mutex_lock+0x8a/0x798
[  319.799818]  [<000000020d75087a>] mutex_lock_nested+0x32/0x40
[  319.799820]  [<000000020cb029a6>] kvm_s390_set_tod_clock+0x36/0x250
[  319.799823]  [<000000020cb14d14>] kvm_s390_handle_b2+0x6cc/0x26f0
[  319.799825]  [<000000020cb09b6e>] kvm_handle_sie_intercept+0x1fe/0xe98
[  319.799827]  [<000000020cb06c28>] kvm_arch_vcpu_ioctl_run+0xec8/0x1880
[  319.799829]  [<000000020caeddc6>] kvm_vcpu_ioctl+0x29e/0x958
[  319.799831]  [<000000020ce4e82e>] __s390x_sys_ioctl+0xbe/0x100
[  319.799833]  [<000000020d744a72>] __do_syscall+0x1da/0x208
[  319.799835]  [<000000020d757322>] system_call+0x82/0xb0
[  319.799836] INFO: lockdep is turned off.


> 
>     LOCKDEP_CIRCULAR (suite: kvm-unit-tests-kvm, case: -)
>                  WARNING: possible circular locking dependency detected
>                  
> 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1 Not tainted
>                  ------------------------------------------------------
>                  qemu-system-s39/161139 is trying to acquire lock:
>                  0000000280dc0b98 (&kvm->lock){+.+.}-{3:3}, at: 
> kvm_s390_set_tod_clock+0x36/0x220 [kvm]
>                  but task is already holding lock:
>                  0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: 
> kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
>                  which lock already depends on the new lock.
>                  the existing dependency chain (in reverse order) is:
>                  -> #1 (&vcpu->mutex){+.+.}-{3:3}:
>                         __lock_acquire+0x604/0xbd8
>                         lock_acquire.part.0+0xe2/0x250
>                         lock_acquire+0xb0/0x200
>                         __mutex_lock+0x9e/0x8a0
>                         mutex_lock_nested+0x32/0x40
>                         kvm_s390_gisa_disable+0xa4/0x130 [kvm]
>                         kvm_s390_handle_pv+0x718/0x778 [kvm]
>                         kvm_arch_vm_ioctl+0x4ac/0x5f8 [kvm]
>                         kvm_vm_ioctl+0x336/0x530 [kvm]
>                         __s390x_sys_ioctl+0xbe/0x100
>                         __do_syscall+0x1da/0x208
>                         system_call+0x82/0xb0
>                  -> #0 (&kvm->lock){+.+.}-{3:3}:
>                         check_prev_add+0xe0/0xed8
>                         validate_chain+0x736/0xb20
>                         __lock_acquire+0x604/0xbd8
>                         lock_acquire.part.0+0xe2/0x250
>                         lock_acquire+0xb0/0x200
>                         __mutex_lock+0x9e/0x8a0
>                         mutex_lock_nested+0x32/0x40
>                         kvm_s390_set_tod_clock+0x36/0x220 [kvm]
>                         kvm_s390_handle_b2+0x378/0x728 [kvm]
>                         kvm_handle_sie_intercept+0x13a/0x448 [kvm]
>                         vcpu_post_run+0x28e/0x560 [kvm]
>                         __vcpu_run+0x266/0x388 [kvm]
>                         kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
>                         kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
>                         __s390x_sys_ioctl+0xbe/0x100
>                         __do_syscall+0x1da/0x208
>                         system_call+0x82/0xb0
>                  other info that might help us debug this:
>                   Possible unsafe locking scenario:
>                         CPU0                    CPU1
>                         ----                    ----
>                    lock(&vcpu->mutex);
>                                                 lock(&kvm->lock);
>                                                 lock(&vcpu->mutex);
>                    lock(&kvm->lock);
>                   *** DEADLOCK ***
>                  2 locks held by qemu-system-s39/161139:
>                   #0: 0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: 
> kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
>                   #1: 0000000280dc47c8 (&kvm->srcu){....}-{0:0}, at: 
> __vcpu_run+0x1d4/0x388 [kvm]
>                  stack backtrace:
>                  CPU: 10 PID: 161139 Comm: qemu-system-s39 Not tainted 
> 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1
>                  Hardware name: IBM 8561 T01 701 (LPAR)
>                  Call Trace:
>                   [<00000001da4e89de>] dump_stack_lvl+0x8e/0xc8
>                   [<00000001d9876c56>] check_noncircular+0x136/0x158
>                   [<00000001d9877c70>] check_prev_add+0xe0/0xed8
>                   [<00000001d987919e>] validate_chain+0x736/0xb20
>                   [<00000001d987b23c>] __lock_acquire+0x604/0xbd8
>                   [<00000001d987c432>] lock_acquire.part.0+0xe2/0x250
>                   [<00000001d987c650>] lock_acquire+0xb0/0x200
>                   [<00000001da4f72ae>] __mutex_lock+0x9e/0x8a0
>                   [<00000001da4f7ae2>] mutex_lock_nested+0x32/0x40
>                   [<000003ff8070cd6e>] kvm_s390_set_tod_clock+0x36/0x220 
> [kvm]
>                   [<000003ff8071dd68>] kvm_s390_handle_b2+0x378/0x728 [kvm]
>                   [<000003ff8071146a>] 
> kvm_handle_sie_intercept+0x13a/0x448 [kvm]
>                   [<000003ff8070dd46>] vcpu_post_run+0x28e/0x560 [kvm]
>                   [<000003ff8070e27e>] __vcpu_run+0x266/0x388 [kvm]
>                   [<000003ff8070eba2>] 
> kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
>                   [<000003ff806f4044>] kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
>                   [<00000001d9b47ac6>] __s390x_sys_ioctl+0xbe/0x100
>                   [<00000001da4ec152>] __do_syscall+0x1da/0x208
>                   [<00000001da4fec42>] system_call+0x82/0xb0
>                  INFO: lockdep is turned off.





[  319.799638] ======================================================
[  319.799639] WARNING: possible circular locking dependency detected
[  319.799641] 5.17.0-rc5-08427-gfd14b6309198 #4661 Not tainted
[  319.799643] ------------------------------------------------------
[  319.799644] qemu-system-s39/14220 is trying to acquire lock:
[  319.799646] 00000000b30c0b50 (&kvm->lock){+.+.}-{3:3}, at: 
kvm_s390_set_tod_clock+0x36/0x250
[  319.799659]
                but task is already holding lock:
[  319.799660] 00000000b5beda60 (&vcpu->mutex){+.+.}-{3:3}, at: 
kvm_vcpu_ioctl+0x9a/0x958
[  319.799665]
                which lock already depends on the new lock.

[  319.799667]
                the existing dependency chain (in reverse order) is:
[  319.799668]
                -> #1 (&vcpu->mutex){+.+.}-{3:3}:
[  319.799671]        __mutex_lock+0x8a/0x798
[  319.799677]        mutex_lock_nested+0x32/0x40
[  319.799679]        kvm_arch_vm_ioctl+0x1902/0x2c58
[  319.799682]        kvm_vm_ioctl+0x5b0/0xa80
[  319.799685]        __s390x_sys_ioctl+0xbe/0x100
[  319.799688]        __do_syscall+0x1da/0x208
[  319.799689]        system_call+0x82/0xb0
[  319.799692]
                -> #0 (&kvm->lock){+.+.}-{3:3}:
[  319.799694]        __lock_acquire+0x1916/0x2e70
[  319.799699]        lock_acquire+0x164/0x388
[  319.799702]        __mutex_lock+0x8a/0x798
[  319.799757]        mutex_lock_nested+0x32/0x40
[  319.799759]        kvm_s390_set_tod_clock+0x36/0x250
[  319.799761]        kvm_s390_handle_b2+0x6cc/0x26f0
[  319.799764]        kvm_handle_sie_intercept+0x1fe/0xe98
[  319.799765]        kvm_arch_vcpu_ioctl_run+0xec8/0x1880
[  319.799768]        kvm_vcpu_ioctl+0x29e/0x958
[  319.799769]        __s390x_sys_ioctl+0xbe/0x100
[  319.799771]        __do_syscall+0x1da/0x208
[  319.799773]        system_call+0x82/0xb0
[  319.799774]
                other info that might help us debug this:

[  319.799776]  Possible unsafe locking scenario:

[  319.799777]        CPU0                    CPU1
[  319.799778]        ----                    ----
[  319.799779]   lock(&vcpu->mutex);
[  319.799780]                                lock(&kvm->lock);
[  319.799782]                                lock(&vcpu->mutex);
[  319.799783]   lock(&kvm->lock);
[  319.799784]
                 *** DEADLOCK ***

[  319.799785] 2 locks held by qemu-system-s39/14220:
[  319.799787]  #0: 00000000b5beda60 (&vcpu->mutex){+.+.}-{3:3}, at: 
kvm_vcpu_ioctl+0x9a/0x958
[  319.799791]  #1: 00000000b30c4588 (&kvm->srcu){....}-{0:0}, at: 
kvm_arch_vcpu_ioctl_run+0x6f2/0x1880
[  319.799796]
                stack backtrace:
[  319.799798] CPU: 5 PID: 14220 Comm: qemu-system-s39 Not tainted 
5.17.0-rc5-08427-gfd14b6309198 #4661
[  319.799801] Hardware name: IBM 8561 T01 701 (LPAR)
[  319.799802] Call Trace:
[  319.799803]  [<000000020d7410de>] dump_stack_lvl+0x76/0x98
[  319.799808]  [<000000020cbbd268>] check_noncircular+0x140/0x160
[  319.799811]  [<000000020cbc0efe>] __lock_acquire+0x1916/0x2e70
[  319.799813]  [<000000020cbc2dbc>] lock_acquire+0x164/0x388
[  319.799816]  [<000000020d75013a>] __mutex_lock+0x8a/0x798
[  319.799818]  [<000000020d75087a>] mutex_lock_nested+0x32/0x40
[  319.799820]  [<000000020cb029a6>] kvm_s390_set_tod_clock+0x36/0x250
[  319.799823]  [<000000020cb14d14>] kvm_s390_handle_b2+0x6cc/0x26f0
[  319.799825]  [<000000020cb09b6e>] kvm_handle_sie_intercept+0x1fe/0xe98
[  319.799827]  [<000000020cb06c28>] kvm_arch_vcpu_ioctl_run+0xec8/0x1880
[  319.799829]  [<000000020caeddc6>] kvm_vcpu_ioctl+0x29e/0x958
[  319.799831]  [<000000020ce4e82e>] __s390x_sys_ioctl+0xbe/0x100
[  319.799833]  [<000000020d744a72>] __do_syscall+0x1da/0x208
[  319.799835]  [<000000020d757322>] system_call+0x82/0xb0
[  319.799836] INFO: lockdep is turned off.

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

* Re: [PATCH v3 1/1] KVM: s390: pv: make use of ultravisor AIV support
  2022-02-22  8:13   ` Christian Borntraeger
  2022-02-22 17:27     ` Michael Mueller
@ 2022-02-24 15:47     ` Michael Mueller
  2022-02-25 13:29       ` Christian Borntraeger
  1 sibling, 1 reply; 8+ messages in thread
From: Michael Mueller @ 2022-02-24 15:47 UTC (permalink / raw)
  To: Christian Borntraeger, kvm
  Cc: cohuck, frankja, thuth, pasic, david, linux-s390, linux-kernel



On 22.02.22 09:13, Christian Borntraeger wrote:
> Am 09.02.22 um 16:22 schrieb Michael Mueller:
>> This patch enables the ultravisor adapter interruption vitualization
>> support indicated by UV feature BIT_UV_FEAT_AIV. This allows ISC
>> interruption injection directly into the GISA IPM for PV kvm guests.
>>
>> Hardware that does not support this feature will continue to use the
>> UV interruption interception method to deliver ISC interruptions to
>> PV kvm guests. For this purpose, the ECA_AIV bit for all guest cpus
>> will be cleared and the GISA will be disabled during PV CPU setup.
>>
>> In addition a check in __inject_io() has been removed. That reduces the
>> required instructions for interruption handling for PV and traditional
>> kvm guests.
>>
>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> 
> The CI said the following with gisa_disable in the calltrace.
> Will drop from next for now.

It turns out this is related to kvm_s390_set_tod_clock() which
is triggered by a kvm-unit-test (sac_PV) and not related directly
to this patch. Please re-apply.

> 
>     LOCKDEP_CIRCULAR (suite: kvm-unit-tests-kvm, case: -)
>                  WARNING: possible circular locking dependency detected
>                  
> 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1 Not tainted
>                  ------------------------------------------------------
>                  qemu-system-s39/161139 is trying to acquire lock:
>                  0000000280dc0b98 (&kvm->lock){+.+.}-{3:3}, at: 
> kvm_s390_set_tod_clock+0x36/0x220 [kvm]
>                  but task is already holding lock:
>                  0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: 
> kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
>                  which lock already depends on the new lock.
>                  the existing dependency chain (in reverse order) is:
>                  -> #1 (&vcpu->mutex){+.+.}-{3:3}:
>                         __lock_acquire+0x604/0xbd8
>                         lock_acquire.part.0+0xe2/0x250
>                         lock_acquire+0xb0/0x200
>                         __mutex_lock+0x9e/0x8a0
>                         mutex_lock_nested+0x32/0x40
>                         kvm_s390_gisa_disable+0xa4/0x130 [kvm]
>                         kvm_s390_handle_pv+0x718/0x778 [kvm]
>                         kvm_arch_vm_ioctl+0x4ac/0x5f8 [kvm]
>                         kvm_vm_ioctl+0x336/0x530 [kvm]
>                         __s390x_sys_ioctl+0xbe/0x100
>                         __do_syscall+0x1da/0x208
>                         system_call+0x82/0xb0
>                  -> #0 (&kvm->lock){+.+.}-{3:3}:
>                         check_prev_add+0xe0/0xed8
>                         validate_chain+0x736/0xb20
>                         __lock_acquire+0x604/0xbd8
>                         lock_acquire.part.0+0xe2/0x250
>                         lock_acquire+0xb0/0x200
>                         __mutex_lock+0x9e/0x8a0
>                         mutex_lock_nested+0x32/0x40
>                         kvm_s390_set_tod_clock+0x36/0x220 [kvm]
>                         kvm_s390_handle_b2+0x378/0x728 [kvm]
>                         kvm_handle_sie_intercept+0x13a/0x448 [kvm]
>                         vcpu_post_run+0x28e/0x560 [kvm]
>                         __vcpu_run+0x266/0x388 [kvm]
>                         kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
>                         kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
>                         __s390x_sys_ioctl+0xbe/0x100
>                         __do_syscall+0x1da/0x208
>                         system_call+0x82/0xb0
>                  other info that might help us debug this:
>                   Possible unsafe locking scenario:
>                         CPU0                    CPU1
>                         ----                    ----
>                    lock(&vcpu->mutex);
>                                                 lock(&kvm->lock);
>                                                 lock(&vcpu->mutex);
>                    lock(&kvm->lock);
>                   *** DEADLOCK ***
>                  2 locks held by qemu-system-s39/161139:
>                   #0: 0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: 
> kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
>                   #1: 0000000280dc47c8 (&kvm->srcu){....}-{0:0}, at: 
> __vcpu_run+0x1d4/0x388 [kvm]
>                  stack backtrace:
>                  CPU: 10 PID: 161139 Comm: qemu-system-s39 Not tainted 
> 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1
>                  Hardware name: IBM 8561 T01 701 (LPAR)
>                  Call Trace:
>                   [<00000001da4e89de>] dump_stack_lvl+0x8e/0xc8
>                   [<00000001d9876c56>] check_noncircular+0x136/0x158
>                   [<00000001d9877c70>] check_prev_add+0xe0/0xed8
>                   [<00000001d987919e>] validate_chain+0x736/0xb20
>                   [<00000001d987b23c>] __lock_acquire+0x604/0xbd8
>                   [<00000001d987c432>] lock_acquire.part.0+0xe2/0x250
>                   [<00000001d987c650>] lock_acquire+0xb0/0x200
>                   [<00000001da4f72ae>] __mutex_lock+0x9e/0x8a0
>                   [<00000001da4f7ae2>] mutex_lock_nested+0x32/0x40
>                   [<000003ff8070cd6e>] kvm_s390_set_tod_clock+0x36/0x220 
> [kvm]
>                   [<000003ff8071dd68>] kvm_s390_handle_b2+0x378/0x728 [kvm]
>                   [<000003ff8071146a>] 
> kvm_handle_sie_intercept+0x13a/0x448 [kvm]
>                   [<000003ff8070dd46>] vcpu_post_run+0x28e/0x560 [kvm]
>                   [<000003ff8070e27e>] __vcpu_run+0x266/0x388 [kvm]
>                   [<000003ff8070eba2>] 
> kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
>                   [<000003ff806f4044>] kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
>                   [<00000001d9b47ac6>] __s390x_sys_ioctl+0xbe/0x100
>                   [<00000001da4ec152>] __do_syscall+0x1da/0x208
>                   [<00000001da4fec42>] system_call+0x82/0xb0
>                  INFO: lockdep is turned off.

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

* Re: [PATCH v3 1/1] KVM: s390: pv: make use of ultravisor AIV support
  2022-02-24 15:47     ` Michael Mueller
@ 2022-02-25 13:29       ` Christian Borntraeger
  0 siblings, 0 replies; 8+ messages in thread
From: Christian Borntraeger @ 2022-02-25 13:29 UTC (permalink / raw)
  To: Michael Mueller, kvm
  Cc: cohuck, frankja, thuth, pasic, david, linux-s390, linux-kernel



Am 24.02.22 um 16:47 schrieb Michael Mueller:
> 
> 
> On 22.02.22 09:13, Christian Borntraeger wrote:
>> Am 09.02.22 um 16:22 schrieb Michael Mueller:
>>> This patch enables the ultravisor adapter interruption vitualization
>>> support indicated by UV feature BIT_UV_FEAT_AIV. This allows ISC
>>> interruption injection directly into the GISA IPM for PV kvm guests.
>>>
>>> Hardware that does not support this feature will continue to use the
>>> UV interruption interception method to deliver ISC interruptions to
>>> PV kvm guests. For this purpose, the ECA_AIV bit for all guest cpus
>>> will be cleared and the GISA will be disabled during PV CPU setup.
>>>
>>> In addition a check in __inject_io() has been removed. That reduces the
>>> required instructions for interruption handling for PV and traditional
>>> kvm guests.
>>>
>>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
>>
>> The CI said the following with gisa_disable in the calltrace.
>> Will drop from next for now.
> 
> It turns out this is related to kvm_s390_set_tod_clock() which
> is triggered by a kvm-unit-test (sac_PV) and not related directly
> to this patch. Please re-apply.

Done. We need to fix the sck handler instead.

> 
>>
>>     LOCKDEP_CIRCULAR (suite: kvm-unit-tests-kvm, case: -)
>>                  WARNING: possible circular locking dependency detected
>> 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1 Not tainted
>>                  ------------------------------------------------------
>>                  qemu-system-s39/161139 is trying to acquire lock:
>>                  0000000280dc0b98 (&kvm->lock){+.+.}-{3:3}, at: kvm_s390_set_tod_clock+0x36/0x220 [kvm]
>>                  but task is already holding lock:
>>                  0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
>>                  which lock already depends on the new lock.
>>                  the existing dependency chain (in reverse order) is:
>>                  -> #1 (&vcpu->mutex){+.+.}-{3:3}:
>>                         __lock_acquire+0x604/0xbd8
>>                         lock_acquire.part.0+0xe2/0x250
>>                         lock_acquire+0xb0/0x200
>>                         __mutex_lock+0x9e/0x8a0
>>                         mutex_lock_nested+0x32/0x40
>>                         kvm_s390_gisa_disable+0xa4/0x130 [kvm]
>>                         kvm_s390_handle_pv+0x718/0x778 [kvm]
>>                         kvm_arch_vm_ioctl+0x4ac/0x5f8 [kvm]
>>                         kvm_vm_ioctl+0x336/0x530 [kvm]
>>                         __s390x_sys_ioctl+0xbe/0x100
>>                         __do_syscall+0x1da/0x208
>>                         system_call+0x82/0xb0
>>                  -> #0 (&kvm->lock){+.+.}-{3:3}:
>>                         check_prev_add+0xe0/0xed8
>>                         validate_chain+0x736/0xb20
>>                         __lock_acquire+0x604/0xbd8
>>                         lock_acquire.part.0+0xe2/0x250
>>                         lock_acquire+0xb0/0x200
>>                         __mutex_lock+0x9e/0x8a0
>>                         mutex_lock_nested+0x32/0x40
>>                         kvm_s390_set_tod_clock+0x36/0x220 [kvm]
>>                         kvm_s390_handle_b2+0x378/0x728 [kvm]
>>                         kvm_handle_sie_intercept+0x13a/0x448 [kvm]
>>                         vcpu_post_run+0x28e/0x560 [kvm]
>>                         __vcpu_run+0x266/0x388 [kvm]
>>                         kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
>>                         kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
>>                         __s390x_sys_ioctl+0xbe/0x100
>>                         __do_syscall+0x1da/0x208
>>                         system_call+0x82/0xb0
>>                  other info that might help us debug this:
>>                   Possible unsafe locking scenario:
>>                         CPU0                    CPU1
>>                         ----                    ----
>>                    lock(&vcpu->mutex);
>>                                                 lock(&kvm->lock);
>>                                                 lock(&vcpu->mutex);
>>                    lock(&kvm->lock);
>>                   *** DEADLOCK ***
>>                  2 locks held by qemu-system-s39/161139:
>>                   #0: 0000000280f4e4b8 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0x9a/0xa40 [kvm]
>>                   #1: 0000000280dc47c8 (&kvm->srcu){....}-{0:0}, at: __vcpu_run+0x1d4/0x388 [kvm]
>>                  stack backtrace:
>>                  CPU: 10 PID: 161139 Comm: qemu-system-s39 Not tainted 5.17.0-20220221.rc5.git1.b8f0356a093a.300.fc35.s390x+debug #1
>>                  Hardware name: IBM 8561 T01 701 (LPAR)
>>                  Call Trace:
>>                   [<00000001da4e89de>] dump_stack_lvl+0x8e/0xc8
>>                   [<00000001d9876c56>] check_noncircular+0x136/0x158
>>                   [<00000001d9877c70>] check_prev_add+0xe0/0xed8
>>                   [<00000001d987919e>] validate_chain+0x736/0xb20
>>                   [<00000001d987b23c>] __lock_acquire+0x604/0xbd8
>>                   [<00000001d987c432>] lock_acquire.part.0+0xe2/0x250
>>                   [<00000001d987c650>] lock_acquire+0xb0/0x200
>>                   [<00000001da4f72ae>] __mutex_lock+0x9e/0x8a0
>>                   [<00000001da4f7ae2>] mutex_lock_nested+0x32/0x40
>>                   [<000003ff8070cd6e>] kvm_s390_set_tod_clock+0x36/0x220 [kvm]
>>                   [<000003ff8071dd68>] kvm_s390_handle_b2+0x378/0x728 [kvm]
>>                   [<000003ff8071146a>] kvm_handle_sie_intercept+0x13a/0x448 [kvm]
>>                   [<000003ff8070dd46>] vcpu_post_run+0x28e/0x560 [kvm]
>>                   [<000003ff8070e27e>] __vcpu_run+0x266/0x388 [kvm]
>>                   [<000003ff8070eba2>] kvm_arch_vcpu_ioctl_run+0x10a/0x270 [kvm]
>>                   [<000003ff806f4044>] kvm_vcpu_ioctl+0x27c/0xa40 [kvm]
>>                   [<00000001d9b47ac6>] __s390x_sys_ioctl+0xbe/0x100
>>                   [<00000001da4ec152>] __do_syscall+0x1da/0x208
>>                   [<00000001da4fec42>] system_call+0x82/0xb0
>>                  INFO: lockdep is turned off.

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

end of thread, other threads:[~2022-02-25 13:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 15:22 [PATCH v3 0/1] KVM: s390: pv: make use of ultravisor AIV support Michael Mueller
2022-02-09 15:22 ` [PATCH v3 1/1] " Michael Mueller
2022-02-15 13:18   ` Janosch Frank
2022-02-15 15:16   ` Christian Borntraeger
2022-02-22  8:13   ` Christian Borntraeger
2022-02-22 17:27     ` Michael Mueller
2022-02-24 15:47     ` Michael Mueller
2022-02-25 13:29       ` Christian Borntraeger

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).