Re: [RFC PATCH v3 2/2] KVM: s390: Extend the USER_SIGP capability

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

On Wed, 2021-11-17 at 08:54 +0100, Christian Borntraeger wrote:
> Am 11.11.21 um 20:05 schrieb Eric Farman:
> > On Thu, 2021-11-11 at 19:29 +0100, David Hildenbrand wrote:
> > > On 11.11.21 18:48, Eric Farman wrote:
> > > > On Thu, 2021-11-11 at 17:13 +0100, Janosch Frank wrote:
> > > > > 
> > > > > Looking at the API I'd like to avoid having two IOCTLs
> > > > 
> > > > Since the order is a single byte, we could have the payload of
> > > > an
> > > > ioctl
> > > > say "0-255 is an order that we're busy processing, anything
> > > > higher
> > > > than
> > > > that resets the busy" or something. That would remove the need
> > > > for
> > > > a
> > > > second IOCTL.
> > > 
> > > Maybe just pass an int and treat a negative (or just -1) value as
> > > clearing the order.
> > > 
> > 
> > Right, that's exactly what I had at one point. I thought it was too
> > cumbersome, but maybe not. Will dust it off, pending my question to
> > Janosch about 0-vs-1 IOCTLs.
> 
> As a totally different idea. Would a sync_reg value called SIGP_BUSY
> work as well?
> 

Hrm... I'm not sure. I played with it a bit, and it's not looking
great. I'm almost certainly missing some serialization, because I was
frequently "losing" one of the toggles (busy/not-busy) when hammering
CPUs with various SIGP orders on this interface and thus getting
incorrect responses from the in-kernel orders.

I also took a stab at David's idea of tying it to KVM_MP_STATE [1]. I
still think it's a little odd, considering the existing states are all
z/Arch-defined CPU states, but it does sound like the sort of thing
we're trying to do (letting userspace announce what the CPU is up to).
One flaw is that most of the rest of QEMU uses s390_cpu_set_state() for
this, which returns the number of running CPUs instead of the return
code from the MP_STATE ioctl (via kvm_s390_set_cpu_state()) that SIGP
would be interested in. Even if I made the ioctl call directly, I still
encounter some system problems that smell like ones I've addressed in
v2 and v3. Possibly fixable, but I didn't pursue them far enough to be
certain.

I ALSO took a stab at folding this into the S390 IRQ paths [2], similar
to what was done with kvm_s390_stop_info. This worked reasonably well,
except the QEMU interface kvm_s390_vcpu_interrupt() returns a void, and
so wouldn't notice an error sent back by KVM. Not a deal breaker, but
having not heard anything to this idea, I didn't go much farther.

Next week is a short week due to the US holiday, so rather than flesh
out any of the above possibilities, I'm going to send a new RFC as v4.
This will be back to a single IOCTL, with a small payload, per Janosch'
feedback. We can have a discussion on that, but if any of the above
alternatives sound more appealing I can try getting one of them working
with more consistency.

[1] 
https://lore.kernel.org/r/ff344676-0c37-610b-eafb-b1477db0f6a1@redhat.com/
[2] 
https://lore.kernel.org/all/b206e7b73696907328bc4338664dea1ef572e8aa.camel@linux.ibm.com/