diff mbox

[3/6] Emulator: Inject #PF when page was not found

Message ID 1242375740-31222-4-git-send-email-agraf@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Alexander Graf May 15, 2009, 8:22 a.m. UTC
If we couldn't find a page on read_emulated, it might be a good
idea to tell the guest about that and inject a #PF.

We do the same already for write faults. I don't know why it was
not implemented for reads.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/x86/kvm/x86.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

Comments

Joerg Roedel May 15, 2009, 1:40 p.m. UTC | #1
On Fri, May 15, 2009 at 10:22:17AM +0200, Alexander Graf wrote:
> If we couldn't find a page on read_emulated, it might be a good
> idea to tell the guest about that and inject a #PF.
> 
> We do the same already for write faults. I don't know why it was
> not implemented for reads.

Have you checked that the emulator will never ever do speculative reads?
This may be the reason why the fault was not injected here.

> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/x86/kvm/x86.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5fcde2c..5aa1219 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2131,10 +2131,13 @@ static int emulator_read_emulated(unsigned long addr,
>  		goto mmio;
>  
>  	if (kvm_read_guest_virt(addr, val, bytes, vcpu)
> -				== X86EMUL_CONTINUE)
> +				== X86EMUL_CONTINUE) {
>  		return X86EMUL_CONTINUE;
> -	if (gpa == UNMAPPED_GVA)
> +	}
> +	if (gpa == UNMAPPED_GVA) {
> +		kvm_inject_page_fault(vcpu, addr, 0);
>  		return X86EMUL_PROPAGATE_FAULT;
> +	}
>  
>  mmio:
>  	/*
> -- 
> 1.6.0.2
> 
>
Avi Kivity May 17, 2009, 7:59 p.m. UTC | #2
Alexander Graf wrote:
> If we couldn't find a page on read_emulated, it might be a good
> idea to tell the guest about that and inject a #PF.
>
> We do the same already for write faults. I don't know why it was
> not implemented for reads.
>
>   

I can't think why it was done for writes.  Normally, a guest page fault 
would be trapped and reflected a long time before emulation, in 
FNAME(page_fault)(), after walk_addr().

Can you give some details on the situation?  What instruction was 
executed, and why kvm tried to emulate it?

(I guess it depends on the relative priority of svm instruction 
intercepts and the page fault intercept?)
Alexander Graf May 17, 2009, 8:25 p.m. UTC | #3
On 17.05.2009, at 21:59, Avi Kivity <avi@redhat.com> wrote:

> Alexander Graf wrote:
>> If we couldn't find a page on read_emulated, it might be a good
>> idea to tell the guest about that and inject a #PF.
>>
>> We do the same already for write faults. I don't know why it was
>> not implemented for reads.
>>
>>
>
> I can't think why it was done for writes.  Normally, a guest page  
> fault would be trapped and reflected a long time before emulation,  
> in FNAME(page_fault)(), after walk_addr().
>
> Can you give some details on the situation?  What instruction was  
> executed, and why kvm tried to emulate it?

I remember it was something about accessing the apic with npt. Maybe  
the real problem was the restricted bit checking that made the  
emulated instruction behave differently from the real mmu.

I really need to start writing down why I did things when doing them :).

I can recheck if it still breaks without the inject.

Alex

>
>
> (I guess it depends on the relative priority of svm instruction  
> intercepts and the page fault intercept?)
>
> -- 
> Do not meddle in the internals of kernels, for they are subtle and  
> quick to panic.
>
--
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
Avi Kivity May 17, 2009, 8:58 p.m. UTC | #4
Alexander Graf wrote:
>>
>> I can't think why it was done for writes.  Normally, a guest page 
>> fault would be trapped and reflected a long time before emulation, in 
>> FNAME(page_fault)(), after walk_addr().
>>
>> Can you give some details on the situation?  What instruction was 
>> executed, and why kvm tried to emulate it?
>
> I remember it was something about accessing the apic with npt. Maybe 
> the real problem was the restricted bit checking that made the 
> emulated instruction behave differently from the real mmu.

The apic should not be mapped by Hyper-V's shadow page tables, so this 
should have been handled by page_fault().
Alexander Graf May 18, 2009, 12:55 p.m. UTC | #5
On 17.05.2009, at 22:58, Avi Kivity wrote:

> Alexander Graf wrote:
>>>
>>> I can't think why it was done for writes.  Normally, a guest page  
>>> fault would be trapped and reflected a long time before emulation,  
>>> in FNAME(page_fault)(), after walk_addr().
>>>
>>> Can you give some details on the situation?  What instruction was  
>>> executed, and why kvm tried to emulate it?
>>
>> I remember it was something about accessing the apic with npt.  
>> Maybe the real problem was the restricted bit checking that made  
>> the emulated instruction behave differently from the real mmu.
>
> The apic should not be mapped by Hyper-V's shadow page tables, so  
> this should have been handled by page_fault().

I think I only had to include this to find out that the restricted bit  
was checked for, so I got a blue screen in the guest :-).
Hyper-V works fine without this patch on NPT.

Alex
--
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 mbox

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5fcde2c..5aa1219 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2131,10 +2131,13 @@  static int emulator_read_emulated(unsigned long addr,
 		goto mmio;
 
 	if (kvm_read_guest_virt(addr, val, bytes, vcpu)
-				== X86EMUL_CONTINUE)
+				== X86EMUL_CONTINUE) {
 		return X86EMUL_CONTINUE;
-	if (gpa == UNMAPPED_GVA)
+	}
+	if (gpa == UNMAPPED_GVA) {
+		kvm_inject_page_fault(vcpu, addr, 0);
 		return X86EMUL_PROPAGATE_FAULT;
+	}
 
 mmio:
 	/*