On Wed, Apr 05, 2017 at 04:11:40PM +0200, Radim Krčmář wrote: > 2017-04-04 19:23+0200, Christoffer Dall: > > On Tue, Apr 04, 2017 at 07:06:00PM +0200, Andrew Jones wrote: > >> On Tue, Apr 04, 2017 at 05:24:03PM +0200, Christoffer Dall wrote: > >> > On Fri, Mar 31, 2017 at 06:06:51PM +0200, Andrew Jones wrote: > >> > > +and will definitely see the request, or is outside guest mode, but has yet > >> > > +to do its final request check, and therefore when it does, it will see the > >> > > +request, then things will work. However, the transition from outside to > >> > > +inside guest mode, after the last request check has been made, opens a > >> > > +window where a request could be made, but the VCPU would not see until it > >> > > +exits guest mode some time later. See the table below. > >> > > >> > This text, and the table below, only deals with the details of entering > >> > the guest. Should we talk about kvm_vcpu_exiting_guest_mode() and > >> > anything related to exiting the guest? > >> > >> I think all !IN_GUEST_MODE should behave the same, so I was avoiding > >> the use of EXITING_GUEST_MODE and OUTSIDE_GUEST_MODE, which wouldn't be > >> hard to address, but then I'd also have to address > >> READING_SHADOW_PAGE_TABLES, which may complicate the document more than > >> necessary. I'm not sure we need to address a VCPU exiting guest mode, > >> other than making sure it's clear that a VCPU that exits must check > >> requests before it enters again. > > > > But the problem is that kvm_make_all_cpus_request() only sends IPIs to > > CPUs where the mode was different from OUTSIDE_GUEST_MODE, so there it's > > about !OUTSIDE_GUEST_MODE rather than !IN_GUEST_MODE, so there's some > > subtlety here which I feel like it's dangerous to paper over. > > Right, that needs fixing in the code. Really? I thought Paolo said that this is the intended behavior and semantics; non-urgent requests that should just be serviced before the next guest entry. Now I'm confused again. What did I miss? > > guest_mode is just an optimization that allows us to skip sending the > IPI when the VCPU is known to handle the request as soon as possible. > > IN_GUEST_MODE: we must force VM exit or the request could never be > handled > EXITING_GUEST_MODE: another request already forces the VM exit and > we're just waiting for the VCPU to notice our request > OUTSIDE_GUEST_MODE: KVM is going to notice our request without any > intervention > READING_SHADOW_PAGE_TABLES: same as OUTSIDE_GUEST_MODE -- rename to > unwieldly OUTSIDE_GUEST_MODE_READING_SHADOW_PAGE_TABLES? Again, I thought Paolo was arguing that EXITING_GUEST_MODE makes the whole thing work because you check that after checking requests? > > The kick is needed only in IN_GUEST_MODE and a wake up is needed in case > where the guest is halted OUTSIDE_GUEST_MODE ... > Hm, maybe we should add a halt state too? Wouldn't that be swait_active(&vcpu->wq) ? You could add a wrapper though. What I think you need is a way to distinguish the semantics of calling kvm_make_all_cpus_request(), perhaps by adding a 'bool wake_up' parameter. I also feel like it would be more reliable or easier to understand if kvm_make_all_cpus_request() called kvm_vcpu_kick() somehow, but there may be such an established and understood use of the differences between the two by other architectures that it's worse to introduce the churn of changing it. I don't know. Thanks, -Christoffer
