[PATCH 0/2] KVM: s390: interrupt: Fix stepping into interrupt handlers

[PATCH 0/2] KVM: s390: interrupt: Fix stepping into interrupt handlers

[Ilya Leoshkevich]

Hi,

I tried to compare the behavior of KVM and TCG by diffing instruction
traces, and found two issues in KVM related to stepping into interrupt
handlers.

I'm not very familiar with the KVM code base, so please let me know if
the fixes can be improved or if these problems need to be handled
completely differently.

Best regards,
Ilya

Ilya Leoshkevich (2):
  KVM: s390: interrupt: Fix stepping into interrupt handlers
  KVM: s390: interrupt: Fix stepping into program interrupt handlers

 arch/s390/kvm/intercept.c | 19 +++++++++++++++++--
 arch/s390/kvm/interrupt.c | 10 ++++++++++
 arch/s390/kvm/kvm-s390.c  |  4 ++--
 3 files changed, 29 insertions(+), 4 deletions(-)

-- 
2.41.0

[PATCH 1/2] KVM: s390: interrupt: Fix stepping into interrupt handlers

[Ilya Leoshkevich]

Currently, after single-stepping an instruction that causes a program
interrupt, GDB ends up on the second instruction of the program
interrupt handler.

The reason is that vcpu_pre_run() manually delivers the program
interrupt, and then __vcpu_run() runs the first handler instruction
using the CPUSTAT_P flag. This causes a KVM_SINGLESTEP exit on the
second handler instruction.

Instead, KVM needs to do a KVM_SINGLESTEP exit after the manual
interrupt delivery.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 arch/s390/kvm/interrupt.c | 10 ++++++++++
 arch/s390/kvm/kvm-s390.c  |  4 ++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 9250fde1f97d..e633ec4f98d8 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1392,6 +1392,7 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu)
 {
 	struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int;
 	int rc = 0;
+	bool delivered = false;
 	unsigned long irq_type;
 	unsigned long irqs;
 
@@ -1465,6 +1466,15 @@ int __must_check kvm_s390_deliver_pending_interrupts(struct kvm_vcpu *vcpu)
 			WARN_ONCE(1, "Unknown pending irq type %ld", irq_type);
 			clear_bit(irq_type, &li->pending_irqs);
 		}
+		delivered |= !rc;
+	}
+
+	if (delivered && guestdbg_sstep_enabled(vcpu)) {
+		struct kvm_debug_exit_arch *debug_exit = &vcpu->run->debug.arch;
+
+		debug_exit->addr = vcpu->arch.sie_block->gpsw.addr;
+		debug_exit->type = KVM_SINGLESTEP;
+		vcpu->guest_debug |= KVM_GUESTDBG_EXIT_PENDING;
 	}
 
 	set_intercept_indicators(vcpu);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 39b36562c043..2d66d83b682c 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4607,7 +4607,7 @@ static int vcpu_pre_run(struct kvm_vcpu *vcpu)
 
 	if (!kvm_is_ucontrol(vcpu->kvm)) {
 		rc = kvm_s390_deliver_pending_interrupts(vcpu);
-		if (rc)
+		if (rc || guestdbg_exit_pending(vcpu))
 			return rc;
 	}
 
@@ -4734,7 +4734,7 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
 
 	do {
 		rc = vcpu_pre_run(vcpu);
-		if (rc)
+		if (rc || guestdbg_exit_pending(vcpu))
 			break;
 
 		kvm_vcpu_srcu_read_unlock(vcpu);
-- 
2.41.0

[PATCH 2/2] KVM: s390: interrupt: Fix stepping into program interrupt handlers

[Ilya Leoshkevich]

Currently, after single-stepping an instruction that causes a
specification exception, GDB ends up on the instruction immediately
following it.

The reason is that vcpu_post_run() injects the interrupt and sets
KVM_GUESTDBG_EXIT_PENDING, causing a KVM_SINGLESTEP exit. The
interrupt is not delivered, however, therefore userspace sees the
address of the next instruction.

Fix by letting the __vcpu_run() loop go into the next iteration,
where vcpu_pre_run() delivers the interrupt and sets
KVM_GUESTDBG_EXIT_PENDING.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 arch/s390/kvm/intercept.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index 0ee02dae14b2..f8adb64ebda1 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -226,7 +226,22 @@ static int handle_itdb(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-#define per_event(vcpu) (vcpu->arch.sie_block->iprcc & PGM_PER)
+static bool should_handle_per_event(struct kvm_vcpu *vcpu)
+{
+	if (!guestdbg_enabled(vcpu))
+		return false;
+	if (!(vcpu->arch.sie_block->iprcc & PGM_PER))
+		return false;
+	if (guestdbg_sstep_enabled(vcpu) &&
+	    vcpu->arch.sie_block->iprcc != PGM_PER) {
+		/*
+		 * __vcpu_run() will exit after delivering the concurrently
+		 * indicated condition.
+		 */
+		return false;
+	}
+	return true;
+}
 
 static int handle_prog(struct kvm_vcpu *vcpu)
 {
@@ -242,7 +257,7 @@ static int handle_prog(struct kvm_vcpu *vcpu)
 	if (kvm_s390_pv_cpu_is_protected(vcpu))
 		return -EOPNOTSUPP;
 
-	if (guestdbg_enabled(vcpu) && per_event(vcpu)) {
+	if (should_handle_per_event(vcpu)) {
 		rc = kvm_s390_handle_per_event(vcpu);
 		if (rc)
 			return rc;
-- 
2.41.0