Re: [PATCH] KVM: s390: fix cc for successful PQAP

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

On Thu, 2023-12-07 at 10:39 -0500, Anthony Krowiak wrote:
> 
> On 12/1/23 1:16 PM, Eric Farman wrote:
> > The various errors that are possible when processing a PQAP
> > instruction (the absence of a driver hook, an error FROM that
> > hook), all correctly set the PSW condition code to 3. But if
> > that processing works successfully, CC0 needs to be set to
> > convey that everything was fine.
> > 
> > Fix the check so that the guest can examine the condition code
> > to determine whether GPR1 has meaningful data.
> > 
> > Fixes: e5282de93105 ("s390: ap: kvm: add PQAP interception for
> > AQIC")
> > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > ---
> >   arch/s390/kvm/priv.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
> > index 621a17fd1a1b..f875a404a0a0 100644
> > --- a/arch/s390/kvm/priv.c
> > +++ b/arch/s390/kvm/priv.c
> > @@ -676,8 +676,12 @@ static int handle_pqap(struct kvm_vcpu *vcpu)
> >         if (vcpu->kvm->arch.crypto.pqap_hook) {
> >                 pqap_hook = *vcpu->kvm->arch.crypto.pqap_hook;
> >                 ret = pqap_hook(vcpu);
> > -               if (!ret && vcpu->run->s.regs.gprs[1] & 0x00ff0000)
> > -                       kvm_s390_set_psw_cc(vcpu, 3);
> > +               if (!ret) {
> > +                       if (vcpu->run->s.regs.gprs[1] & 0x00ff0000)
> > +                               kvm_s390_set_psw_cc(vcpu, 3);
> > +                       else
> > +                               kvm_s390_set_psw_cc(vcpu, 0);
> > +               }
> 
> 
> The cc is not set if pqap_hook returns a non-zero rc; however, this 
> point may be moot given the only non-zero rc is -EOPNOTSUPP. I'm a
> bit 
> foggy on what happens when non-zero return codes are passed up the
> stack.

Right, a non-zero RC will get reflected to the interception handlers,
where EOPNOTSUPP instructs control to be given to userspace. So not
setting a condition code is correct here, as userspace will be expected
to do that.

> 
> 
> >                 up_read(&vcpu->kvm->arch.crypto.pqap_hook_rwsem);
> >                 return ret;
> >         }