| Message ID | 5032F926.7020801@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/21/2012 05:57 AM, Xiao Guangrong wrote: > Currently, we reexecute all unhandleable instructions if they do not > access on the mmio, however, it can not work if host map the readonly > memory to guest. If the instruction try to write this kind of memory, > it will fault again when guest retry it, then we will goto a infinite > loop: retry instruction -> write #PF -> emulation fail -> > retry instruction -> ... > > Fix it by retrying the instruction only when it faults on the writable > memory > > Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> > --- > arch/x86/kvm/x86.c | 12 +++++++++++- > 1 files changed, 11 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index fb0d937..704680d 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4473,6 +4473,7 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu) > static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) > { > gpa_t gpa; > + pfn_t pfn; > > if (tdp_enabled) > return false; > @@ -4490,8 +4491,17 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) > if (gpa == UNMAPPED_GVA) > return true; /* let cpu generate fault */ > > - if (!kvm_is_error_hva(gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT))) > + /* > + * Do not retry the unhandleable instruction if it faults on the > + * readonly host memory, otherwise it will goto a infinite loop: > + * retry instruction -> write #PF -> emulation fail -> retry > + * instruction -> ... > + */ > + pfn = gfn_to_pfn(vcpu->kvm, gpa_to_gfn(gpa)); > + if (!is_error_pfn(pfn)) { > + kvm_release_pfn_clean(pfn); > return true; > + } > > return false; > } > Good catch. Did this actually happen or did you find it by code inspection?
On 08/22/2012 08:01 PM, Avi Kivity wrote: > On 08/21/2012 05:57 AM, Xiao Guangrong wrote: >> Currently, we reexecute all unhandleable instructions if they do not >> access on the mmio, however, it can not work if host map the readonly >> memory to guest. If the instruction try to write this kind of memory, >> it will fault again when guest retry it, then we will goto a infinite >> loop: retry instruction -> write #PF -> emulation fail -> >> retry instruction -> ... >> >> Fix it by retrying the instruction only when it faults on the writable >> memory >> >> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> >> --- >> arch/x86/kvm/x86.c | 12 +++++++++++- >> 1 files changed, 11 insertions(+), 1 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index fb0d937..704680d 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -4473,6 +4473,7 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu) >> static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) >> { >> gpa_t gpa; >> + pfn_t pfn; >> >> if (tdp_enabled) >> return false; >> @@ -4490,8 +4491,17 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) >> if (gpa == UNMAPPED_GVA) >> return true; /* let cpu generate fault */ >> >> - if (!kvm_is_error_hva(gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT))) >> + /* >> + * Do not retry the unhandleable instruction if it faults on the >> + * readonly host memory, otherwise it will goto a infinite loop: >> + * retry instruction -> write #PF -> emulation fail -> retry >> + * instruction -> ... >> + */ >> + pfn = gfn_to_pfn(vcpu->kvm, gpa_to_gfn(gpa)); >> + if (!is_error_pfn(pfn)) { >> + kvm_release_pfn_clean(pfn); >> return true; >> + } >> >> return false; >> } >> > > Good catch. Did this actually happen or did you find it by code inspection? > It did not happen in my test. Actually, it is reported by Marcelo. -- 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/x86.c b/arch/x86/kvm/x86.c index fb0d937..704680d 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4473,6 +4473,7 @@ static int handle_emulation_failure(struct kvm_vcpu *vcpu) static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) { gpa_t gpa; + pfn_t pfn; if (tdp_enabled) return false; @@ -4490,8 +4491,17 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) if (gpa == UNMAPPED_GVA) return true; /* let cpu generate fault */ - if (!kvm_is_error_hva(gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT))) + /* + * Do not retry the unhandleable instruction if it faults on the + * readonly host memory, otherwise it will goto a infinite loop: + * retry instruction -> write #PF -> emulation fail -> retry + * instruction -> ... + */ + pfn = gfn_to_pfn(vcpu->kvm, gpa_to_gfn(gpa)); + if (!is_error_pfn(pfn)) { + kvm_release_pfn_clean(pfn); return true; + } return false; }
Currently, we reexecute all unhandleable instructions if they do not access on the mmio, however, it can not work if host map the readonly memory to guest. If the instruction try to write this kind of memory, it will fault again when guest retry it, then we will goto a infinite loop: retry instruction -> write #PF -> emulation fail -> retry instruction -> ... Fix it by retrying the instruction only when it faults on the writable memory Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> --- arch/x86/kvm/x86.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)