Re: [PATCH v4 09/10] KVM: s390: add and wire function gib_alert_irq_handler()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 */




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux