On 04/24/2012 05:19 PM, Avi Kivity wrote: >>> Turned out to be simpler than expected. However, I think there's a problem >>> with make_all_cpus_request() possible reading an incorrect vcpu->cpu. >> >> >> It seems possible. >> >> Can we fix it by reading vcpu->cpu when the vcpu is in GUEST_MODE or >> EXITING_GUEST_MODE (IIRC, in these modes, interrupt is disabled)? >> >> Like: >> >> if (kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE) >> cpumask_set_cpu(vcpu->cpu, cpus); > > I think it is actually okay. We are only vulnerable if lockless shadow > walk started during prepare_zap_page(), and extends past > kvm_flush_remote_tlbs(), yes? But in that case, vcpu->cpu is stable > since local_irq_disable() kills preemption. > This case can happen? VCPU 0 VCPU 1 kvm_for_each_vcpu(i, vcpu, kvm) { kvm_make_request(req, vcpu); VCPU1 is running on CPU 1 out of guest mode cpu = vcpu->cpu; /* Set ->requests bit before we read ->mode */ smp_mb(); if (cpus != NULL && cpu != -1 && cpu != me && VCPU1 is scheduled to CPU 2, and running in guest mode kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE) cpumask_set_cpu(cpu, cpus); } VCPU 0 send IPI to CPU1, but actually, VCPU1 is running on CPU 2. -- 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