On Wed, Feb 17, 2010 at 08:17:28PM +0100, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Wed, Feb 17, 2010 at 12:23:39PM +0100, Jan Kiszka wrote: > >> Gleb Natapov wrote: > >>> On Wed, Feb 17, 2010 at 01:13:29PM +0200, Avi Kivity wrote: > >>>> On 02/17/2010 12:43 PM, Gleb Natapov wrote: > >>>>>> And, again: This is an _existing_ user space ABI. We could only provide > >>>>>> an alternative, but we have to maintain what is there at least for some > >>>>>> longer grace period. > >>>>>> > >>>>> But it was always broken for SVM and was broken for VMX for a year and > >>>>> nobody noticed, so may be instead of reintroducing old interface we should > >>>>> do it right this time? > >>>> We need to fix the existing interface first, and then think long and > >>>> hard if we want yet another interface, since we're likely to screw > >>>> it up as well. > >>>> > >>>> The more interfaces we introduce, the harder maintenance becomes. > >>>> > >>> We are in a sad state if we cannot improve interface. The current one > >>> outsource part of CPU functionality into userspace. This should be a big > >>> no-no. > >> I still disagree on this. Moving the decision logic to user space > >> prevented to re-implement a gdbstub in kernel space. I oversaw that > >> re-injecting #BP over older SVM was broken, but it is now fixed for all > >> vendors. So moving it back to kernel has actually no long-term reason. > >> > > There were patches to implement gdbstub in kernel space! And not so long > > time ago :) > > Yes, a good reason to implement yet another one. :) > We can you unify them later :). But seriously I am not proposing anything like gdbstub in kernel, just track inserted breakpoints in kernel. > > But I want to move only a tiny bit of logic into the kernel space. > > And #BP reinjection brokenness is a different issue. It should be fixed > > anyway no matter where decision about reinfection happens. > > > > If maintainers think that we should not have improved interface and we > > should support reinjection of #DB from userspace then this patch should > > be applied. I don't have other objections to it. But I, at least, would > > prefer the old interface for #DB reinjection (KVM_GUESTDBG_INJECT_DB) > > and not the new one. The old one makes it explicit what we are doing, > > the new one allows injection of any event and should be used only during > > migration or CPU reset. It would be event good idea to fail setting > > events if CPU is running. > > Event injection is well supported by both vendors (except for those > software-triggered events). Just because QEMU mostly uses it for reset > and migration doesn't mean we have to restrict other users to only those > cases as well. Yes we have too! Qemu implements device model and the way devices communicates with CPU is well defined and called interrupts, so we have a way to inject interrupts (KVM_IRQ_LINE/KVM_INTERRUPT). Input is validated and passed into VCPU in the right time, we do not inject interrupts directly into VCPU using event injection. Exceptions, on the other hand, is completely internal CPU thing. QEMU shouldn't be a part of CPU emulation. > > And as we have true event injection now, and as it naturally conflicts Now we have a bug that should be fixed ASAP. We should allow setting of some VCPU state only when VCPU is stopped and only for migration/reset purposes. > with the special KVM_SET_GUEST_DEBUG interface, I have a patch that > consolidates this usage for QEMU: use the old interface of > SET_GUEST_DEBUG for pre-2.6.33 kernels, switch to SET_VCPU_EVENTS on > recent ones. Don't do that please, this will encourage use of SET_VCPU_EVENTS for something it shouldn't be used for. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
