[PATCH 1/10] KVM MMU: fix for calculating gpa in invlpg code

[PATCH 1/10] KVM MMU: fix for calculating gpa in invlpg code

[Xiao Guangrong]

If the guest is 32-bit, we should use 'quadrant' to adjust gpa
offset

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/x86/kvm/paging_tmpl.h |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index d0cc07e..46d80d6 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -478,9 +478,14 @@ static void FNAME(invlpg)(struct kvm_vcpu *vcpu, gva_t gva)
 		    ((level == PT_DIRECTORY_LEVEL && is_large_pte(*sptep))) ||
 		    ((level == PT_PDPE_LEVEL && is_large_pte(*sptep)))) {
 			struct kvm_mmu_page *sp = page_header(__pa(sptep));
+			int offset = 0;
+
+			if (PTTYPE == 32)
+				offset = sp->role.quadrant << PT64_LEVEL_BITS;;
 
 			pte_gpa = (sp->gfn << PAGE_SHIFT);
-			pte_gpa += (sptep - sp->spt) * sizeof(pt_element_t);
+			pte_gpa += (sptep - sp->spt + offset) *
+					sizeof(pt_element_t);
 
 			if (is_shadow_present_pte(*sptep)) {
 				rmap_remove(vcpu->kvm, sptep);
-- 
1.6.1.2

[PATCH 2/10] KVM MMU: convert mmu tracepoints

[Xiao Guangrong]

Convert mmu tracepoints by using DECLARE_EVENT_CLASS

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/x86/kvm/mmutrace.h |   69 +++++++++++++++++-----------------------------
 1 files changed, 26 insertions(+), 43 deletions(-)

diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index bc4f7f0..d860a03 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -92,15 +92,15 @@ TRACE_EVENT(
 	TP_printk("pte %llx level %u", __entry->pte, __entry->level)
 );
 
-/* We set a pte accessed bit */
-TRACE_EVENT(
-	kvm_mmu_set_accessed_bit,
+DECLARE_EVENT_CLASS(kvm_mmu_set_bit_class,
+
 	TP_PROTO(unsigned long table_gfn, unsigned index, unsigned size),
+
 	TP_ARGS(table_gfn, index, size),
 
 	TP_STRUCT__entry(
 		__field(__u64, gpa)
-		),
+	),
 
 	TP_fast_assign(
 		__entry->gpa = ((u64)table_gfn << PAGE_SHIFT)
@@ -110,22 +110,20 @@ TRACE_EVENT(
 	TP_printk("gpa %llx", __entry->gpa)
 );
 
-/* We set a pte dirty bit */
-TRACE_EVENT(
-	kvm_mmu_set_dirty_bit,
+/* We set a pte accessed bit */
+DEFINE_EVENT(kvm_mmu_set_bit_class, kvm_mmu_set_accessed_bit,
+
 	TP_PROTO(unsigned long table_gfn, unsigned index, unsigned size),
-	TP_ARGS(table_gfn, index, size),
 
-	TP_STRUCT__entry(
-		__field(__u64, gpa)
-		),
+	TP_ARGS(table_gfn, index, size)
+);
 
-	TP_fast_assign(
-		__entry->gpa = ((u64)table_gfn << PAGE_SHIFT)
-				+ index * size;
-		),
+/* We set a pte dirty bit */
+DEFINE_EVENT(kvm_mmu_set_bit_class, kvm_mmu_set_dirty_bit,
 
-	TP_printk("gpa %llx", __entry->gpa)
+	TP_PROTO(unsigned long table_gfn, unsigned index, unsigned size),
+
+	TP_ARGS(table_gfn, index, size)
 );
 
 TRACE_EVENT(
@@ -164,54 +162,39 @@ TRACE_EVENT(
 		  __entry->created ? "new" : "existing")
 );
 
-TRACE_EVENT(
-	kvm_mmu_sync_page,
+DECLARE_EVENT_CLASS(kvm_mmu_page_class,
+
 	TP_PROTO(struct kvm_mmu_page *sp),
 	TP_ARGS(sp),
 
 	TP_STRUCT__entry(
 		KVM_MMU_PAGE_FIELDS
-		),
+	),
 
 	TP_fast_assign(
 		KVM_MMU_PAGE_ASSIGN(sp)
-		),
+	),
 
 	TP_printk("%s", KVM_MMU_PAGE_PRINTK())
 );
 
-TRACE_EVENT(
-	kvm_mmu_unsync_page,
+DEFINE_EVENT(kvm_mmu_page_class, kvm_mmu_sync_page,
 	TP_PROTO(struct kvm_mmu_page *sp),
-	TP_ARGS(sp),
-
-	TP_STRUCT__entry(
-		KVM_MMU_PAGE_FIELDS
-		),
-
-	TP_fast_assign(
-		KVM_MMU_PAGE_ASSIGN(sp)
-		),
 
-	TP_printk("%s", KVM_MMU_PAGE_PRINTK())
+	TP_ARGS(sp)
 );
 
-TRACE_EVENT(
-	kvm_mmu_zap_page,
+DEFINE_EVENT(kvm_mmu_page_class, kvm_mmu_unsync_page,
 	TP_PROTO(struct kvm_mmu_page *sp),
-	TP_ARGS(sp),
 
-	TP_STRUCT__entry(
-		KVM_MMU_PAGE_FIELDS
-		),
+	TP_ARGS(sp)
+);
 
-	TP_fast_assign(
-		KVM_MMU_PAGE_ASSIGN(sp)
-		),
+DEFINE_EVENT(kvm_mmu_page_class, kvm_mmu_zap_page,
+	TP_PROTO(struct kvm_mmu_page *sp),
 
-	TP_printk("%s", KVM_MMU_PAGE_PRINTK())
+	TP_ARGS(sp)
 );
-
 #endif /* _TRACE_KVMMMU_H */
 
 #undef TRACE_INCLUDE_PATH
-- 
1.6.1.2

[PATCH 3/10] KVM MMU: move unsync/sync tracpoints to proper place

[Xiao Guangrong]

Move unsync/sync tracepoints to the proper place, it's good
for us to obtain unsync page live time

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/x86/kvm/mmu.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ddfa865..abf8bd4 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1189,6 +1189,7 @@ static struct kvm_mmu_page *kvm_mmu_lookup_page(struct kvm *kvm, gfn_t gfn)
 static void kvm_unlink_unsync_page(struct kvm *kvm, struct kvm_mmu_page *sp)
 {
 	WARN_ON(!sp->unsync);
+	trace_kvm_mmu_sync_page(sp);
 	sp->unsync = 0;
 	--kvm->stat.mmu_unsync;
 }
@@ -1202,7 +1203,6 @@ static int kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 		return 1;
 	}
 
-	trace_kvm_mmu_sync_page(sp);
 	if (rmap_write_protect(vcpu->kvm, sp->gfn))
 		kvm_flush_remote_tlbs(vcpu->kvm);
 	kvm_unlink_unsync_page(vcpu->kvm, sp);
@@ -1730,7 +1730,6 @@ static int kvm_unsync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 	struct kvm_mmu_page *s;
 	struct hlist_node *node, *n;
 
-	trace_kvm_mmu_unsync_page(sp);
 	index = kvm_page_table_hashfn(sp->gfn);
 	bucket = &vcpu->kvm->arch.mmu_page_hash[index];
 	/* don't unsync if pagetable is shadowed with multiple roles */
@@ -1740,6 +1739,7 @@ static int kvm_unsync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 		if (s->role.word != sp->role.word)
 			return 1;
 	}
+	trace_kvm_mmu_unsync_page(sp);
 	++vcpu->kvm->stat.mmu_unsync;
 	sp->unsync = 1;
 
-- 
1.6.1.2

[PATCH 4/10] KVM MMU: Move invlpg code out of paging_tmpl.h

[Xiao Guangrong]

Using '!sp->role.cr4_pae' replaces 'PTTYPE == 32' and using
'pte_size = sp->role.cr4_pae ? 8 : 4' replaces sizeof(pt_element_t)

Then no need compile twice for this code

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/x86/kvm/mmu.c         |   60 ++++++++++++++++++++++++++++++++++++++++++-
 arch/x86/kvm/paging_tmpl.h |   56 -----------------------------------------
 2 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index abf8bd4..fac7c09 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2256,6 +2256,62 @@ static bool is_rsvd_bits_set(struct kvm_vcpu *vcpu, u64 gpte, int level)
 	return (gpte & vcpu->arch.mmu.rsvd_bits_mask[bit7][level-1]) != 0;
 }
 
+static void paging_invlpg(struct kvm_vcpu *vcpu, gva_t gva)
+{
+	struct kvm_shadow_walk_iterator iterator;
+	gpa_t pte_gpa = -1;
+	int level;
+	u64 *sptep;
+	int need_flush = 0;
+	unsigned pte_size = 0;
+
+	spin_lock(&vcpu->kvm->mmu_lock);
+
+	for_each_shadow_entry(vcpu, gva, iterator) {
+		level = iterator.level;
+		sptep = iterator.sptep;
+
+		if (level == PT_PAGE_TABLE_LEVEL  ||
+		    ((level == PT_DIRECTORY_LEVEL && is_large_pte(*sptep))) ||
+		    ((level == PT_PDPE_LEVEL && is_large_pte(*sptep)))) {
+			struct kvm_mmu_page *sp = page_header(__pa(sptep));
+			int offset = 0;
+
+			if (!sp->role.cr4_pae)
+				offset = sp->role.quadrant << PT64_LEVEL_BITS;;
+			pte_size = sp->role.cr4_pae ? 8 : 4;
+			pte_gpa = (sp->gfn << PAGE_SHIFT);
+			pte_gpa += (sptep - sp->spt + offset) * pte_size;
+
+			if (is_shadow_present_pte(*sptep)) {
+				rmap_remove(vcpu->kvm, sptep);
+				if (is_large_pte(*sptep))
+					--vcpu->kvm->stat.lpages;
+				need_flush = 1;
+			}
+			__set_spte(sptep, shadow_trap_nonpresent_pte);
+			break;
+		}
+
+		if (!is_shadow_present_pte(*sptep))
+			break;
+	}
+
+	if (need_flush)
+		kvm_flush_remote_tlbs(vcpu->kvm);
+
+	atomic_inc(&vcpu->kvm->arch.invlpg_counter);
+
+	spin_unlock(&vcpu->kvm->mmu_lock);
+
+	if (pte_gpa == -1)
+		return;
+
+	if (mmu_topup_memory_caches(vcpu))
+		return;
+	kvm_mmu_pte_write(vcpu, pte_gpa, NULL, pte_size, 0);
+}
+
 #define PTTYPE 64
 #include "paging_tmpl.h"
 #undef PTTYPE
@@ -2335,7 +2391,7 @@ static int paging64_init_context_common(struct kvm_vcpu *vcpu, int level)
 	context->gva_to_gpa = paging64_gva_to_gpa;
 	context->prefetch_page = paging64_prefetch_page;
 	context->sync_page = paging64_sync_page;
-	context->invlpg = paging64_invlpg;
+	context->invlpg = paging_invlpg;
 	context->free = paging_free;
 	context->root_level = level;
 	context->shadow_root_level = level;
@@ -2360,7 +2416,7 @@ static int paging32_init_context(struct kvm_vcpu *vcpu)
 	context->free = paging_free;
 	context->prefetch_page = paging32_prefetch_page;
 	context->sync_page = paging32_sync_page;
-	context->invlpg = paging32_invlpg;
+	context->invlpg = paging_invlpg;
 	context->root_level = PT32_ROOT_LEVEL;
 	context->shadow_root_level = PT32E_ROOT_LEVEL;
 	context->root_hpa = INVALID_PAGE;
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 46d80d6..d0df9cd 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -460,62 +460,6 @@ out_unlock:
 	return 0;
 }
 
-static void FNAME(invlpg)(struct kvm_vcpu *vcpu, gva_t gva)
-{
-	struct kvm_shadow_walk_iterator iterator;
-	gpa_t pte_gpa = -1;
-	int level;
-	u64 *sptep;
-	int need_flush = 0;
-
-	spin_lock(&vcpu->kvm->mmu_lock);
-
-	for_each_shadow_entry(vcpu, gva, iterator) {
-		level = iterator.level;
-		sptep = iterator.sptep;
-
-		if (level == PT_PAGE_TABLE_LEVEL  ||
-		    ((level == PT_DIRECTORY_LEVEL && is_large_pte(*sptep))) ||
-		    ((level == PT_PDPE_LEVEL && is_large_pte(*sptep)))) {
-			struct kvm_mmu_page *sp = page_header(__pa(sptep));
-			int offset = 0;
-
-			if (PTTYPE == 32)
-				offset = sp->role.quadrant << PT64_LEVEL_BITS;;
-
-			pte_gpa = (sp->gfn << PAGE_SHIFT);
-			pte_gpa += (sptep - sp->spt + offset) *
-					sizeof(pt_element_t);
-
-			if (is_shadow_present_pte(*sptep)) {
-				rmap_remove(vcpu->kvm, sptep);
-				if (is_large_pte(*sptep))
-					--vcpu->kvm->stat.lpages;
-				need_flush = 1;
-			}
-			__set_spte(sptep, shadow_trap_nonpresent_pte);
-			break;
-		}
-
-		if (!is_shadow_present_pte(*sptep))
-			break;
-	}
-
-	if (need_flush)
-		kvm_flush_remote_tlbs(vcpu->kvm);
-
-	atomic_inc(&vcpu->kvm->arch.invlpg_counter);
-
-	spin_unlock(&vcpu->kvm->mmu_lock);
-
-	if (pte_gpa == -1)
-		return;
-
-	if (mmu_topup_memory_caches(vcpu))
-		return;
-	kvm_mmu_pte_write(vcpu, pte_gpa, NULL, sizeof(pt_element_t), 0);
-}
-
 static gpa_t FNAME(gva_to_gpa)(struct kvm_vcpu *vcpu, gva_t vaddr, u32 access,
 			       u32 *error)
 {
-- 
1.6.1.2

Re: [PATCH 1/10] KVM MMU: fix for calculating gpa in invlpg code

[Avi Kivity]

On 04/22/2010 09:12 AM, Xiao Guangrong wrote:
> If the guest is 32-bit, we should use 'quadrant' to adjust gpa
> offset
>
>    

Good catch.  Only affects kvm_mmu_pte_write(), so I don't think this had 
ill effects other than not prefetching the correct address?

> @@ -478,9 +478,14 @@ static void FNAME(invlpg)(struct kvm_vcpu *vcpu, gva_t gva)
>   		    ((level == PT_DIRECTORY_LEVEL&&  is_large_pte(*sptep))) ||
>   		    ((level == PT_PDPE_LEVEL&&  is_large_pte(*sptep)))) {
>   			struct kvm_mmu_page *sp = page_header(__pa(sptep));
> +			int offset = 0;
> +
> +			if (PTTYPE == 32)
> +				offset = sp->role.quadrant<<  PT64_LEVEL_BITS;;
>    

Wrong for PT_DIRECTORY_LEVEL (should be q << 8).  Also, too many 
semicolons.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 4/10] KVM MMU: Move invlpg code out of paging_tmpl.h

[Avi Kivity]

On 04/22/2010 09:12 AM, Xiao Guangrong wrote:
> Using '!sp->role.cr4_pae' replaces 'PTTYPE == 32' and using
> 'pte_size = sp->role.cr4_pae ? 8 : 4' replaces sizeof(pt_element_t)
>
> Then no need compile twice for this code
>
> Signed-off-by: Xiao Guangrong<xiaoguangrong@cn.fujitsu.com>
> ---
>   arch/x86/kvm/mmu.c         |   60 ++++++++++++++++++++++++++++++++++++++++++-
>   arch/x86/kvm/paging_tmpl.h |   56 -----------------------------------------
>   2 files changed, 58 insertions(+), 58 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index abf8bd4..fac7c09 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2256,6 +2256,62 @@ static bool is_rsvd_bits_set(struct kvm_vcpu *vcpu, u64 gpte, int level)
>   	return (gpte&  vcpu->arch.mmu.rsvd_bits_mask[bit7][level-1]) != 0;
>   }
>
> +static void paging_invlpg(struct kvm_vcpu *vcpu, gva_t gva)
> +{
> +	struct kvm_shadow_walk_iterator iterator;
> +	gpa_t pte_gpa = -1;
> +	int level;
> +	u64 *sptep;
> +	int need_flush = 0;
> +	unsigned pte_size = 0;
> +
> +	spin_lock(&vcpu->kvm->mmu_lock);
> +
> +	for_each_shadow_entry(vcpu, gva, iterator) {
> +		level = iterator.level;
> +		sptep = iterator.sptep;
> +
> +		if (level == PT_PAGE_TABLE_LEVEL  ||
> +		    ((level == PT_DIRECTORY_LEVEL&&  is_large_pte(*sptep))) ||
> +		    ((level == PT_PDPE_LEVEL&&  is_large_pte(*sptep)))) {
> +			struct kvm_mmu_page *sp = page_header(__pa(sptep));
> +			int offset = 0;
> +
> +			if (!sp->role.cr4_pae)
> +				offset = sp->role.quadrant<<  PT64_LEVEL_BITS;;
> +			pte_size = sp->role.cr4_pae ? 8 : 4;
> +			pte_gpa = (sp->gfn<<  PAGE_SHIFT);
> +			pte_gpa += (sptep - sp->spt + offset) * pte_size;
> +
> +			if (is_shadow_present_pte(*sptep)) {
> +				rmap_remove(vcpu->kvm, sptep);
> +				if (is_large_pte(*sptep))
> +					--vcpu->kvm->stat.lpages;
> +				need_flush = 1;
> +			}
> +			__set_spte(sptep, shadow_trap_nonpresent_pte);
> +			break;
> +		}
> +
> +		if (!is_shadow_present_pte(*sptep))
> +			break;
> +	}
> +
> +	if (need_flush)
> +		kvm_flush_remote_tlbs(vcpu->kvm);
> +
> +	atomic_inc(&vcpu->kvm->arch.invlpg_counter);
> +
> +	spin_unlock(&vcpu->kvm->mmu_lock);
> +
> +	if (pte_gpa == -1)
> +		return;
> +
> +	if (mmu_topup_memory_caches(vcpu))
> +		return;
> +	kvm_mmu_pte_write(vcpu, pte_gpa, NULL, pte_size, 0);
> +}
> +
>    

I think we should keep it in - kvm_mmu_pte_write() calls back to 
FNAME(update_pte), we could make the call directly from here speed 
things up, since we already have the spte and don't need to look it up.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 4/10] KVM MMU: Move invlpg code out of paging_tmpl.h

[Avi Kivity]

On 04/23/2010 02:27 PM, Avi Kivity wrote:
> On 04/22/2010 09:12 AM, Xiao Guangrong wrote:
>> Using '!sp->role.cr4_pae' replaces 'PTTYPE == 32' and using
>> 'pte_size = sp->role.cr4_pae ? 8 : 4' replaces sizeof(pt_element_t)
>>
>> Then no need compile twice for this code
>>
> I think we should keep it in - kvm_mmu_pte_write() calls back to 
> FNAME(update_pte), we could make the call directly from here speed 
> things up, since we already have the spte and don't need to look it up.
>

I see you do this in patches 9, 10 - but is it possible to use 
update_pte directly?  I think we'll need to make 
guess_page_from_pte_write() part of paging_tmpl.h (in general anything 
that depends on pte size is better off in paging_tmpl.h).

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 1/10] KVM MMU: fix for calculating gpa in invlpg code

