On Tue, 14 Jan 2025 18:34:13 +0100
"Christoph Schlameuss" <schlameuss@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxx>
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
>
