Re: [PATCH v1] KVM: s390: store DXC/VXC in fpc on DATA/Vector-processing exceptions

[Christian Borntraeger]

On 08/22/2018 11:53 AM, David Hildenbrand wrote:
> When DATA exceptions and vector-processing exceptions (program interrupts)
> are injected, the DXC/VXC is also to be stored in the fpc, if AFP is
> enabled in CR0.
> 
> This can happen inside KVM when reinjecting an interrupt during program
> interrupt intercepts. These are triggered for example when debugging the
> guest (concurrent PER events result in an intercept instead of an
> injection of such interrupts).
> 
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
> 
> Only compile-tested.

I checked the Linux code (arch/s390/kernel/traps.c) and Linux uses the FPC (and
not the lowcore field) to decide about the signal (SIGFPE) and si_code. So we want
to have the correct DXC/VXC value.

Now, I wrote a short test program that does
feenableexcept(FE_DIVBYZERO);
and a division by zero.
and attached gdb to that guest together with a breakpoint on the divide (and the instruction
after).
I get the pint exit for the instruction after (as it is suppressing) and at this point in
time the guest fpc already contains the correct DXC value. So you patch will certainly not
hurt, but it seems not necessary.

Still trying to look further if I missed something.

> 
>  arch/s390/include/asm/ctl_reg.h | 1 +
>  arch/s390/kvm/interrupt.c       | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/arch/s390/include/asm/ctl_reg.h b/arch/s390/include/asm/ctl_reg.h
> index 4600453536c2..88f3f14baee9 100644
> --- a/arch/s390/include/asm/ctl_reg.h
> +++ b/arch/s390/include/asm/ctl_reg.h
> @@ -11,6 +11,7 @@
>  #include <linux/const.h>
>  
>  #define CR0_CLOCK_COMPARATOR_SIGN	_BITUL(63 - 10)
> +#define CR0_AFP_REGISTER_CONTROL	_BITUL(63 - 45)
>  #define CR0_EMERGENCY_SIGNAL_SUBMASK	_BITUL(63 - 49)
>  #define CR0_EXTERNAL_CALL_SUBMASK	_BITUL(63 - 50)
>  #define CR0_CLOCK_COMPARATOR_SUBMASK	_BITUL(63 - 52)
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index fcb55b02990e..5b5754d8f460 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -765,6 +765,14 @@ static int __must_check __deliver_prog(struct kvm_vcpu *vcpu)
>  		break;
>  	case PGM_VECTOR_PROCESSING:
>  	case PGM_DATA:
> +		if (vcpu->arch.sie_block->gcr[0] & CR0_AFP_REGISTER_CONTROL) {
> +			/* make sure the new fpc will be lazily loaded */
> +			save_fpu_regs();
> +			/* the DXC/VXC cannot make the fpc invalid */
> +			current->thread.fpu.fpc &= ~0xff00u;
> +			current->thread.fpu.fpc |= (pgm_info.data_exc_code << 8)
> +						   & 0xff00u;

maybe reuse  FPC_DXC_MASK instead of 0xff00 ?




> +		}
>  		rc = put_guest_lc(vcpu, pgm_info.data_exc_code,
>  				  (u32 *)__LC_DATA_EXC_CODE);
>  		break;
>