Re: [PATCH v1 03/13] KVM: s390: use __kvm_faultin_pfn()

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

On Tue, 14 Jan 2025 18:34:13 +0100
"Christoph Schlameuss" <schlameuss@linux.ibm.com> wrote:

> On Wed Jan 8, 2025 at 7:14 PM CET, Claudio Imbrenda wrote:
> > Refactor the existing page fault handling code to use __kvm_faultin_pfn().
> >
> > This possible now that memslots are always present.
> >
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > ---
> >  arch/s390/kvm/kvm-s390.c | 92 +++++++++++++++++++++++++++++++---------
> >  arch/s390/mm/gmap.c      |  1 +
> >  2 files changed, 73 insertions(+), 20 deletions(-)  
> 
> With nits resolved:
> 
> Reviewed-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

I'll send a v2 with some more substantial differences, so maybe you
will want to look at that first :) 

[...]

> > +		gfn = gpa_to_gfn(gaddr);
> > +		if (kvm_is_ucontrol(vcpu->kvm)) {
> > +			/*
> > +			 * This translates the per-vCPU guest address into a
> > +			 * fake guest address, which can then be used with the
> > +			 * fake memslots that are identity mapping userspace.
> > +			 * This allows ucontrol VMs to use the normal fault
> > +			 * resolution path, like normal VMs.
> > +			 */
> > +			gaddr_tmp = gmap_translate(vcpu->arch.gmap, gaddr);
> > +			if (gaddr_tmp == -EFAULT) {
> >  				vcpu->run->exit_reason = KVM_EXIT_S390_UCONTROL;
> >  				vcpu->run->s390_ucontrol.trans_exc_code = gaddr;
> >  				vcpu->run->s390_ucontrol.pgm_code = 0x10;  
> 
> nit: s/0x10/PGM_SEGMENT_TRANSLATION/

the original code has 0x10, I wanted to keep it as it is, but I'll
change it, since I'm refactoring more code in v2

> 
> >  				return -EREMOTE;
> >  			}
> > -			return vcpu_post_run_addressing_exception(vcpu);
> > +			gfn = gpa_to_gfn(gaddr_tmp);
> >  		}
> > +		return kvm_s390_handle_dat_fault(vcpu, gfn, gaddr, flags);
> >  		break;  
> 
> nit: Remove the break after the return here?

yes

> 
> >  	default:
> >  		KVM_BUG(1, vcpu->kvm, "Unexpected program interrupt 0x%x, TEID 0x%016lx",
> > @@ -4880,7 +4932,7 @@ static int vcpu_post_run_handle_fault(struct kvm_vcpu *vcpu)
> >  		send_sig(SIGSEGV, current, 0);
> >  		break;
> >  	}
> > -	return rc;
> > +	return 0;
> >  }
> >  
> >  static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason)
> > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
> > index 16b8a36c56de..3aacef77c174 100644
> > --- a/arch/s390/mm/gmap.c
> > +++ b/arch/s390/mm/gmap.c
> > @@ -605,6 +605,7 @@ int __gmap_link(struct gmap *gmap, unsigned long gaddr, unsigned long vmaddr)
> >  	radix_tree_preload_end();
> >  	return rc;
> >  }
> > +EXPORT_SYMBOL(__gmap_link);
> >  
> >  /**
> >   * fixup_user_fault_nowait - manually resolve a user page fault without waiting  
>