Re: [PATCH] [2/2] KVM: Add VT-x machine check support
On Thu, Jun 04, 2009 at 02:48:17PM +0300, Avi Kivity wrote: Andi Kleen wrote: [Avi could you please still consider this patch for your 2.6.31 patchqueue? It's fairly simple, but important to handle memory errors in guests] Oh yes, and it'll be needed for -stable. IIUC, right now a machine check is trapped by the guest, so the guest is killed instead of the host? Yes the guest will receive int 18. But it will not kill itmelf because the guest cannot access the machine check MSRs, so it will not see any machine check. So it's kind of ignored, which is pretty bad. +/* + * Trigger machine check on the host. We assume all the MSRs are already set up + * by the CPU and that we still run on the same CPU as the MCE occurred on. + * We pass a fake environment to the machine check handler because we want + * the guest to be always treated like user space, no matter what context + * it used internally. + */ This assumption is incorrect. This code is executed after preemption has been enabled, and we may have even slept before reaching it. The only thing that counts here is the context before the machine check event. If there was a vmexit we know it was in guest context. The only requirement we have is that we're running still on the same CPU. I assume that's true, otherwise the vmcb accesses wouldn't work? [EXIT_REASON_EPT_VIOLATION] = handle_ept_violation, +[EXIT_REASON_MACHINE_CHECK] = handle_machine_check, }; static const int kvm_vmx_max_exit_handlers = We get both an explicit EXIT_REASON and an exception? These are different cases. The exception is #MC in guest context, the EXIT_REASON is when a #MC happens while the CPU is executing the VM entry microcode. -Andi -- a...@linux.intel.com -- Speaking for myself only. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2/2] KVM: Add VT-x machine check support
Andi Kleen wrote: This assumption is incorrect. This code is executed after preemption has been enabled, and we may have even slept before reaching it. The only thing that counts here is the context before the machine check event. If there was a vmexit we know it was in guest context. The only requirement we have is that we're running still on the same CPU. I assume that's true, otherwise the vmcb accesses wouldn't work? It's not true, we're in preemptible context and may have even slept. vmcs access work because we have a preempt notifier called when we are scheduled in, and will execute vmclear/vmptrld as necessary. Look at kvm_preempt_ops in virt/kvm_main.c. We get both an explicit EXIT_REASON and an exception? These are different cases. The exception is #MC in guest context, the EXIT_REASON is when a #MC happens while the CPU is executing the VM entry microcode. I see, thanks. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2/2] KVM: Add VT-x machine check support
On Thu, Jun 04, 2009 at 03:49:03PM +0300, Avi Kivity wrote: Andi Kleen wrote: This assumption is incorrect. This code is executed after preemption has been enabled, and we may have even slept before reaching it. The only thing that counts here is the context before the machine check event. If there was a vmexit we know it was in guest context. The only requirement we have is that we're running still on the same CPU. I assume that's true, otherwise the vmcb accesses wouldn't work? It's not true, we're in preemptible context and may have even slept. vmcs access work because we have a preempt notifier called when we are scheduled in, and will execute vmclear/vmptrld as necessary. Look at kvm_preempt_ops in virt/kvm_main.c. I see. So we need to move that check earlier. Do you have a preference where it should be? -Andi -- a...@linux.intel.com -- Speaking for myself only. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2/2] KVM: Add VT-x machine check support
Andi Kleen wrote: vmcs access work because we have a preempt notifier called when we are scheduled in, and will execute vmclear/vmptrld as necessary. Look at kvm_preempt_ops in virt/kvm_main.c. I see. So we need to move that check earlier. Do you have a preference where it should be? There's no good place as it breaks the nice exit handler table. You could put it in vmx_complete_interrupts() next to NMI handling. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2/2] KVM: Add VT-x machine check support
Andi Kleen wrote: There's no good place as it breaks the nice exit handler table. You could put it in vmx_complete_interrupts() next to NMI handling. I think I came up with a easy cheesy but not too bad solution now that should work. It simply remembers the CPU in the vcpu structure and schedules back to it. That's fine for this purpose. We might be able schedule back in a timely manner. Why not hack vmx_complete_interrupts()? You're still in the critical section so you're guaranteed no delays or surprises. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [2/2] KVM: Add VT-x machine check support
On Thu, Jun 04, 2009 at 04:49:50PM +0300, Avi Kivity wrote: Andi Kleen wrote: There's no good place as it breaks the nice exit handler table. You could put it in vmx_complete_interrupts() next to NMI handling. I think I came up with a easy cheesy but not too bad solution now that should work. It simply remembers the CPU in the vcpu structure and schedules back to it. That's fine for this purpose. We might be able schedule back in a timely manner. Why not hack vmx_complete_interrupts()? You're still in the critical section so you're guaranteed no delays or surprises. Yes, have to do that. My original scheme was too risky because the Machine checks have synchronization mechanisms now and preemption has no time limit. I'll hack on it later today, hope fully have a patch tomorrow. -Andi -- a...@linux.intel.com -- Speaking for myself only. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
