On Mon, 2021-10-11 at 11:21 +0200, David Hildenbrand wrote: > On 11.10.21 10:40, Christian Borntraeger wrote: > > > > Am 11.10.21 um 09:09 schrieb David Hildenbrand: > > > On 08.10.21 22:38, Eric Farman wrote: > > > > When building a Stop IRQ to pass to KVM, we should incorporate > > > > the flags if handling the SIGP Stop and Store Status order. > > > > With that, KVM can reject other orders that are submitted for > > > > the same CPU while the operation is fully processed. > > > > > > > > Signed-off-by: Eric Farman <far...@linux.ibm.com> > > > > Acked-by: Janosch Frank <fran...@linux.ibm.com> > > > > --- > > > > target/s390x/kvm/kvm.c | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c > > > > index 5b1fdb55c4..701b9ddc88 100644 > > > > --- a/target/s390x/kvm/kvm.c > > > > +++ b/target/s390x/kvm/kvm.c > > > > @@ -2555,6 +2555,10 @@ void kvm_s390_stop_interrupt(S390CPU > > > > *cpu) > > > > .type = KVM_S390_SIGP_STOP, > > > > }; > > > > + if (cpu->env.sigp_order == SIGP_STOP_STORE_STATUS) { > > > > + irq.u.stop.flags = KVM_S390_STOP_FLAG_STORE_STATUS; > > > > + } > > > > + > > > > > > KVM_S390_STOP_FLAG_STORE_STATUS tells KVM to perform the store > > > status as well ... is that really what we want? > > At least it should not hurt I guess. QEMU then does it again? > > The thing is, that before we officially completed the action in user > space (and let other SIGP actions actually succeed in user space on > the > CPU), the target CPU will be reported as !busy in the kernel > already. > And before we even inject the stop interrupt, the CPU will be > detected > as !busy in the kernel. I guess it will fix some cases where we poll > via > SENSE if the stop and store happened, because the store *did* happen > in > the kernel and we'll simply store again in user space. > > However, I wonder if we want to handle it more generically: Properly > flag a CPU as busy for SIGP when we start processing the order until > we > completed processing the order. That would allow to handle other > SIGP > operations in user space cleanly, without any chance for races with > SENSE code running in the kernel. >
I think a generic solution would be ideal, but I'm wrestling with the race with the kernel's SENSE code. Today, handle_sigp_single_dst already checks to see if a CPU is currently processing an order and returns a CC2 when it does, but of course the kernel's SENSE code doesn't know that. We could flag the CPU as busy in the kernel when sending a SIGP to userspace, so that the SENSE code indicates BUSY, but then how do we know when userspace is finished and the CPU is no longer BUSY? Eric