| Message ID | 20240712144841.1230591-1-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Suppress MMIO that is triggered during task switch emulation | expand |
On Fri, Jul 12, 2024 at 07:48:41AM -0700, Sean Christopherson wrote: [...] > See commit 0dc902267cb3 ("KVM: x86: Suppress pending MMIO write exits if > emulator detects exception") for more details on KVM's limitations with > respect to emulated MMIO during complex emulator flows. > I try to understand the changelog of commit 0dc902267cb3 but I’m confused with the MMIO read. The commit said, "For MMIO reads, KVM immediately exits to userspace upon detecting MMIO as userspace provides the to-be-read value in a buffer, and so KVM can safely (more or less) restart the instruction from the beginning." But in read_emulated(), mc->end is adjusted after checking rc, i.e., although the value will be saved in the buffer, mc->end is not adjusted after existing to userspace. Maybe this would really support a buffer for multiple MMIO read instructions (e.g. POPA)? diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 5d4c86133453..841d5b6f21b0 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1367,8 +1367,11 @@ static int read_emulated(struct x86_emulate_ctxt *ctxt, rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size, &ctxt->exception); - if (rc != X86EMUL_CONTINUE) + if (rc != X86EMUL_CONTINUE) { + if (rc == X86EMUL_IO_NEEDED) + mc->end += size; return rc; + } mc->end += size; [...]
Queued, thanks. Paolo
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 994743266480..47bd8a9fdb21 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11803,7 +11803,13 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int idt_index, ret = emulator_task_switch(ctxt, tss_selector, idt_index, reason, has_error_code, error_code); - if (ret) { + + /* + * Report an error userspace if MMIO is needed, as KVM doesn't support + * MMIO during a task switch (or any other complex operation). + */ + if (ret || vcpu->mmio_needed) { + vcpu->mmio_needed = false; vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR; vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION; vcpu->run->internal.ndata = 0;
Explicitly suppress userspace emulated MMIO exits that are triggered when emulating a task switch as KVM doesn't support userspace MMIO during complex (multi-step) emulation. Silently ignoring the exit request can result in the WARN_ON_ONCE(vcpu->mmio_needed) firing if KVM exits to userspace for some other reason prior to purging mmio_needed. See commit 0dc902267cb3 ("KVM: x86: Suppress pending MMIO write exits if emulator detects exception") for more details on KVM's limitations with respect to emulated MMIO during complex emulator flows. Reported-by: syzbot+2fb9f8ed752c01bc9a3f@syzkaller.appspotmail.com Signed-off-by: Sean Christopherson <seanjc@google.com> --- This is from a syzkaller report on a Google-internal kernel, but it repros on upstream (obviously). There are unfortunately an absurd number of upstream reports with "WARNING in kvm_arch_vcpu_ioctl_run" as the title, so I haven't been able to hunt down an upstream report. arch/x86/kvm/x86.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) base-commit: 771df9ffadb8204e61d3e98f36c5067102aab78f