Re: qemu-kvm: do not allow vcpu stop with in progress PIO

[Marcelo Tosatti <mtosatti@redhat.com>]

On Wed, Feb 10, 2010 at 09:02:00AM +0200, Avi Kivity wrote:
> On 02/09/2010 10:58 PM, Marcelo Tosatti wrote:
> >You're right... this should be enough to avoid a stop with uncomplete
> >PIO (and this is what happens for MMIO already). The signal will not
> >be dequeued, so KVM will complete_pio and exit before entering with
> >-EAGAIN. Please review and queue for stable.
> >
> 
> Not right enough.  This is very fragile, we depend on the kernel
> noticing the signal after completing pio but before starting
> execution.  I don't think we guarantee that.

As long as the signal is blocked, we do (and for older kernels too).

> Maybe we should turn complete_pio/complete_mmio to an ioctl, so that
> we can control what happens exactly.  Or maybe it's simplest to
> document it as a feature and guarantee it.  There's some merit in it
> - only guest execution is the nonatomic part, so we only interrupt
> that.

Right. So would you like a patch to x86.c to comment on this, on top of
complete_pio / mmio completion?

> >qemu upstream needs a bit more work.
> 
> Could be as simple as raising a blocked exception that is unmasked
> by kvm, then entering the guest.

The vcpu inner loop is not atomic in upstream as it is in qemu-kvm. It
breaks out to process pending events way too easily.

> 
> >-------
> >
> >Re-enter the kernel to complete in progress PIO. Otherwise the
> >operation can be lost during migration.
> >
> >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> >
> >Index: qemu-kvm/qemu-kvm.c
> >===================================================================
> >--- qemu-kvm.orig/qemu-kvm.c
> >+++ qemu-kvm/qemu-kvm.c
> >@@ -967,6 +967,7 @@ int kvm_run(CPUState *env)
> >                                  run->io.direction,
> >                                  run->io.size,
> >                                  run->io.count);
> >+            r = 0;
> >              break;
> >          case KVM_EXIT_DEBUG:
> >              r = handle_debug(env);
> 
> 
> -- 
> Do not meddle in the internals of kernels, for they are subtle and quick to panic.