Re: [RFC PATCH v1 3/6] KVM: s390: Simplify SIGP Restart

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

On Tue, 2021-10-12 at 17:23 +0200, Thomas Huth wrote:
> On 11/10/2021 09.45, Christian Borntraeger wrote:
> > 
> > Am 08.10.21 um 22:31 schrieb Eric Farman:
> > > Now that we check for the STOP IRQ injection at the top of the
> > > SIGP
> > > handler (before the userspace/kernelspace check), we don't need
> > > to do
> > > it down here for the Restart order.
> > > 
> > > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > > ---
> > >   arch/s390/kvm/sigp.c | 11 +----------
> > >   1 file changed, 1 insertion(+), 10 deletions(-)
> > > 
> > > diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
> > > index 6ca01bbc72cf..0c08927ca7c9 100644
> > > --- a/arch/s390/kvm/sigp.c
> > > +++ b/arch/s390/kvm/sigp.c
> > > @@ -240,17 +240,8 @@ static int __sigp_sense_running(struct
> > > kvm_vcpu *vcpu,
> > >   static int __prepare_sigp_re_start(struct kvm_vcpu *vcpu,
> > >                      struct kvm_vcpu *dst_vcpu, u8 order_code)
> > >   {
> > > -    struct kvm_s390_local_interrupt *li = &dst_vcpu-
> > > >arch.local_int;
> > >       /* handle (RE)START in user space */
> > > -    int rc = -EOPNOTSUPP;
> > > -
> > > -    /* make sure we don't race with STOP irq injection */
> > > -    spin_lock(&li->lock);
> > > -    if (kvm_s390_is_stop_irq_pending(dst_vcpu))
> > > -        rc = SIGP_CC_BUSY;
> > > -    spin_unlock(&li->lock);
> > > -
> > > -    return rc;
> > > +    return -EOPNOTSUPP;
> > >   }
> > >   static int __prepare_sigp_cpu_reset(struct kvm_vcpu *vcpu,
> > > 
> > 
> > @thuth?
> > Question is, does it make sense to merge patch 2 and 3 to make
> > things more 
> > obvious?
> 
> Maybe.
> 
> Anyway: Would it make sense to remove __prepare_sigp_re_start()
> completely 
> now and let __prepare_sigp_unknown() set the return code in the
> "default:" case?

We could, but that would affect the SIGP START case which also uses the
re_start routine. And if we're going down that path, we could remove
(INITIAL) CPU RESET handled in __prepare_sigp_cpu_reset, which does the
same thing (nothing). Not sure it buys us much, other than losing the
details in the different counters of which SIGP orders are processed.

Eric

> 
>   Thomas
>