* [PATCH] KVM: MMU: Don't flush shadow when enabling dirty tracking
@ 2010-12-26 11:23 Avi Kivity
2010-12-27 4:06 ` Takuya Yoshikawa
0 siblings, 1 reply; 4+ messages in thread
From: Avi Kivity @ 2010-12-26 11:23 UTC (permalink / raw)
To: Marcelo Tosatti, kvm
Instead, drop large mappings, which were the reason we dropped shadow.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/kvm/mmu.c | 10 ++++++----
virt/kvm/kvm_main.c | 7 +------
2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 43bd5e3..1bbe2c0 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3444,14 +3444,16 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
if (!test_bit(slot, sp->slot_bitmap))
continue;
- if (sp->role.level != PT_PAGE_TABLE_LEVEL)
- continue;
-
pt = sp->spt;
- for (i = 0; i < PT64_ENT_PER_PAGE; ++i)
+ for (i = 0; i < PT64_ENT_PER_PAGE; ++i) {
+ if (sp->role.level != PT_PAGE_TABLE_LEVEL
+ && is_large_pte(pt[i]))
+ drop_spte(kvm, &pt[i],
+ shadow_trap_nonpresent_pte);
/* avoid RMW */
if (is_writable_pte(pt[i]))
update_spte(&pt[i], pt[i] & ~PT_WRITABLE_MASK);
+ }
}
kvm_flush_remote_tlbs(kvm);
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index b1b6cbb..b3bfeb8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -586,7 +586,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
struct kvm_userspace_memory_region *mem,
int user_alloc)
{
- int r, flush_shadow = 0;
+ int r;
gfn_t base_gfn;
unsigned long npages;
unsigned long i;
@@ -706,8 +706,6 @@ skip_lpage:
if (kvm_create_dirty_bitmap(&new) < 0)
goto out_free;
/* destroy any largepage mappings for dirty tracking */
- if (old.npages)
- flush_shadow = 1;
}
#else /* not defined CONFIG_S390 */
new.user_alloc = user_alloc;
@@ -778,9 +776,6 @@ skip_lpage:
kvm_free_physmem_slot(&old, &new);
kfree(old_memslots);
- if (flush_shadow)
- kvm_arch_flush_shadow(kvm);
-
return 0;
out_free:
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: MMU: Don't flush shadow when enabling dirty tracking
2010-12-26 11:23 [PATCH] KVM: MMU: Don't flush shadow when enabling dirty tracking Avi Kivity
@ 2010-12-27 4:06 ` Takuya Yoshikawa
2010-12-27 9:16 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Takuya Yoshikawa @ 2010-12-27 4:06 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm
Avi Kivity <avi@redhat.com> wrote:
> Instead, drop large mappings, which were the reason we dropped shadow.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> arch/x86/kvm/mmu.c | 10 ++++++----
> virt/kvm/kvm_main.c | 7 +------
> 2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 43bd5e3..1bbe2c0 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3444,14 +3444,16 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
> if (!test_bit(slot, sp->slot_bitmap))
> continue;
>
> - if (sp->role.level != PT_PAGE_TABLE_LEVEL)
> - continue;
> -
> pt = sp->spt;
> - for (i = 0; i < PT64_ENT_PER_PAGE; ++i)
> + for (i = 0; i < PT64_ENT_PER_PAGE; ++i) {
> + if (sp->role.level != PT_PAGE_TABLE_LEVEL
> + && is_large_pte(pt[i]))
> + drop_spte(kvm, &pt[i],
> + shadow_trap_nonpresent_pte);
> /* avoid RMW */
> if (is_writable_pte(pt[i]))
> update_spte(&pt[i], pt[i] & ~PT_WRITABLE_MASK);
> + }
> }
> kvm_flush_remote_tlbs(kvm);
> }
What is the difference from the similar part in rmap_write_protect()?
=== from rmap_write_protect() ===
/* check for huge page mappings */
for (i = PT_DIRECTORY_LEVEL;
i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
rmapp = gfn_to_rmap(kvm, gfn, i);
spte = rmap_next(kvm, rmapp, NULL);
while (spte) {
BUG_ON(!spte);
BUG_ON(!(*spte & PT_PRESENT_MASK));
BUG_ON((*spte & (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK)) != (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK));
pgprintk("rmap_write_protect(large): spte %p %llx %lld\n", spte, *spte, gfn);
if (is_writable_pte(*spte)) {
drop_spte(kvm, spte,
shadow_trap_nonpresent_pte);
--kvm->stat.lpages;
spte = NULL;
write_protected = 1;
}
spte = rmap_next(kvm, rmapp, spte);
}
}
===
Takuya
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: MMU: Don't flush shadow when enabling dirty tracking
2010-12-27 4:06 ` Takuya Yoshikawa
@ 2010-12-27 9:16 ` Avi Kivity
2010-12-27 9:25 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Avi Kivity @ 2010-12-27 9:16 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: Marcelo Tosatti, kvm
On 12/27/2010 06:06 AM, Takuya Yoshikawa wrote:
> Avi Kivity<avi@redhat.com> wrote:
>
> > Instead, drop large mappings, which were the reason we dropped shadow.
> >
> > Signed-off-by: Avi Kivity<avi@redhat.com>
> > ---
> > arch/x86/kvm/mmu.c | 10 ++++++----
> > virt/kvm/kvm_main.c | 7 +------
> > 2 files changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index 43bd5e3..1bbe2c0 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -3444,14 +3444,16 @@ void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
> > if (!test_bit(slot, sp->slot_bitmap))
> > continue;
> >
> > - if (sp->role.level != PT_PAGE_TABLE_LEVEL)
> > - continue;
> > -
> > pt = sp->spt;
> > - for (i = 0; i< PT64_ENT_PER_PAGE; ++i)
> > + for (i = 0; i< PT64_ENT_PER_PAGE; ++i) {
> > + if (sp->role.level != PT_PAGE_TABLE_LEVEL
> > + && is_large_pte(pt[i]))
> > + drop_spte(kvm,&pt[i],
> > + shadow_trap_nonpresent_pte);
> > /* avoid RMW */
> > if (is_writable_pte(pt[i]))
> > update_spte(&pt[i], pt[i]& ~PT_WRITABLE_MASK);
> > + }
> > }
> > kvm_flush_remote_tlbs(kvm);
> > }
>
> What is the difference from the similar part in rmap_write_protect()?
>
> === from rmap_write_protect() ===
>
> /* check for huge page mappings */
> for (i = PT_DIRECTORY_LEVEL;
> i< PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
> rmapp = gfn_to_rmap(kvm, gfn, i);
> spte = rmap_next(kvm, rmapp, NULL);
> while (spte) {
> BUG_ON(!spte);
> BUG_ON(!(*spte& PT_PRESENT_MASK));
> BUG_ON((*spte& (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK)) != (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK));
> pgprintk("rmap_write_protect(large): spte %p %llx %lld\n", spte, *spte, gfn);
> if (is_writable_pte(*spte)) {
> drop_spte(kvm, spte,
> shadow_trap_nonpresent_pte);
> --kvm->stat.lpages;
> spte = NULL;
> write_protected = 1;
> }
> spte = rmap_next(kvm, rmapp, spte);
> }
> }
> ===
Shouldn't be any. I forgot to rmap_remove() and to update kvm_stat,
thanks for pointing it out.
I'll post an updated patch. We'll want to share code between the two
later on.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: MMU: Don't flush shadow when enabling dirty tracking
2010-12-27 9:16 ` Avi Kivity
@ 2010-12-27 9:25 ` Avi Kivity
0 siblings, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2010-12-27 9:25 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: Marcelo Tosatti, kvm
On 12/27/2010 11:16 AM, Avi Kivity wrote:
> On 12/27/2010 06:06 AM, Takuya Yoshikawa wrote:
>> Avi Kivity<avi@redhat.com> wrote:
>>
>> > Instead, drop large mappings, which were the reason we dropped
>> shadow.
>> >
>> > Signed-off-by: Avi Kivity<avi@redhat.com>
>> > ---
>> > arch/x86/kvm/mmu.c | 10 ++++++----
>> > virt/kvm/kvm_main.c | 7 +------
>> > 2 files changed, 7 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> > index 43bd5e3..1bbe2c0 100644
>> > --- a/arch/x86/kvm/mmu.c
>> > +++ b/arch/x86/kvm/mmu.c
>> > @@ -3444,14 +3444,16 @@ void
>> kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot)
>> > if (!test_bit(slot, sp->slot_bitmap))
>> > continue;
>> >
>> > - if (sp->role.level != PT_PAGE_TABLE_LEVEL)
>> > - continue;
>> > -
>> > pt = sp->spt;
>> > - for (i = 0; i< PT64_ENT_PER_PAGE; ++i)
>> > + for (i = 0; i< PT64_ENT_PER_PAGE; ++i) {
>> > + if (sp->role.level != PT_PAGE_TABLE_LEVEL
>> > + && is_large_pte(pt[i]))
>> > + drop_spte(kvm,&pt[i],
>> > + shadow_trap_nonpresent_pte);
>> > /* avoid RMW */
>> > if (is_writable_pte(pt[i]))
>> > update_spte(&pt[i], pt[i]& ~PT_WRITABLE_MASK);
>> > + }
>> > }
>> > kvm_flush_remote_tlbs(kvm);
>> > }
>>
>> What is the difference from the similar part in rmap_write_protect()?
>>
>> === from rmap_write_protect() ===
>>
>> /* check for huge page mappings */
>> for (i = PT_DIRECTORY_LEVEL;
>> i< PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
>> rmapp = gfn_to_rmap(kvm, gfn, i);
>> spte = rmap_next(kvm, rmapp, NULL);
>> while (spte) {
>> BUG_ON(!spte);
>> BUG_ON(!(*spte& PT_PRESENT_MASK));
>> BUG_ON((*spte& (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK)) !=
>> (PT_PAGE_SIZE_MASK|PT_PRESENT_MASK));
>> pgprintk("rmap_write_protect(large): spte %p %llx
>> %lld\n", spte, *spte, gfn);
>> if (is_writable_pte(*spte)) {
>> drop_spte(kvm, spte,
>> shadow_trap_nonpresent_pte);
>> --kvm->stat.lpages;
>> spte = NULL;
>> write_protected = 1;
>> }
>> spte = rmap_next(kvm, rmapp, spte);
>> }
>> }
>> ===
>
> Shouldn't be any. I forgot to rmap_remove() and to update kvm_stat,
> thanks for pointing it out.
>
> I'll post an updated patch. We'll want to share code between the two
> later on.
>
Actually, drop_spte() calls rmap_remove(), so all that's broken is
kvm->stat.lpages accounting.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-12-27 9:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-26 11:23 [PATCH] KVM: MMU: Don't flush shadow when enabling dirty tracking Avi Kivity
2010-12-27 4:06 ` Takuya Yoshikawa
2010-12-27 9:16 ` Avi Kivity
2010-12-27 9:25 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox