* [PATCH v3 0/1] KVM: s390: pv: fix external interruption loop not always detected @ 2023-02-13 8:55 Nico Boehr 2023-02-13 8:55 ` [PATCH v3 1/1] " Nico Boehr 0 siblings, 1 reply; 6+ messages in thread From: Nico Boehr @ 2023-02-13 8:55 UTC (permalink / raw) To: borntraeger, frankja, imbrenda; +Cc: kvm, linux-s390 v3: --- * fix some checkpatch warnings To determine whether the guest has caused an external interruption loop upon code 20 (external interrupt) intercepts, the ext_new_psw needs to be inspected to see whether external interrupts are enabled. Under non-PV, ext_new_psw can simply be taken from guest lowcore. Under PV, KVM can only access the encrypted guest lowcore and hence the ext_new_psw must not be taken from guest lowcore. handle_external_interrupt() incorrectly did that and hence was not able to reliably tell whether an external interruption loop is happening or not. False negatives cause spurious failures of my kvm-unit-test for extint loops[1] under PV. Since code 20 is only caused under PV if and only if the guest's ext_new_psw is enabled for external interrupts, false positive detection of a external interruption loop can not happen. Fix this issue by instead looking at the guest PSW in the state description. Since the PSW swap for external interrupt is done by the ultravisor before the intercept is caused, this reliably tells whether the guest is enabled for external interrupts in the ext_new_psw. Also update the comments to explain better what is happening. [1] https://lore.kernel.org/kvm/20220812062151.1980937-4-nrb@linux.ibm.com/ Nico Boehr (1): KVM: s390: pv: fix external interruption loop not always detected arch/s390/kvm/intercept.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) -- 2.39.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] KVM: s390: pv: fix external interruption loop not always detected 2023-02-13 8:55 [PATCH v3 0/1] KVM: s390: pv: fix external interruption loop not always detected Nico Boehr @ 2023-02-13 8:55 ` Nico Boehr 2023-02-13 9:06 ` Christian Borntraeger 2023-02-13 9:21 ` Janosch Frank 0 siblings, 2 replies; 6+ messages in thread From: Nico Boehr @ 2023-02-13 8:55 UTC (permalink / raw) To: borntraeger, frankja, imbrenda; +Cc: kvm, linux-s390 To determine whether the guest has caused an external interruption loop upon code 20 (external interrupt) intercepts, the ext_new_psw needs to be inspected to see whether external interrupts are enabled. Under non-PV, ext_new_psw can simply be taken from guest lowcore. Under PV, KVM can only access the encrypted guest lowcore and hence the ext_new_psw must not be taken from guest lowcore. handle_external_interrupt() incorrectly did that and hence was not able to reliably tell whether an external interruption loop is happening or not. False negatives cause spurious failures of my kvm-unit-test for extint loops[1] under PV. Since code 20 is only caused under PV if and only if the guest's ext_new_psw is enabled for external interrupts, false positive detection of a external interruption loop can not happen. Fix this issue by instead looking at the guest PSW in the state description. Since the PSW swap for external interrupt is done by the ultravisor before the intercept is caused, this reliably tells whether the guest is enabled for external interrupts in the ext_new_psw. Also update the comments to explain better what is happening. [1] https://lore.kernel.org/kvm/20220812062151.1980937-4-nrb@linux.ibm.com/ Signed-off-by: Nico Boehr <nrb@linux.ibm.com> --- arch/s390/kvm/intercept.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c index 0ee02dae14b2..2cda8d9d7c6e 100644 --- a/arch/s390/kvm/intercept.c +++ b/arch/s390/kvm/intercept.c @@ -271,10 +271,18 @@ static int handle_prog(struct kvm_vcpu *vcpu) * handle_external_interrupt - used for external interruption interceptions * @vcpu: virtual cpu * - * This interception only occurs if the CPUSTAT_EXT_INT bit was set, or if - * the new PSW does not have external interrupts disabled. In the first case, - * we've got to deliver the interrupt manually, and in the second case, we - * drop to userspace to handle the situation there. + * This interception occurs if: + * - the CPUSTAT_EXT_INT bit was already set when the external interrupt + * occurred. In this case, the interrupt needs to be injected manually to + * preserve interrupt priority. + * - the external new PSW has external interrupts enabled, which will cause an + * interruption loop. We drop to userspace in this case. + * + * The latter case can be detected by inspecting the external mask bit in the + * external new psw. + * + * Under PV, only the latter case can occur, since interrupt priorities are + * handled in the ultravisor. */ static int handle_external_interrupt(struct kvm_vcpu *vcpu) { @@ -285,10 +293,18 @@ static int handle_external_interrupt(struct kvm_vcpu *vcpu) vcpu->stat.exit_external_interrupt++; - rc = read_guest_lc(vcpu, __LC_EXT_NEW_PSW, &newpsw, sizeof(psw_t)); - if (rc) - return rc; - /* We can not handle clock comparator or timer interrupt with bad PSW */ + if (kvm_s390_pv_cpu_is_protected(vcpu)) { + newpsw = vcpu->arch.sie_block->gpsw; + } else { + rc = read_guest_lc(vcpu, __LC_EXT_NEW_PSW, &newpsw, sizeof(psw_t)); + if (rc) + return rc; + } + + /* + * Clock comparator or timer interrupt with external interrupt enabled + * will cause interrupt loop. Drop to userspace. + */ if ((eic == EXT_IRQ_CLK_COMP || eic == EXT_IRQ_CPU_TIMER) && (newpsw.mask & PSW_MASK_EXT)) return -EOPNOTSUPP; -- 2.39.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] KVM: s390: pv: fix external interruption loop not always detected 2023-02-13 8:55 ` [PATCH v3 1/1] " Nico Boehr @ 2023-02-13 9:06 ` Christian Borntraeger 2023-02-13 9:21 ` Janosch Frank 1 sibling, 0 replies; 6+ messages in thread From: Christian Borntraeger @ 2023-02-13 9:06 UTC (permalink / raw) To: Nico Boehr, frankja, imbrenda; +Cc: kvm, linux-s390 Am 13.02.23 um 09:55 schrieb Nico Boehr: > To determine whether the guest has caused an external interruption loop > upon code 20 (external interrupt) intercepts, the ext_new_psw needs to > be inspected to see whether external interrupts are enabled. > > Under non-PV, ext_new_psw can simply be taken from guest lowcore. Under > PV, KVM can only access the encrypted guest lowcore and hence the > ext_new_psw must not be taken from guest lowcore. > > handle_external_interrupt() incorrectly did that and hence was not able > to reliably tell whether an external interruption loop is happening or > not. False negatives cause spurious failures of my kvm-unit-test > for extint loops[1] under PV. > > Since code 20 is only caused under PV if and only if the guest's > ext_new_psw is enabled for external interrupts, false positive detection > of a external interruption loop can not happen. > > Fix this issue by instead looking at the guest PSW in the state > description. Since the PSW swap for external interrupt is done by the > ultravisor before the intercept is caused, this reliably tells whether > the guest is enabled for external interrupts in the ext_new_psw. > > Also update the comments to explain better what is happening. > > [1] https://lore.kernel.org/kvm/20220812062151.1980937-4-nrb@linux.ibm.com/ > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> > --- > arch/s390/kvm/intercept.c | 32 ++++++++++++++++++++++++-------- > 1 file changed, 24 insertions(+), 8 deletions(-) > > diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c > index 0ee02dae14b2..2cda8d9d7c6e 100644 > --- a/arch/s390/kvm/intercept.c > +++ b/arch/s390/kvm/intercept.c > @@ -271,10 +271,18 @@ static int handle_prog(struct kvm_vcpu *vcpu) > * handle_external_interrupt - used for external interruption interceptions > * @vcpu: virtual cpu > * > - * This interception only occurs if the CPUSTAT_EXT_INT bit was set, or if > - * the new PSW does not have external interrupts disabled. In the first case, > - * we've got to deliver the interrupt manually, and in the second case, we > - * drop to userspace to handle the situation there. > + * This interception occurs if: > + * - the CPUSTAT_EXT_INT bit was already set when the external interrupt > + * occurred. In this case, the interrupt needs to be injected manually to > + * preserve interrupt priority. > + * - the external new PSW has external interrupts enabled, which will cause an > + * interruption loop. We drop to userspace in this case. > + * > + * The latter case can be detected by inspecting the external mask bit in the > + * external new psw. > + * > + * Under PV, only the latter case can occur, since interrupt priorities are > + * handled in the ultravisor. > */ > static int handle_external_interrupt(struct kvm_vcpu *vcpu) > { > @@ -285,10 +293,18 @@ static int handle_external_interrupt(struct kvm_vcpu *vcpu) > > vcpu->stat.exit_external_interrupt++; > > - rc = read_guest_lc(vcpu, __LC_EXT_NEW_PSW, &newpsw, sizeof(psw_t)); > - if (rc) > - return rc; > - /* We can not handle clock comparator or timer interrupt with bad PSW */ > + if (kvm_s390_pv_cpu_is_protected(vcpu)) { > + newpsw = vcpu->arch.sie_block->gpsw; > + } else { > + rc = read_guest_lc(vcpu, __LC_EXT_NEW_PSW, &newpsw, sizeof(psw_t)); > + if (rc) > + return rc; > + } > + > + /* > + * Clock comparator or timer interrupt with external interrupt enabled > + * will cause interrupt loop. Drop to userspace. > + */ > if ((eic == EXT_IRQ_CLK_COMP || eic == EXT_IRQ_CPU_TIMER) && > (newpsw.mask & PSW_MASK_EXT)) > return -EOPNOTSUPP; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] KVM: s390: pv: fix external interruption loop not always detected 2023-02-13 8:55 ` [PATCH v3 1/1] " Nico Boehr 2023-02-13 9:06 ` Christian Borntraeger @ 2023-02-13 9:21 ` Janosch Frank 2023-02-13 12:00 ` Janosch Frank 1 sibling, 1 reply; 6+ messages in thread From: Janosch Frank @ 2023-02-13 9:21 UTC (permalink / raw) To: Nico Boehr, borntraeger, imbrenda; +Cc: kvm, linux-s390 On 2/13/23 09:55, Nico Boehr wrote: > To determine whether the guest has caused an external interruption loop > upon code 20 (external interrupt) intercepts, the ext_new_psw needs to > be inspected to see whether external interrupts are enabled. > > Under non-PV, ext_new_psw can simply be taken from guest lowcore. Under > PV, KVM can only access the encrypted guest lowcore and hence the > ext_new_psw must not be taken from guest lowcore. > > handle_external_interrupt() incorrectly did that and hence was not able > to reliably tell whether an external interruption loop is happening or > not. False negatives cause spurious failures of my kvm-unit-test > for extint loops[1] under PV. > > Since code 20 is only caused under PV if and only if the guest's > ext_new_psw is enabled for external interrupts, false positive detection > of a external interruption loop can not happen. > > Fix this issue by instead looking at the guest PSW in the state > description. Since the PSW swap for external interrupt is done by the > ultravisor before the intercept is caused, this reliably tells whether > the guest is enabled for external interrupts in the ext_new_psw. > > Also update the comments to explain better what is happening. > > [1] https://lore.kernel.org/kvm/20220812062151.1980937-4-nrb@linux.ibm.com/ > > Signed-off-by: Nico Boehr <nrb@linux.ibm.com> > --- Reviewed-by: Janosch Frank <frankja@linux.ibm.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] KVM: s390: pv: fix external interruption loop not always detected 2023-02-13 9:21 ` Janosch Frank @ 2023-02-13 12:00 ` Janosch Frank 2023-02-13 13:02 ` Nico Boehr 0 siblings, 1 reply; 6+ messages in thread From: Janosch Frank @ 2023-02-13 12:00 UTC (permalink / raw) To: Nico Boehr, borntraeger, imbrenda; +Cc: kvm, linux-s390 On 2/13/23 10:21, Janosch Frank wrote: > On 2/13/23 09:55, Nico Boehr wrote: >> To determine whether the guest has caused an external interruption loop >> upon code 20 (external interrupt) intercepts, the ext_new_psw needs to >> be inspected to see whether external interrupts are enabled. >> >> Under non-PV, ext_new_psw can simply be taken from guest lowcore. Under >> PV, KVM can only access the encrypted guest lowcore and hence the >> ext_new_psw must not be taken from guest lowcore. >> >> handle_external_interrupt() incorrectly did that and hence was not able >> to reliably tell whether an external interruption loop is happening or >> not. False negatives cause spurious failures of my kvm-unit-test >> for extint loops[1] under PV. >> >> Since code 20 is only caused under PV if and only if the guest's >> ext_new_psw is enabled for external interrupts, false positive detection >> of a external interruption loop can not happen. >> >> Fix this issue by instead looking at the guest PSW in the state >> description. Since the PSW swap for external interrupt is done by the >> ultravisor before the intercept is caused, this reliably tells whether >> the guest is enabled for external interrupts in the ext_new_psw. >> >> Also update the comments to explain better what is happening. >> >> [1] https://lore.kernel.org/kvm/20220812062151.1980937-4-nrb@linux.ibm.com/ >> >> Signed-off-by: Nico Boehr <nrb@linux.ibm.com> >> --- > > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > I'll add this when picking: Fixes: 201ae986ead7 ("KVM: s390: protvirt: Implement interrupt injection") ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] KVM: s390: pv: fix external interruption loop not always detected 2023-02-13 12:00 ` Janosch Frank @ 2023-02-13 13:02 ` Nico Boehr 0 siblings, 0 replies; 6+ messages in thread From: Nico Boehr @ 2023-02-13 13:02 UTC (permalink / raw) To: Janosch Frank, borntraeger, imbrenda; +Cc: kvm, linux-s390 Quoting Janosch Frank (2023-02-13 13:00:25) [...] > I'll add this when picking: > Fixes: 201ae986ead7 ("KVM: s390: protvirt: Implement interrupt injection") Fine for me, thanks and sorry for the troubles. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-13 13:02 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-13 8:55 [PATCH v3 0/1] KVM: s390: pv: fix external interruption loop not always detected Nico Boehr 2023-02-13 8:55 ` [PATCH v3 1/1] " Nico Boehr 2023-02-13 9:06 ` Christian Borntraeger 2023-02-13 9:21 ` Janosch Frank 2023-02-13 12:00 ` Janosch Frank 2023-02-13 13:02 ` Nico Boehr
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox