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@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.
"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@xxxxxxxxxxxxx>
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
