On Thu, 24 Jan 2019 13:59:38 +0100
Michael Mueller <mimu@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
+/**
+ * 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@xxxxxxxxxxxxx>