Re: [PATCH v1] s390/kvm: pv: don't present the ecall interrupt twice

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Mon, 18 Jul 2022 15:04:34 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> When the SIGP interpretation facility is present and a VCPU sends an
> ecall to another VCPU in enabled wait, the sending VCPU receives a 56
> intercept (partial execution), so KVM can wake up the receiving CPU.
> Note that the SIGP interpretation facility will take care of the
> interrupt delivery and KVM's only job is to wake the receiving VCPU.
> 
> For PV, the sending VCPU will receive a 108 intercept (pv notify) and
> should continue like in the non-PV case, i.e. wake the receiving VCPU.
> 
> For PV and non-PV guests the interrupt delivery will occur through the
> SIGP interpretation facility on SIE entry when SIE finds the X bit in
> the status field set.
> 
> However, in handle_pv_notification(), there was no special handling for
> SIGP, which leads to interrupt injection being requested by KVM for the
> next SIE entry. This results in the interrupt being delivered twice:
> once by the SIGP interpretation facility and once by KVM through the
> IICTL.
> 
> Add the necessary special handling in handle_pv_notification(), similar
> to handle_partial_execution(), which simply wakes the receiving VCPU and
> leave interrupt delivery to the SIGP interpretation facility.
> 
> In contrast to external calls, emergency calls are not interpreted but
> also cause a 108 intercept, which is why we still need to call
> handle_instruction() for SIGP orders other than ecall.
> 
> Since kvm_s390_handle_sigp_pei() is now called for all SIGP orders which
> cause a 108 intercept - even if they are actually handled by
> handle_instruction() - move the tracepoint in kvm_s390_handle_sigp_pei()
> to avoid possibly confusing trace messages.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> Cc: <stable@vger.kernel.org> # 5.7
> Fixes: da24a0cc58ed ("KVM: s390: protvirt: Instruction emulation")
> ---
>  arch/s390/kvm/intercept.c | 15 +++++++++++++++
>  arch/s390/kvm/sigp.c      |  4 ++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
> index 8bd42a20d924..88112065d941 100644
> --- a/arch/s390/kvm/intercept.c
> +++ b/arch/s390/kvm/intercept.c
> @@ -528,12 +528,27 @@ static int handle_pv_uvc(struct kvm_vcpu *vcpu)
>  
>  static int handle_pv_notification(struct kvm_vcpu *vcpu)
>  {
> +	int ret;
> +
>  	if (vcpu->arch.sie_block->ipa == 0xb210)
>  		return handle_pv_spx(vcpu);
>  	if (vcpu->arch.sie_block->ipa == 0xb220)
>  		return handle_pv_sclp(vcpu);
>  	if (vcpu->arch.sie_block->ipa == 0xb9a4)
>  		return handle_pv_uvc(vcpu);
> +	if (vcpu->arch.sie_block->ipa >> 8 == 0xae) {
> +		/*
> +		 * Besides external call, other SIGP orders also cause a
> +		 * 108 (pv notify) intercept. In contrast to external call,
> +		 * these orders need to be emulated and hence the appropriate
> +		 * place to handle them is in handle_instruction().
> +		 * So first try kvm_s390_handle_sigp_pei() and if that isn't
> +		 * successful, go on with handle_instruction().
> +		 */
> +		ret = kvm_s390_handle_sigp_pei(vcpu);
> +		if (!ret)
> +			return ret;
> +	}
>  
>  	return handle_instruction(vcpu);
>  }
> diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
> index 8aaee2892ec3..cb747bf6c798 100644
> --- a/arch/s390/kvm/sigp.c
> +++ b/arch/s390/kvm/sigp.c
> @@ -480,9 +480,9 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu)
>  	struct kvm_vcpu *dest_vcpu;
>  	u8 order_code = kvm_s390_get_base_disp_rs(vcpu, NULL);
>  
> -	trace_kvm_s390_handle_sigp_pei(vcpu, order_code, cpu_addr);
> -
>  	if (order_code == SIGP_EXTERNAL_CALL) {
> +		trace_kvm_s390_handle_sigp_pei(vcpu, order_code, cpu_addr);
> +
>  		dest_vcpu = kvm_get_vcpu_by_id(vcpu->kvm, cpu_addr);
>  		BUG_ON(dest_vcpu == NULL);
>