On Fri, Feb 09, 2024 at 09:45:38PM +0100, Eric Farman wrote: > The routine ar_translation() is called by get_vcpu_asce(), which is > called by both the instruction intercept path (where the access > registers had been loaded with the guest's values), and the MEM_OP > ioctl (which hadn't). This means that any ALET the guest expects to > be used would be ignored. > > Furthermore, the logic in ar_translation() will store the contents > of the access registers back to the KVM_RUN struct. This unexpected > change of AR values can lead to problems after invoking the MEM_OP, > for example an ALET Specification Exception. > > Fix this by swapping the host/guest access registers around the > MEM_OP ioctl, in the same way that the KVM_RUN ioctl does with > sync_regs()/store_regs(). The full register swap isn't needed here, > since only the access registers are used in this interface. > > Suggested-by: Christian Borntraeger <[email protected]> > Signed-off-by: Eric Farman <[email protected]> > --- > arch/s390/kvm/kvm-s390.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index ea63ac769889..c2dfeea55dcf 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -5391,6 +5391,10 @@ static long kvm_s390_vcpu_mem_op(struct kvm_vcpu *vcpu, > return -ENOMEM; > } > > + /* Swap host/guest access registers in the event of a MEM_OP with AR > specified */ > + save_access_regs(vcpu->arch.host_acrs); > + restore_access_regs(vcpu->run->s.regs.acrs); > + > acc_mode = mop->op == KVM_S390_MEMOP_LOGICAL_READ ? GACC_FETCH : > GACC_STORE; > if (mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY) { > r = check_gva_range(vcpu, mop->gaddr, mop->ar, mop->size, > @@ -5420,6 +5424,8 @@ static long kvm_s390_vcpu_mem_op(struct kvm_vcpu *vcpu, > kvm_s390_inject_prog_irq(vcpu, &vcpu->arch.pgm); > > out_free: > + save_access_regs(vcpu->run->s.regs.acrs); > + restore_access_regs(vcpu->arch.host_acrs);
I guess we will end up with more and more of such constructs until nobody knows when which register contents are loaded. I would higly prefer a TIF flag which indicates if the access registers contain the host or guest register contents, and actual users grab the required content from the correct location - or better: always take it from guest save area, and write to the guest save area if the to be invented TIF flag indicates that access registers contain guest registers... Or maybe a TIF flag with different semantics: "guest save area does not reflect current state - which is within registers".