[Xiao Guangrong]

Avi Kivity wrote:
> On 04/22/2010 09:12 AM, Xiao Guangrong wrote:
>> If the guest is 32-bit, we should use 'quadrant' to adjust gpa
>> offset
>>
>>    
> 
> Good catch.  Only affects kvm_mmu_pte_write(), so I don't think this had
> ill effects other than not prefetching the correct address?
> 

Yes

>> @@ -478,9 +478,14 @@ static void FNAME(invlpg)(struct kvm_vcpu *vcpu,
>> gva_t gva)
>>               ((level == PT_DIRECTORY_LEVEL&&  is_large_pte(*sptep))) ||
>>               ((level == PT_PDPE_LEVEL&&  is_large_pte(*sptep)))) {
>>               struct kvm_mmu_page *sp = page_header(__pa(sptep));
>> +            int offset = 0;
>> +
>> +            if (PTTYPE == 32)
>> +                offset = sp->role.quadrant<<  PT64_LEVEL_BITS;;
>>    
> 
> Wrong for PT_DIRECTORY_LEVEL (should be q << 8).  Also, too many
> semicolons.
> 

I guess you mean 'PT64_LEVEL_BITS' not 'PT_DIRECTORY_LEVEL' here :-)

It should be q << 8 here? it hardly understand, take leve = 1 for example,
32-bit guest PTE page table mapping range is 2^(10+12), PAE's PTE page table
mapping range is 2^(9+12), so, i think it's quadrant << 9 here, and other
function like FNAME(prefetch_page), FNAME(sync_page) also are q << 9

