Re: [RFC 10/10] KVM: s390: Cleanup kvm_s390_store_status_unloaded

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

On Mon, 16 Mar 2026 16:23:57 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Fixup comments, use gpa_t and replace magic constants.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 24 ++++++++++++++++--------
>  arch/s390/kvm/kvm-s390.h |  4 ++--
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 1668580008c6..c76f83b38d27 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4993,11 +4993,12 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  
>  /*
>   * store status at address
> - * we use have two special cases:
> - * KVM_S390_STORE_STATUS_NOADDR: -> 0x1200 on 64 bit
> - * KVM_S390_STORE_STATUS_PREFIXED: -> prefix
> + *
> + * We have two special cases:
> + * - KVM_S390_STORE_STATUS_NOADDR: -> 0x1200 on 64 bit
> + * - KVM_S390_STORE_STATUS_PREFIXED: -> prefix
>   */

since you're touching this comment block, make it proper kerneldoc,
describing parameters, return value, etc

the rest of the patch is good

> -int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long gpa)
> +int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, gpa_t gpa)
>  {
>  	unsigned char archmode = 1;
>  	freg_t fprs[NUM_FPRS];
> @@ -5007,15 +5008,22 @@ int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long gpa)
>  
>  	px = kvm_s390_get_prefix(vcpu);
>  	if (gpa == KVM_S390_STORE_STATUS_NOADDR) {
> -		if (write_guest_abs(vcpu, 163, &archmode, 1))
> +		if (write_guest_abs(vcpu, __LC_AR_MODE_ID, &archmode, 1))
>  			return -EFAULT;
>  		gpa = 0;
>  	} else if (gpa == KVM_S390_STORE_STATUS_PREFIXED) {
> -		if (write_guest_real(vcpu, 163, &archmode, 1))
> +		if (write_guest_real(vcpu, __LC_AR_MODE_ID, &archmode, 1))
>  			return -EFAULT;
>  		gpa = px;
> -	} else
> +	} else {
> +		/*
> +		 * Store status at address does NOT store vrs and arch
> +		 * indication. Since we add __LC_FPREGS_SAVE_AREA to
> +		 * the address when writing, we need to subtract it
> +		 * here.
> +		 */
>  		gpa -= __LC_FPREGS_SAVE_AREA;
> +	}
>  
>  	/* manually convert vector registers if necessary */
>  	if (cpu_has_vx()) {
> @@ -5049,7 +5057,7 @@ int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long gpa)
>  	return rc ? -EFAULT : 0;
>  }
>  
> -int kvm_s390_vcpu_store_status(struct kvm_vcpu *vcpu, unsigned long addr)
> +int kvm_s390_vcpu_store_status(struct kvm_vcpu *vcpu, gpa_t addr)
>  {
>  	/*
>  	 * The guest FPRS and ACRS are in the host FPRS/ACRS due to the lazy
> diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
> index 1ffaec723a30..9cfc3caee334 100644
> --- a/arch/s390/kvm/kvm-s390.h
> +++ b/arch/s390/kvm/kvm-s390.h
> @@ -450,8 +450,8 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu);
>  
>  /* implemented in kvm-s390.c */
>  int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod);
> -int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long addr);
> -int kvm_s390_vcpu_store_status(struct kvm_vcpu *vcpu, unsigned long addr);
> +int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, gpa_t addr);
> +int kvm_s390_vcpu_store_status(struct kvm_vcpu *vcpu, gpa_t addr);
>  int kvm_s390_vcpu_start(struct kvm_vcpu *vcpu);
>  int kvm_s390_vcpu_stop(struct kvm_vcpu *vcpu);
>  void kvm_s390_vcpu_block(struct kvm_vcpu *vcpu);