> On 11 Nov 2019, at 15:40, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > > On 11/11/19 10:16, Liran Alon wrote: >> - /* INITs are latched while in SMM */ >> - if ((is_smm(vcpu) || vcpu->arch.smi_pending) && >> + /* INITs are latched while CPU is in specific states */ >> + if ((kvm_vcpu_latch_init(vcpu) || vcpu->arch.smi_pending) && >> (mp_state->mp_state == KVM_MP_STATE_SIPI_RECEIVED || >> mp_state->mp_state == KVM_MP_STATE_INIT_RECEIVED)) >> goto out; > > Just a small doc clarification: > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 318046647fda..cacfe14717d6 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2707,7 +2707,8 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu) > return; > > /* > - * INITs are latched while CPU is in specific states. > + * INITs are latched while CPU is in specific states > + * (SMM, VMX non-root mode, SVM with GIF=0). I didn’t want this line of comment as it may diverge from the implementation of kvm_vcpu_latch_init(). That’s why I removed it. > * Because a CPU cannot be in these states immediately > * after it has processed an INIT signal (and thus in > * KVM_MP_STATE_INIT_RECEIVED state), just eat SIPIs > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 681544f8db31..11746534e209 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -8706,7 +8706,11 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, > mp_state->mp_state != KVM_MP_STATE_RUNNABLE) > goto out; > > - /* INITs are latched while CPU is in specific states */ > + /* > + * KVM_MP_STATE_INIT_RECEIVED means the processor is in > + * INIT state; latched init should be reported using > + * KVM_SET_VCPU_EVENTS, so reject it here. > + */ Yes this is a good comment. Thanks for adding it. > if ((kvm_vcpu_latch_init(vcpu) || vcpu->arch.smi_pending) && > (mp_state->mp_state == KVM_MP_STATE_SIPI_RECEIVED || > mp_state->mp_state == KVM_MP_STATE_INIT_RECEIVED)) > > > I'm not sure why you're removing the first hunk, it's just meant to > explain why it needs to be a kvm_x86_ops in case the reader is not > thinking about nested virtualization. > > Paolo >