Sorry for the double semicolons here, will fix it

Thanks,
Xiao

Re: [PATCH 1/10] KVM MMU: fix for calculating gpa in invlpg code

[Avi Kivity]

On 04/23/2010 03:05 PM, Xiao Guangrong wrote:
>
>
>>> @@ -478,9 +478,14 @@ static void FNAME(invlpg)(struct kvm_vcpu *vcpu,
>>> gva_t gva)
>>>                ((level == PT_DIRECTORY_LEVEL&&   is_large_pte(*sptep))) ||
>>>                ((level == PT_PDPE_LEVEL&&   is_large_pte(*sptep)))) {
>>>                struct kvm_mmu_page *sp = page_header(__pa(sptep));
>>> +            int offset = 0;
>>> +
>>> +            if (PTTYPE == 32)
>>> +                offset = sp->role.quadrant<<   PT64_LEVEL_BITS;;
>>>
>>>        
>> Wrong for PT_DIRECTORY_LEVEL (should be q<<  8).  Also, too many
>> semicolons.
>>
>>      
> I guess you mean 'PT64_LEVEL_BITS' not 'PT_DIRECTORY_LEVEL' here :-)
>    

No, I mean if level == PT_DIRECTORY_LEVEL, then we want role.quadrant << 
8, not 9.

