On Fri, Oct 17, 2008 at 07:40:00PM +0200, Jan Kiszka wrote: > Sheng Yang wrote: > > On Fri, Oct 17, 2008 at 07:23:01PM +0200, Jan Kiszka wrote: > >> Sheng Yang wrote: > >>> On Wed, Oct 15, 2008 at 04:27:51PM +0200, Jan Kiszka wrote: > >>>> As suggested by Avi, this patch introduces a counter of VCPUs that have > >>>> LVT0 set to NMI mode. Only if the counter > 0, we push the PIT ticks via > >>>> all LAPIC LVT0 lines to enable NMI watchdog support. > >>>> > >>> I feel a little strange about: if *counter > 0*, we push to *all*. Can we > >>> only push NMIs to the ones that set NMI for LVT0? > >> We don't do that due to !kvm_apic_accept_pic_intr(). The counter is only > >> about optimizing that case where we don't have to walk the whole chain, > >> asking every vcpu if it would like to receive the IRQ. > > > > I don't agree to use kvm_apic_accept_pic_intr() here, as I explained in the > > first mail. It's not a normal path, and current KVM handle it well. > > Current KVM only support PIC Mode, which is fine, but not sufficient for > NMI watchdog support. We need to get the Virtual Wire Mode in, but > correctly. > > >>> How about add a field in struct kvm_lapic? We can quickly know if we need > >>> to > >>> inject NMI for this vcpu. Well, though kernel mostly enable NMI watchdog > >>> on > >>> all vcpu, I think this is more precise, and match the logic, and avoid one > >>> more field in kvm_arch... > >> The point of this patch is to avoid touching vcpu structures AT ALL when > >> there is no interest in the NMI watchdog (normally, OSes will either > >> enable the WD trick for all CPUSs or keep it off). > > > > Logically, I think lapic is more proper place. And put a bool there won't > > affect much. I think we can do it more straightly here. > > If you have dozens of lapics, you don't want to check them all if they > are ALL switched of anyway. That information is better encoded in a > single, (virtual) system-wide bool. That's the most common case we want > to speed up. And it is the core of the optimization Avi suggested > (unless I totally misunderstood him).
Yeah, I am agree on this point now. But for the above one, NO... :) Using apic_local_deliver() also means I ignored PIC and make it transpent. Please don't involve it in again. It's *not* the normal usage. I want to keep the impact as small as possible. -- regards Yang, Sheng > Jan > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
