| Message ID | 1409670830-14544-3-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Ah, here you add emulation of these bits. On Tue, Sep 02, 2014 at 05:13:48PM +0200, Paolo Bonzini wrote: > This is similar to what the EPT code does with the exit qualification. > This allows the guest to see a valid value for bits 33:32. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/paging_tmpl.h | 6 ++++++ > arch/x86/kvm/svm.c | 26 ++++++++++++++++++++++---- > 2 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index 410776528265..99d4c4e836a0 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -322,8 +322,14 @@ retry_walk: > > real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn), > PFERR_USER_MASK|PFERR_WRITE_MASK); > + > + /* > + * Can this happen (except if the guest is playing TOCTTOU games)? > + * We should have gotten a nested page fault on table_gfn instead. > + */ Comment is true, but doesn't make the check below obsolete, no? > if (unlikely(real_gfn == UNMAPPED_GVA)) > goto error; > @@ -1974,10 +1974,28 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu, > { > struct vcpu_svm *svm = to_svm(vcpu); > > - svm->vmcb->control.exit_code = SVM_EXIT_NPF; > - svm->vmcb->control.exit_code_hi = 0; > - svm->vmcb->control.exit_info_1 = fault->error_code; > - svm->vmcb->control.exit_info_2 = fault->address; > + /* > + * We can keep the value that the processor stored in the VMCB, > + * but make up something sensible if we hit the WARN. > + */ > + if (WARN_ON(svm->vmcb->control.exit_code != SVM_EXIT_NPF)) { > + svm->vmcb->control.exit_code = SVM_EXIT_NPF; > + svm->vmcb->control.exit_code_hi = 0; > + svm->vmcb->control.exit_info_1 = (1ULL << 32); > + svm->vmcb->control.exit_info_2 = fault->address; > + } Its been a while since I looked into this, but is an injected NPF exit always the result of a real NPF exit? How about an io-port emulated on L1 but passed through to L2 by the nested hypervisor. On emulation of INS or OUTS, KVM would need to read/write to an L2 address space, maybe causing NPF faults to be injected. In this case an IOIO exit would cause an injected NPF exit for L1. Joerg -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 02/09/2014 18:33, Joerg Roedel ha scritto: > Ah, here you add emulation of these bits. > > On Tue, Sep 02, 2014 at 05:13:48PM +0200, Paolo Bonzini wrote: >> This is similar to what the EPT code does with the exit qualification. >> This allows the guest to see a valid value for bits 33:32. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> arch/x86/kvm/paging_tmpl.h | 6 ++++++ >> arch/x86/kvm/svm.c | 26 ++++++++++++++++++++++---- >> 2 files changed, 28 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h >> index 410776528265..99d4c4e836a0 100644 >> --- a/arch/x86/kvm/paging_tmpl.h >> +++ b/arch/x86/kvm/paging_tmpl.h >> @@ -322,8 +322,14 @@ retry_walk: >> >> real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn), >> PFERR_USER_MASK|PFERR_WRITE_MASK); >> + >> + /* >> + * Can this happen (except if the guest is playing TOCTTOU games)? >> + * We should have gotten a nested page fault on table_gfn instead. >> + */ > > Comment is true, but doesn't make the check below obsolete, no? No, it doesn't. I'll rewrite it as /* * This cannot happen unless the guest is playing TOCTTOU games, * because we would have gotten a nested page fault on table_gfn * instead. If this happens, the exit qualification / exit info * field will incorrectly have "guest page access" as the * nested page fault's cause, instead of "guest page structure * access". */ >> if (unlikely(real_gfn == UNMAPPED_GVA)) >> goto error; >> @@ -1974,10 +1974,28 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu, >> { >> struct vcpu_svm *svm = to_svm(vcpu); >> >> - svm->vmcb->control.exit_code = SVM_EXIT_NPF; >> - svm->vmcb->control.exit_code_hi = 0; >> - svm->vmcb->control.exit_info_1 = fault->error_code; >> - svm->vmcb->control.exit_info_2 = fault->address; >> + /* >> + * We can keep the value that the processor stored in the VMCB, >> + * but make up something sensible if we hit the WARN. >> + */ >> + if (WARN_ON(svm->vmcb->control.exit_code != SVM_EXIT_NPF)) { >> + svm->vmcb->control.exit_code = SVM_EXIT_NPF; >> + svm->vmcb->control.exit_code_hi = 0; >> + svm->vmcb->control.exit_info_1 = (1ULL << 32); >> + svm->vmcb->control.exit_info_2 = fault->address; >> + } > > Its been a while since I looked into this, but is an injected NPF exit > always the result of a real NPF exit? I think so, but that's why I CCed you. :) > How about an io-port emulated on > L1 but passed through to L2 by the nested hypervisor. On emulation of > INS or OUTS, KVM would need to read/write to an L2 address space, It would need to read/write to *L1* (that's where the VMCB's IOIO map lies), which could result into a regular page fault injected into L1. Paolo > maybe > causing NPF faults to be injected. In this case an IOIO exit would cause > an injected NPF exit for L1. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 02/09/2014 18:46, Paolo Bonzini ha scritto: >> > How about an io-port emulated on >> > L1 but passed through to L2 by the nested hypervisor. On emulation of >> > INS or OUTS, KVM would need to read/write to an L2 address space, > It would need to read/write to *L1* (that's where the VMCB's IOIO map > lies), which could result into a regular page fault injected into L1. Nevermind, the VMCB's IO bitmap is a guest physical address. I see what you mean now. nested_svm_intercept_ioio would return NESTED_EXIT_HOST and proceed to emulate the read/write. This could indeed cause a NPF. For now I'll remove the WARN_ON. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 02, 2014 at 06:46:06PM +0200, Paolo Bonzini wrote: > Il 02/09/2014 18:33, Joerg Roedel ha scritto: > > Comment is true, but doesn't make the check below obsolete, no? > > No, it doesn't. I'll rewrite it as > > /* > * This cannot happen unless the guest is playing TOCTTOU games, > * because we would have gotten a nested page fault on table_gfn > * instead. If this happens, the exit qualification / exit info > * field will incorrectly have "guest page access" as the > * nested page fault's cause, instead of "guest page structure > * access". > */ Sounds good. > > Its been a while since I looked into this, but is an injected NPF exit > > always the result of a real NPF exit? > > I think so, but that's why I CCed you. :) > > > How about an io-port emulated on > > L1 but passed through to L2 by the nested hypervisor. On emulation of > > INS or OUTS, KVM would need to read/write to an L2 address space, > > It would need to read/write to *L1* (that's where the VMCB's IOIO map > lies), which could result into a regular page fault injected into L1. Hmm, INS and OUTS read/write to a virtual memory location, right? So if we have an io-port intercepted by bare-metal KVM but not by the L1 hypervisor, and the L2 guest accesses this io-port, bare-metal KVM would get an IOIO exit it has to emulate itself (it can't inject an IOIO exit to L1 anyway, since L1 does not intercept the io-port). During emulation the bare-metal KVM would read/write at an L2 virtual address, because that is the context the instruction is executed in. And while resolving this L2 virtual address, an NPF fault could be detected that needs to be injected into the L1 hypervisor, right? Joerg -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/02/2014 07:46 PM, Paolo Bonzini wrote: > */ >>> if (unlikely(real_gfn == UNMAPPED_GVA)) >>> goto error; >>> @@ -1974,10 +1974,28 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu, >>> { >>> struct vcpu_svm *svm = to_svm(vcpu); >>> >>> - svm->vmcb->control.exit_code = SVM_EXIT_NPF; >>> - svm->vmcb->control.exit_code_hi = 0; >>> - svm->vmcb->control.exit_info_1 = fault->error_code; >>> - svm->vmcb->control.exit_info_2 = fault->address; >>> + /* >>> + * We can keep the value that the processor stored in the VMCB, >>> + * but make up something sensible if we hit the WARN. >>> + */ >>> + if (WARN_ON(svm->vmcb->control.exit_code != SVM_EXIT_NPF)) { >>> + svm->vmcb->control.exit_code = SVM_EXIT_NPF; >>> + svm->vmcb->control.exit_code_hi = 0; >>> + svm->vmcb->control.exit_info_1 = (1ULL << 32); >>> + svm->vmcb->control.exit_info_2 = fault->address; >>> + } >> Its been a while since I looked into this, but is an injected NPF exit >> always the result of a real NPF exit? > I think so, but that's why I CCed you. :) It could always be the result of emulation into which L0 was tricked. I don't think it's a safe assumption. >> How about an io-port emulated on >> L1 but passed through to L2 by the nested hypervisor. On emulation of >> INS or OUTS, KVM would need to read/write to an L2 address space, > It would need to read/write to *L1* (that's where the VMCB's IOIO map > lies), which could result into a regular page fault injected into L1. > > Paolo > >> maybe >> causing NPF faults to be injected. In this case an IOIO exit would cause >> an injected NPF exit for L1. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h index 410776528265..99d4c4e836a0 100644 --- a/arch/x86/kvm/paging_tmpl.h +++ b/arch/x86/kvm/paging_tmpl.h @@ -322,8 +322,14 @@ retry_walk: real_gfn = mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn), PFERR_USER_MASK|PFERR_WRITE_MASK); + + /* + * Can this happen (except if the guest is playing TOCTTOU games)? + * We should have gotten a nested page fault on table_gfn instead. + */ if (unlikely(real_gfn == UNMAPPED_GVA)) goto error; + real_gfn = gpa_to_gfn(real_gfn); host_addr = gfn_to_hva_prot(vcpu->kvm, real_gfn, diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 8ef704037370..bd78a3baca31 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1974,10 +1974,28 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu, { struct vcpu_svm *svm = to_svm(vcpu); - svm->vmcb->control.exit_code = SVM_EXIT_NPF; - svm->vmcb->control.exit_code_hi = 0; - svm->vmcb->control.exit_info_1 = fault->error_code; - svm->vmcb->control.exit_info_2 = fault->address; + /* + * We can keep the value that the processor stored in the VMCB, + * but make up something sensible if we hit the WARN. + */ + if (WARN_ON(svm->vmcb->control.exit_code != SVM_EXIT_NPF)) { + svm->vmcb->control.exit_code = SVM_EXIT_NPF; + svm->vmcb->control.exit_code_hi = 0; + svm->vmcb->control.exit_info_1 = (1ULL << 32); + svm->vmcb->control.exit_info_2 = fault->address; + } + + /* + * The present bit is always zero for page structure faults on real + * hardware, but we compute it as 1 in fault->error_code. We can + * just keep the original exit info for page structure faults---but + * not for other faults, because the processor's error code might + * be wrong if we have just created a shadow PTE. + */ + if (svm->vmcb->control.exit_info_1 & (1ULL << 32)) { + svm->vmcb->control.exit_info_1 &= ~0xffffffffULL; + svm->vmcb->control.exit_info_1 |= fault->error_code; + } nested_svm_vmexit(svm); }
This is similar to what the EPT code does with the exit qualification. This allows the guest to see a valid value for bits 33:32. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/kvm/paging_tmpl.h | 6 ++++++ arch/x86/kvm/svm.c | 26 ++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-)