[PATCH v6 00/13] KVM: s390: make use of the GIB

[PATCH v6 00/13] KVM: s390: make use of the GIB

[Michael Mueller]

The Guest Information Area (GIB) and its mechanics is part of
the AIV facility. It provides hardware support to process
Adapter Interruptions (AI) for pagable storage mode guests. 

Whenever a guest cannot process an AI because none of its
vcpus is running in SIE context, a GIB alert interruption is
sent to and handled by the host program to dispatch a vcpu of
the respective guest.

Eventually, the AI is handled by the guest.

v5->v6:
  Futher significant change was introduced with this version
  now:

  a) During the alert list processing now a hrtimer is
     started to kick an idle vcpu with open I/O interrupt
     mask. It then is restarted to monitor the interruption
     consumption and when the IPM is clean (no relevant
     ISCs pending anymore) atomically restores the IAM
     and don't restarts or if new ISCs arrive kick other
     idle vcpus.

  patches 1/13 to 7/13:
     prepare the existing Interruption and GISA code for
     the introduction of the GIB code.
  patches 8/13 to 10/13:
     kept from v5.
  patch 11/13:
     Some comments added.
  patch 12/13:
     Significatly changed due to the hrtimer control.
  patch 13/13:
     Basically kept from v5 except for a slight reordering
     that shortens the patch.

v4->v5:
  Between these two versions some significant change was
  introduced:

  a) During IPM look-up, the IAM gets atomically restored
     if the IPM is clean. (The main strategy is: As soon no
     airqs are pending anymore, get prepared for new ones
     arriving).

  b) Kick a set of vcpus in WAIT state, that will be able
     to handle all ISCs in IPM if possible. The main loop
     that processes the GIB alert list is unchanged.

  patches 1/15 to 6/15:
     prepare the existing Interruption and GISA code for
     the introduction of the GIB code.
  patches 7/15 to 9/15:
     kept from v4. 
  patch 10/15:
     restores the IAM in get_ipm() when clean on request
     with additional parameter irq_flags
  patch 12/15:
     modifies kvm_s390_deliver_pending_interrupts() such
     that the IAM is not restored during the deliverable_irqs()
     test as we are about to enter the SIE. Restoring the
     IAM would trigger the alert mechanism unnecessarily
     on SIE entry.
  patch 13/15:
     process_gib_alert_list() now triggers a minimal set
     of idle vcpus capable the handle all pending ISCs.
  patch 14/15:
     the wiring is logical similar to v4 but uses different
     routines
  patch 15/15: 
     kept from v4. 

v3->v4:
  The central change of v4 is that the metric "vcpus_in_sie"
  has been dropped. A least agressive IAM restore strategy
  is being followed now. The consequence is that potentially
  more GAL interruptions are to be handled by the host program.
  The assuption as made that as soon the first vcpu is trying
  to go into wait state, this could be the last vcpu being
  open for I/O interruptions. Thus, to not loose the initiative
  to deliver I/O interruptions as soon as possible, the IAM
  is restored. Adding better heuristics can be done in future.

  I expect this to be the last version of the series.
 
  patch 01/12: Kernel msg now printed when FLIC fails
               to register during arch inititialization.
  patch 03/12: Commit message slightly changed with the
  	       hint that gib initialization is called
	       by last patch of this serias.
  patch 09/12: IAM now restored during kvm_s390_handle_wait()
  patch 10/12: gib destroy case now handled upon unsuccessful
  	       arch inititialization.

All other patches are unchanged.

v2->v3:
  patch 01/12: Fixes a resource dealocation issue upon
  	       unsuccessful exit from kvm_arch_init().
	       It is relevant for patch 11/12 as well. 
  patch 08/12: The function process_gib_alert_list() has
               been rewritten to reduce the number of GAL
	       interruptions on the host for many guest
	       scenarios.
  patch 10/12: The IAM is now never cleared before SIE entry.
               It will be cleared by millicode as soon the
	       first guest interruption is made pending and
	       not directly processed in SIE. The IAM gets
	       restored as soon a vm is idle, i.e. no vcpu
	       of that guest is in SIE context anymore.
  patch 11/12: now integrates with new patch 01/12
  patch 12/12: This patch is just experimental and shall not
               be part of the final series.

The first patch of series v2: "leave AIs in IPM of GISA
during vcpu_pre_run()" has been dropped.

All other patches are unchanged.

v1->v2:
  patch 01/12: New patch. Tt can go also standalone without the
  	       rest of the GIB series presented here but is a
	       requirement 
  patch 03/12: kvm_s390_gib_init() now has a return code
  patch 06/12: unlink_gib_alert_list() now uses cmpxchg() instead
    	       of atomic_xchg()
  patch 08/12: New patch. Foctors out __find_vcpu_for_floating_irq()
  patch 09/12: process_gib_alert_list() has been simplified
               the GISA clear/destroy cases have been removed
  patch 11/12: kvm_s390_gisa_clear/destroy() now clear the IAM
               and process the alert list directly
	       The IAM now gets restored in vcpu_post_run() only if
	       the GISA is not in alert list
  patch 12/12: during kvm_arch_init() now the return code of
    	       kvm_s390_gib_init() is honored.

All other patches are unchanged.

Michael Mueller (13):
  KVM: s390: drop obsolete else path
  KVM: s390: make bitmap declaration consitent
  KVM: s390: move bitmap idle_mask into arch struct top level
  KVM: s390: coding style kvm_s390_gisa_init/clear()
  KVM: s390: use pending_irqs_no_gisa() where appropriate
  KVM: s390: remove kvm_s390_ from gisa static inline functions
  KVM: s390: introduce struct kvm_s390_gisa_interrupt
  s390/cio: add function chsc_sgib()
  KVM: s390: add the GIB and its related life-cyle functions
  KVM: s390: add kvm reference to struct sie_page2
  KVM: s390: add functions to (un)register GISC with GISA
  KVM: s390: add gib_alert_irq_handler()
  KVM: s390: start using the GIB

 arch/s390/include/asm/cio.h      |   1 +
 arch/s390/include/asm/irq.h      |   1 +
 arch/s390/include/asm/isc.h      |   1 +
 arch/s390/include/asm/kvm_host.h |  36 +++-
 arch/s390/kernel/irq.c           |   1 +
 arch/s390/kvm/interrupt.c        | 422 +++++++++++++++++++++++++++++++++++----
 arch/s390/kvm/kvm-s390.c         |  13 +-
 arch/s390/kvm/kvm-s390.h         |   4 +-
 drivers/s390/cio/chsc.c          |  37 ++++
 drivers/s390/cio/chsc.h          |   1 +
 10 files changed, 472 insertions(+), 45 deletions(-)

-- 
2.13.4

[PATCH v6 01/13] KVM: s390: drop obsolete else path

[Michael Mueller]

