Re: [PATCH v3 1/2] KVM: s390: fix access register usage in ioctls

[Christian Borntraeger <borntraeger@linux.ibm.com>]

Am 16.02.24 um 22:36 schrieb Eric Farman:
> The routine ar_translation() can be reached by both the instruction
> intercept path (where the access registers had been loaded with the
> guest register contents), and the MEM_OP ioctls (which hadn't).
> Since this routine saves the current registers to vcpu->run,
> this routine erroneously saves host registers into the guest space.
> 
> Introduce a boolean in the kvm_vcpu_arch struct to indicate whether
> the registers contain guest contents. If they do (the instruction
> intercept path), the save can be performed and the AR translation
> is done just as it is today. If they don't (the MEM_OP path), the
> AR can be read from vcpu->run without stashing the current contents.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>

Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>   arch/s390/include/asm/kvm_host.h | 1 +
>   arch/s390/kvm/gaccess.c          | 3 ++-
>   arch/s390/kvm/kvm-s390.c         | 3 +++
>   3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 52664105a473..c86215eb4ca7 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -765,6 +765,7 @@ struct kvm_vcpu_arch {
>   	__u64 cputm_start;
>   	bool gs_enabled;
>   	bool skey_enabled;
> +	bool acrs_loaded;
>   	struct kvm_s390_pv_vcpu pv;
>   	union diag318_info diag318_info;
>   };
> diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> index 5bfcc50c1a68..b4c805092021 100644
> --- a/arch/s390/kvm/gaccess.c
> +++ b/arch/s390/kvm/gaccess.c
> @@ -391,7 +391,8 @@ static int ar_translation(struct kvm_vcpu *vcpu, union asce *asce, u8 ar,
>   	if (ar >= NUM_ACRS)
>   		return -EINVAL;
>   
> -	save_access_regs(vcpu->run->s.regs.acrs);
> +	if (vcpu->arch.acrs_loaded)
> +		save_access_regs(vcpu->run->s.regs.acrs);
>   	alet.val = vcpu->run->s.regs.acrs[ar];
>   
>   	if (ar == 0 || alet.val == 0) {
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ea63ac769889..61092f0e0a66 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3951,6 +3951,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>   				    KVM_SYNC_ARCH0 |
>   				    KVM_SYNC_PFAULT |
>   				    KVM_SYNC_DIAG318;
> +	vcpu->arch.acrs_loaded = false;
>   	kvm_s390_set_prefix(vcpu, 0);
>   	if (test_kvm_facility(vcpu->kvm, 64))
>   		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;
> @@ -4951,6 +4952,7 @@ static void sync_regs(struct kvm_vcpu *vcpu)
>   	}
>   	save_access_regs(vcpu->arch.host_acrs);
>   	restore_access_regs(vcpu->run->s.regs.acrs);
> +	vcpu->arch.acrs_loaded = true;
>   	/* save host (userspace) fprs/vrs */
>   	save_fpu_regs();
>   	vcpu->arch.host_fpregs.fpc = current->thread.fpu.fpc;
> @@ -5021,6 +5023,7 @@ static void store_regs(struct kvm_vcpu *vcpu)
>   	kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
>   	save_access_regs(vcpu->run->s.regs.acrs);
>   	restore_access_regs(vcpu->arch.host_acrs);
> +	vcpu->arch.acrs_loaded = false;
>   	/* Save guest register state */
>   	save_fpu_regs();
>   	vcpu->run->s.regs.fpc = current->thread.fpu.fpc;