Re: [PATCH 16/22] KVM: MMU: Track page fault data in struct vcpu

[Joerg Roedel <joerg.roedel@amd.com>]

On Tue, Apr 27, 2010 at 03:58:36PM +0300, Avi Kivity wrote:
> On 04/27/2010 01:38 PM, Joerg Roedel wrote:
> >This patch introduces two fields in vcpu_arch for x86:
> >
> >	* fault_address
> >	* fault_error_code
> >
> >This will be used to correctly propagate page faults back
> >into the guest when we could have either an ordinary page
> >fault or a nested page fault. In the case of a nested page
> >fault the fault-address is different from the original
> >address that should be walked. So we need to keep track
> >about the real fault-address.
> >We could also remove the current path of the error_code to
> >the fault. But this change is too invasive and outside the
> >scope of this patch set. It will be changed and tested
> >seperatly.
> >
> >Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> >---
> >  arch/x86/include/asm/kvm_host.h |    3 +++
> >  arch/x86/kvm/paging_tmpl.h      |    4 ++++
> >  2 files changed, 7 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> >index d9dfc8c..8426870 100644
> >--- a/arch/x86/include/asm/kvm_host.h
> >+++ b/arch/x86/include/asm/kvm_host.h
> >@@ -298,6 +298,9 @@ struct kvm_vcpu_arch {
> >  	/* Used for two dimensional paging emulation */
> >  	struct kvm_mmu nested_mmu;
> >
> >+	unsigned long fault_address;
> 
> Probably a problem on i386.  How does npt handle faults when the
> guest is using pae paging and the host (in our case the guest...)
> isn't?  I see it uses exit_info_2 for the address, which is a u64.

This shouldn't be an issue. If we run on 32bit host with nested paging
the guest can't have more than 4gb of addressable memory because of the
page table limitations (nested page table is always in host format).

> So we probably need to upgrade gva_t to a u64.  Please send this as
> a separate patch, and test on i386 hosts.
> 
> >+	int fault_error_code;
> 
> unsigned.

Right. I'll fix it.

> Maybe put the two in a struct, easier to pass around.

Yeah, makes it easier to read.

	Joerg