* [patch 0/4] mmu audit update @ 2009-06-02 21:36 Marcelo Tosatti 2009-06-02 21:36 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti ` (5 more replies) 0 siblings, 6 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-02 21:36 UTC (permalink / raw) To: avi, sheng; +Cc: kvm Some updates to the MMU audit code. The third patch is "guessy" because I could not find the notrap spte documentation, all I can see is the page-fault error code mask and match fields in the VMCS, but can't see the link of that to sptes. Can someone point it out please? -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps 2009-06-02 21:36 [patch 0/4] mmu audit update Marcelo Tosatti @ 2009-06-02 21:36 ` Marcelo Tosatti 2009-06-08 9:24 ` Avi Kivity 2009-06-02 21:36 ` [patch 2/4] KVM: MMU audit: update audit_write_protection Marcelo Tosatti ` (4 subsequent siblings) 5 siblings, 1 reply; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-02 21:36 UTC (permalink / raw) To: avi, sheng; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-1 --] [-- Type: text/plain, Size: 3975 bytes --] Under testing, count_writable_mappings returns a value that is 2 integers larger than what count_rmaps returns. Suspicion is that either of the two functions is counting a duplicate (either positively or negatively). Modifying check_writable_mappings_rmap to check for rmap existance on all present MMU pages fails to trigger an error, which should keep Avi happy. Also introduce mmu_spte_walk to invoke a callback on all present sptes visible to the current vcpu, might be useful in the future. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3017,6 +3017,55 @@ static gva_t canonicalize(gva_t gva) return gva; } + +typedef void (*inspect_spte_fn) (struct kvm *kvm, struct kvm_mmu_page *sp, + u64 *sptep); + +static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp, + inspect_spte_fn fn) +{ + int i; + + for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { + u64 ent = sp->spt[i]; + + if (is_shadow_present_pte(ent)) { + if (sp->role.level > 1) { + struct kvm_mmu_page *child; + child = page_header(ent & PT64_BASE_ADDR_MASK); + __mmu_spte_walk(kvm, child, fn); + } + if (sp->role.level == 1) + fn(kvm, sp, &sp->spt[i]); + } + } +} + +static void mmu_spte_walk(struct kvm_vcpu *vcpu, inspect_spte_fn fn) +{ + int i; + struct kvm_mmu_page *sp; + + if (!VALID_PAGE(vcpu->arch.mmu.root_hpa)) + return; + if (vcpu->arch.mmu.shadow_root_level == PT64_ROOT_LEVEL) { + hpa_t root = vcpu->arch.mmu.root_hpa; + sp = page_header(root); + __mmu_spte_walk(vcpu->kvm, sp, fn); + return; + } + for (i = 0; i < 4; ++i) { + hpa_t root = vcpu->arch.mmu.pae_root[i]; + + if (root && VALID_PAGE(root)) { + root &= PT64_BASE_ADDR_MASK; + sp = page_header(root); + __mmu_spte_walk(vcpu->kvm, sp, fn); + } + } + return; +} + static void audit_mappings_page(struct kvm_vcpu *vcpu, u64 page_pte, gva_t va, int level) { @@ -3109,9 +3158,43 @@ static int count_rmaps(struct kvm_vcpu * return nmaps; } -static int count_writable_mappings(struct kvm_vcpu *vcpu) +void inspect_spte_has_rmap(struct kvm *kvm, struct kvm_mmu_page *sp, u64 *sptep) +{ + unsigned long *rmapp; + struct kvm_mmu_page *rev_sp; + gfn_t gfn; + + if (*sptep & PT_WRITABLE_MASK) { + rev_sp = page_header(__pa(sptep)); + gfn = rev_sp->gfns[sptep - rev_sp->spt]; + + if (!gfn_to_memslot(kvm, gfn)) { + printk(KERN_ERR "%s: no memslot for gfn %ld\n", + audit_msg, gfn); + printk(KERN_ERR "%s: index %ld of sp (gfn=%lx)\n", + audit_msg, sptep - rev_sp->spt, + rev_sp->gfn); + dump_stack(); + return; + } + + rmapp = gfn_to_rmap(kvm, rev_sp->gfns[sptep - rev_sp->spt], 0); + if (!*rmapp) { + printk(KERN_ERR "%s: no rmap for writable spte %llx\n", + audit_msg, *sptep); + dump_stack(); + } + } + +} + +void audit_writable_sptes_have_rmaps(struct kvm_vcpu *vcpu) +{ + mmu_spte_walk(vcpu, inspect_spte_has_rmap); +} + +static void check_writable_mappings_rmap(struct kvm_vcpu *vcpu) { - int nmaps = 0; struct kvm_mmu_page *sp; int i; @@ -3128,20 +3211,16 @@ static int count_writable_mappings(struc continue; if (!(ent & PT_WRITABLE_MASK)) continue; - ++nmaps; + inspect_spte_has_rmap(vcpu->kvm, sp, &pt[i]); } } - return nmaps; + return; } static void audit_rmap(struct kvm_vcpu *vcpu) { - int n_rmap = count_rmaps(vcpu); - int n_actual = count_writable_mappings(vcpu); - - if (n_rmap != n_actual) - printk(KERN_ERR "%s: (%s) rmap %d actual %d\n", - __func__, audit_msg, n_rmap, n_actual); + check_writable_mappings_rmap(vcpu); + count_rmaps(vcpu); } static void audit_write_protection(struct kvm_vcpu *vcpu) @@ -3175,6 +3254,7 @@ static void kvm_mmu_audit(struct kvm_vcp audit_rmap(vcpu); audit_write_protection(vcpu); audit_mappings(vcpu); + audit_writable_sptes_have_rmaps(vcpu); dbg = olddbg; } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps 2009-06-02 21:36 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti @ 2009-06-08 9:24 ` Avi Kivity 2009-06-09 12:33 ` Marcelo Tosatti 0 siblings, 1 reply; 23+ messages in thread From: Avi Kivity @ 2009-06-08 9:24 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: sheng, kvm Marcelo Tosatti wrote: > Under testing, count_writable_mappings returns a value that is 2 integers > larger than what count_rmaps returns. > > Suspicion is that either of the two functions is counting a duplicate (either > positively or negatively). > > Modifying check_writable_mappings_rmap to check for rmap existance on > all present MMU pages fails to trigger an error, which should keep Avi > happy. > > Also introduce mmu_spte_walk to invoke a callback on all present sptes visible > to the current vcpu, might be useful in the future. > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > Index: kvm/arch/x86/kvm/mmu.c > =================================================================== > --- kvm.orig/arch/x86/kvm/mmu.c > +++ kvm/arch/x86/kvm/mmu.c > @@ -3017,6 +3017,55 @@ static gva_t canonicalize(gva_t gva) > return gva; > } > > + > +typedef void (*inspect_spte_fn) (struct kvm *kvm, struct kvm_mmu_page *sp, > + u64 *sptep); > + > +static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp, > + inspect_spte_fn fn) > +{ > + int i; > + > + for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { > + u64 ent = sp->spt[i]; > + > + if (is_shadow_present_pte(ent)) { > + if (sp->role.level > 1) { > I think this is broken wrt large pages. We should recurse if role.level > 1 or the G bit is set. > + if (*sptep & PT_WRITABLE_MASK) { > + rev_sp = page_header(__pa(sptep)); > + gfn = rev_sp->gfns[sptep - rev_sp->spt]; > + > + if (!gfn_to_memslot(kvm, gfn)) { > + printk(KERN_ERR "%s: no memslot for gfn %ld\n", > + audit_msg, gfn); > + printk(KERN_ERR "%s: index %ld of sp (gfn=%lx)\n", > + audit_msg, sptep - rev_sp->spt, > + rev_sp->gfn); > + dump_stack(); > + return; > + } > + > + rmapp = gfn_to_rmap(kvm, rev_sp->gfns[sptep - rev_sp->spt], 0); > + if (!*rmapp) { > + printk(KERN_ERR "%s: no rmap for writable spte %llx\n", > + audit_msg, *sptep); > + dump_stack(); > + } > + } > Semi-related: we should set up a new exit code to halt the VM so it can be inspected, otherwise all those printks and dump_stack()s will quickly overwhelm the logging facilities. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps 2009-06-08 9:24 ` Avi Kivity @ 2009-06-09 12:33 ` Marcelo Tosatti 2009-06-09 12:40 ` Avi Kivity 0 siblings, 1 reply; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-09 12:33 UTC (permalink / raw) To: Avi Kivity; +Cc: sheng, kvm On Mon, Jun 08, 2009 at 12:24:08PM +0300, Avi Kivity wrote: >> +static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp, >> + inspect_spte_fn fn) >> +{ >> + int i; >> + >> + for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { >> + u64 ent = sp->spt[i]; >> + >> + if (is_shadow_present_pte(ent)) { >> + if (sp->role.level > 1) { >> > > I think this is broken wrt large pages. We should recurse if role.level > > 1 or the G bit is set. Yes, fixed. Plan to add largepages validity checks later. > Semi-related: we should set up a new exit code to halt the VM so it can > be inspected, otherwise all those printks and dump_stack()s will quickly > overwhelm the logging facilities. Can you clarify on the halt exit code? Because for other exit codes which similar behaviour is wanted, say, unhandled vm exit, the policy can be handled in userspace (and the decision to halt or not seems better suited to happen there). So perhaps KVM_EXIT_MMU_AUDIT_FAILED? I wondered before whether it would be good to stop auditing on the first error, but gave up on the idea. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps 2009-06-09 12:33 ` Marcelo Tosatti @ 2009-06-09 12:40 ` Avi Kivity 0 siblings, 0 replies; 23+ messages in thread From: Avi Kivity @ 2009-06-09 12:40 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: sheng, kvm Marcelo Tosatti wrote: >> Semi-related: we should set up a new exit code to halt the VM so it can >> be inspected, otherwise all those printks and dump_stack()s will quickly >> overwhelm the logging facilities. >> > > Can you clarify on the halt exit code? > set a bit that tells KVM_RUN to quit (like KVM_EXIT_UNKNOWN), when userspace sees it it vm_stop()s, waiting for someone to dig in. Clean logs, clean state. > Because for other exit codes which similar behaviour is wanted, say, > unhandled vm exit, the policy can be handled in userspace (and the > decision to halt or not seems better suited to happen there). So perhaps > KVM_EXIT_MMU_AUDIT_FAILED? > Yes, the name was bad. We can do KVM_EXIT_INTERNAL_ERROR and have a maintainer_name field in the union to choose who will handle it. > I wondered before whether it would be good to stop auditing on the first > error, but gave up on the idea. > It's harder without exceptions. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 2/4] KVM: MMU audit: update audit_write_protection 2009-06-02 21:36 [patch 0/4] mmu audit update Marcelo Tosatti 2009-06-02 21:36 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti @ 2009-06-02 21:36 ` Marcelo Tosatti 2009-06-02 21:36 ` [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti ` (3 subsequent siblings) 5 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-02 21:36 UTC (permalink / raw) To: avi, sheng; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-2 --] [-- Type: text/plain, Size: 1225 bytes --] - Unsync pages contain writable sptes in the rmap. - rmaps do not exclusively contain writable sptes anymore. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3228,20 +3228,28 @@ static void audit_write_protection(struc struct kvm_mmu_page *sp; struct kvm_memory_slot *slot; unsigned long *rmapp; + u64 *spte; gfn_t gfn; list_for_each_entry(sp, &vcpu->kvm->arch.active_mmu_pages, link) { if (sp->role.direct) continue; + if (sp->unsync) + continue; gfn = unalias_gfn(vcpu->kvm, sp->gfn); slot = gfn_to_memslot_unaliased(vcpu->kvm, sp->gfn); rmapp = &slot->rmap[gfn - slot->base_gfn]; - if (*rmapp) - printk(KERN_ERR "%s: (%s) shadow page has writable" - " mappings: gfn %lx role %x\n", + + spte = rmap_next(vcpu->kvm, rmapp, NULL); + while (spte) { + if (*spte & PT_WRITABLE_MASK) + printk(KERN_ERR "%s: (%s) shadow page has " + "writable mappings: gfn %lx role %x\n", __func__, audit_msg, sp->gfn, sp->role.word); + spte = rmap_next(vcpu->kvm, rmapp, spte); + } } } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level 2009-06-02 21:36 [patch 0/4] mmu audit update Marcelo Tosatti 2009-06-02 21:36 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti 2009-06-02 21:36 ` [patch 2/4] KVM: MMU audit: update audit_write_protection Marcelo Tosatti @ 2009-06-02 21:36 ` Marcelo Tosatti 2009-06-02 21:36 ` [patch 4/4] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti ` (2 subsequent siblings) 5 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-02 21:36 UTC (permalink / raw) To: avi, sheng; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-3 --] [-- Type: text/plain, Size: 849 bytes --] update_pte sets nonleaf sptes as notrap, so either it or the audit code is broken. Choose the audit code. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3081,12 +3081,7 @@ static void audit_mappings_page(struct k va = canonicalize(va); if (level > 1) { - if (ent == shadow_notrap_nonpresent_pte) - printk(KERN_ERR "audit: (%s) nontrapping pte" - " in nonleaf level: levels %d gva %lx" - " level %d pte %llx\n", audit_msg, - vcpu->arch.mmu.root_level, va, level, ent); - else + if (is_shadow_present_pte(ent)) audit_mappings_page(vcpu, ent, va, level - 1); } else { gpa_t gpa = vcpu->arch.mmu.gva_to_gpa(vcpu, va); -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 4/4] KVM: MMU audit: audit_mappings tweaks 2009-06-02 21:36 [patch 0/4] mmu audit update Marcelo Tosatti ` (2 preceding siblings ...) 2009-06-02 21:36 ` [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti @ 2009-06-02 21:36 ` Marcelo Tosatti 2009-06-08 9:29 ` Avi Kivity 2009-06-07 7:14 ` [patch 0/4] mmu audit update Avi Kivity 2009-06-09 13:13 ` [patch 0/4] mmu audit update v2 Marcelo Tosatti 5 siblings, 1 reply; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-02 21:36 UTC (permalink / raw) To: avi, sheng; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-4 --] [-- Type: text/plain, Size: 1103 bytes --] - Fail early in case gfn_to_pfn returns is_error_pfn. - For the pre pte write case, avoid spurious "gva is valid but spte is notrap" messages (the emulation code does the guest write first, so this particular case is OK). Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3089,6 +3089,11 @@ static void audit_mappings_page(struct k pfn_t pfn = gfn_to_pfn(vcpu->kvm, gfn); hpa_t hpa = (hpa_t)pfn << PAGE_SHIFT; + if (is_error_pfn(pfn)) { + kvm_release_pfn_clean(pfn); + continue; + } + if (is_shadow_present_pte(ent) && (ent & PT64_BASE_ADDR_MASK) != hpa) printk(KERN_ERR "xx audit error: (%s) levels %d" @@ -3256,7 +3261,8 @@ static void kvm_mmu_audit(struct kvm_vcp audit_msg = msg; audit_rmap(vcpu); audit_write_protection(vcpu); - audit_mappings(vcpu); + if (strcmp("pre pte write", audit_msg)) + audit_mappings(vcpu); audit_writable_sptes_have_rmaps(vcpu); dbg = olddbg; } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch 4/4] KVM: MMU audit: audit_mappings tweaks 2009-06-02 21:36 ` [patch 4/4] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti @ 2009-06-08 9:29 ` Avi Kivity 0 siblings, 0 replies; 23+ messages in thread From: Avi Kivity @ 2009-06-08 9:29 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: sheng, kvm Marcelo Tosatti wrote: > @@ -3256,7 +3261,8 @@ static void kvm_mmu_audit(struct kvm_vcp > audit_msg = msg; > audit_rmap(vcpu); > audit_write_protection(vcpu); > - audit_mappings(vcpu); > + if (strcmp("pre pte write", audit_msg)) > + audit_mappings(vcpu); > audit_writable_sptes_have_rmaps(vcpu); > dbg = olddbg; > } > strcmp() doesn't return a truth value, please add != 0. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch 0/4] mmu audit update 2009-06-02 21:36 [patch 0/4] mmu audit update Marcelo Tosatti ` (3 preceding siblings ...) 2009-06-02 21:36 ` [patch 4/4] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti @ 2009-06-07 7:14 ` Avi Kivity 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti 2009-06-09 13:13 ` [patch 0/4] mmu audit update v2 Marcelo Tosatti 5 siblings, 1 reply; 23+ messages in thread From: Avi Kivity @ 2009-06-07 7:14 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: sheng, kvm Marcelo Tosatti wrote: > Some updates to the MMU audit code. > > The third patch is "guessy" because I could not find the notrap spte > documentation, all I can see is the page-fault error code mask and match > fields in the VMCS, but can't see the link of that to sptes. Can someone > point it out please? > When bypass_guest_pf is set, we tell vmx not to trap if the fault is due to page-not-present. So if we know gpte.p == 0, we set spte.p = 0 and allow not-present page faults to go directly to the guest without trapping. Of course, we still need to trap cases where gpte.p = 1 but we haven't mapped the page yet. So we set a reserved bit in the spte and trap on that. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 0/6] mmu audit update v4 2009-06-07 7:14 ` [patch 0/4] mmu audit update Avi Kivity @ 2009-06-10 15:27 ` Marcelo Tosatti 2009-06-10 15:27 ` [patch 1/6] KVM: MMU: introduce is_last_spte helper Marcelo Tosatti ` (6 more replies) 0 siblings, 7 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-10 15:27 UTC (permalink / raw) To: avi; +Cc: kvm Addressing comments, introducing a new helper, handling largepages. -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 1/6] KVM: MMU: introduce is_last_spte helper 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti @ 2009-06-10 15:27 ` Marcelo Tosatti 2009-06-10 15:27 ` [patch 2/6] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti ` (5 subsequent siblings) 6 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-10 15:27 UTC (permalink / raw) To: avi; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: last-spte --] [-- Type: text/plain, Size: 1816 bytes --] Hiding some of the last largepage / level interaction (which is useful for gbpages and for zero based levels). Also merge the PT_PAGE_TABLE_LEVEL clearing loop in unlink_children. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -250,6 +250,15 @@ static int is_rmap_spte(u64 pte) return is_shadow_present_pte(pte); } +static int is_last_spte(u64 pte, int level) +{ + if (level == PT_PAGE_TABLE_LEVEL) + return 1; + if (level == PT_DIRECTORY_LEVEL && is_large_pte(pte)) + return 1; + return 0; +} + static pfn_t spte_to_pfn(u64 pte) { return (pte & PT64_BASE_ADDR_MASK) >> PAGE_SHIFT; @@ -1293,25 +1302,17 @@ static void kvm_mmu_page_unlink_children pt = sp->spt; - if (sp->role.level == PT_PAGE_TABLE_LEVEL) { - for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { - if (is_shadow_present_pte(pt[i])) - rmap_remove(kvm, &pt[i]); - pt[i] = shadow_trap_nonpresent_pte; - } - return; - } - for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { ent = pt[i]; if (is_shadow_present_pte(ent)) { - if (!is_large_pte(ent)) { + if (!is_last_spte(ent, sp->role.level)) { ent &= PT64_BASE_ADDR_MASK; mmu_page_remove_parent_pte(page_header(ent), &pt[i]); } else { - --kvm->stat.lpages; + if (is_large_pte(ent)) + --kvm->stat.lpages; rmap_remove(kvm, &pt[i]); } } @@ -2357,8 +2358,7 @@ static void mmu_pte_write_zap_pte(struct pte = *spte; if (is_shadow_present_pte(pte)) { - if (sp->role.level == PT_PAGE_TABLE_LEVEL || - is_large_pte(pte)) + if (is_last_spte(pte, sp->role.level)) rmap_remove(vcpu->kvm, spte); else { child = page_header(pte & PT64_BASE_ADDR_MASK); -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 2/6] KVM: MMU audit: update count_writable_mappings / count_rmaps 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti 2009-06-10 15:27 ` [patch 1/6] KVM: MMU: introduce is_last_spte helper Marcelo Tosatti @ 2009-06-10 15:27 ` Marcelo Tosatti 2009-06-10 15:27 ` [patch 3/6] KVM: MMU audit: update audit_write_protection Marcelo Tosatti ` (4 subsequent siblings) 6 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-10 15:27 UTC (permalink / raw) To: avi; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-1 --] [-- Type: text/plain, Size: 4081 bytes --] Under testing, count_writable_mappings returns a value that is 2 integers larger than what count_rmaps returns. Suspicion is that either of the two functions is counting a duplicate (either positively or negatively). Modifying check_writable_mappings_rmap to check for rmap existance on all present MMU pages fails to trigger an error, which should keep Avi happy. Also introduce mmu_spte_walk to invoke a callback on all present sptes visible to the current vcpu, might be useful in the future. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3020,6 +3020,55 @@ static gva_t canonicalize(gva_t gva) return gva; } + +typedef void (*inspect_spte_fn) (struct kvm *kvm, struct kvm_mmu_page *sp, + u64 *sptep); + +static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp, + inspect_spte_fn fn) +{ + int i; + + for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { + u64 ent = sp->spt[i]; + + if (is_shadow_present_pte(ent)) { + if (sp->role.level > 1 && !is_large_pte(ent)) { + struct kvm_mmu_page *child; + child = page_header(ent & PT64_BASE_ADDR_MASK); + __mmu_spte_walk(kvm, child, fn); + } + if (sp->role.level == 1) + fn(kvm, sp, &sp->spt[i]); + } + } +} + +static void mmu_spte_walk(struct kvm_vcpu *vcpu, inspect_spte_fn fn) +{ + int i; + struct kvm_mmu_page *sp; + + if (!VALID_PAGE(vcpu->arch.mmu.root_hpa)) + return; + if (vcpu->arch.mmu.shadow_root_level == PT64_ROOT_LEVEL) { + hpa_t root = vcpu->arch.mmu.root_hpa; + sp = page_header(root); + __mmu_spte_walk(vcpu->kvm, sp, fn); + return; + } + for (i = 0; i < 4; ++i) { + hpa_t root = vcpu->arch.mmu.pae_root[i]; + + if (root && VALID_PAGE(root)) { + root &= PT64_BASE_ADDR_MASK; + sp = page_header(root); + __mmu_spte_walk(vcpu->kvm, sp, fn); + } + } + return; +} + static void audit_mappings_page(struct kvm_vcpu *vcpu, u64 page_pte, gva_t va, int level) { @@ -3112,9 +3161,47 @@ static int count_rmaps(struct kvm_vcpu * return nmaps; } -static int count_writable_mappings(struct kvm_vcpu *vcpu) +void inspect_spte_has_rmap(struct kvm *kvm, struct kvm_mmu_page *sp, u64 *sptep) +{ + unsigned long *rmapp; + struct kvm_mmu_page *rev_sp; + gfn_t gfn; + + if (*sptep & PT_WRITABLE_MASK) { + rev_sp = page_header(__pa(sptep)); + gfn = rev_sp->gfns[sptep - rev_sp->spt]; + + if (!gfn_to_memslot(kvm, gfn)) { + if (!printk_ratelimit()) + return; + printk(KERN_ERR "%s: no memslot for gfn %ld\n", + audit_msg, gfn); + printk(KERN_ERR "%s: index %ld of sp (gfn=%lx)\n", + audit_msg, sptep - rev_sp->spt, + rev_sp->gfn); + dump_stack(); + return; + } + + rmapp = gfn_to_rmap(kvm, rev_sp->gfns[sptep - rev_sp->spt], 0); + if (!*rmapp) { + if (!printk_ratelimit()) + return; + printk(KERN_ERR "%s: no rmap for writable spte %llx\n", + audit_msg, *sptep); + dump_stack(); + } + } + +} + +void audit_writable_sptes_have_rmaps(struct kvm_vcpu *vcpu) +{ + mmu_spte_walk(vcpu, inspect_spte_has_rmap); +} + +static void check_writable_mappings_rmap(struct kvm_vcpu *vcpu) { - int nmaps = 0; struct kvm_mmu_page *sp; int i; @@ -3131,20 +3218,16 @@ static int count_writable_mappings(struc continue; if (!(ent & PT_WRITABLE_MASK)) continue; - ++nmaps; + inspect_spte_has_rmap(vcpu->kvm, sp, &pt[i]); } } - return nmaps; + return; } static void audit_rmap(struct kvm_vcpu *vcpu) { - int n_rmap = count_rmaps(vcpu); - int n_actual = count_writable_mappings(vcpu); - - if (n_rmap != n_actual) - printk(KERN_ERR "%s: (%s) rmap %d actual %d\n", - __func__, audit_msg, n_rmap, n_actual); + check_writable_mappings_rmap(vcpu); + count_rmaps(vcpu); } static void audit_write_protection(struct kvm_vcpu *vcpu) @@ -3178,6 +3261,7 @@ static void kvm_mmu_audit(struct kvm_vcp audit_rmap(vcpu); audit_write_protection(vcpu); audit_mappings(vcpu); + audit_writable_sptes_have_rmaps(vcpu); dbg = olddbg; } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 3/6] KVM: MMU audit: update audit_write_protection 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti 2009-06-10 15:27 ` [patch 1/6] KVM: MMU: introduce is_last_spte helper Marcelo Tosatti 2009-06-10 15:27 ` [patch 2/6] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti @ 2009-06-10 15:27 ` Marcelo Tosatti 2009-06-10 15:27 ` [patch 4/6] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti ` (3 subsequent siblings) 6 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-10 15:27 UTC (permalink / raw) To: avi; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-2 --] [-- Type: text/plain, Size: 1225 bytes --] - Unsync pages contain writable sptes in the rmap. - rmaps do not exclusively contain writable sptes anymore. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3235,20 +3235,28 @@ static void audit_write_protection(struc struct kvm_mmu_page *sp; struct kvm_memory_slot *slot; unsigned long *rmapp; + u64 *spte; gfn_t gfn; list_for_each_entry(sp, &vcpu->kvm->arch.active_mmu_pages, link) { if (sp->role.direct) continue; + if (sp->unsync) + continue; gfn = unalias_gfn(vcpu->kvm, sp->gfn); slot = gfn_to_memslot_unaliased(vcpu->kvm, sp->gfn); rmapp = &slot->rmap[gfn - slot->base_gfn]; - if (*rmapp) - printk(KERN_ERR "%s: (%s) shadow page has writable" - " mappings: gfn %lx role %x\n", + + spte = rmap_next(vcpu->kvm, rmapp, NULL); + while (spte) { + if (*spte & PT_WRITABLE_MASK) + printk(KERN_ERR "%s: (%s) shadow page has " + "writable mappings: gfn %lx role %x\n", __func__, audit_msg, sp->gfn, sp->role.word); + spte = rmap_next(vcpu->kvm, rmapp, spte); + } } } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 4/6] KVM: MMU audit: nontrapping ptes in nonleaf level 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti ` (2 preceding siblings ...) 2009-06-10 15:27 ` [patch 3/6] KVM: MMU audit: update audit_write_protection Marcelo Tosatti @ 2009-06-10 15:27 ` Marcelo Tosatti 2009-06-10 15:27 ` [patch 5/6] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti ` (2 subsequent siblings) 6 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-10 15:27 UTC (permalink / raw) To: avi; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-3 --] [-- Type: text/plain, Size: 787 bytes --] It is valid to set non leaf sptes as notrap. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3084,12 +3084,7 @@ static void audit_mappings_page(struct k va = canonicalize(va); if (level > 1) { - if (ent == shadow_notrap_nonpresent_pte) - printk(KERN_ERR "audit: (%s) nontrapping pte" - " in nonleaf level: levels %d gva %lx" - " level %d pte %llx\n", audit_msg, - vcpu->arch.mmu.root_level, va, level, ent); - else + if (is_shadow_present_pte(ent)) audit_mappings_page(vcpu, ent, va, level - 1); } else { gpa_t gpa = vcpu->arch.mmu.gva_to_gpa(vcpu, va); -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 5/6] KVM: MMU audit: audit_mappings tweaks 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti ` (3 preceding siblings ...) 2009-06-10 15:27 ` [patch 4/6] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti @ 2009-06-10 15:27 ` Marcelo Tosatti 2009-06-10 15:27 ` [patch 6/6] KVM: MMU audit: largepage handling Marcelo Tosatti 2009-06-11 14:24 ` [patch 0/6] mmu audit update v4 Avi Kivity 6 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-10 15:27 UTC (permalink / raw) To: avi; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-4 --] [-- Type: text/plain, Size: 1108 bytes --] - Fail early in case gfn_to_pfn returns is_error_pfn. - For the pre pte write case, avoid spurious "gva is valid but spte is notrap" messages (the emulation code does the guest write first, so this particular case is OK). Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3092,6 +3092,11 @@ static void audit_mappings_page(struct k pfn_t pfn = gfn_to_pfn(vcpu->kvm, gfn); hpa_t hpa = (hpa_t)pfn << PAGE_SHIFT; + if (is_error_pfn(pfn)) { + kvm_release_pfn_clean(pfn); + continue; + } + if (is_shadow_present_pte(ent) && (ent & PT64_BASE_ADDR_MASK) != hpa) printk(KERN_ERR "xx audit error: (%s) levels %d" @@ -3263,7 +3268,8 @@ static void kvm_mmu_audit(struct kvm_vcp audit_msg = msg; audit_rmap(vcpu); audit_write_protection(vcpu); - audit_mappings(vcpu); + if (strcmp("pre pte write", audit_msg) != 0) + audit_mappings(vcpu); audit_writable_sptes_have_rmaps(vcpu); dbg = olddbg; } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 6/6] KVM: MMU audit: largepage handling 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti ` (4 preceding siblings ...) 2009-06-10 15:27 ` [patch 5/6] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti @ 2009-06-10 15:27 ` Marcelo Tosatti 2009-06-11 14:24 ` [patch 0/6] mmu audit update v4 Avi Kivity 6 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-10 15:27 UTC (permalink / raw) To: avi; +Cc: kvm, Marcelo Tosatti [-- Attachment #1: mmu-audit-last-pte --] [-- Type: text/plain, Size: 1483 bytes --] Make the audit code aware of largepages. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3033,12 +3033,11 @@ static void __mmu_spte_walk(struct kvm * u64 ent = sp->spt[i]; if (is_shadow_present_pte(ent)) { - if (sp->role.level > 1 && !is_large_pte(ent)) { + if (!is_last_spte(ent, sp->role.level)) { struct kvm_mmu_page *child; child = page_header(ent & PT64_BASE_ADDR_MASK); __mmu_spte_walk(kvm, child, fn); - } - if (sp->role.level == 1) + } else fn(kvm, sp, &sp->spt[i]); } } @@ -3083,10 +3082,9 @@ static void audit_mappings_page(struct k continue; va = canonicalize(va); - if (level > 1) { - if (is_shadow_present_pte(ent)) - audit_mappings_page(vcpu, ent, va, level - 1); - } else { + if (is_shadow_present_pte(ent) && !is_last_spte(ent, level)) + audit_mappings_page(vcpu, ent, va, level - 1); + else { gpa_t gpa = vcpu->arch.mmu.gva_to_gpa(vcpu, va); gfn_t gfn = gpa >> PAGE_SHIFT; pfn_t pfn = gfn_to_pfn(vcpu->kvm, gfn); @@ -3183,7 +3181,8 @@ void inspect_spte_has_rmap(struct kvm *k return; } - rmapp = gfn_to_rmap(kvm, rev_sp->gfns[sptep - rev_sp->spt], 0); + rmapp = gfn_to_rmap(kvm, rev_sp->gfns[sptep - rev_sp->spt], + is_large_pte(*sptep)); if (!*rmapp) { if (!printk_ratelimit()) return; -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch 0/6] mmu audit update v4 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti ` (5 preceding siblings ...) 2009-06-10 15:27 ` [patch 6/6] KVM: MMU audit: largepage handling Marcelo Tosatti @ 2009-06-11 14:24 ` Avi Kivity 6 siblings, 0 replies; 23+ messages in thread From: Avi Kivity @ 2009-06-11 14:24 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm Marcelo Tosatti wrote: > Addressing comments, introducing a new helper, handling largepages. > > Applied, thanks. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 0/4] mmu audit update v2 2009-06-02 21:36 [patch 0/4] mmu audit update Marcelo Tosatti ` (4 preceding siblings ...) 2009-06-07 7:14 ` [patch 0/4] mmu audit update Avi Kivity @ 2009-06-09 13:13 ` Marcelo Tosatti 2009-06-09 13:13 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti ` (3 more replies) 5 siblings, 4 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-09 13:13 UTC (permalink / raw) To: kvm; +Cc: avi Addressing comments. -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps 2009-06-09 13:13 ` [patch 0/4] mmu audit update v2 Marcelo Tosatti @ 2009-06-09 13:13 ` Marcelo Tosatti 2009-06-09 13:13 ` [patch 2/4] KVM: MMU audit: update audit_write_protection Marcelo Tosatti ` (2 subsequent siblings) 3 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-09 13:13 UTC (permalink / raw) To: kvm; +Cc: avi, Marcelo Tosatti [-- Attachment #1: mmu-audit-1 --] [-- Type: text/plain, Size: 3997 bytes --] Under testing, count_writable_mappings returns a value that is 2 integers larger than what count_rmaps returns. Suspicion is that either of the two functions is counting a duplicate (either positively or negatively). Modifying check_writable_mappings_rmap to check for rmap existance on all present MMU pages fails to trigger an error, which should keep Avi happy. Also introduce mmu_spte_walk to invoke a callback on all present sptes visible to the current vcpu, might be useful in the future. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3020,6 +3020,55 @@ static gva_t canonicalize(gva_t gva) return gva; } + +typedef void (*inspect_spte_fn) (struct kvm *kvm, struct kvm_mmu_page *sp, + u64 *sptep); + +static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp, + inspect_spte_fn fn) +{ + int i; + + for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { + u64 ent = sp->spt[i]; + + if (is_shadow_present_pte(ent)) { + if (sp->role.level > 1 && !is_large_pte(ent)) { + struct kvm_mmu_page *child; + child = page_header(ent & PT64_BASE_ADDR_MASK); + __mmu_spte_walk(kvm, child, fn); + } + if (sp->role.level == 1) + fn(kvm, sp, &sp->spt[i]); + } + } +} + +static void mmu_spte_walk(struct kvm_vcpu *vcpu, inspect_spte_fn fn) +{ + int i; + struct kvm_mmu_page *sp; + + if (!VALID_PAGE(vcpu->arch.mmu.root_hpa)) + return; + if (vcpu->arch.mmu.shadow_root_level == PT64_ROOT_LEVEL) { + hpa_t root = vcpu->arch.mmu.root_hpa; + sp = page_header(root); + __mmu_spte_walk(vcpu->kvm, sp, fn); + return; + } + for (i = 0; i < 4; ++i) { + hpa_t root = vcpu->arch.mmu.pae_root[i]; + + if (root && VALID_PAGE(root)) { + root &= PT64_BASE_ADDR_MASK; + sp = page_header(root); + __mmu_spte_walk(vcpu->kvm, sp, fn); + } + } + return; +} + static void audit_mappings_page(struct kvm_vcpu *vcpu, u64 page_pte, gva_t va, int level) { @@ -3112,9 +3161,43 @@ static int count_rmaps(struct kvm_vcpu * return nmaps; } -static int count_writable_mappings(struct kvm_vcpu *vcpu) +void inspect_spte_has_rmap(struct kvm *kvm, struct kvm_mmu_page *sp, u64 *sptep) +{ + unsigned long *rmapp; + struct kvm_mmu_page *rev_sp; + gfn_t gfn; + + if (*sptep & PT_WRITABLE_MASK) { + rev_sp = page_header(__pa(sptep)); + gfn = rev_sp->gfns[sptep - rev_sp->spt]; + + if (!gfn_to_memslot(kvm, gfn)) { + printk(KERN_ERR "%s: no memslot for gfn %ld\n", + audit_msg, gfn); + printk(KERN_ERR "%s: index %ld of sp (gfn=%lx)\n", + audit_msg, sptep - rev_sp->spt, + rev_sp->gfn); + dump_stack(); + return; + } + + rmapp = gfn_to_rmap(kvm, rev_sp->gfns[sptep - rev_sp->spt], 0); + if (!*rmapp) { + printk(KERN_ERR "%s: no rmap for writable spte %llx\n", + audit_msg, *sptep); + dump_stack(); + } + } + +} + +void audit_writable_sptes_have_rmaps(struct kvm_vcpu *vcpu) +{ + mmu_spte_walk(vcpu, inspect_spte_has_rmap); +} + +static void check_writable_mappings_rmap(struct kvm_vcpu *vcpu) { - int nmaps = 0; struct kvm_mmu_page *sp; int i; @@ -3131,20 +3214,16 @@ static int count_writable_mappings(struc continue; if (!(ent & PT_WRITABLE_MASK)) continue; - ++nmaps; + inspect_spte_has_rmap(vcpu->kvm, sp, &pt[i]); } } - return nmaps; + return; } static void audit_rmap(struct kvm_vcpu *vcpu) { - int n_rmap = count_rmaps(vcpu); - int n_actual = count_writable_mappings(vcpu); - - if (n_rmap != n_actual) - printk(KERN_ERR "%s: (%s) rmap %d actual %d\n", - __func__, audit_msg, n_rmap, n_actual); + check_writable_mappings_rmap(vcpu); + count_rmaps(vcpu); } static void audit_write_protection(struct kvm_vcpu *vcpu) @@ -3178,6 +3257,7 @@ static void kvm_mmu_audit(struct kvm_vcp audit_rmap(vcpu); audit_write_protection(vcpu); audit_mappings(vcpu); + audit_writable_sptes_have_rmaps(vcpu); dbg = olddbg; } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 2/4] KVM: MMU audit: update audit_write_protection 2009-06-09 13:13 ` [patch 0/4] mmu audit update v2 Marcelo Tosatti 2009-06-09 13:13 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti @ 2009-06-09 13:13 ` Marcelo Tosatti 2009-06-09 13:13 ` [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti 2009-06-09 13:13 ` [patch 4/4] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti 3 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-09 13:13 UTC (permalink / raw) To: kvm; +Cc: avi, Marcelo Tosatti [-- Attachment #1: mmu-audit-2 --] [-- Type: text/plain, Size: 1225 bytes --] - Unsync pages contain writable sptes in the rmap. - rmaps do not exclusively contain writable sptes anymore. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3231,20 +3231,28 @@ static void audit_write_protection(struc struct kvm_mmu_page *sp; struct kvm_memory_slot *slot; unsigned long *rmapp; + u64 *spte; gfn_t gfn; list_for_each_entry(sp, &vcpu->kvm->arch.active_mmu_pages, link) { if (sp->role.direct) continue; + if (sp->unsync) + continue; gfn = unalias_gfn(vcpu->kvm, sp->gfn); slot = gfn_to_memslot_unaliased(vcpu->kvm, sp->gfn); rmapp = &slot->rmap[gfn - slot->base_gfn]; - if (*rmapp) - printk(KERN_ERR "%s: (%s) shadow page has writable" - " mappings: gfn %lx role %x\n", + + spte = rmap_next(vcpu->kvm, rmapp, NULL); + while (spte) { + if (*spte & PT_WRITABLE_MASK) + printk(KERN_ERR "%s: (%s) shadow page has " + "writable mappings: gfn %lx role %x\n", __func__, audit_msg, sp->gfn, sp->role.word); + spte = rmap_next(vcpu->kvm, rmapp, spte); + } } } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level 2009-06-09 13:13 ` [patch 0/4] mmu audit update v2 Marcelo Tosatti 2009-06-09 13:13 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti 2009-06-09 13:13 ` [patch 2/4] KVM: MMU audit: update audit_write_protection Marcelo Tosatti @ 2009-06-09 13:13 ` Marcelo Tosatti 2009-06-09 13:13 ` [patch 4/4] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti 3 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-09 13:13 UTC (permalink / raw) To: kvm; +Cc: avi, Marcelo Tosatti [-- Attachment #1: mmu-audit-3 --] [-- Type: text/plain, Size: 849 bytes --] update_pte sets nonleaf sptes as notrap, so either it or the audit code is broken. Choose the audit code. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3084,12 +3084,7 @@ static void audit_mappings_page(struct k va = canonicalize(va); if (level > 1) { - if (ent == shadow_notrap_nonpresent_pte) - printk(KERN_ERR "audit: (%s) nontrapping pte" - " in nonleaf level: levels %d gva %lx" - " level %d pte %llx\n", audit_msg, - vcpu->arch.mmu.root_level, va, level, ent); - else + if (is_shadow_present_pte(ent)) audit_mappings_page(vcpu, ent, va, level - 1); } else { gpa_t gpa = vcpu->arch.mmu.gva_to_gpa(vcpu, va); -- ^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch 4/4] KVM: MMU audit: audit_mappings tweaks 2009-06-09 13:13 ` [patch 0/4] mmu audit update v2 Marcelo Tosatti ` (2 preceding siblings ...) 2009-06-09 13:13 ` [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti @ 2009-06-09 13:13 ` Marcelo Tosatti 3 siblings, 0 replies; 23+ messages in thread From: Marcelo Tosatti @ 2009-06-09 13:13 UTC (permalink / raw) To: kvm; +Cc: avi, Marcelo Tosatti [-- Attachment #1: mmu-audit-4 --] [-- Type: text/plain, Size: 1108 bytes --] - Fail early in case gfn_to_pfn returns is_error_pfn. - For the pre pte write case, avoid spurious "gva is valid but spte is notrap" messages (the emulation code does the guest write first, so this particular case is OK). Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm/arch/x86/kvm/mmu.c =================================================================== --- kvm.orig/arch/x86/kvm/mmu.c +++ kvm/arch/x86/kvm/mmu.c @@ -3092,6 +3092,11 @@ static void audit_mappings_page(struct k pfn_t pfn = gfn_to_pfn(vcpu->kvm, gfn); hpa_t hpa = (hpa_t)pfn << PAGE_SHIFT; + if (is_error_pfn(pfn)) { + kvm_release_pfn_clean(pfn); + continue; + } + if (is_shadow_present_pte(ent) && (ent & PT64_BASE_ADDR_MASK) != hpa) printk(KERN_ERR "xx audit error: (%s) levels %d" @@ -3259,7 +3264,8 @@ static void kvm_mmu_audit(struct kvm_vcp audit_msg = msg; audit_rmap(vcpu); audit_write_protection(vcpu); - audit_mappings(vcpu); + if (strcmp("pre pte write", audit_msg) != 0) + audit_mappings(vcpu); audit_writable_sptes_have_rmaps(vcpu); dbg = olddbg; } -- ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2009-06-11 14:24 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-06-02 21:36 [patch 0/4] mmu audit update Marcelo Tosatti 2009-06-02 21:36 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti 2009-06-08 9:24 ` Avi Kivity 2009-06-09 12:33 ` Marcelo Tosatti 2009-06-09 12:40 ` Avi Kivity 2009-06-02 21:36 ` [patch 2/4] KVM: MMU audit: update audit_write_protection Marcelo Tosatti 2009-06-02 21:36 ` [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti 2009-06-02 21:36 ` [patch 4/4] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti 2009-06-08 9:29 ` Avi Kivity 2009-06-07 7:14 ` [patch 0/4] mmu audit update Avi Kivity 2009-06-10 15:27 ` [patch 0/6] mmu audit update v4 Marcelo Tosatti 2009-06-10 15:27 ` [patch 1/6] KVM: MMU: introduce is_last_spte helper Marcelo Tosatti 2009-06-10 15:27 ` [patch 2/6] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti 2009-06-10 15:27 ` [patch 3/6] KVM: MMU audit: update audit_write_protection Marcelo Tosatti 2009-06-10 15:27 ` [patch 4/6] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti 2009-06-10 15:27 ` [patch 5/6] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti 2009-06-10 15:27 ` [patch 6/6] KVM: MMU audit: largepage handling Marcelo Tosatti 2009-06-11 14:24 ` [patch 0/6] mmu audit update v4 Avi Kivity 2009-06-09 13:13 ` [patch 0/4] mmu audit update v2 Marcelo Tosatti 2009-06-09 13:13 ` [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps Marcelo Tosatti 2009-06-09 13:13 ` [patch 2/4] KVM: MMU audit: update audit_write_protection Marcelo Tosatti 2009-06-09 13:13 ` [patch 3/4] KVM: MMU audit: nontrapping ptes in nonleaf level Marcelo Tosatti 2009-06-09 13:13 ` [patch 4/4] KVM: MMU audit: audit_mappings tweaks Marcelo Tosatti
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).