Re: [PATCH v1 1/2] KVM: s390: load guest access registers in MEM_OP ioctl

[Eric Farman <farman@linux.ibm.com>]

On Mon, 2024-02-12 at 12:52 +0100, Heiko Carstens wrote:
> > On Mon, Feb 12, 2024 at 11:21:30AM +0100, Heiko Carstens wrote:
> > > > Or maybe a TIF flag with different semantics: "guest save area
> > > > does
> > > > not
> > > > reflect current state - which is within registers".
> > 
> > Something like the below; untested of course.

Ooops, yeah. Christian suggested something similar in his first
response to the RFC which I'd overlooked.

> >  But I guess there must be
> > some arch specific vcpu flags, which can be used to achieve the
> > same?

Agreed. Putting something there probably makes sense to keep it in the
KVM sphere

> > 
> > diff --git a/arch/s390/include/asm/thread_info.h
> > b/arch/s390/include/asm/thread_info.h
> > index a674c7d25da5..b9ff8b125fb8 100644
> > --- a/arch/s390/include/asm/thread_info.h
> > +++ b/arch/s390/include/asm/thread_info.h
> > @@ -69,6 +69,7 @@ void arch_setup_new_exec(void);
> >  #define TIF_PATCH_PENDING 5 /* pending live patching update */
> >  #define TIF_PGSTE 6 /* New mm's will use 4K page tables */
> >  #define TIF_NOTIFY_SIGNAL 7 /* signal notifications exist */
> > +#define TIF_KVM_ACRS 8 /* access registers contain guest content
> > */
> >  #define TIF_ISOLATE_BP_GUEST 9 /* Run KVM guests with isolated BP
> > */
> >  #define TIF_PER_TRAP 10 /* Need to handle PER trap on exit to
> > usermode */
> >  
> > diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
> > index 5bfcc50c1a68..b0ef242d2371 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 (test_thread_flag(TIF_KVM_ACRS))
> > + save_access_regs(vcpu->run->s.regs.acrs);

...or WARN if not set, so that we know of the missing path. Will send
this all as a v2. Thanks.

> >   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..3ee0913639d5 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -4951,6 +4951,7 @@ static void sync_regs(struct kvm_vcpu *vcpu)
> >   }
> >   save_access_regs(vcpu->arch.host_acrs);
> >   restore_access_regs(vcpu->run->s.regs.acrs);
> > + set_thread_flag(TIF_KVM_ACRS);
> >   /* save host (userspace) fprs/vrs */
> >   save_fpu_regs();
> >   vcpu->arch.host_fpregs.fpc = current->thread.fpu.fpc;
> > @@ -5020,6 +5021,7 @@ static void store_regs(struct kvm_vcpu *vcpu)
> >   kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
> >   kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
> >   save_access_regs(vcpu->run->s.regs.acrs);
> > + clear_thread_flag(TIF_KVM_ACRS);
> >   restore_access_regs(vcpu->arch.host_acrs);
> >   /* Save guest register state */
> >   save_fpu_regs();