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 <[email protected]>
---
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
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<[email protected]>
> ---
> 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.
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.
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
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.