The explicit else path specified in set_intercept_indicators_io
is not required as the function returns in case the first branch
is taken anyway.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index fcb55b02990e..19d556512452 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -345,7 +345,7 @@ static void set_intercept_indicators_io(struct kvm_vcpu *vcpu)
 {
 	if (!(pending_irqs_no_gisa(vcpu) & IRQ_PEND_IO_MASK))
 		return;
-	else if (psw_ioint_disabled(vcpu))
+	if (psw_ioint_disabled(vcpu))
 		kvm_s390_set_cpuflags(vcpu, CPUSTAT_IO_INT);
 	else
 		vcpu->arch.sie_block->lctl |= LCTL_CR6;
-- 
2.13.4

[PATCH v6 02/13] KVM: s390: make bitmap declaration consitent

[Michael Mueller]

Use a consistent bitmap declaration throughout the code.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h | 2 +-
 arch/s390/kvm/interrupt.c        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index d5d24889c3bc..3cba08f73dc6 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -591,7 +591,7 @@ struct kvm_s390_float_interrupt {
 	struct kvm_s390_mchk_info mchk;
 	struct kvm_s390_ext_info srv_signal;
 	int next_rr_cpu;
-	unsigned long idle_mask[BITS_TO_LONGS(KVM_MAX_VCPUS)];
+	DECLARE_BITMAP(idle_mask, KVM_MAX_VCPUS);
 	struct mutex ais_lock;
 	u8 simm;
 	u8 nimm;
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 19d556512452..167a3068ef84 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2831,7 +2831,7 @@ static void store_local_irq(struct kvm_s390_local_interrupt *li,
 int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
 {
 	int scn;
-	unsigned long sigp_emerg_pending[BITS_TO_LONGS(KVM_MAX_VCPUS)];
+	DECLARE_BITMAP(sigp_emerg_pending, KVM_MAX_VCPUS);
 	struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int;
 	unsigned long pending_irqs;
 	struct kvm_s390_irq irq;
-- 
2.13.4

[PATCH v6 03/13] KVM: s390: move bitmap idle_mask into arch struct top level

[Michael Mueller]

The vcpu idle_mask state is used by but not specific
to the emulated floating interruptions. The state is
relevant to gisa related interruptions as well.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  2 +-
 arch/s390/kvm/interrupt.c        | 11 +++++------
 arch/s390/kvm/kvm-s390.h         |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 3cba08f73dc6..c259a67c4298 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -591,7 +591,6 @@ struct kvm_s390_float_interrupt {
 	struct kvm_s390_mchk_info mchk;
 	struct kvm_s390_ext_info srv_signal;
 	int next_rr_cpu;
-	DECLARE_BITMAP(idle_mask, KVM_MAX_VCPUS);
 	struct mutex ais_lock;
 	u8 simm;
 	u8 nimm;
@@ -838,6 +837,7 @@ struct kvm_arch{
 	/* subset of available cpu features enabled by user space */
 	DECLARE_BITMAP(cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS);
 	struct kvm_s390_gisa *gisa;
+	DECLARE_BITMAP(idle_mask, KVM_MAX_VCPUS);
 };
 
 #define KVM_HVA_ERR_BAD		(-1UL)
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 167a3068ef84..2a3eb9f076c3 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -318,13 +318,13 @@ static unsigned long deliverable_irqs(struct kvm_vcpu *vcpu)
 static void __set_cpu_idle(struct kvm_vcpu *vcpu)
 {
 	kvm_s390_set_cpuflags(vcpu, CPUSTAT_WAIT);
-	set_bit(vcpu->vcpu_id, vcpu->kvm->arch.float_int.idle_mask);
+	set_bit(vcpu->vcpu_id, vcpu->kvm->arch.idle_mask);
 }
 
 static void __unset_cpu_idle(struct kvm_vcpu *vcpu)
 {
 	kvm_s390_clear_cpuflags(vcpu, CPUSTAT_WAIT);
-	clear_bit(vcpu->vcpu_id, vcpu->kvm->arch.float_int.idle_mask);
+	clear_bit(vcpu->vcpu_id, vcpu->kvm->arch.idle_mask);
 }
 
 static void __reset_intercept_indicators(struct kvm_vcpu *vcpu)
@@ -1726,7 +1726,6 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
  */
 static void __floating_irq_kick(struct kvm *kvm, u64 type)
 {
-	struct kvm_s390_float_interrupt *fi = &kvm->arch.float_int;
 	struct kvm_vcpu *dst_vcpu;
 	int sigcpu, online_vcpus, nr_tries = 0;
 
@@ -1735,11 +1734,11 @@ static void __floating_irq_kick(struct kvm *kvm, u64 type)
 		return;
 
 	/* find idle VCPUs first, then round robin */
-	sigcpu = find_first_bit(fi->idle_mask, online_vcpus);
+	sigcpu = find_first_bit(kvm->arch.idle_mask, online_vcpus);
 	if (sigcpu == online_vcpus) {
 		do {
-			sigcpu = fi->next_rr_cpu;
-			fi->next_rr_cpu = (fi->next_rr_cpu + 1) % online_vcpus;
+			sigcpu = kvm->arch.float_int.next_rr_cpu++;
+			kvm->arch.float_int.next_rr_cpu %= online_vcpus;
 			/* avoid endless loops if all vcpus are stopped */
 			if (nr_tries++ >= online_vcpus)
 				return;
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 1f6e36cdce0d..72ef87799593 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -67,7 +67,7 @@ static inline int is_vcpu_stopped(struct kvm_vcpu *vcpu)
 
 static inline int is_vcpu_idle(struct kvm_vcpu *vcpu)
 {
-	return test_bit(vcpu->vcpu_id, vcpu->kvm->arch.float_int.idle_mask);
+	return test_bit(vcpu->vcpu_id, vcpu->kvm->arch.idle_mask);
 }
 
 static inline int kvm_is_ucontrol(struct kvm *kvm)
-- 
2.13.4

[PATCH v6 04/13] KVM: s390: coding style kvm_s390_gisa_init/clear()

[Michael Mueller]

The change helps to reduce line length and
increases code readability.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 2a3eb9f076c3..005dbe7252e7 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2885,20 +2885,20 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
 
 void kvm_s390_gisa_clear(struct kvm *kvm)
 {
-	if (kvm->arch.gisa) {
-		memset(kvm->arch.gisa, 0, sizeof(struct kvm_s390_gisa));
-		kvm->arch.gisa->next_alert = (u32)(u64)kvm->arch.gisa;
-		VM_EVENT(kvm, 3, "gisa 0x%pK cleared", kvm->arch.gisa);
-	}
+	if (!kvm->arch.gisa)
+		return;
+	memset(kvm->arch.gisa, 0, sizeof(struct kvm_s390_gisa));
+	kvm->arch.gisa->next_alert = (u32)(u64)kvm->arch.gisa;
+	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", kvm->arch.gisa);
 }
 
 void kvm_s390_gisa_init(struct kvm *kvm)
 {
-	if (css_general_characteristics.aiv) {
-		kvm->arch.gisa = &kvm->arch.sie_page2->gisa;
-		VM_EVENT(kvm, 3, "gisa 0x%pK initialized", kvm->arch.gisa);
-		kvm_s390_gisa_clear(kvm);
-	}
+	if (!css_general_characteristics.aiv)
+		return;
+	kvm->arch.gisa = &kvm->arch.sie_page2->gisa;
+	kvm_s390_gisa_clear(kvm);
+	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", kvm->arch.gisa);
 }
 
 void kvm_s390_gisa_destroy(struct kvm *kvm)
-- 
2.13.4

[PATCH v6 05/13] KVM: s390: use pending_irqs_no_gisa() where appropriate

[Michael Mueller]

Interruption types that are not represented in GISA shall
use pending_irqs_no_gisa() to test pending interruptions.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 005dbe7252e7..cb48736867ed 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -353,7 +353,7 @@ static void set_intercept_indicators_io(struct kvm_vcpu *vcpu)
 
 static void set_intercept_indicators_ext(struct kvm_vcpu *vcpu)
 {
-	if (!(pending_irqs(vcpu) & IRQ_PEND_EXT_MASK))
+	if (!(pending_irqs_no_gisa(vcpu) & IRQ_PEND_EXT_MASK))
 		return;
 	if (psw_extint_disabled(vcpu))
 		kvm_s390_set_cpuflags(vcpu, CPUSTAT_EXT_INT);
@@ -363,7 +363,7 @@ static void set_intercept_indicators_ext(struct kvm_vcpu *vcpu)
 
 static void set_intercept_indicators_mchk(struct kvm_vcpu *vcpu)
 {
-	if (!(pending_irqs(vcpu) & IRQ_PEND_MCHK_MASK))
+	if (!(pending_irqs_no_gisa(vcpu) & IRQ_PEND_MCHK_MASK))
 		return;
 	if (psw_mchk_disabled(vcpu))
 		vcpu->arch.sie_block->ictl |= ICTL_LPSW;
-- 
2.13.4

[PATCH v6 06/13] KVM: s390: remove kvm_s390_ from gisa static inline functions

[Michael Mueller]

This will shorten the length of code lines. All GISA related
static inline functions are local to interrupt.c.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index cb48736867ed..942cc7d33766 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -217,22 +217,22 @@ static inline u8 int_word_to_isc(u32 int_word)
  */
 #define IPM_BIT_OFFSET (offsetof(struct kvm_s390_gisa, ipm) * BITS_PER_BYTE)
 
-static inline void kvm_s390_gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
+static inline void gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
 {
 	set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
 }
 
-static inline u8 kvm_s390_gisa_get_ipm(struct kvm_s390_gisa *gisa)
+static inline u8 gisa_get_ipm(struct kvm_s390_gisa *gisa)
 {
 	return READ_ONCE(gisa->ipm);
 }
 
-static inline void kvm_s390_gisa_clear_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
+static inline void gisa_clear_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
 {
 	clear_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
 }
 
-static inline int kvm_s390_gisa_tac_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
+static inline int gisa_tac_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
 {
 	return test_and_clear_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
 }
@@ -246,7 +246,7 @@ static inline unsigned long pending_irqs_no_gisa(struct kvm_vcpu *vcpu)
 static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
 {
 	return pending_irqs_no_gisa(vcpu) |
-		kvm_s390_gisa_get_ipm(vcpu->kvm->arch.gisa) << IRQ_PEND_IO_ISC_7;
+		gisa_get_ipm(vcpu->kvm->arch.gisa) << IRQ_PEND_IO_ISC_7;
 }
 
 static inline int isc_to_irq_type(unsigned long isc)
@@ -999,7 +999,7 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu,
 	}
 
 	if (vcpu->kvm->arch.gisa &&
-	    kvm_s390_gisa_tac_ipm_gisc(vcpu->kvm->arch.gisa, isc)) {
+	    gisa_tac_ipm_gisc(vcpu->kvm->arch.gisa, isc)) {
 		/*
 		 * in case an adapter interrupt was not delivered
 		 * in SIE context KVM will handle the delivery
@@ -1541,10 +1541,10 @@ static int get_top_gisa_isc(struct kvm *kvm, u64 isc_mask, u32 schid)
 	if (!kvm->arch.gisa)
 		goto out;
 
-	active_mask = (isc_mask & kvm_s390_gisa_get_ipm(kvm->arch.gisa) << 24) << 32;
+	active_mask = (isc_mask & gisa_get_ipm(kvm->arch.gisa) << 24) << 32;
 	while (active_mask) {
 		isc = __fls(active_mask) ^ (BITS_PER_LONG - 1);
-		if (kvm_s390_gisa_tac_ipm_gisc(kvm->arch.gisa, isc))
+		if (gisa_tac_ipm_gisc(kvm->arch.gisa, isc))
 			return isc;
 		clear_bit_inv(isc, &active_mask);
 	}
@@ -1584,7 +1584,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
 	/* both types of interrupts present */
 	if (int_word_to_isc(inti->io.io_int_word) <= isc) {
 		/* classical IO int with higher priority */
-		kvm_s390_gisa_set_ipm_gisc(kvm->arch.gisa, isc);
+		gisa_set_ipm_gisc(kvm->arch.gisa, isc);
 		goto out;
 	}
 gisa_out:
@@ -1596,7 +1596,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
 			kvm_s390_reinject_io_int(kvm, inti);
 		inti = tmp_inti;
 	} else
-		kvm_s390_gisa_set_ipm_gisc(kvm->arch.gisa, isc);
+		gisa_set_ipm_gisc(kvm->arch.gisa, isc);
 out:
 	return inti;
 }
@@ -1694,7 +1694,7 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
 
 	if (kvm->arch.gisa && inti->type & KVM_S390_INT_IO_AI_MASK) {
 		VM_EVENT(kvm, 4, "%s isc %1u", "inject: I/O (AI/gisa)", isc);
-		kvm_s390_gisa_set_ipm_gisc(kvm->arch.gisa, isc);
+		gisa_set_ipm_gisc(kvm->arch.gisa, isc);
 		kfree(inti);
 		return 0;
 	}
@@ -2025,15 +2025,14 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
 
 	max_irqs = len / sizeof(struct kvm_s390_irq);
 
-	if (kvm->arch.gisa &&
-	    kvm_s390_gisa_get_ipm(kvm->arch.gisa)) {
+	if (kvm->arch.gisa && gisa_get_ipm(kvm->arch.gisa)) {
 		for (i = 0; i <= MAX_ISC; i++) {
 			if (n == max_irqs) {
 				/* signal userspace to try again */
 				ret = -ENOMEM;
 				goto out_nolock;
 			}
-			if (kvm_s390_gisa_tac_ipm_gisc(kvm->arch.gisa, i)) {
+			if (gisa_tac_ipm_gisc(kvm->arch.gisa, i)) {
 				irq = (struct kvm_s390_irq *) &buf[n];
 				irq->type = KVM_S390_INT_IO(1, 0, 0, 0);
 				irq->u.io.io_int_word = isc_to_int_word(i);
-- 
2.13.4

[PATCH v6 07/13] KVM: s390: introduce struct kvm_s390_gisa_interrupt

[Michael Mueller]

Use this struct analog to the kvm interruption structs
for kvm emulated floating and local interruptions.
Further fields will be added with this series as
required.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  6 ++++-
 arch/s390/kvm/interrupt.c        | 52 +++++++++++++++++++++++-----------------
 arch/s390/kvm/kvm-s390.c         |  2 +-
 3 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index c259a67c4298..0941571d34eb 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -803,6 +803,10 @@ struct kvm_s390_vsie {
 	struct page *pages[KVM_MAX_VCPUS];
 };
 
+struct kvm_s390_gisa_interrupt {
+	struct kvm_s390_gisa *origin;
+};
+
 struct kvm_arch{
 	void *sca;
 	int use_esca;
@@ -836,8 +840,8 @@ struct kvm_arch{
 	atomic64_t cmma_dirty_pages;
 	/* subset of available cpu features enabled by user space */
 	DECLARE_BITMAP(cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS);
-	struct kvm_s390_gisa *gisa;
 	DECLARE_BITMAP(idle_mask, KVM_MAX_VCPUS);
+	struct kvm_s390_gisa_interrupt gisa_int;
 };
 
 #define KVM_HVA_ERR_BAD		(-1UL)
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 942cc7d33766..ee91d1de0036 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -246,7 +246,8 @@ static inline unsigned long pending_irqs_no_gisa(struct kvm_vcpu *vcpu)
 static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
 {
 	return pending_irqs_no_gisa(vcpu) |
-		gisa_get_ipm(vcpu->kvm->arch.gisa) << IRQ_PEND_IO_ISC_7;
+		gisa_get_ipm(vcpu->kvm->arch.gisa_int.origin) <<
+			IRQ_PEND_IO_ISC_7;
 }
 
 static inline int isc_to_irq_type(unsigned long isc)
@@ -956,6 +957,7 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu,
 {
 	struct list_head *isc_list;
 	struct kvm_s390_float_interrupt *fi;
+	struct kvm_s390_gisa_interrupt *gi = &vcpu->kvm->arch.gisa_int;
 	struct kvm_s390_interrupt_info *inti = NULL;
 	struct kvm_s390_io_info io;
 	u32 isc;
@@ -998,8 +1000,7 @@ static int __must_check __deliver_io(struct kvm_vcpu *vcpu,
 		goto out;
 	}
 
-	if (vcpu->kvm->arch.gisa &&
-	    gisa_tac_ipm_gisc(vcpu->kvm->arch.gisa, isc)) {
+	if (gi->origin && gisa_tac_ipm_gisc(gi->origin, isc)) {
 		/*
 		 * in case an adapter interrupt was not delivered
 		 * in SIE context KVM will handle the delivery
@@ -1533,18 +1534,19 @@ static struct kvm_s390_interrupt_info *get_top_io_int(struct kvm *kvm,
 
 static int get_top_gisa_isc(struct kvm *kvm, u64 isc_mask, u32 schid)
 {
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
 	unsigned long active_mask;
 	int isc;
 
 	if (schid)
 		goto out;
-	if (!kvm->arch.gisa)
+	if (!gi->origin)
 		goto out;
 
-	active_mask = (isc_mask & gisa_get_ipm(kvm->arch.gisa) << 24) << 32;
+	active_mask = (isc_mask & gisa_get_ipm(gi->origin) << 24) << 32;
 	while (active_mask) {
 		isc = __fls(active_mask) ^ (BITS_PER_LONG - 1);
-		if (gisa_tac_ipm_gisc(kvm->arch.gisa, isc))
+		if (gisa_tac_ipm_gisc(gi->origin, isc))
 			return isc;
 		clear_bit_inv(isc, &active_mask);
 	}
@@ -1567,6 +1569,7 @@ static int get_top_gisa_isc(struct kvm *kvm, u64 isc_mask, u32 schid)
 struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
 						    u64 isc_mask, u32 schid)
 {
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
 	struct kvm_s390_interrupt_info *inti, *tmp_inti;
 	int isc;
 
@@ -1584,7 +1587,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
 	/* both types of interrupts present */
 	if (int_word_to_isc(inti->io.io_int_word) <= isc) {
 		/* classical IO int with higher priority */
-		gisa_set_ipm_gisc(kvm->arch.gisa, isc);
+		gisa_set_ipm_gisc(gi->origin, isc);
 		goto out;
 	}
 gisa_out:
@@ -1596,7 +1599,7 @@ struct kvm_s390_interrupt_info *kvm_s390_get_io_int(struct kvm *kvm,
 			kvm_s390_reinject_io_int(kvm, inti);
 		inti = tmp_inti;
 	} else
-		gisa_set_ipm_gisc(kvm->arch.gisa, isc);
+		gisa_set_ipm_gisc(gi->origin, isc);
 out:
 	return inti;
 }
@@ -1685,6 +1688,7 @@ static int __inject_float_mchk(struct kvm *kvm,
 
 static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
 {
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
 	struct kvm_s390_float_interrupt *fi;
 	struct list_head *list;
 	int isc;
@@ -1692,9 +1696,9 @@ static int __inject_io(struct kvm *kvm, struct kvm_s390_interrupt_info *inti)
 	kvm->stat.inject_io++;
 	isc = int_word_to_isc(inti->io.io_int_word);
 
-	if (kvm->arch.gisa && 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(kvm->arch.gisa, isc);
+		gisa_set_ipm_gisc(gi->origin, isc);
 		kfree(inti);
 		return 0;
 	}
@@ -1752,7 +1756,8 @@ static void __floating_irq_kick(struct kvm *kvm, u64 type)
 		kvm_s390_set_cpuflags(dst_vcpu, CPUSTAT_STOP_INT);
 		break;
 	case KVM_S390_INT_IO_MIN...KVM_S390_INT_IO_MAX:
-		if (!(type & KVM_S390_INT_IO_AI_MASK && kvm->arch.gisa))
+		if (!(type & KVM_S390_INT_IO_AI_MASK &&
+		      kvm->arch.gisa_int.origin))
 			kvm_s390_set_cpuflags(dst_vcpu, CPUSTAT_IO_INT);
 		break;
 	default:
@@ -2002,6 +2007,7 @@ void kvm_s390_clear_float_irqs(struct kvm *kvm)
 
 static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
 {
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
 	struct kvm_s390_interrupt_info *inti;
 	struct kvm_s390_float_interrupt *fi;
 	struct kvm_s390_irq *buf;
@@ -2025,14 +2031,14 @@ static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
 
 	max_irqs = len / sizeof(struct kvm_s390_irq);
 
-	if (kvm->arch.gisa && gisa_get_ipm(kvm->arch.gisa)) {
+	if (gi->origin && gisa_get_ipm(gi->origin)) {
 		for (i = 0; i <= MAX_ISC; i++) {
 			if (n == max_irqs) {
 				/* signal userspace to try again */
 				ret = -ENOMEM;
 				goto out_nolock;
 			}
-			if (gisa_tac_ipm_gisc(kvm->arch.gisa, i)) {
+			if (gisa_tac_ipm_gisc(gi->origin, i)) {
 				irq = (struct kvm_s390_irq *) &buf[n];
 				irq->type = KVM_S390_INT_IO(1, 0, 0, 0);
 				irq->u.io.io_int_word = isc_to_int_word(i);
@@ -2884,25 +2890,27 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
 
 void kvm_s390_gisa_clear(struct kvm *kvm)
 {
-	if (!kvm->arch.gisa)
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+
+	if (!gi->origin)
 		return;
-	memset(kvm->arch.gisa, 0, sizeof(struct kvm_s390_gisa));
-	kvm->arch.gisa->next_alert = (u32)(u64)kvm->arch.gisa;
-	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", kvm->arch.gisa);
+	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
+	gi->origin->next_alert = (u32)(u64)gi->origin;
+	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", gi->origin);
 }
 
 void kvm_s390_gisa_init(struct kvm *kvm)
 {
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+
 	if (!css_general_characteristics.aiv)
 		return;
-	kvm->arch.gisa = &kvm->arch.sie_page2->gisa;
+	gi->origin = &kvm->arch.sie_page2->gisa;
 	kvm_s390_gisa_clear(kvm);
-	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", kvm->arch.gisa);
+	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
 }
 
 void kvm_s390_gisa_destroy(struct kvm *kvm)
 {
-	if (!kvm->arch.gisa)
-		return;
-	kvm->arch.gisa = NULL;
+	kvm->arch.gisa_int.origin = NULL;
 }
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 7f4bc58a53b9..4c855cb67699 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2812,7 +2812,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
 
 	vcpu->arch.sie_block->icpua = id;
 	spin_lock_init(&vcpu->arch.local_int.lock);
-	vcpu->arch.sie_block->gd = (u32)(u64)kvm->arch.gisa;
+	vcpu->arch.sie_block->gd = (u32)(u64)kvm->arch.gisa_int.origin;
 	if (vcpu->arch.sie_block->gd && sclp.has_gisaf)
 		vcpu->arch.sie_block->gd |= GISA_FORMAT1;
 	seqcount_init(&vcpu->arch.cputm_seqcount);
-- 
2.13.4

[PATCH v6 08/13] s390/cio: add function chsc_sgib()

[Michael Mueller]

This patch implements the Set Guest Information Block operation
to request association or disassociation of a Guest Information
Block (GIB) with the Adapter Interruption Facility. The operation
is required to receive GIB alert interrupts for guest adapters
in conjunction with AIV and GISA.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
Reviewed-by: Sebastian Ott <sebott@linux.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Halil Pasic <pasic@linux.ibm.com>
Acked-by: Janosch Frank <frankja@linux.ibm.com>
Acked-by: Cornelia Huck <cohuck@redhat.com>
---
 arch/s390/include/asm/cio.h |  1 +
 drivers/s390/cio/chsc.c     | 37 +++++++++++++++++++++++++++++++++++++
 drivers/s390/cio/chsc.h     |  1 +
 3 files changed, 39 insertions(+)

diff --git a/arch/s390/include/asm/cio.h b/arch/s390/include/asm/cio.h
index 225667652069..1727180e8ca1 100644
--- a/arch/s390/include/asm/cio.h
+++ b/arch/s390/include/asm/cio.h
@@ -331,5 +331,6 @@ extern void css_schedule_reprobe(void);
 /* Function from drivers/s390/cio/chsc.c */
 int chsc_sstpc(void *page, unsigned int op, u16 ctrl, u64 *clock_delta);
 int chsc_sstpi(void *page, void *result, size_t size);
+int chsc_sgib(u32 origin);
 
 #endif
diff --git a/drivers/s390/cio/chsc.c b/drivers/s390/cio/chsc.c
index a0baee25134c..4159c63a5fd2 100644
--- a/drivers/s390/cio/chsc.c
+++ b/drivers/s390/cio/chsc.c
@@ -1382,3 +1382,40 @@ int chsc_pnso_brinfo(struct subchannel_id schid,
 	return chsc_error_from_response(brinfo_area->response.code);
 }
 EXPORT_SYMBOL_GPL(chsc_pnso_brinfo);
+
+int chsc_sgib(u32 origin)
+{
+	struct {
+		struct chsc_header request;
+		u16 op;
+		u8  reserved01[2];
+		u8  reserved02:4;
+		u8  fmt:4;
+		u8  reserved03[7];
+		/* operation data area begin */
+		u8  reserved04[4];
+		u32 gib_origin;
+		u8  reserved05[10];
+		u8  aix;
+		u8  reserved06[4029];
+		struct chsc_header response;
+		u8  reserved07[4];
+	} *sgib_area;
+	int ret;
+
+	spin_lock_irq(&chsc_page_lock);
+	memset(chsc_page, 0, PAGE_SIZE);
+	sgib_area = chsc_page;
+	sgib_area->request.length = 0x0fe0;
+	sgib_area->request.code = 0x0021;
+	sgib_area->op = 0x1;
+	sgib_area->gib_origin = origin;
+
+	ret = chsc(sgib_area);
+	if (ret == 0)
+		ret = chsc_error_from_response(sgib_area->response.code);
+	spin_unlock_irq(&chsc_page_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(chsc_sgib);
diff --git a/drivers/s390/cio/chsc.h b/drivers/s390/cio/chsc.h
index 78aba8d94eec..e57d68e325a3 100644
--- a/drivers/s390/cio/chsc.h
+++ b/drivers/s390/cio/chsc.h
@@ -164,6 +164,7 @@ int chsc_get_channel_measurement_chars(struct channel_path *chp);
 int chsc_ssqd(struct subchannel_id schid, struct chsc_ssqd_area *ssqd);
 int chsc_sadc(struct subchannel_id schid, struct chsc_scssc_area *scssc,
 	      u64 summary_indicator_addr, u64 subchannel_indicator_addr);
+int chsc_sgib(u32 origin);
 int chsc_error_from_response(int response);
 
 int chsc_siosl(struct subchannel_id schid);
-- 
2.13.4

[PATCH v6 09/13] KVM: s390: add the GIB and its related life-cyle functions

[Michael Mueller]

The Guest Information Block (GIB) links the GISA of all guests
that have adapter interrupts pending. These interrupts cannot be
delivered because all vcpus of these guests are currently in WAIT
state or have masked the respective Innterruption Sub Class (ISC).
If enabled, a GIB alert is issued on the host to schedule these
guests to run suitable vcpus to consume the pending interruptions.

This mechanism allows to process adapter interrupts for currently
not running guests.

The GIB is created during host initialization and associated with
the Adapter Interruption Facility in case an Adapter Interruption
Virtualization Facility is available.

The GIB initialization and thus the activation of the related code
will be done in an upcoming patch of this series.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  9 +++++++++
 arch/s390/kvm/interrupt.c        | 43 ++++++++++++++++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.c         |  1 +
 arch/s390/kvm/kvm-s390.h         |  2 ++
 4 files changed, 55 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 0941571d34eb..397af0d33539 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -784,6 +784,15 @@ struct kvm_s390_gisa {
 	};
 };
 
+struct kvm_s390_gib {
+	u32 alert_list_origin;
+	u32 reserved01;
+	u8:5;
+	u8  nisc:3;
+	u8  reserved03[3];
+	u32 reserved04[5];
+};
+
 /*
  * sie_page2 has to be allocated as DMA because fac_list, crycb and
  * gisa need 31bit addresses in the sie control block.
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index ee91d1de0036..5efcd9e2cf8f 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -7,6 +7,9 @@
  *    Author(s): Carsten Otte <cotte@de.ibm.com>
  */
 
+#define KMSG_COMPONENT "kvm-s390"
+#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
+
 #include <linux/interrupt.h>
 #include <linux/kvm_host.h>
 #include <linux/hrtimer.h>
@@ -31,6 +34,8 @@
 #define PFAULT_DONE 0x0680
 #define VIRTIO_PARAM 0x0d00
 
+static struct kvm_s390_gib *gib;
+
 /* handle external calls via sigp interpretation facility */
 static int sca_ext_call_pending(struct kvm_vcpu *vcpu, int *src_id)
 {
@@ -2914,3 +2919,41 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
 {
 	kvm->arch.gisa_int.origin = NULL;
 }
+
+void kvm_s390_gib_destroy(void)
+{
+	if (!gib)
+		return;
+	chsc_sgib(0);
+	free_page((unsigned long)gib);
+	gib = NULL;
+}
+
+int kvm_s390_gib_init(u8 nisc)
+{
+	int rc = 0;
+
+	if (!css_general_characteristics.aiv) {
+		KVM_EVENT(3, "%s", "gib not initialized, no AIV facility");
+		goto out;
+	}
+
+	gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+	if (!gib) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	gib->nisc = nisc;
+	if (chsc_sgib((u32)(u64)gib)) {
+		pr_err("Associating the GIB with the AIV facility failed\n");
+		free_page((unsigned long)gib);
+		gib = NULL;
+		rc = -EIO;
+		goto out;
+	}
+
+	KVM_EVENT(3, "gib 0x%pK (nisc=%d) initialized", gib, gib->nisc);
+out:
+	return rc;
+}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4c855cb67699..118d4c0bbb8e 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -444,6 +444,7 @@ int kvm_arch_init(void *opaque)
 
 void kvm_arch_exit(void)
 {
+	kvm_s390_gib_destroy();
 	debug_unregister(kvm_s390_dbf);
 }
 
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 72ef87799593..6d9448dbd052 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -381,6 +381,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);
+int kvm_s390_gib_init(u8 nisc);
+void kvm_s390_gib_destroy(void);
 
 /* implemented in guestdbg.c */
 void kvm_s390_backup_guest_per_regs(struct kvm_vcpu *vcpu);
-- 
2.13.4

[PATCH v6 10/13] KVM: s390: add kvm reference to struct sie_page2

[Michael Mueller]

Adding the kvm reference to struct sie_page2 will allow to
determine the kvm a given gisa belongs to:

  container_of(gisa, struct sie_page2, gisa)->kvm

This functionality will be required to process a gisa in
gib alert interruption context.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 arch/s390/include/asm/kvm_host.h | 3 ++-
 arch/s390/kvm/kvm-s390.c         | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 397af0d33539..7077762ab460 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -801,7 +801,8 @@ struct sie_page2 {
 	__u64 fac_list[S390_ARCH_FAC_LIST_SIZE_U64];	/* 0x0000 */
 	struct kvm_s390_crypto_cb crycb;		/* 0x0800 */
 	struct kvm_s390_gisa gisa;			/* 0x0900 */
-	u8 reserved920[0x1000 - 0x920];			/* 0x0920 */
+	struct kvm *kvm;				/* 0x0920 */
+	u8 reserved928[0x1000 - 0x928];			/* 0x0928 */
 };
 
 struct kvm_s390_vsie {
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 118d4c0bbb8e..67023d5656fd 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2210,6 +2210,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	if (!kvm->arch.sie_page2)
 		goto out_err;
 
+	kvm->arch.sie_page2->kvm = kvm;
 	kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list;
 
 	for (i = 0; i < kvm_s390_fac_size(); i++) {
-- 
2.13.4

[PATCH v6 11/13] KVM: s390: add functions to (un)register GISC with GISA

[Michael Mueller]

Add the Interruption Alert Mask (IAM) to the architecture specific
kvm struct. This mask in the GISA is used to define for which ISC
a GIB alert will be issued.

The functions kvm_s390_gisc_register() and kvm_s390_gisc_unregister()
are used to (un)register a GISC (guest ISC) with a virtual machine and
its GISA.

Upon successful completion, kvm_s390_gisc_register() returns the
ISC to be used for GIB alert interruptions. A negative return code
indicates an error during registration.

Theses functions will be used by other adapter types like AP and PCI to
request pass-through interruption support.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  13 +++++
 arch/s390/kvm/interrupt.c        | 117 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 7077762ab460..2cfff617cb21 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -781,6 +781,9 @@ struct kvm_s390_gisa {
 			u8  reserved03[11];
 			u32 airq_count;
 		} g1;
+		struct {
+			u64 word[4];
+		} u64;
 	};
 };
 
@@ -813,8 +816,15 @@ struct kvm_s390_vsie {
 	struct page *pages[KVM_MAX_VCPUS];
 };
 
+struct kvm_s390_gisa_iam {
+	u8 mask;
+	spinlock_t ref_lock;
+	u32 ref_count[MAX_ISC + 1];
+};
+
 struct kvm_s390_gisa_interrupt {
 	struct kvm_s390_gisa *origin;
+	struct kvm_s390_gisa_iam alert;
 };
 
 struct kvm_arch{
@@ -885,6 +895,9 @@ void kvm_arch_crypto_set_masks(struct kvm *kvm, unsigned long *apm,
 extern int sie64a(struct kvm_s390_sie_block *, u64 *);
 extern char sie_exit;
 
+extern int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc);
+extern int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc);
+
 static inline void kvm_arch_hardware_disable(void) {}
 static inline void kvm_arch_check_processor_compat(void *rtn) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 5efcd9e2cf8f..6bc9dab6d352 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -222,6 +222,33 @@ static inline u8 int_word_to_isc(u32 int_word)
  */
 #define IPM_BIT_OFFSET (offsetof(struct kvm_s390_gisa, ipm) * BITS_PER_BYTE)
 
+/**
+ * gisa_set_iam - change the GISA interruption alert mask
+ *
+ * @gisa: gisa to operate on
+ * @iam: new IAM value to use
+ *
+ * Change the IAM atomically with the next alert address and the IPM
+ * of the GISA if the GISA is not part of the GIB alert list. All three
+ * fields are located in the first long word of the GISA.
+ *
+ * Returns: 0 on success
+ *          -EBUSY in case the gisa is part of the alert list
+ */
+static inline int gisa_set_iam(struct kvm_s390_gisa *gisa, u8 iam)
+{
+	u64 word, _word;
+
+	do {
+		word = READ_ONCE(gisa->u64.word[0]);
+		if ((u64)gisa != word >> 32)
+			return -EBUSY;
+		_word = (word & ~0xffUL) | iam;
+	} while (cmpxchg(&gisa->u64.word[0], word, _word) != word);
+
+	return 0;
+}
+
 static inline void gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
 {
 	set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
@@ -2911,6 +2938,8 @@ void kvm_s390_gisa_init(struct kvm *kvm)
 	if (!css_general_characteristics.aiv)
 		return;
 	gi->origin = &kvm->arch.sie_page2->gisa;
+	gi->alert.mask = 0;
+	spin_lock_init(&gi->alert.ref_lock);
 	kvm_s390_gisa_clear(kvm);
 	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
 }
@@ -2920,6 +2949,94 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
 	kvm->arch.gisa_int.origin = NULL;
 }
 
+/**
+ * kvm_s390_gisc_register - register a guest ISC
+ *
+ * @kvm:  the kernel vm to work with
+ * @gisc: the guest interruption sub class to register
+ *
+ * The function extends the vm specific alert mask to use.
+ * The effectve IAM mask in the GISA is updated as well
+ * in case the GISA is not part of the GIB alert list.
+ * It will be updated latest when the IAM gets restored
+ * by gisa_get_ipm_or_restore_iam().
+ *
+ * Returns: the nonspecific ISC (NISC) the gib alert mechanism
+ *          has registered with the channel subsystem.
+ *          -ENODEV in case the vm uses no GISA
+ *          -ERANGE in case the guest ISC is invalid
+ */
+int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc)
+{
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+	u8 alert_mask;
+
+	if (!gi->origin)
+		return -ENODEV;
+	if (gisc > MAX_ISC)
+		return -ERANGE;
+
+	spin_lock(&gi->alert.ref_lock);
+	if (gi->alert.ref_count[gisc] == 0) {
+		alert_mask = gi->alert.mask | 0x80 >> gisc;
+		WRITE_ONCE(gi->alert.mask, alert_mask);
+	}
+	gi->alert.ref_count[gisc]++;
+	if (gi->alert.ref_count[gisc] == 1)
+		gisa_set_iam(gi->origin, alert_mask);
+	spin_unlock(&gi->alert.ref_lock);
+
+	return gib->nisc;
+}
+EXPORT_SYMBOL_GPL(kvm_s390_gisc_register);
+
+/**
+ * kvm_s390_gisc_unregister - unregister a guest ISC
+ *
+ * @kvm:  the kernel vm to work with
+ * @gisc: the guest interruption sub class to register
+ *
+ * The function reduces the vm specific alert mask to use.
+ * The effectve IAM mask in the GISA is updated as well
+ * in case the GISA is not part of the GIB alert list.
+ * It will be updated latest when the IAM gets restored
+ * by gisa_get_ipm_or_restore_iam().
+ *
+ * Returns: the nonspecific ISC (NISC) the gib alert mechanism
+ *          has registered with the channel subsystem.
+ *          -ENODEV in case the vm uses no GISA
+ *          -ERANGE in case the guest ISC is invalid
+ *          -EINVAL in case the guest ISC is not registered
+ */
+int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc)
+{
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+	u8 alert_mask;
+	int rc = 0;
+
+	if (!gi->origin)
+		return -ENODEV;
+	if (gisc > MAX_ISC)
+		return -ERANGE;
+
+	spin_lock(&gi->alert.ref_lock);
+	if (gi->alert.ref_count[gisc] == 0) {
+		rc = -EINVAL;
+		goto out;
+	}
+	gi->alert.ref_count[gisc]--;
+	if (gi->alert.ref_count[gisc] == 0) {
+		alert_mask = gi->alert.mask & ~(0x80 >> gisc);
+		WRITE_ONCE(gi->alert.mask, alert_mask);
+		gisa_set_iam(gi->origin, alert_mask);
+	}
+out:
+	spin_unlock(&gi->alert.ref_lock);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
+
 void kvm_s390_gib_destroy(void)
 {
 	if (!gib)
-- 
2.13.4

[PATCH v6 12/13] KVM: s390: add gib_alert_irq_handler()

[Michael Mueller]

The patch implements a handler for GIB alert interruptions
on the host. Its task is to alert guests that interrupts are
pending for them.

A GIB alert interrupt statistic counter is added as well:

$ cat /proc/interrupts
          CPU0       CPU1
  ...
  GAL:      23         37   [I/O] GIB Alert
  ...

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
---
 arch/s390/include/asm/irq.h      |   1 +
 arch/s390/include/asm/isc.h      |   1 +
 arch/s390/include/asm/kvm_host.h |   3 +
 arch/s390/kernel/irq.c           |   1 +
 arch/s390/kvm/interrupt.c        | 186 +++++++++++++++++++++++++++++++++++++--
 arch/s390/kvm/kvm-s390.c         |   2 +
 6 files changed, 189 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/irq.h b/arch/s390/include/asm/irq.h
index 2f7f27e5493f..afaf5e3c57fd 100644
--- a/arch/s390/include/asm/irq.h
+++ b/arch/s390/include/asm/irq.h
@@ -62,6 +62,7 @@ enum interruption_class {
 	IRQIO_MSI,
 	IRQIO_VIR,
 	IRQIO_VAI,
+	IRQIO_GAL,
 	NMI_NMI,
 	CPU_RST,
 	NR_ARCH_IRQS
diff --git a/arch/s390/include/asm/isc.h b/arch/s390/include/asm/isc.h
index 6cb9e2ed05b6..b2cc1ec78d06 100644
--- a/arch/s390/include/asm/isc.h
+++ b/arch/s390/include/asm/isc.h
@@ -21,6 +21,7 @@
 /* Adapter interrupts. */
 #define QDIO_AIRQ_ISC IO_SCH_ISC	/* I/O subchannel in qdio mode */
 #define PCI_ISC 2			/* PCI I/O subchannels */
+#define GAL_ISC 5			/* GIB alert */
 #define AP_ISC 6			/* adjunct processor (crypto) devices */
 
 /* Functions for registration of I/O interruption subclasses */
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 2cfff617cb21..c5f51566ecd6 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -825,6 +825,9 @@ struct kvm_s390_gisa_iam {
 struct kvm_s390_gisa_interrupt {
 	struct kvm_s390_gisa *origin;
 	struct kvm_s390_gisa_iam alert;
+	struct hrtimer timer;
+	u64 expires;
+	DECLARE_BITMAP(kicked_mask, KVM_MAX_VCPUS);
 };
 
 struct kvm_arch{
diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index 0e8d68bac82c..0cd5a5f96729 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -88,6 +88,7 @@ static const struct irq_class irqclass_sub_desc[] = {
 	{.irq = IRQIO_MSI,  .name = "MSI", .desc = "[I/O] MSI Interrupt" },
 	{.irq = IRQIO_VIR,  .name = "VIR", .desc = "[I/O] Virtual I/O Devices"},
 	{.irq = IRQIO_VAI,  .name = "VAI", .desc = "[I/O] Virtual I/O Devices AI"},
+	{.irq = IRQIO_GAL,  .name = "GAL", .desc = "[I/O] GIB Alert"},
 	{.irq = NMI_NMI,    .name = "NMI", .desc = "[NMI] Machine Check"},
 	{.irq = CPU_RST,    .name = "RST", .desc = "[CPU] CPU Restart"},
 };
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 6bc9dab6d352..3294f77f4fce 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -26,6 +26,7 @@
 #include <asm/gmap.h>
 #include <asm/switch_to.h>
 #include <asm/nmi.h>
+#include <asm/airq.h>
 #include "kvm-s390.h"
 #include "gaccess.h"
 #include "trace-s390.h"
@@ -249,6 +250,57 @@ static inline int gisa_set_iam(struct kvm_s390_gisa *gisa, u8 iam)
 	return 0;
 }
 
+/**
+ * gisa_clear_ipm - clear the GISA interruption pending mask
+ *
+ * @gisa: gisa to operate on
+ *
+ * Clear the IPM atomically with the next alert address and the IAM
+ * of the GISA unconditionally. All three fields are located in the
+ * first long word of the GISA.
+ */
+static inline void gisa_clear_ipm(struct kvm_s390_gisa *gisa)
+{
+	u64 word, _word;
+
+	do {
+		word = READ_ONCE(gisa->u64.word[0]);
+		_word = word & ~(0xffUL << 24);
+	} while (cmpxchg(&gisa->u64.word[0], word, _word) != word);
+}
+
+/**
+ * gisa_get_ipm_or_restore_iam - return IPM or restore GISA IAM
+ *
+ * @gi: gisa interrupt struct to work on
+ *
+ * Atomically restores the interruption alert mask if none of the
+ * relevant ISCs are pending and return the IPM.
+ *
+ * Returns: the relevant pending ISCs
+ */
+static inline u8 gisa_get_ipm_or_restore_iam(struct kvm_s390_gisa_interrupt *gi)
+{
+	u8 pending_mask, alert_mask;
+	u64 word, _word;
+
+	do {
+		word = READ_ONCE(gi->origin->u64.word[0]);
+		alert_mask = READ_ONCE(gi->alert.mask);
+		pending_mask = (u8)(word >> 24) & alert_mask;
+		if (pending_mask)
+			return pending_mask;
+		_word = (word & ~0xffUL) | alert_mask;
+	} while (cmpxchg(&gi->origin->u64.word[0], word, _word) != word);
+
+	return 0;
+}
+
+static inline int gisa_in_alert_list(struct kvm_s390_gisa *gisa)
+{
+	return READ_ONCE(gisa->next_alert) != (u32)(u64)gisa;
+}
+
 static inline void gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
 {
 	set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
@@ -2920,14 +2972,100 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
 	return n;
 }
 
+static void __airqs_kick_single_vcpu(struct kvm *kvm, u8 deliverable_mask)
+{
+	int vcpu_id, online_vcpus = atomic_read(&kvm->online_vcpus);
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+	struct kvm_vcpu *vcpu;
+
+	for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
+		vcpu = kvm_get_vcpu(kvm, vcpu_id);
+		if (psw_ioint_disabled(vcpu))
+			continue;
+		deliverable_mask &= (u8)(vcpu->arch.sie_block->gcr[6] >> 24);
+		if (deliverable_mask) {
+			/* lately kicked but not yet running */
+			if (test_and_set_bit(vcpu_id, gi->kicked_mask))
+				return;
+			kvm_s390_vcpu_wakeup(vcpu);
+			return;
+		}
+	}
+}
+
+static enum hrtimer_restart gisa_vcpu_kicker(struct hrtimer *timer)
+{
+	struct kvm_s390_gisa_interrupt *gi =
+		container_of(timer, struct kvm_s390_gisa_interrupt, timer);
+	struct kvm *kvm =
+		container_of(gi->origin, struct sie_page2, gisa)->kvm;
+	u8 pending_mask;
+
+	pending_mask = gisa_get_ipm_or_restore_iam(gi);
+	if (pending_mask) {
+		__airqs_kick_single_vcpu(kvm, pending_mask);
+		hrtimer_forward_now(timer, ns_to_ktime(gi->expires));
+		return HRTIMER_RESTART;
+	};
+
+	return HRTIMER_NORESTART;
+}
+
+#define NULL_GISA_ADDR 0x00000000UL
+#define NONE_GISA_ADDR 0x00000001UL
+#define GISA_ADDR_MASK 0xfffff000UL
+
+static void process_gib_alert_list(void)
+{
+	struct kvm_s390_gisa_interrupt *gi;
+	struct kvm_s390_gisa *gisa;
+	struct kvm *kvm;
+	u32 final, origin = 0UL;
+
+	do {
+		/*
+		 * If the NONE_GISA_ADDR is still stored in the alert list
+		 * origin, we will leave the outer loop. No further GISA has
+		 * been added to the alert list by millicode while processing
+		 * the current alert list.
+		 */
+		final = (origin & NONE_GISA_ADDR);
+		/*
+		 * Cut off the alert list and store the NONE_GISA_ADDR in the
+		 * alert list origin to avoid further GAL interruptions.
+		 * A new alert list can be build up by millicode in parallel
+		 * for guests not in the yet cut-off alert list. When in the
+		 * final loop, store the NULL_GISA_ADDR instead. This will re-
+		 * enable GAL interruptions on the host again.
+		 */
+		origin = xchg(&gib->alert_list_origin,
+			      (!final) ? NONE_GISA_ADDR : NULL_GISA_ADDR);
+		/*
+		 * Loop through the just cut-off alert list and start the
+		 * gisa timers to kick idle vcpus to consume the pending
+		 * interruptions asap.
+		 */
+		while (origin & GISA_ADDR_MASK) {
+			gisa = (struct kvm_s390_gisa *)(u64)origin;
+			origin = gisa->next_alert;
+			gisa->next_alert = (u32)(u64)gisa;
+			kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
+			gi = &kvm->arch.gisa_int;
+			if (hrtimer_active(&gi->timer))
+				hrtimer_cancel(&gi->timer);
+			hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
+		}
+	} while (!final);
+
+}
+
 void kvm_s390_gisa_clear(struct kvm *kvm)
 {
 	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
 
 	if (!gi->origin)
 		return;
-	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
-	gi->origin->next_alert = (u32)(u64)gi->origin;
+	gisa_clear_ipm(gi->origin);
 	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", gi->origin);
 }
 
@@ -2940,13 +3078,25 @@ void kvm_s390_gisa_init(struct kvm *kvm)
 	gi->origin = &kvm->arch.sie_page2->gisa;
 	gi->alert.mask = 0;
 	spin_lock_init(&gi->alert.ref_lock);
-	kvm_s390_gisa_clear(kvm);
+	gi->expires = 50 * 1000; /* 50 usec */
+	hrtimer_init(&gi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	gi->timer.function = gisa_vcpu_kicker;
+	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
+	gi->origin->next_alert = (u32)(u64)gi->origin;
 	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
 }
 
 void kvm_s390_gisa_destroy(struct kvm *kvm)
 {
-	kvm->arch.gisa_int.origin = NULL;
+	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+
+	if (!gi->origin)
+		return;
+	hrtimer_cancel(&gi->timer);
+	WRITE_ONCE(gi->alert.mask, 0);
+	while (gisa_in_alert_list(gi->origin))
+		cpu_relax();
+	gi->origin = NULL;
 }
 
 /**
@@ -3037,11 +3187,23 @@ int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc)
 }
 EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
 
+static void gib_alert_irq_handler(struct airq_struct *airq)
+{
+	inc_irq_stat(IRQIO_GAL);
+	process_gib_alert_list();
+}
+
+static struct airq_struct gib_alert_irq = {
+	.handler = gib_alert_irq_handler,
+	.lsi_ptr = &gib_alert_irq.lsi_mask,
+};
+
 void kvm_s390_gib_destroy(void)
 {
 	if (!gib)
 		return;
 	chsc_sgib(0);
+	unregister_adapter_interrupt(&gib_alert_irq);
 	free_page((unsigned long)gib);
 	gib = NULL;
 }
@@ -3061,16 +3223,30 @@ int kvm_s390_gib_init(u8 nisc)
 		goto out;
 	}
 
+	gib_alert_irq.isc = nisc;
+	if (register_adapter_interrupt(&gib_alert_irq)) {
+		pr_err("Registering the GIB alert interruption handler failed\n");
+		rc = -EIO;
+		goto out_free_gib;
+	}
+
 	gib->nisc = nisc;
 	if (chsc_sgib((u32)(u64)gib)) {
 		pr_err("Associating the GIB with the AIV facility failed\n");
 		free_page((unsigned long)gib);
 		gib = NULL;
 		rc = -EIO;
-		goto out;
+		goto out_unreg_gal;
 	}
 
 	KVM_EVENT(3, "gib 0x%pK (nisc=%d) initialized", gib, gib->nisc);
+	goto out;
+
+out_unreg_gal:
+	unregister_adapter_interrupt(&gib_alert_irq);
+out_free_gib:
+	free_page((unsigned long)gib);
+	gib = NULL;
 out:
 	return rc;
 }
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 67023d5656fd..0e6ba4d17207 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3460,6 +3460,8 @@ static int vcpu_pre_run(struct kvm_vcpu *vcpu)
 		kvm_s390_patch_guest_per_regs(vcpu);
 	}
 
+	clear_bit(vcpu->vcpu_id, vcpu->kvm->arch.gisa_int.kicked_mask);
+
 	vcpu->arch.sie_block->icptcode = 0;
 	cpuflags = atomic_read(&vcpu->arch.sie_block->cpuflags);
 	VCPU_EVENT(vcpu, 6, "entering sie flags %x", cpuflags);
-- 
2.13.4

[PATCH v6 13/13] KVM: s390: start using the GIB

[Michael Mueller]

By initializing the GIB, it will be used by the kvm host.

Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 0e6ba4d17207..dcf6d62b4e80 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -435,8 +435,15 @@ int kvm_arch_init(void *opaque)
 		pr_err("Failed to register FLIC rc=%d\n", rc);
 		goto out_debug_unreg;
 	}
+
+	rc = kvm_s390_gib_init(GAL_ISC);
+	if (rc)
+		goto out_gib_destroy;
+
 	return 0;
 
+out_gib_destroy:
+	kvm_s390_gib_destroy();
 out_debug_unreg:
 	debug_unregister(kvm_s390_dbf);
 	return rc;
-- 
2.13.4

Re: [PATCH v6 02/13] KVM: s390: make bitmap declaration consitent

[Cornelia Huck]

On Thu, 24 Jan 2019 13:59:28 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

In the subject: s/consitent/consistent/

> Use a consistent bitmap declaration throughout the code.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h | 2 +-
>  arch/s390/kvm/interrupt.c        | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v6 06/13] KVM: s390: remove kvm_s390_ from gisa static inline functions

[Cornelia Huck]

On Thu, 24 Jan 2019 13:59:32 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> This will shorten the length of code lines. All GISA related
> static inline functions are local to interrupt.c.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>  arch/s390/kvm/interrupt.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v6 01/13] KVM: s390: drop obsolete else path

[Cornelia Huck]

On Thu, 24 Jan 2019 13:59:27 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> The explicit else path specified in set_intercept_indicators_io
> is not required as the function returns in case the first branch
> is taken anyway.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>  arch/s390/kvm/interrupt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v6 07/13] KVM: s390: introduce struct kvm_s390_gisa_interrupt

[Cornelia Huck]

On Thu, 24 Jan 2019 13:59:33 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Use this struct analog to the kvm interruption structs
> for kvm emulated floating and local interruptions.

I guess that makes sense.

> Further fields will be added with this series as
> required.

A reference to "this series" will look a bit strange if you look at the
committed patch later. What about:

"GIB handling will add further fields to this structure as required."

?

> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h |  6 ++++-
>  arch/s390/kvm/interrupt.c        | 52 +++++++++++++++++++++++-----------------
>  arch/s390/kvm/kvm-s390.c         |  2 +-
>  3 files changed, 36 insertions(+), 24 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v6 07/13] KVM: s390: introduce struct kvm_s390_gisa_interrupt

[Michael Mueller]

On 24.01.19 16:06, Cornelia Huck wrote:
> On Thu, 24 Jan 2019 13:59:33 +0100
> Michael Mueller <mimu@linux.ibm.com> wrote:
> 
>> Use this struct analog to the kvm interruption structs
>> for kvm emulated floating and local interruptions.
> 
> I guess that makes sense.
> 
>> Further fields will be added with this series as
>> required.
> 
> A reference to "this series" will look a bit strange if you look at the
> committed patch later. What about:
> 
> "GIB handling will add further fields to this structure as required."

Yes, that reads pretty well. I will replace the sentence.

> 
> ?
> 
>>
>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
>> ---
>>   arch/s390/include/asm/kvm_host.h |  6 ++++-
>>   arch/s390/kvm/interrupt.c        | 52 +++++++++++++++++++++++-----------------
>>   arch/s390/kvm/kvm-s390.c         |  2 +-
>>   3 files changed, 36 insertions(+), 24 deletions(-)
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 

Re: [PATCH v6 06/13] KVM: s390: remove kvm_s390_ from gisa static inline functions

[Pierre Morel]

On 24/01/2019 13:59, Michael Mueller wrote:
> This will shorten the length of code lines. All GISA related
> static inline functions are local to interrupt.c.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>   arch/s390/kvm/interrupt.c | 27 +++++++++++++--------------
>   1 file changed, 13 insertions(+), 14 deletions(-)

Reviewed-by: Pierre Morel<pmorel@linux.ibm.com>


-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

Re: [PATCH v6 01/13] KVM: s390: drop obsolete else path

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:27 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> The explicit else path specified in set_intercept_indicators_io
> is not required as the function returns in case the first branch
> is taken anyway.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

> ---
>  arch/s390/kvm/interrupt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index fcb55b02990e..19d556512452 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -345,7 +345,7 @@ static void set_intercept_indicators_io(struct kvm_vcpu *vcpu)
>  {
>  	if (!(pending_irqs_no_gisa(vcpu) & IRQ_PEND_IO_MASK))
>  		return;
> -	else if (psw_ioint_disabled(vcpu))
> +	if (psw_ioint_disabled(vcpu))
>  		kvm_s390_set_cpuflags(vcpu, CPUSTAT_IO_INT);
>  	else
>  		vcpu->arch.sie_block->lctl |= LCTL_CR6;

Re: [PATCH v6 02/13] KVM: s390: make bitmap declaration consitent

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:28 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Use a consistent bitmap declaration throughout the code.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

> ---
>  arch/s390/include/asm/kvm_host.h | 2 +-
>  arch/s390/kvm/interrupt.c        | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index d5d24889c3bc..3cba08f73dc6 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -591,7 +591,7 @@ struct kvm_s390_float_interrupt {
>  	struct kvm_s390_mchk_info mchk;
>  	struct kvm_s390_ext_info srv_signal;
>  	int next_rr_cpu;
> -	unsigned long idle_mask[BITS_TO_LONGS(KVM_MAX_VCPUS)];
> +	DECLARE_BITMAP(idle_mask, KVM_MAX_VCPUS);
>  	struct mutex ais_lock;
>  	u8 simm;
>  	u8 nimm;
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 19d556512452..167a3068ef84 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2831,7 +2831,7 @@ static void store_local_irq(struct kvm_s390_local_interrupt *li,
>  int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
>  {
>  	int scn;
> -	unsigned long sigp_emerg_pending[BITS_TO_LONGS(KVM_MAX_VCPUS)];
> +	DECLARE_BITMAP(sigp_emerg_pending, KVM_MAX_VCPUS);
>  	struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int;
>  	unsigned long pending_irqs;
>  	struct kvm_s390_irq irq;

Re: [PATCH v6 04/13] KVM: s390: coding style kvm_s390_gisa_init/clear()

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:30 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> The change helps to reduce line length and
> increases code readability.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

> ---
>  arch/s390/kvm/interrupt.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 2a3eb9f076c3..005dbe7252e7 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2885,20 +2885,20 @@ int kvm_s390_get_irq_state(struct kvm_vcpu *vcpu, __u8 __user *buf, int len)
>  
>  void kvm_s390_gisa_clear(struct kvm *kvm)
>  {
> -	if (kvm->arch.gisa) {
> -		memset(kvm->arch.gisa, 0, sizeof(struct kvm_s390_gisa));
> -		kvm->arch.gisa->next_alert = (u32)(u64)kvm->arch.gisa;
> -		VM_EVENT(kvm, 3, "gisa 0x%pK cleared", kvm->arch.gisa);
> -	}
> +	if (!kvm->arch.gisa)
> +		return;
> +	memset(kvm->arch.gisa, 0, sizeof(struct kvm_s390_gisa));
> +	kvm->arch.gisa->next_alert = (u32)(u64)kvm->arch.gisa;
> +	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", kvm->arch.gisa);
>  }
>  
>  void kvm_s390_gisa_init(struct kvm *kvm)
>  {
> -	if (css_general_characteristics.aiv) {
> -		kvm->arch.gisa = &kvm->arch.sie_page2->gisa;
> -		VM_EVENT(kvm, 3, "gisa 0x%pK initialized", kvm->arch.gisa);
> -		kvm_s390_gisa_clear(kvm);
> -	}
> +	if (!css_general_characteristics.aiv)
> +		return;
> +	kvm->arch.gisa = &kvm->arch.sie_page2->gisa;
> +	kvm_s390_gisa_clear(kvm);
> +	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", kvm->arch.gisa);
>  }
>  
>  void kvm_s390_gisa_destroy(struct kvm *kvm)

Re: [PATCH v6 06/13] KVM: s390: remove kvm_s390_ from gisa static inline functions

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:32 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> This will shorten the length of code lines. All GISA related
> static inline functions are local to interrupt.c.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

[..]

Re: [PATCH v6 03/13] KVM: s390: move bitmap idle_mask into arch struct top level

[Pierre Morel]

On 24/01/2019 13:59, Michael Mueller wrote:
> The vcpu idle_mask state is used by but not specific
> to the emulated floating interruptions. The state is
> relevant to gisa related interruptions as well.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>   arch/s390/include/asm/kvm_host.h |  2 +-
>   arch/s390/kvm/interrupt.c        | 11 +++++------
>   arch/s390/kvm/kvm-s390.h         |  2 +-
>   3 files changed, 7 insertions(+), 8 deletions(-)


Seems logical to me.

Reviewed-by: Pierre Morel<pmorel@linux.ibm.com>

Pierre

-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

Re: [PATCH v6 11/13] KVM: s390: add functions to (un)register GISC with GISA

[Pierre Morel]

On 24/01/2019 13:59, Michael Mueller wrote:
> Add the Interruption Alert Mask (IAM) to the architecture specific
> kvm struct. This mask in the GISA is used to define for which ISC
> a GIB alert will be issued.
> 
> The functions kvm_s390_gisc_register() and kvm_s390_gisc_unregister()
> are used to (un)register a GISC (guest ISC) with a virtual machine and
> its GISA.
> 
> Upon successful completion, kvm_s390_gisc_register() returns the
> ISC to be used for GIB alert interruptions. A negative return code
> indicates an error during registration.
> 
> Theses functions will be used by other adapter types like AP and PCI to
> request pass-through interruption support.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>   arch/s390/include/asm/kvm_host.h |  13 +++++
>   arch/s390/kvm/interrupt.c        | 117 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 130 insertions(+)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 7077762ab460..2cfff617cb21 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -781,6 +781,9 @@ struct kvm_s390_gisa {
>   			u8  reserved03[11];
>   			u32 airq_count;
>   		} g1;
> +		struct {
> +			u64 word[4];
> +		} u64;
>   	};
>   };
>   
> @@ -813,8 +816,15 @@ struct kvm_s390_vsie {
>   	struct page *pages[KVM_MAX_VCPUS];
>   };
>   
> +struct kvm_s390_gisa_iam {
> +	u8 mask;
> +	spinlock_t ref_lock;
> +	u32 ref_count[MAX_ISC + 1];
> +};
> +
>   struct kvm_s390_gisa_interrupt {
>   	struct kvm_s390_gisa *origin;
> +	struct kvm_s390_gisa_iam alert;
>   };
>   
>   struct kvm_arch{
> @@ -885,6 +895,9 @@ void kvm_arch_crypto_set_masks(struct kvm *kvm, unsigned long *apm,
>   extern int sie64a(struct kvm_s390_sie_block *, u64 *);
>   extern char sie_exit;
>   
> +extern int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc);
> +extern int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc);
> +
>   static inline void kvm_arch_hardware_disable(void) {}
>   static inline void kvm_arch_check_processor_compat(void *rtn) {}
>   static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 5efcd9e2cf8f..6bc9dab6d352 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -222,6 +222,33 @@ static inline u8 int_word_to_isc(u32 int_word)
>    */
>   #define IPM_BIT_OFFSET (offsetof(struct kvm_s390_gisa, ipm) * BITS_PER_BYTE)
>   
> +/**
> + * gisa_set_iam - change the GISA interruption alert mask
> + *
> + * @gisa: gisa to operate on
> + * @iam: new IAM value to use
> + *
> + * Change the IAM atomically with the next alert address and the IPM
> + * of the GISA if the GISA is not part of the GIB alert list. All three
> + * fields are located in the first long word of the GISA.
> + *
> + * Returns: 0 on success
> + *          -EBUSY in case the gisa is part of the alert list
> + */
> +static inline int gisa_set_iam(struct kvm_s390_gisa *gisa, u8 iam)
> +{
> +	u64 word, _word;
> +
> +	do {
> +		word = READ_ONCE(gisa->u64.word[0]);
> +		if ((u64)gisa != word >> 32)
> +			return -EBUSY;
> +		_word = (word & ~0xffUL) | iam;
> +	} while (cmpxchg(&gisa->u64.word[0], word, _word) != word);
> +
> +	return 0;
> +}
> +
>   static inline void gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
>   {
>   	set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
> @@ -2911,6 +2938,8 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>   	if (!css_general_characteristics.aiv)
>   		return;
>   	gi->origin = &kvm->arch.sie_page2->gisa;
> +	gi->alert.mask = 0;
> +	spin_lock_init(&gi->alert.ref_lock);
>   	kvm_s390_gisa_clear(kvm);
>   	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>   }
> @@ -2920,6 +2949,94 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
>   	kvm->arch.gisa_int.origin = NULL;
>   }
>   
> +/**
> + * kvm_s390_gisc_register - register a guest ISC
> + *
> + * @kvm:  the kernel vm to work with
> + * @gisc: the guest interruption sub class to register
> + *
> + * The function extends the vm specific alert mask to use.
> + * The effectve IAM mask in the GISA is updated as well
> + * in case the GISA is not part of the GIB alert list.
> + * It will be updated latest when the IAM gets restored
> + * by gisa_get_ipm_or_restore_iam().
> + *
> + * Returns: the nonspecific ISC (NISC) the gib alert mechanism
> + *          has registered with the channel subsystem.
> + *          -ENODEV in case the vm uses no GISA
> + *          -ERANGE in case the guest ISC is invalid
> + */
> +int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc)
> +{
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	u8 alert_mask;
> +
> +	if (!gi->origin)
> +		return -ENODEV;
> +	if (gisc > MAX_ISC)
> +		return -ERANGE;
> +
> +	spin_lock(&gi->alert.ref_lock);
> +	if (gi->alert.ref_count[gisc] == 0) {
> +		alert_mask = gi->alert.mask | 0x80 >> gisc;
> +		WRITE_ONCE(gi->alert.mask, alert_mask);

not sure WRITE_ONCE is useful.



> +	}
> +	gi->alert.ref_count[gisc]++;
> +	if (gi->alert.ref_count[gisc] == 1)
> +		gisa_set_iam(gi->origin, alert_mask);

This will trigger a GAL interrupt on first AP interrupt in all cases.
However setting the iam here is simple and the price is not so high.
so... without the WRITE_ONCE in register and in unregister:

Acked-by: Pierre Morel<pmorel@linux.ibm.com>

Pierre

-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

Re: [PATCH v6 05/13] KVM: s390: use pending_irqs_no_gisa() where appropriate

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:31 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Interruption types that are not represented in GISA shall
> use pending_irqs_no_gisa() to test pending interruptions.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>

I guess this is just a tiny optimization.

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

> ---
>  arch/s390/kvm/interrupt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 005dbe7252e7..cb48736867ed 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -353,7 +353,7 @@ static void set_intercept_indicators_io(struct kvm_vcpu *vcpu)
>  
>  static void set_intercept_indicators_ext(struct kvm_vcpu *vcpu)
>  {
> -	if (!(pending_irqs(vcpu) & IRQ_PEND_EXT_MASK))
> +	if (!(pending_irqs_no_gisa(vcpu) & IRQ_PEND_EXT_MASK))
>  		return;
>  	if (psw_extint_disabled(vcpu))
>  		kvm_s390_set_cpuflags(vcpu, CPUSTAT_EXT_INT);
> @@ -363,7 +363,7 @@ static void set_intercept_indicators_ext(struct kvm_vcpu *vcpu)
>  
>  static void set_intercept_indicators_mchk(struct kvm_vcpu *vcpu)
>  {
> -	if (!(pending_irqs(vcpu) & IRQ_PEND_MCHK_MASK))
> +	if (!(pending_irqs_no_gisa(vcpu) & IRQ_PEND_MCHK_MASK))
>  		return;
>  	if (psw_mchk_disabled(vcpu))
>  		vcpu->arch.sie_block->ictl |= ICTL_LPSW;

Re: [PATCH v6 06/13] KVM: s390: remove kvm_s390_ from gisa static inline functions

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:32 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Tags: inv, patch, s390 
> From: Michael Mueller <mimu@linux.ibm.com>
> To: KVM Mailing List <kvm@vger.kernel.org>
> Cc: Linux-S390 Mailing List <linux-s390@vger.kernel.org>, linux-kernel@vger.kernel.org, Martin Schwidefsky <schwidefsky@de.ibm.com>, Heiko Carstens <heiko.carstens@de.ibm.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Janosch Frank <frankja@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Pierre Morel <pmorel@linux.ibm.com>, Michael Mueller <mimu@linux.ibm.com>
> Subject: [PATCH v6 06/13] KVM: s390: remove kvm_s390_ from gisa static inline functions
> Date: Thu, 24 Jan 2019 13:59:32 +0100
> X-Mailer: git-send-email 2.13.4
> 
> This will shorten the length of code lines. All GISA related
> static inline functions are local to interrupt.c.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

Re: [PATCH v6 11/13] KVM: s390: add functions to (un)register GISC with GISA

[Michael Mueller]

On 28.01.19 16:45, Pierre Morel wrote:
> On 24/01/2019 13:59, Michael Mueller wrote:
>> Add the Interruption Alert Mask (IAM) to the architecture specific
>> kvm struct. This mask in the GISA is used to define for which ISC
>> a GIB alert will be issued.
>>
>> The functions kvm_s390_gisc_register() and kvm_s390_gisc_unregister()
>> are used to (un)register a GISC (guest ISC) with a virtual machine and
>> its GISA.
>>
>> Upon successful completion, kvm_s390_gisc_register() returns the
>> ISC to be used for GIB alert interruptions. A negative return code
>> indicates an error during registration.
>>
>> Theses functions will be used by other adapter types like AP and PCI to
>> request pass-through interruption support.
>>
>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
>> ---
>>   arch/s390/include/asm/kvm_host.h |  13 +++++
>>   arch/s390/kvm/interrupt.c        | 117 
>> +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 130 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h 
>> b/arch/s390/include/asm/kvm_host.h
>> index 7077762ab460..2cfff617cb21 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -781,6 +781,9 @@ struct kvm_s390_gisa {
>>               u8  reserved03[11];
>>               u32 airq_count;
>>           } g1;
>> +        struct {
>> +            u64 word[4];
>> +        } u64;
>>       };
>>   };
>> @@ -813,8 +816,15 @@ struct kvm_s390_vsie {
>>       struct page *pages[KVM_MAX_VCPUS];
>>   };
>> +struct kvm_s390_gisa_iam {
>> +    u8 mask;
>> +    spinlock_t ref_lock;
>> +    u32 ref_count[MAX_ISC + 1];
>> +};
>> +
>>   struct kvm_s390_gisa_interrupt {
>>       struct kvm_s390_gisa *origin;
>> +    struct kvm_s390_gisa_iam alert;
>>   };
>>   struct kvm_arch{
>> @@ -885,6 +895,9 @@ void kvm_arch_crypto_set_masks(struct kvm *kvm, 
>> unsigned long *apm,
>>   extern int sie64a(struct kvm_s390_sie_block *, u64 *);
>>   extern char sie_exit;
>> +extern int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc);
>> +extern int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc);
>> +
>>   static inline void kvm_arch_hardware_disable(void) {}
>>   static inline void kvm_arch_check_processor_compat(void *rtn) {}
>>   static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>> index 5efcd9e2cf8f..6bc9dab6d352 100644
>> --- a/arch/s390/kvm/interrupt.c
>> +++ b/arch/s390/kvm/interrupt.c
>> @@ -222,6 +222,33 @@ static inline u8 int_word_to_isc(u32 int_word)
>>    */
>>   #define IPM_BIT_OFFSET (offsetof(struct kvm_s390_gisa, ipm) * 
>> BITS_PER_BYTE)
>> +/**
>> + * gisa_set_iam - change the GISA interruption alert mask
>> + *
>> + * @gisa: gisa to operate on
>> + * @iam: new IAM value to use
>> + *
>> + * Change the IAM atomically with the next alert address and the IPM
>> + * of the GISA if the GISA is not part of the GIB alert list. All three
>> + * fields are located in the first long word of the GISA.
>> + *
>> + * Returns: 0 on success
>> + *          -EBUSY in case the gisa is part of the alert list
>> + */
>> +static inline int gisa_set_iam(struct kvm_s390_gisa *gisa, u8 iam)
>> +{
>> +    u64 word, _word;
>> +
>> +    do {
>> +        word = READ_ONCE(gisa->u64.word[0]);
>> +        if ((u64)gisa != word >> 32)
>> +            return -EBUSY;
>> +        _word = (word & ~0xffUL) | iam;
>> +    } while (cmpxchg(&gisa->u64.word[0], word, _word) != word);
>> +
>> +    return 0;
>> +}
>> +
>>   static inline void gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 
>> gisc)
>>   {
>>       set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
>> @@ -2911,6 +2938,8 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>>       if (!css_general_characteristics.aiv)
>>           return;
>>       gi->origin = &kvm->arch.sie_page2->gisa;
>> +    gi->alert.mask = 0;
>> +    spin_lock_init(&gi->alert.ref_lock);
>>       kvm_s390_gisa_clear(kvm);
>>       VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>>   }
>> @@ -2920,6 +2949,94 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
>>       kvm->arch.gisa_int.origin = NULL;
>>   }
>> +/**
>> + * kvm_s390_gisc_register - register a guest ISC
>> + *
>> + * @kvm:  the kernel vm to work with
>> + * @gisc: the guest interruption sub class to register
>> + *
>> + * The function extends the vm specific alert mask to use.
>> + * The effectve IAM mask in the GISA is updated as well
>> + * in case the GISA is not part of the GIB alert list.
>> + * It will be updated latest when the IAM gets restored
>> + * by gisa_get_ipm_or_restore_iam().
>> + *
>> + * Returns: the nonspecific ISC (NISC) the gib alert mechanism
>> + *          has registered with the channel subsystem.
>> + *          -ENODEV in case the vm uses no GISA
>> + *          -ERANGE in case the guest ISC is invalid
>> + */
>> +int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc)
>> +{
>> +    struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>> +    u8 alert_mask;
>> +
>> +    if (!gi->origin)
>> +        return -ENODEV;
>> +    if (gisc > MAX_ISC)
>> +        return -ERANGE;
>> +
>> +    spin_lock(&gi->alert.ref_lock);
>> +    if (gi->alert.ref_count[gisc] == 0) {
>> +        alert_mask = gi->alert.mask | 0x80 >> gisc;
>> +        WRITE_ONCE(gi->alert.mask, alert_mask);
> 
> not sure WRITE_ONCE is useful.

I dropped that in both routines.

> 
> 
> 
>> +    }
>> +    gi->alert.ref_count[gisc]++;
>> +    if (gi->alert.ref_count[gisc] == 1)
>> +        gisa_set_iam(gi->origin, alert_mask);
> 
> This will trigger a GAL interrupt on first AP interrupt in all cases.
> However setting the iam here is simple and the price is not so high.
> so... without the WRITE_ONCE in register and in unregister:


> 
> Acked-by: Pierre Morel<pmorel@linux.ibm.com>
> 
> Pierre
> 

Cheers,
Michael

Re: [PATCH v6 07/13] KVM: s390: introduce struct kvm_s390_gisa_interrupt

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:33 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Use this struct analog to the kvm interruption structs
> for kvm emulated floating and local interruptions.
> Further fields will be added with this series as
> required.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

While looking at this I was asking myself what guards against invalid
gisa pointer dereference e.g. when pending_irqs() is called (see below).

AFAIU we set up gisa_int.origin only if we have
css_general_characteristics.aiv. Opinions?

Anyway if it is a problem, it is a pre-existing one (should be fixed
ASAP but does not affect the validity of this patch).

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

> ---
>  arch/s390/include/asm/kvm_host.h |  6 ++++-
>  arch/s390/kvm/interrupt.c        | 52 +++++++++++++++++++++++-----------------
>  arch/s390/kvm/kvm-s390.c         |  2 +-
>  3 files changed, 36 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index c259a67c4298..0941571d34eb 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -803,6 +803,10 @@ struct kvm_s390_vsie {
>  	struct page *pages[KVM_MAX_VCPUS];
>  };
>  

[..]

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 942cc7d33766..ee91d1de0036 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -246,7 +246,8 @@ static inline unsigned long pending_irqs_no_gisa(struct kvm_vcpu *vcpu)
>  static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
>  {
>  	return pending_irqs_no_gisa(vcpu) |
> -		gisa_get_ipm(vcpu->kvm->arch.gisa) << IRQ_PEND_IO_ISC_7;
> +		gisa_get_ipm(vcpu->kvm->arch.gisa_int.origin) <<

Unconditional call to gisa_get_ipm(), and get ipm just accesses ->ipm.

> +			IRQ_PEND_IO_ISC_7;
>  }
>  

[..]

>  void kvm_s390_gisa_init(struct kvm *kvm)
>  {
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +
>  	if (!css_general_characteristics.aiv)
>  		return;
> -	kvm->arch.gisa = &kvm->arch.sie_page2->gisa;
> +	gi->origin = &kvm->arch.sie_page2->gisa;

Should stay NULL if !css_general_characteristics.aiv.

>  	kvm_s390_gisa_clear(kvm);
> -	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", kvm->arch.gisa);
> +	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>  }
>  

Re: [PATCH v6 09/13] KVM: s390: add the GIB and its related life-cyle functions

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:35 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> The Guest Information Block (GIB) links the GISA of all guests
> that have adapter interrupts pending. These interrupts cannot be
> delivered because all vcpus of these guests are currently in WAIT
> state or have masked the respective Innterruption Sub Class (ISC).
> If enabled, a GIB alert is issued on the host to schedule these
> guests to run suitable vcpus to consume the pending interruptions.
> 
> This mechanism allows to process adapter interrupts for currently
> not running guests.
> 
> The GIB is created during host initialization and associated with
> the Adapter Interruption Facility in case an Adapter Interruption
> Virtualization Facility is available.
> 
> The GIB initialization and thus the activation of the related code
> will be done in an upcoming patch of this series.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>

I don't agree with the commit message to 100%, but I'd rather not start
a discussion.

Acked-by: Halil Pasic <pasic@linux.ibm.com>

[..]

Re: [PATCH v6 11/13] KVM: s390: add functions to (un)register GISC with GISA

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:37 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Add the Interruption Alert Mask (IAM) to the architecture specific
> kvm struct. This mask in the GISA is used to define for which ISC
> a GIB alert will be issued.
> 
> The functions kvm_s390_gisc_register() and kvm_s390_gisc_unregister()
> are used to (un)register a GISC (guest ISC) with a virtual machine and
> its GISA.
> 
> Upon successful completion, kvm_s390_gisc_register() returns the
> ISC to be used for GIB alert interruptions. A negative return code
> indicates an error during registration.
> 
> Theses functions will be used by other adapter types like AP and PCI to
> request pass-through interruption support.


I'm not sure this interface is going to to fit PCI that well. But IMHO
no reason to delay the whole series -- we can think about zPCI later.
Same goes for some of the names.

Another idea for later would be to sanity check in gisa destroy that
alert.mask is back to all zero -- to catch any corresponding driver
bugs.

Acked-by: Halil Pasic <pasic@linux.ibm.com>

> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>

[..]

>  static inline void gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc)
>  {
>  	set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa);
> @@ -2911,6 +2938,8 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>  	if (!css_general_characteristics.aiv)
>  		return;
>  	gi->origin = &kvm->arch.sie_page2->gisa;
> +	gi->alert.mask = 0;

I don't think this is necessary. Otherwise you would need to
zero the alert.ref[] too, or?

Regards,
Halil

> +	spin_lock_init(&gi->alert.ref_lock);
>  	kvm_s390_gisa_clear(kvm);
>  	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>  }

Re: [PATCH v6 10/13] KVM: s390: add kvm reference to struct sie_page2

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:36 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Adding the kvm reference to struct sie_page2 will allow to
> determine the kvm a given gisa belongs to:
> 
>   container_of(gisa, struct sie_page2, gisa)->kvm
> 
> This functionality will be required to process a gisa in
> gib alert interruption context.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>


Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

[..]

Re: [PATCH v6 11/13] KVM: s390: add functions to (un)register GISC with GISA

[Cornelia Huck]

On Thu, 24 Jan 2019 13:59:37 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> Add the Interruption Alert Mask (IAM) to the architecture specific
> kvm struct. This mask in the GISA is used to define for which ISC
> a GIB alert will be issued.
> 
> The functions kvm_s390_gisc_register() and kvm_s390_gisc_unregister()
> are used to (un)register a GISC (guest ISC) with a virtual machine and
> its GISA.
> 
> Upon successful completion, kvm_s390_gisc_register() returns the
> ISC to be used for GIB alert interruptions. A negative return code
> indicates an error during registration.
> 
> Theses functions will be used by other adapter types like AP and PCI to
> request pass-through interruption support.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h |  13 +++++
>  arch/s390/kvm/interrupt.c        | 117 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 130 insertions(+)
> 

> +int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc)
> +{
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	u8 alert_mask;
> +
> +	if (!gi->origin)
> +		return -ENODEV;
> +	if (gisc > MAX_ISC)
> +		return -ERANGE;
> +
> +	spin_lock(&gi->alert.ref_lock);
> +	if (gi->alert.ref_count[gisc] == 0) {

If the ref_count is 0 here...

> +		alert_mask = gi->alert.mask | 0x80 >> gisc;
> +		WRITE_ONCE(gi->alert.mask, alert_mask);
> +	}
> +	gi->alert.ref_count[gisc]++;
> +	if (gi->alert.ref_count[gisc] == 1)

...it will certainly be 1 here, won't it?

Can you do all of the manipulations in a single if branch?

> +		gisa_set_iam(gi->origin, alert_mask);
> +	spin_unlock(&gi->alert.ref_lock);
> +
> +	return gib->nisc;
> +}

Re: [PATCH v6 07/13] KVM: s390: introduce struct kvm_s390_gisa_interrupt

[Cornelia Huck]

On Mon, 28 Jan 2019 17:50:54 +0100
Halil Pasic <pasic@linux.ibm.com> wrote:

> On Thu, 24 Jan 2019 13:59:33 +0100
> Michael Mueller <mimu@linux.ibm.com> wrote:
> 
> > Use this struct analog to the kvm interruption structs
> > for kvm emulated floating and local interruptions.
> > Further fields will be added with this series as
> > required.
> > 
> > Signed-off-by: Michael Mueller <mimu@linux.ibm.com>  
> 
> While looking at this I was asking myself what guards against invalid
> gisa pointer dereference e.g. when pending_irqs() is called (see below).
> 
> AFAIU we set up gisa_int.origin only if we have
> css_general_characteristics.aiv. Opinions?

I think you're right that this is a (pre-existing) problem.

> > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> > index 942cc7d33766..ee91d1de0036 100644
> > --- a/arch/s390/kvm/interrupt.c
> > +++ b/arch/s390/kvm/interrupt.c
> > @@ -246,7 +246,8 @@ static inline unsigned long pending_irqs_no_gisa(struct kvm_vcpu *vcpu)
> >  static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
> >  {
> >  	return pending_irqs_no_gisa(vcpu) |
> > -		gisa_get_ipm(vcpu->kvm->arch.gisa) << IRQ_PEND_IO_ISC_7;
> > +		gisa_get_ipm(vcpu->kvm->arch.gisa_int.origin) <<  
> 
> Unconditional call to gisa_get_ipm(), and get ipm just accesses ->ipm.

All other callers of this function check for gisa != NULL first, so
either we should check here as well or move the check into the
gisa_get_ipm() function.

> 
> > +			IRQ_PEND_IO_ISC_7;
> >  }
> >    

Re: [PATCH v6 12/13] KVM: s390: add gib_alert_irq_handler()

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:38 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> The patch implements a handler for GIB alert interruptions
> on the host. Its task is to alert guests that interrupts are
> pending for them.
> 
> A GIB alert interrupt statistic counter is added as well:
> 
> $ cat /proc/interrupts
>           CPU0       CPU1
>   ...
>   GAL:      23         37   [I/O] GIB Alert
>   ...
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
[..]
> +/**
> + * gisa_get_ipm_or_restore_iam - return IPM or restore GISA IAM
> + *
> + * @gi: gisa interrupt struct to work on
> + *
> + * Atomically restores the interruption alert mask if none of the
> + * relevant ISCs are pending and return the IPM.

The word 'relevant' probably reflects some previous state. It does not
bother me too much.

[..]

>  
> +static void __airqs_kick_single_vcpu(struct kvm *kvm, u8 deliverable_mask)
> +{
> +	int vcpu_id, online_vcpus = atomic_read(&kvm->online_vcpus);
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +	struct kvm_vcpu *vcpu;
> +
> +	for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
> +		vcpu = kvm_get_vcpu(kvm, vcpu_id);
> +		if (psw_ioint_disabled(vcpu))
> +			continue;
> +		deliverable_mask &= (u8)(vcpu->arch.sie_block->gcr[6] >> 24);
> +		if (deliverable_mask) {
> +			/* lately kicked but not yet running */

How about /* was kicked but didn't run yet */?

> +			if (test_and_set_bit(vcpu_id, gi->kicked_mask))
> +				return;
> +			kvm_s390_vcpu_wakeup(vcpu);
> +			return;
> +		}
> +	}
> +}
> +

[..]

> +static void process_gib_alert_list(void)
> +{
> +	struct kvm_s390_gisa_interrupt *gi;
> +	struct kvm_s390_gisa *gisa;
> +	struct kvm *kvm;
> +	u32 final, origin = 0UL;
> +
> +	do {
> +		/*
> +		 * If the NONE_GISA_ADDR is still stored in the alert list
> +		 * origin, we will leave the outer loop. No further GISA has
> +		 * been added to the alert list by millicode while processing
> +		 * the current alert list.
> +		 */
> +		final = (origin & NONE_GISA_ADDR);
> +		/*
> +		 * Cut off the alert list and store the NONE_GISA_ADDR in the
> +		 * alert list origin to avoid further GAL interruptions.
> +		 * A new alert list can be build up by millicode in parallel
> +		 * for guests not in the yet cut-off alert list. When in the
> +		 * final loop, store the NULL_GISA_ADDR instead. This will re-
> +		 * enable GAL interruptions on the host again.
> +		 */
> +		origin = xchg(&gib->alert_list_origin,
> +			      (!final) ? NONE_GISA_ADDR : NULL_GISA_ADDR);
> +		/*
> +		 * Loop through the just cut-off alert list and start the
> +		 * gisa timers to kick idle vcpus to consume the pending
> +		 * interruptions asap.
> +		 */
> +		while (origin & GISA_ADDR_MASK) {
> +			gisa = (struct kvm_s390_gisa *)(u64)origin;
> +			origin = gisa->next_alert;
> +			gisa->next_alert = (u32)(u64)gisa;
> +			kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
> +			gi = &kvm->arch.gisa_int;
> +			if (hrtimer_active(&gi->timer))
> +				hrtimer_cancel(&gi->timer);
> +			hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
> +		}
> +	} while (!final);
> +
> +}
> +
>  void kvm_s390_gisa_clear(struct kvm *kvm)
>  {
>  	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>  
>  	if (!gi->origin)
>  		return;
> -	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
> -	gi->origin->next_alert = (u32)(u64)gi->origin;
> +	gisa_clear_ipm(gi->origin);

This could be a separate patch. I would like little more explanation
to this.

>  	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", gi->origin);
>  }
>  
> @@ -2940,13 +3078,25 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>  	gi->origin = &kvm->arch.sie_page2->gisa;
>  	gi->alert.mask = 0;
>  	spin_lock_init(&gi->alert.ref_lock);
> -	kvm_s390_gisa_clear(kvm);
> +	gi->expires = 50 * 1000; /* 50 usec */

I blindly trust your choice here ;) 

> +	hrtimer_init(&gi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	gi->timer.function = gisa_vcpu_kicker;
> +	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
> +	gi->origin->next_alert = (u32)(u64)gi->origin;
>  	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>  }
>  
>  void kvm_s390_gisa_destroy(struct kvm *kvm)
>  {
> -	kvm->arch.gisa_int.origin = NULL;
> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +
> +	if (!gi->origin)
> +		return;
> +	hrtimer_cancel(&gi->timer);

I'm not sure this cancel here is sufficient. 

> +	WRITE_ONCE(gi->alert.mask, 0);
> +	while (gisa_in_alert_list(gi->origin))
> +		cpu_relax();

If you end up waiting here, I guess, it's likely that a new
timer is going to get set up right after we do
gisa->next_alert = (u32)(u64)gisa;
in  process_gib_alert_list().

> +	gi->origin = NULL;
>  }
>  
>  /**
> @@ -3037,11 +3187,23 @@ int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc)
>  }
>  EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
>  


Overall, there are couple of things I would prefer done differently,
but better something working today that something prefect in 6 months.
In that sense, provided my comment regarding destroy is addressed:

Acked-by: Halil Pasic <pasic@linux.ibm.com>

Re: [PATCH v6 13/13] KVM: s390: start using the GIB

[Halil Pasic]

On Thu, 24 Jan 2019 13:59:39 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> By initializing the GIB, it will be used by the kvm host.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>

Reviewed-by: Halil Pasic <pasic@linux.ibm.com>

Re: [PATCH v6 11/13] KVM: s390: add functions to (un)register GISC with GISA

[Michael Mueller]

On 29.01.19 14:09, Cornelia Huck wrote:
> On Thu, 24 Jan 2019 13:59:37 +0100
> Michael Mueller <mimu@linux.ibm.com> wrote:
> 
>> Add the Interruption Alert Mask (IAM) to the architecture specific
>> kvm struct. This mask in the GISA is used to define for which ISC
>> a GIB alert will be issued.
>>
>> The functions kvm_s390_gisc_register() and kvm_s390_gisc_unregister()
>> are used to (un)register a GISC (guest ISC) with a virtual machine and
>> its GISA.
>>
>> Upon successful completion, kvm_s390_gisc_register() returns the
>> ISC to be used for GIB alert interruptions. A negative return code
>> indicates an error during registration.
>>
>> Theses functions will be used by other adapter types like AP and PCI to
>> request pass-through interruption support.
>>
>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
>> ---
>>   arch/s390/include/asm/kvm_host.h |  13 +++++
>>   arch/s390/kvm/interrupt.c        | 117 +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 130 insertions(+)
>>
> 
>> +int kvm_s390_gisc_register(struct kvm *kvm, u32 gisc)
>> +{
>> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>> +	u8 alert_mask;
>> +
>> +	if (!gi->origin)
>> +		return -ENODEV;
>> +	if (gisc > MAX_ISC)
>> +		return -ERANGE;
>> +
>> +	spin_lock(&gi->alert.ref_lock);
>> +	if (gi->alert.ref_count[gisc] == 0) {
> 
> If the ref_count is 0 here...
> 
>> +		alert_mask = gi->alert.mask | 0x80 >> gisc;
>> +		WRITE_ONCE(gi->alert.mask, alert_mask);
>> +	}
>> +	gi->alert.ref_count[gisc]++;
>> +	if (gi->alert.ref_count[gisc] == 1)
> 
> ...it will certainly be 1 here, won't it?

Sure, the increment is unconditional and can be done right away.
Thus it is logically the same and now symmetric to the unregister
function:

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index ba314da746b7..f37dfb01c63c 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2976,11 +2976,11 @@ int kvm_s390_gisc_register(struct kvm *kvm, u32 
gisc)
                 return -ERANGE;

         spin_lock(&gi->alert.ref_lock);
-       if (gi->alert.ref_count[gisc] == 0)
-               gi->alert.mask |= 0x80 >> gisc;
         gi->alert.ref_count[gisc]++;
-       if (gi->alert.ref_count[gisc] == 1)
+       if (gi->alert.ref_count[gisc] == 1) {
+               gi->alert.mask |= 0x80 >> gisc;
                 gisa_set_iam(gi->origin, gi->alert.mask);
+       }
         spin_unlock(&gi->alert.ref_lock);

         return gib->nisc;


> 
> Can you do all of the manipulations in a single if branch?
> 
>> +		gisa_set_iam(gi->origin, alert_mask);
>> +	spin_unlock(&gi->alert.ref_lock);
>> +
>> +	return gib->nisc;
>> +}
> 

Thanks,
Michael

Re: [PATCH v6 03/13] KVM: s390: move bitmap idle_mask into arch struct top level

[Cornelia Huck]

On Thu, 24 Jan 2019 13:59:29 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> The vcpu idle_mask state is used by but not specific
> to the emulated floating interruptions. The state is
> relevant to gisa related interruptions as well.
> 
> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h |  2 +-
>  arch/s390/kvm/interrupt.c        | 11 +++++------
>  arch/s390/kvm/kvm-s390.h         |  2 +-
>  3 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v6 12/13] KVM: s390: add gib_alert_irq_handler()

[Michael Mueller]

On 29.01.19 14:26, Halil Pasic wrote:
> On Thu, 24 Jan 2019 13:59:38 +0100
> Michael Mueller <mimu@linux.ibm.com> wrote:
> 
>> The patch implements a handler for GIB alert interruptions
>> on the host. Its task is to alert guests that interrupts are
>> pending for them.
>>
>> A GIB alert interrupt statistic counter is added as well:
>>
>> $ cat /proc/interrupts
>>            CPU0       CPU1
>>    ...
>>    GAL:      23         37   [I/O] GIB Alert
>>    ...
>>
>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> [..]
>> +/**
>> + * gisa_get_ipm_or_restore_iam - return IPM or restore GISA IAM
>> + *
>> + * @gi: gisa interrupt struct to work on
>> + *
>> + * Atomically restores the interruption alert mask if none of the
>> + * relevant ISCs are pending and return the IPM.
> 
> The word 'relevant' probably reflects some previous state. It does not
> bother me too much.

"relevant" refers to the ISCs handled by the GAL mechanism, i.e those
registered in the kvm->arch.gisa_int.alert.mask.

> 
> [..]
> 
>>   
>> +static void __airqs_kick_single_vcpu(struct kvm *kvm, u8 deliverable_mask)
>> +{
>> +	int vcpu_id, online_vcpus = atomic_read(&kvm->online_vcpus);
>> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>> +	struct kvm_vcpu *vcpu;
>> +
>> +	for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
>> +		vcpu = kvm_get_vcpu(kvm, vcpu_id);
>> +		if (psw_ioint_disabled(vcpu))
>> +			continue;
>> +		deliverable_mask &= (u8)(vcpu->arch.sie_block->gcr[6] >> 24);
>> +		if (deliverable_mask) {
>> +			/* lately kicked but not yet running */
> 
> How about /* was kicked but didn't run yet */?

what is the difference here...

> 
>> +			if (test_and_set_bit(vcpu_id, gi->kicked_mask))
>> +				return;
>> +			kvm_s390_vcpu_wakeup(vcpu);
>> +			return;
>> +		}
>> +	}
>> +}
>> +
> 
> [..]
> 
>> +static void process_gib_alert_list(void)
>> +{
>> +	struct kvm_s390_gisa_interrupt *gi;
>> +	struct kvm_s390_gisa *gisa;
>> +	struct kvm *kvm;
>> +	u32 final, origin = 0UL;
>> +
>> +	do {
>> +		/*
>> +		 * If the NONE_GISA_ADDR is still stored in the alert list
>> +		 * origin, we will leave the outer loop. No further GISA has
>> +		 * been added to the alert list by millicode while processing
>> +		 * the current alert list.
>> +		 */
>> +		final = (origin & NONE_GISA_ADDR);
>> +		/*
>> +		 * Cut off the alert list and store the NONE_GISA_ADDR in the
>> +		 * alert list origin to avoid further GAL interruptions.
>> +		 * A new alert list can be build up by millicode in parallel
>> +		 * for guests not in the yet cut-off alert list. When in the
>> +		 * final loop, store the NULL_GISA_ADDR instead. This will re-
>> +		 * enable GAL interruptions on the host again.
>> +		 */
>> +		origin = xchg(&gib->alert_list_origin,
>> +			      (!final) ? NONE_GISA_ADDR : NULL_GISA_ADDR);
>> +		/*
>> +		 * Loop through the just cut-off alert list and start the
>> +		 * gisa timers to kick idle vcpus to consume the pending
>> +		 * interruptions asap.
>> +		 */
>> +		while (origin & GISA_ADDR_MASK) {
>> +			gisa = (struct kvm_s390_gisa *)(u64)origin;
>> +			origin = gisa->next_alert;
>> +			gisa->next_alert = (u32)(u64)gisa;
>> +			kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
>> +			gi = &kvm->arch.gisa_int;
>> +			if (hrtimer_active(&gi->timer))
>> +				hrtimer_cancel(&gi->timer);
>> +			hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
>> +		}
>> +	} while (!final);
>> +
>> +}
>> +
>>   void kvm_s390_gisa_clear(struct kvm *kvm)
>>   {
>>   	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>>   
>>   	if (!gi->origin)
>>   		return;
>> -	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
>> -	gi->origin->next_alert = (u32)(u64)gi->origin;
>> +	gisa_clear_ipm(gi->origin);
> 
> This could be a separate patch. I would like little more explanation
> to this.

I can break at out as I had before... ;)

> 
>>   	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", gi->origin);
>>   }
>>   
>> @@ -2940,13 +3078,25 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>>   	gi->origin = &kvm->arch.sie_page2->gisa;
>>   	gi->alert.mask = 0;
>>   	spin_lock_init(&gi->alert.ref_lock);
>> -	kvm_s390_gisa_clear(kvm);
>> +	gi->expires = 50 * 1000; /* 50 usec */
> 
> I blindly trust your choice here ;)

You know I will increase it to 1 ms together with the change that I
proposed. (gisa_get_ipm_or_restore_iam() in kvm_s390_handle_wait()).

> 
>> +	hrtimer_init(&gi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> +	gi->timer.function = gisa_vcpu_kicker;
>> +	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
>> +	gi->origin->next_alert = (u32)(u64)gi->origin;
>>   	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
>>   }
>>   
>>   void kvm_s390_gisa_destroy(struct kvm *kvm)
>>   {
>> -	kvm->arch.gisa_int.origin = NULL;
>> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>> +
>> +	if (!gi->origin)
>> +		return;
>> +	hrtimer_cancel(&gi->timer);
> 
> I'm not sure this cancel here is sufficient.
> 
>> +	WRITE_ONCE(gi->alert.mask, 0);
>> +	while (gisa_in_alert_list(gi->origin))
>> +		cpu_relax();
> 
> If you end up waiting here, I guess, it's likely that a new
> timer is going to get set up right after we do
> gisa->next_alert = (u32)(u64)gisa;
> in  process_gib_alert_list().

There will be no vcpus available anymore at this time, whence
none will be kicked by the timer function. Thus canceling the
timer will be sufficient after the loop.

I have addressed the message as well, but will write it into
the KVM trace.

  void kvm_s390_gisa_destroy(struct kvm *kvm)
  {
-       kvm->arch.gisa_int.origin = NULL;
+       struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
+
+       if (!gi->origin)
+               return;
+       if (gi->alert.mask)
+               KVM_EVENT(3, "vm 0x%pK has unexpected iam 0x%02x",
+                         kvm, gi->alert.mask);
+       while (gisa_in_alert_list(gi->origin))
+               cpu_relax();
+       hrtimer_cancel(&gi->timer);
+       gi->origin = NULL;
  }


> 
>> +	gi->origin = NULL;
>>   }
>>   
>>   /**
>> @@ -3037,11 +3187,23 @@ int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc)
>>   }
>>   EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
>>   
> 
> 
> Overall, there are couple of things I would prefer done differently,
> but better something working today that something prefect in 6 months.
> In that sense, provided my comment regarding destroy is addressed:
> 
> Acked-by: Halil Pasic <pasic@linux.ibm.com>
> 

Michael

Re: [PATCH v6 07/13] KVM: s390: introduce struct kvm_s390_gisa_interrupt

[Michael Mueller]

On 29.01.19 14:22, Cornelia Huck wrote:
> On Mon, 28 Jan 2019 17:50:54 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
>> On Thu, 24 Jan 2019 13:59:33 +0100
>> Michael Mueller <mimu@linux.ibm.com> wrote:
>>
>>> Use this struct analog to the kvm interruption structs
>>> for kvm emulated floating and local interruptions.
>>> Further fields will be added with this series as
>>> required.
>>>
>>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
>>
>> While looking at this I was asking myself what guards against invalid
>> gisa pointer dereference e.g. when pending_irqs() is called (see below).
>>
>> AFAIU we set up gisa_int.origin only if we have
>> css_general_characteristics.aiv. Opinions?
> 
> I think you're right that this is a (pre-existing) problem.
> 
>>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>>> index 942cc7d33766..ee91d1de0036 100644
>>> --- a/arch/s390/kvm/interrupt.c
>>> +++ b/arch/s390/kvm/interrupt.c
>>> @@ -246,7 +246,8 @@ static inline unsigned long pending_irqs_no_gisa(struct kvm_vcpu *vcpu)
>>>   static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
>>>   {
>>>   	return pending_irqs_no_gisa(vcpu) |
>>> -		gisa_get_ipm(vcpu->kvm->arch.gisa) << IRQ_PEND_IO_ISC_7;
>>> +		gisa_get_ipm(vcpu->kvm->arch.gisa_int.origin) <<
>>
>> Unconditional call to gisa_get_ipm(), and get ipm just accesses ->ipm.
> 
> All other callers of this function check for gisa != NULL first, so
> either we should check here as well or move the check into the
> gisa_get_ipm() function.

I suggest to use an explicit test like this.

  static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
  {
-       return pending_irqs_no_gisa(vcpu) |
-               gisa_get_ipm(vcpu->kvm->arch.gisa_int.origin) <<
-                       IRQ_PEND_IO_ISC_7;
+       struct kvm_s390_gisa_int *gi = &vcpu->kvm->arch.gisa_int;
+       unsigned long pending_mask;
+
+       pending_mask = pending_irqs_no_gisa(vcpu);
+       if (gi->origin)
+               pending_mask |= gisa_get_ipm(gi->origin) << 
IRQ_PEND_IO_ISC_7;
+       return pending_mask;
  }

Michael


> 
>>
>>> +			IRQ_PEND_IO_ISC_7;
>>>   }
>>>     
> 

Re: [PATCH v6 07/13] KVM: s390: introduce struct kvm_s390_gisa_interrupt

[Halil Pasic]

On Tue, 29 Jan 2019 16:47:10 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> 
> 
> On 29.01.19 14:22, Cornelia Huck wrote:
> > On Mon, 28 Jan 2019 17:50:54 +0100
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> > 
> >> On Thu, 24 Jan 2019 13:59:33 +0100
> >> Michael Mueller <mimu@linux.ibm.com> wrote:
> >>
> >>> Use this struct analog to the kvm interruption structs
> >>> for kvm emulated floating and local interruptions.
> >>> Further fields will be added with this series as
> >>> required.
> >>>
> >>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> >>
> >> While looking at this I was asking myself what guards against invalid
> >> gisa pointer dereference e.g. when pending_irqs() is called (see below).
> >>
> >> AFAIU we set up gisa_int.origin only if we have
> >> css_general_characteristics.aiv. Opinions?
> > 
> > I think you're right that this is a (pre-existing) problem.
> > 
> >>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> >>> index 942cc7d33766..ee91d1de0036 100644
> >>> --- a/arch/s390/kvm/interrupt.c
> >>> +++ b/arch/s390/kvm/interrupt.c
> >>> @@ -246,7 +246,8 @@ static inline unsigned long pending_irqs_no_gisa(struct kvm_vcpu *vcpu)
> >>>   static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
> >>>   {
> >>>   	return pending_irqs_no_gisa(vcpu) |
> >>> -		gisa_get_ipm(vcpu->kvm->arch.gisa) << IRQ_PEND_IO_ISC_7;
> >>> +		gisa_get_ipm(vcpu->kvm->arch.gisa_int.origin) <<
> >>
> >> Unconditional call to gisa_get_ipm(), and get ipm just accesses ->ipm.
> > 
> > All other callers of this function check for gisa != NULL first, so
> > either we should check here as well or move the check into the
> > gisa_get_ipm() function.
> 
> I suggest to use an explicit test like this.
> 
>   static inline unsigned long pending_irqs(struct kvm_vcpu *vcpu)
>   {
> -       return pending_irqs_no_gisa(vcpu) |
> -               gisa_get_ipm(vcpu->kvm->arch.gisa_int.origin) <<
> -                       IRQ_PEND_IO_ISC_7;
> +       struct kvm_s390_gisa_int *gi = &vcpu->kvm->arch.gisa_int;
> +       unsigned long pending_mask;
> +
> +       pending_mask = pending_irqs_no_gisa(vcpu);
> +       if (gi->origin)
> +               pending_mask |= gisa_get_ipm(gi->origin) << 
> IRQ_PEND_IO_ISC_7;
> +       return pending_mask;
>   }
> 

Works with me! Send a stand alone patch?

Regards,
Halil

Re: [PATCH v6 12/13] KVM: s390: add gib_alert_irq_handler()

[Halil Pasic]

On Tue, 29 Jan 2019 16:29:40 +0100
Michael Mueller <mimu@linux.ibm.com> wrote:

> 
> 
> On 29.01.19 14:26, Halil Pasic wrote:
> > On Thu, 24 Jan 2019 13:59:38 +0100
> > Michael Mueller <mimu@linux.ibm.com> wrote:
> > 
> >> The patch implements a handler for GIB alert interruptions
> >> on the host. Its task is to alert guests that interrupts are
> >> pending for them.
> >>
> >> A GIB alert interrupt statistic counter is added as well:
> >>
> >> $ cat /proc/interrupts
> >>            CPU0       CPU1
> >>    ...
> >>    GAL:      23         37   [I/O] GIB Alert
> >>    ...
> >>
> >> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
> > [..]
> >> +/**
> >> + * gisa_get_ipm_or_restore_iam - return IPM or restore GISA IAM
> >> + *
> >> + * @gi: gisa interrupt struct to work on
> >> + *
> >> + * Atomically restores the interruption alert mask if none of the
> >> + * relevant ISCs are pending and return the IPM.
> > 
> > The word 'relevant' probably reflects some previous state. It does not
> > bother me too much.
> 
> "relevant" refers to the ISCs handled by the GAL mechanism, i.e those
> registered in the kvm->arch.gisa_int.alert.mask.

Sorry it was me who overlooked the & with the mask.

> > 
> > [..]
> > 
> >>   
> >> +static void __airqs_kick_single_vcpu(struct kvm *kvm, u8 deliverable_mask)
> >> +{
> >> +	int vcpu_id, online_vcpus = atomic_read(&kvm->online_vcpus);
> >> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> >> +	struct kvm_vcpu *vcpu;
> >> +
> >> +	for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
> >> +		vcpu = kvm_get_vcpu(kvm, vcpu_id);
> >> +		if (psw_ioint_disabled(vcpu))
> >> +			continue;
> >> +		deliverable_mask &= (u8)(vcpu->arch.sie_block->gcr[6] >> 24);
> >> +		if (deliverable_mask) {
> >> +			/* lately kicked but not yet running */
> > 
> > How about /* was kicked but didn't run yet */?
> 
> what is the difference here...

I read you comment like the vcpu is either not running yet or running.
However the vcpu could have went into sie processed the interrupt and
gone back to wait state: the bit in the kicked_mask would be clear
in this case, and we would do the right thing kick it again.

I'm not a grammar expert but that continuous does bother me. I may be
wrong.


> > [..]
> > 
> >> +static void process_gib_alert_list(void)
> >> +{
> >> +	struct kvm_s390_gisa_interrupt *gi;
> >> +	struct kvm_s390_gisa *gisa;
> >> +	struct kvm *kvm;
> >> +	u32 final, origin = 0UL;
> >> +
> >> +	do {
> >> +		/*
> >> +		 * If the NONE_GISA_ADDR is still stored in the alert list
> >> +		 * origin, we will leave the outer loop. No further GISA has
> >> +		 * been added to the alert list by millicode while processing
> >> +		 * the current alert list.
> >> +		 */
> >> +		final = (origin & NONE_GISA_ADDR);
> >> +		/*
> >> +		 * Cut off the alert list and store the NONE_GISA_ADDR in the
> >> +		 * alert list origin to avoid further GAL interruptions.
> >> +		 * A new alert list can be build up by millicode in parallel
> >> +		 * for guests not in the yet cut-off alert list. When in the
> >> +		 * final loop, store the NULL_GISA_ADDR instead. This will re-
> >> +		 * enable GAL interruptions on the host again.
> >> +		 */
> >> +		origin = xchg(&gib->alert_list_origin,
> >> +			      (!final) ? NONE_GISA_ADDR : NULL_GISA_ADDR);
> >> +		/*
> >> +		 * Loop through the just cut-off alert list and start the
> >> +		 * gisa timers to kick idle vcpus to consume the pending
> >> +		 * interruptions asap.
> >> +		 */
> >> +		while (origin & GISA_ADDR_MASK) {
> >> +			gisa = (struct kvm_s390_gisa *)(u64)origin;
> >> +			origin = gisa->next_alert;
> >> +			gisa->next_alert = (u32)(u64)gisa;
> >> +			kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
> >> +			gi = &kvm->arch.gisa_int;
> >> +			if (hrtimer_active(&gi->timer))
> >> +				hrtimer_cancel(&gi->timer);
> >> +			hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
> >> +		}
> >> +	} while (!final);
> >> +
> >> +}
> >> +
> >>   void kvm_s390_gisa_clear(struct kvm *kvm)
> >>   {
> >>   	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> >>   
> >>   	if (!gi->origin)
> >>   		return;
> >> -	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
> >> -	gi->origin->next_alert = (u32)(u64)gi->origin;
> >> +	gisa_clear_ipm(gi->origin);
> > 
> > This could be a separate patch. I would like little more explanation
> > to this.

nice

> 
> I can break at out as I had before... ;)
> 
> > 
> >>   	VM_EVENT(kvm, 3, "gisa 0x%pK cleared", gi->origin);
> >>   }
> >>   
> >> @@ -2940,13 +3078,25 @@ void kvm_s390_gisa_init(struct kvm *kvm)
> >>   	gi->origin = &kvm->arch.sie_page2->gisa;
> >>   	gi->alert.mask = 0;
> >>   	spin_lock_init(&gi->alert.ref_lock);
> >> -	kvm_s390_gisa_clear(kvm);
> >> +	gi->expires = 50 * 1000; /* 50 usec */
> > 
> > I blindly trust your choice here ;)
> 
> You know I will increase it to 1 ms together with the change that I
> proposed. (gisa_get_ipm_or_restore_iam() in kvm_s390_handle_wait()).
> 

Is probably OK with just one gsic registered. I will think about
it a bit more.

> > 
> >> +	hrtimer_init(&gi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> >> +	gi->timer.function = gisa_vcpu_kicker;
> >> +	memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
> >> +	gi->origin->next_alert = (u32)(u64)gi->origin;
> >>   	VM_EVENT(kvm, 3, "gisa 0x%pK initialized", gi->origin);
> >>   }
> >>   
> >>   void kvm_s390_gisa_destroy(struct kvm *kvm)
> >>   {
> >> -	kvm->arch.gisa_int.origin = NULL;
> >> +	struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> >> +
> >> +	if (!gi->origin)
> >> +		return;
> >> +	hrtimer_cancel(&gi->timer);
> > 
> > I'm not sure this cancel here is sufficient.
> > 
> >> +	WRITE_ONCE(gi->alert.mask, 0);
> >> +	while (gisa_in_alert_list(gi->origin))
> >> +		cpu_relax();
> > 
> > If you end up waiting here, I guess, it's likely that a new
> > timer is going to get set up right after we do
> > gisa->next_alert = (u32)(u64)gisa;
> > in  process_gib_alert_list().
> 
> There will be no vcpus available anymore at this time, whence
> none will be kicked by the timer function. Thus canceling the
> timer will be sufficient after the loop.
> 

Hm I'm not 100% convinced this is race free. I guess, I simply
don't understand enough of the tear-down. I don't want to delay
the series because of this. If the last interrupt arrived kind of
long ago we should be fine -- probably. Keep my ack ;)

> I have addressed the message as well, but will write it into
> the KVM trace.
> 
>   void kvm_s390_gisa_destroy(struct kvm *kvm)
>   {
> -       kvm->arch.gisa_int.origin = NULL;
> +       struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
> +
> +       if (!gi->origin)
> +               return;
> +       if (gi->alert.mask)
> +               KVM_EVENT(3, "vm 0x%pK has unexpected iam 0x%02x",
> +                         kvm, gi->alert.mask);
> +       while (gisa_in_alert_list(gi->origin))
> +               cpu_relax();
> +       hrtimer_cancel(&gi->timer);
> +       gi->origin = NULL;
>   }
> 
> > 
> >> +	gi->origin = NULL;
> >>   }
> >>   
> >>   /**
> >> @@ -3037,11 +3187,23 @@ int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc)
> >>   }
> >>   EXPORT_SYMBOL_GPL(kvm_s390_gisc_unregister);
> >>   
> > 
> > 
> > Overall, there are couple of things I would prefer done differently,
> > but better something working today that something prefect in 6 months.
> > In that sense, provided my comment regarding destroy is addressed:
> > 
> > Acked-by: Halil Pasic <pasic@linux.ibm.com>
> > 
> 
> Michael

Re: [PATCH v6 12/13] KVM: s390: add gib_alert_irq_handler()

[Pierre Morel]

On 29/01/2019 16:29, Michael Mueller wrote:
> 
> 
> On 29.01.19 14:26, Halil Pasic wrote:
>> On Thu, 24 Jan 2019 13:59:38 +0100
>> Michael Mueller <mimu@linux.ibm.com> wrote:
>>
>>> The patch implements a handler for GIB alert interruptions
>>> on the host. Its task is to alert guests that interrupts are
>>> pending for them.
>>>
>>> A GIB alert interrupt statistic counter is added as well:
>>>
>>> $ cat /proc/interrupts
>>>            CPU0       CPU1
>>>    ...
>>>    GAL:      23         37   [I/O] GIB Alert
>>>    ...
>>>
>>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
>> [..]
>>> +/**
>>> + * gisa_get_ipm_or_restore_iam - return IPM or restore GISA IAM
>>> + *
>>> + * @gi: gisa interrupt struct to work on
>>> + *
>>> + * Atomically restores the interruption alert mask if none of the
>>> + * relevant ISCs are pending and return the IPM.
>>
>> The word 'relevant' probably reflects some previous state. It does not
>> bother me too much.
> 
> "relevant" refers to the ISCs handled by the GAL mechanism, i.e those
> registered in the kvm->arch.gisa_int.alert.mask.
> 
>>
>> [..]
>>
>>> +static void __airqs_kick_single_vcpu(struct kvm *kvm, u8 
>>> deliverable_mask)
>>> +{
>>> +    int vcpu_id, online_vcpus = atomic_read(&kvm->online_vcpus);
>>> +    struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>>> +    struct kvm_vcpu *vcpu;
>>> +
>>> +    for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
>>> +        vcpu = kvm_get_vcpu(kvm, vcpu_id);
>>> +        if (psw_ioint_disabled(vcpu))
>>> +            continue;
>>> +        deliverable_mask &= (u8)(vcpu->arch.sie_block->gcr[6] >> 24);
>>> +        if (deliverable_mask) {
>>> +            /* lately kicked but not yet running */
>>
>> How about /* was kicked but didn't run yet */?
> 
> what is the difference here...
> 
>>
>>> +            if (test_and_set_bit(vcpu_id, gi->kicked_mask))
>>> +                return;
>>> +            kvm_s390_vcpu_wakeup(vcpu);
>>> +            return;
>>> +        }
>>> +    }
>>> +}
>>> +
>>
>> [..]
>>
>>> +static void process_gib_alert_list(void)
>>> +{
>>> +    struct kvm_s390_gisa_interrupt *gi;
>>> +    struct kvm_s390_gisa *gisa;
>>> +    struct kvm *kvm;
>>> +    u32 final, origin = 0UL;
>>> +
>>> +    do {
>>> +        /*
>>> +         * If the NONE_GISA_ADDR is still stored in the alert list
>>> +         * origin, we will leave the outer loop. No further GISA has
>>> +         * been added to the alert list by millicode while processing
>>> +         * the current alert list.
>>> +         */
>>> +        final = (origin & NONE_GISA_ADDR);
>>> +        /*
>>> +         * Cut off the alert list and store the NONE_GISA_ADDR in the
>>> +         * alert list origin to avoid further GAL interruptions.
>>> +         * A new alert list can be build up by millicode in parallel
>>> +         * for guests not in the yet cut-off alert list. When in the
>>> +         * final loop, store the NULL_GISA_ADDR instead. This will re-
>>> +         * enable GAL interruptions on the host again.
>>> +         */
>>> +        origin = xchg(&gib->alert_list_origin,
>>> +                  (!final) ? NONE_GISA_ADDR : NULL_GISA_ADDR);
>>> +        /*
>>> +         * Loop through the just cut-off alert list and start the
>>> +         * gisa timers to kick idle vcpus to consume the pending
>>> +         * interruptions asap.
>>> +         */
>>> +        while (origin & GISA_ADDR_MASK) {
>>> +            gisa = (struct kvm_s390_gisa *)(u64)origin;
>>> +            origin = gisa->next_alert;
>>> +            gisa->next_alert = (u32)(u64)gisa;
>>> +            kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
>>> +            gi = &kvm->arch.gisa_int;
>>> +            if (hrtimer_active(&gi->timer))
>>> +                hrtimer_cancel(&gi->timer);
>>> +            hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
>>> +        }
>>> +    } while (!final);
>>> +
>>> +}
>>> +
>>>   void kvm_s390_gisa_clear(struct kvm *kvm)
>>>   {
>>>       struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>>>       if (!gi->origin)
>>>           return;
>>> -    memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
>>> -    gi->origin->next_alert = (u32)(u64)gi->origin;
>>> +    gisa_clear_ipm(gi->origin);
>>
>> This could be a separate patch. I would like little more explanation
>> to this.
> 
> I can break at out as I had before... ;)
> 
>>
>>>       VM_EVENT(kvm, 3, "gisa 0x%pK cleared", gi->origin);
>>>   }
>>> @@ -2940,13 +3078,25 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>>>       gi->origin = &kvm->arch.sie_page2->gisa;
>>>       gi->alert.mask = 0;
>>>       spin_lock_init(&gi->alert.ref_lock);
>>> -    kvm_s390_gisa_clear(kvm);
>>> +    gi->expires = 50 * 1000; /* 50 usec */
>>
>> I blindly trust your choice here ;)
> 
> You know I will increase it to 1 ms together with the change that I
> proposed. (gisa_get_ipm_or_restore_iam() in kvm_s390_handle_wait()).

With this.
Reviewed-by: Pierre Morel<pmorel@linux.ibm.com>



-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

Re: [PATCH v6 12/13] KVM: s390: add gib_alert_irq_handler()

[Michael Mueller]

On 30.01.19 17:24, Pierre Morel wrote:
> On 29/01/2019 16:29, Michael Mueller wrote:
>>
>>
>> On 29.01.19 14:26, Halil Pasic wrote:
>>> On Thu, 24 Jan 2019 13:59:38 +0100
>>> Michael Mueller <mimu@linux.ibm.com> wrote:
>>>
>>>> The patch implements a handler for GIB alert interruptions
>>>> on the host. Its task is to alert guests that interrupts are
>>>> pending for them.
>>>>
>>>> A GIB alert interrupt statistic counter is added as well:
>>>>
>>>> $ cat /proc/interrupts
>>>>            CPU0       CPU1
>>>>    ...
>>>>    GAL:      23         37   [I/O] GIB Alert
>>>>    ...
>>>>
>>>> Signed-off-by: Michael Mueller <mimu@linux.ibm.com>
>>> [..]
>>>> +/**
>>>> + * gisa_get_ipm_or_restore_iam - return IPM or restore GISA IAM
>>>> + *
>>>> + * @gi: gisa interrupt struct to work on
>>>> + *
>>>> + * Atomically restores the interruption alert mask if none of the
>>>> + * relevant ISCs are pending and return the IPM.
>>>
>>> The word 'relevant' probably reflects some previous state. It does not
>>> bother me too much.
>>
>> "relevant" refers to the ISCs handled by the GAL mechanism, i.e those
>> registered in the kvm->arch.gisa_int.alert.mask.
>>
>>>
>>> [..]
>>>
>>>> +static void __airqs_kick_single_vcpu(struct kvm *kvm, u8 
>>>> deliverable_mask)
>>>> +{
>>>> +    int vcpu_id, online_vcpus = atomic_read(&kvm->online_vcpus);
>>>> +    struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>>>> +    struct kvm_vcpu *vcpu;
>>>> +
>>>> +    for_each_set_bit(vcpu_id, kvm->arch.idle_mask, online_vcpus) {
>>>> +        vcpu = kvm_get_vcpu(kvm, vcpu_id);
>>>> +        if (psw_ioint_disabled(vcpu))
>>>> +            continue;
>>>> +        deliverable_mask &= (u8)(vcpu->arch.sie_block->gcr[6] >> 24);
>>>> +        if (deliverable_mask) {
>>>> +            /* lately kicked but not yet running */
>>>
>>> How about /* was kicked but didn't run yet */?
>>
>> what is the difference here...
>>
>>>
>>>> +            if (test_and_set_bit(vcpu_id, gi->kicked_mask))
>>>> +                return;
>>>> +            kvm_s390_vcpu_wakeup(vcpu);
>>>> +            return;
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>
>>> [..]
>>>
>>>> +static void process_gib_alert_list(void)
>>>> +{
>>>> +    struct kvm_s390_gisa_interrupt *gi;
>>>> +    struct kvm_s390_gisa *gisa;
>>>> +    struct kvm *kvm;
>>>> +    u32 final, origin = 0UL;
>>>> +
>>>> +    do {
>>>> +        /*
>>>> +         * If the NONE_GISA_ADDR is still stored in the alert list
>>>> +         * origin, we will leave the outer loop. No further GISA has
>>>> +         * been added to the alert list by millicode while processing
>>>> +         * the current alert list.
>>>> +         */
>>>> +        final = (origin & NONE_GISA_ADDR);
>>>> +        /*
>>>> +         * Cut off the alert list and store the NONE_GISA_ADDR in the
>>>> +         * alert list origin to avoid further GAL interruptions.
>>>> +         * A new alert list can be build up by millicode in parallel
>>>> +         * for guests not in the yet cut-off alert list. When in the
>>>> +         * final loop, store the NULL_GISA_ADDR instead. This will re-
>>>> +         * enable GAL interruptions on the host again.
>>>> +         */
>>>> +        origin = xchg(&gib->alert_list_origin,
>>>> +                  (!final) ? NONE_GISA_ADDR : NULL_GISA_ADDR);
>>>> +        /*
>>>> +         * Loop through the just cut-off alert list and start the
>>>> +         * gisa timers to kick idle vcpus to consume the pending
>>>> +         * interruptions asap.
>>>> +         */
>>>> +        while (origin & GISA_ADDR_MASK) {
>>>> +            gisa = (struct kvm_s390_gisa *)(u64)origin;
>>>> +            origin = gisa->next_alert;
>>>> +            gisa->next_alert = (u32)(u64)gisa;
>>>> +            kvm = container_of(gisa, struct sie_page2, gisa)->kvm;
>>>> +            gi = &kvm->arch.gisa_int;
>>>> +            if (hrtimer_active(&gi->timer))
>>>> +                hrtimer_cancel(&gi->timer);
>>>> +            hrtimer_start(&gi->timer, 0, HRTIMER_MODE_REL);
>>>> +        }
>>>> +    } while (!final);
>>>> +
>>>> +}
>>>> +
>>>>   void kvm_s390_gisa_clear(struct kvm *kvm)
>>>>   {
>>>>       struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int;
>>>>       if (!gi->origin)
>>>>           return;
>>>> -    memset(gi->origin, 0, sizeof(struct kvm_s390_gisa));
>>>> -    gi->origin->next_alert = (u32)(u64)gi->origin;
>>>> +    gisa_clear_ipm(gi->origin);
>>>
>>> This could be a separate patch. I would like little more explanation
>>> to this.
>>
>> I can break at out as I had before... ;)
>>
>>>
>>>>       VM_EVENT(kvm, 3, "gisa 0x%pK cleared", gi->origin);
>>>>   }
>>>> @@ -2940,13 +3078,25 @@ void kvm_s390_gisa_init(struct kvm *kvm)
>>>>       gi->origin = &kvm->arch.sie_page2->gisa;
>>>>       gi->alert.mask = 0;
>>>>       spin_lock_init(&gi->alert.ref_lock);
>>>> -    kvm_s390_gisa_clear(kvm);
>>>> +    gi->expires = 50 * 1000; /* 50 usec */
>>>
>>> I blindly trust your choice here ;)
>>
>> You know I will increase it to 1 ms together with the change that I
>> proposed. (gisa_get_ipm_or_restore_iam() in kvm_s390_handle_wait()).
> 
> With this.
> Reviewed-by: Pierre Morel<pmorel@linux.ibm.com>

Pierre,

please see my mail with the measurements that I have done. Up to that
I can't take your Reviewed-by. I will keep the 50 usec.

Michael

> 
> 
>