Re: [PATCH] KVM: VMX: Update instruction length on intercepted BP

Gleb Natapov
2010-02-14T17:26:15Z

On Sun, Feb 14, 2010 at 06:06:31PM +0100, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > On Sun, Feb 14, 2010 at 05:37:39PM +0100, Jan Kiszka wrote:
> >> Gleb Natapov wrote:
> >>> On Sun, Feb 14, 2010 at 12:39:14PM +0100, Jan Kiszka wrote:
> >>>> Gleb Natapov wrote:
> >>>>> On Sun, Feb 14, 2010 at 11:47:58AM +0100, Jan Kiszka wrote:
> >>>>>> Gleb Natapov wrote:
> >>>>>>> On Sun, Feb 14, 2010 at 11:26:31AM +0100, Jan Kiszka wrote:
> >>>>>>>> Gleb Natapov wrote:
> >>>>>>>>> On Sat, Feb 13, 2010 at 10:31:12AM +0100, Jan Kiszka wrote:
> >>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>>>
> >>>>>>>>>> We intercept #BP while in guest debugging mode. As VM exists due to
> >>>>>>>>>> intercepted exceptions do not necessarily come with valid
> >>>>>>>>>> idt_vectoring, we have to update event_exit_inst_len explicitly in such
> >>>>>>>>>> cases. At least in the absence of migration, this ensures that
> >>>>>>>>>> re-injections of #BP will find and use the correct instruction length.
> >>>>>>>>>>
> >>>>>>>>> event_exit_inst_len is only used for event reinjection. Since event
> >>>>>>>>> intercepted here will not be reinjected why updating event_exit_inst_len
> >>>>>>>>> is needed here?
> >>>>>>>> In guest debugging mode a #BP exception is always reported to user space
> >>>>>>>> to find out what caused it. If it was the guest itself, the exception is
> >>>>>>>> reinjected, on older kernels via KVM_SET_GUEST_DEBUG and since 2.6.33
> >>>>>>>> via KVM_SET_VCPU_EVENTS (the latter requires some qemu patch that I will
> >>>>>>>> post later).
> >>>>>>>>
> >>>>>>>> As we currently do not update event_exit_inst_len on #BP exits,
> >>>>>>>> reinjecting fails unless event_exit_inst_len happens to be 1 from some
> >>>>>>>> other exit.
> >>>>>>>>
> >>>>>>> Hmm, how does it work on SVM then where we do not have
> >>>>>>> event_exit_inst_len so execution will resume on the same rip that caused
> >>>>>>> #BP after event reinjection?
> >>>>>>>
> >>>>>> Maybe not at all. I don't think I've tested this scenario on amd so far.
> >>>>>> Guess it needs some special handling in svm to move rip after the int3
> >>>>>> when requesting to inject #BP.
> >>>>>>
> >>>>> This will work for VMX too, no? So may be we should design something
> >>>>> that will work for both VMX and SVM before applying patches that make
> >>>>> oly VMX work?
> >>>> VMX used to work, so my patch is actually a regression fix. I bet this
> >>>> was accidentally broken while cleaning up the interrupt handling of VMX.
> >>>>
> >>> VMX used to always reexecute instruction.
> >> ...since 66fd3f7f90. And that was what broke this guest debugging corner
> >> case.
> >>
> > I see. And I see why it worked, but it shouldn't have been working for
> > SVM. I prefer to look for general solution here that works for SVM/VMX.
>
> I don't see the need to emulate INT3 for the sake of unification. VMX
> works today (with this patch), and SVM might work without further
> efforts, at least on modern hosts:
>
> "Software interrupts cannot be properly injected if the processor does
> not support the NextRIP field, indicated by EDX[3] = 1 as returned by
> CPUID function 8000_000A. Hypervisor software should emulate the event
> injection of software interrupts if NextRIP is not supported."
>
> (right below the paragraph I cited before)
>
> I assume, INT3 can be considered as software interrupt as well in this
> context.
>
Lets check if SVM works. I can do that if you tell me how. My concern is
that if SVM doesn't work we will write another way to handle this
situation that will work on SVM and VMX and then we will have code in
VMX that is not needed, but we have to support it.

--
Gleb.