Am 19.10.21 um 23:35 schrieb Christian Borntraeger:
Am 19.10.21 um 19:54 schrieb Halil Pasic:
The idea behind kicked mask is that we should not re-kick a vcpu
from __airqs_kick_single_vcpu() that is already in the middle of
being kicked by the same function.
If however the vcpu that was idle before when the idle_mask was
examined, is not idle any more after the kicked_mask is set, that
means that we don't need to kick, and that we need to clear the
bit we just set because we may be beyond the point where it would
get cleared in the wake-up process. Since the time window is short,
this is probably more a theoretical than a practical thing: the race
window is small.
To get things harmonized let us also move the clear from vcpu_pre_run()
to __unset_cpu_idle().
this part makes sense.
Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
Fixes: 9f30f6216378 ("KVM: s390: add gib_alert_irq_handler()")
---
arch/s390/kvm/interrupt.c | 7 ++++++-
arch/s390/kvm/kvm-s390.c | 2 --
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 2245f4b8d362..3c80a2237ef5 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -426,6 +426,7 @@ static void __unset_cpu_idle(struct kvm_vcpu *vcpu)
{
kvm_s390_clear_cpuflags(vcpu, CPUSTAT_WAIT);
clear_bit(vcpu->vcpu_idx, vcpu->kvm->arch.idle_mask);
+ clear_bit(vcpu->vcpu_idx, vcpu->kvm->arch.gisa_int.kicked_mask);
}
static void __reset_intercept_indicators(struct kvm_vcpu *vcpu)
@@ -3064,7 +3065,11 @@ static void __airqs_kick_single_vcpu(struct kvm *kvm, u8 deliverable_mask)
/* lately kicked but not yet running */
if (test_and_set_bit(vcpu_idx, gi->kicked_mask))
return;
- kvm_s390_vcpu_wakeup(vcpu);
+ /* if meanwhile not idle: clear and don't kick */
+ if (test_bit(vcpu_idx, kvm->arch.idle_mask))
+ kvm_s390_vcpu_wakeup(vcpu);
+ else
+ clear_bit(vcpu_idx, gi->kicked_mask);
I think this is now a bug. We should not return but continue in that case, no?
Thinking again about this, it might be ok. If we went from idle to non-idle we
likely were in SIE and the interrupt should have been delivered. But I would rather
wake up too often than too less.
I think it might be safer to also clear kicked_mask in __set_cpu_idle
From a CPUs perspective: We have been running and are on our way to become idle.
There is no way that someone kicked us for a wakeup. In other words as long as we
are running, there is no point in kicking us but when going idle we should get rid
of old kick_mask bit.
Doesnt this cover your scenario?
return;
}
}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 1c97493d21e1..6b779ef9f5fb 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4067,8 +4067,6 @@ static int vcpu_pre_run(struct kvm_vcpu *vcpu)
kvm_s390_patch_guest_per_regs(vcpu);
}
- clear_bit(vcpu->vcpu_idx, 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);