> It should be q<<  8 here? it hardly understand, take leve = 1 for example,
> 32-bit guest PTE page table mapping range is 2^(10+12), PAE's PTE page table
> mapping range is 2^(9+12),

For level == PT_DIRECTORY_LEVEL, quadrant is in the range 0..3.  Each sp 
maps 1GB, while the guest page table maps 4GB.  So the upper two bits 
become the quadrant.

>   so, i think it's quadrant<<  9 here, and other
> function like FNAME(prefetch_page), FNAME(sync_page) also are q<<  9
>    

They only work for PT_PAGE_TABLE_LEVEL, so for them 9 is correct.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 4/10] KVM MMU: Move invlpg code out of paging_tmpl.h

[Xiao Guangrong]

Avi Kivity wrote:
> On 04/23/2010 02:27 PM, Avi Kivity wrote:
>> On 04/22/2010 09:12 AM, Xiao Guangrong wrote:
>>> Using '!sp->role.cr4_pae' replaces 'PTTYPE == 32' and using
>>> 'pte_size = sp->role.cr4_pae ? 8 : 4' replaces sizeof(pt_element_t)
>>>
>>> Then no need compile twice for this code
>>>
>> I think we should keep it in - kvm_mmu_pte_write() calls back to
>> FNAME(update_pte), we could make the call directly from here speed
>> things up, since we already have the spte and don't need to look it up.
>>
> 
> I see you do this in patches 9, 10 - but is it possible to use
> update_pte directly?  I think we'll need to make
> guess_page_from_pte_write() part of paging_tmpl.h (in general anything
> that depends on pte size is better off in paging_tmpl.h).
> 

OK, i'll keep invlpg code in paging_tmpl.h and directly call FNAME(update_pte).

But, i don't see mmu_guess_page_from_pte_write() code depends on pte size. :-(

Xiao

Re: [PATCH 4/10] KVM MMU: Move invlpg code out of paging_tmpl.h

[Avi Kivity]

On 04/23/2010 04:21 PM, Xiao Guangrong wrote:
>
> OK, i'll keep invlpg code in paging_tmpl.h and directly call FNAME(update_pte).
>
> But, i don't see mmu_guess_page_from_pte_write() code depends on pte size. :-(
>    

It doesn't indeed, I misremembered.  It's mmu_pte_write_new_pte() (which 
is no longer needed).

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.