On Sun, Feb 24, 2013 at 01:55:07PM +0000, Zhang, Yang Z wrote:
> > contexts, but only two use locks.
> See following logic, I think the problem you mentioned will not happened with
> current logic.
>
> get lock
> if test_pir (this will ensure there is no in flight IPI for same interrupt.
> Since we are taking the lock now, no IPI will be sent before we release the
> lock and no pir->irr is performed by hardware for same interrupt.)
> return coalesced
> if test_irr
> return coalesced
> set_pir
> injected=true
> if t_a_s(on) && in guest mode
> send ipi
> unlock
>
>
> >>> I'd rather think about proper way to do lockless injection before
> >>> committing anything. The case where we care about correct injection
> >>> status is rare, but we always care about scalability and since we
> >>> violate the spec by reading vapic page while vcpu is in non-root
> >>> operation anyway the result may be incorrect with or without the lock.
> >>> Our use can was not in HW designers mind when they designed this thing
> >>> obviously :(
> >>
> >> Zhang, can you comment on whether reading vapic page with CPU in
> >> VMX-non root accessing it is safe?
> See 24.11.4
> In addition to data in the VMCS region itself, VMX non-root operation can be
> controlled by data structures that are
> referenced by pointers in a VMCS (for example, the I/O bitmaps). While the
> pointers to these data structures are
> parts of the VMCS, the data structures themselves are not. They are not
> accessible using VMREAD and VMWRITE
> but by ordinary memory writes.
> Software should ensure that each such data structure is modified only when no
> logical processor with a current
> VMCS that references it is in VMX non-root operation. Doing otherwise may
> lead to unpredictable behavior.
>
> This means the initial design of KVM is wrong. It should not to modify vIRR
> directly.
>
> The good thing is that reading is ok.
OK.
> >> Gleb, yes, a comment mentioning the race (instead of the spinlock) and
> >> explanation why its believed to be benign (given how the injection
> >> return value is interpreted) could also work. Its ugly, though... murphy
> >> is around.
> > The race above is not benign. It will report interrupt as coalesced
> > while in reality it is injected. This may cause to many interrupt to be
> > injected. If this happens rare enough ntp may be able to fix time drift
> > resulted from this.
> Please check the above logic. Does it have same problem? If yes, please point
> out.
1) set_pir must be test_and_set_bit() (so the lock at vcpu entry path can
be dropped).
2) if t_a_s(on) && in guest mode
send ipi
should be inverted:
if guest mode && t_a_s(on)
send ipi
So you avoid setting ON bit if guest is outside of guest mode.
--
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
