Re: [RFCv2 35/37] KVM: s390: protvirt: Mask PSW interrupt bits for interception 104 and 112

[Thomas Huth <thuth@redhat.com>]

On 03/02/2020 14.19, Christian Borntraeger wrote:
> From: Janosch Frank <frankja@linux.ibm.com>
> 
> We're not allowed to inject interrupts on those intercept codes. As our
> PSW is just a copy of the real one that will be replaced on the next
> exit, we can mask out the interrupt bits in the PSW to make sure that we
> do not inject anything.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index d4dc156e2c3e..137ae5dc9101 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4050,6 +4050,7 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason)
>  	return vcpu_post_run_fault_in_sie(vcpu);
>  }
>  
> +#define PSW_INT_MASK (PSW_MASK_EXT | PSW_MASK_IO | PSW_MASK_MCHECK)
>  static int __vcpu_run(struct kvm_vcpu *vcpu)
>  {
>  	int rc, exit_reason;
> @@ -4082,10 +4083,15 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>  		}
>  		exit_reason = sie64a(vcpu->arch.sie_block,
>  				     vcpu->run->s.regs.gprs);
> +		/* This will likely be moved into a new function. */

As already mentioned by Cornelia: Please remove the above comment.

>  		if (kvm_s390_pv_is_protected(vcpu->kvm)) {
>  			memcpy(vcpu->run->s.regs.gprs,
>  			       sie_page->pv_grregs,
>  			       sizeof(sie_page->pv_grregs));

... but I'd like to suggest to add a comment before the following
if-statement with some information from the patch description. Otherwise
this will later be one of the code spots where you scratch your head
while looking at it and wonder why this masking of bits is done here.

> +			if (vcpu->arch.sie_block->icptcode == ICPT_PV_INSTR ||
> +			    vcpu->arch.sie_block->icptcode == ICPT_PV_PREF) {
> +				vcpu->arch.sie_block->gpsw.mask &= ~PSW_INT_MASK;
> +			}
>  		}
>  		local_irq_disable();
>  		__enable_cpu_timer_accounting(vcpu);
> 

 Thomas