On Fri, 30 Nov 2018 15:32:14 +0100
Michael Mueller <mimu@xxxxxxxxxxxxx> wrote:
> The patch implements a handler for GIB alert interruptions
> on the host. Its task is to alert storage backed 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>
> ---
[..]
> @@ -1099,6 +1100,17 @@ int kvm_s390_handle_wait(struct kvm_vcpu *vcpu)
> if (kvm_arch_vcpu_runnable(vcpu))
> return 0;
>
> + /*
> + * This could be the single vcpu of the guest or the last vcpu
> + * open for I/O interruptons going into wait state.
> + */
> + if (vcpu->kvm->arch.gib_in_use &&
> + !in_alert_list(vcpu->kvm->arch.gisa)) {
> + if (unlikely(kvm_s390_gisa_get_ipm(vcpu->kvm->arch.gisa)))
> + return 0;
I don't agree with this condition. If I read this correctly, this makes
the vcpu go back into SIE if we have GISA interrupts pending. This can
basically happen in two ways. First way: we got a new bit in the IPM set
between the call to kvm_arch_vcpu_runnable() and here, and there is a
hope that the newly set (ISC) is not masked. Second way: there is no new
bit in IPM since kvm_arch_vcpu_runnable(). In this case we are certain
that the vCPU can not take the GISA interrupts pending. Second way seems
more likely to happen to me.
> + vcpu->kvm->arch.gisa->iam = vcpu->kvm->arch.iam;
Even without the condition above, I'm struggling with understanding your
algorithm that controls alerting. For instance, I don't quite understand
the !in_alert_list() condition either -- I'm under the impression this
is the only place where gisa->iam is set to non-zero. Maybe explaining
are the invariants you seek to maintain would help me getting it.
Regards,
Halil
> + }
> +
> if (psw_interrupts_disabled(vcpu)) {
> VCPU_EVENT(vcpu, 3, "%s", "disabled wait");
> return -EOPNOTSUPP; /* disabled wait */
