| Message ID | 20190809160047.8319-71-alazar@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VM introspection | expand |
On 09/08/19 18:00, Adalbert Lazăr wrote: > It should complete the commit fd34a9518173 ("kvm: x86: consult the page tracking from kvm_mmu_get_page() and __direct_map()") > > Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com> > --- > arch/x86/kvm/mmu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 65b6acba82da..fd64cf1115da 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -2660,6 +2660,9 @@ static void clear_sp_write_flooding_count(u64 *spte) > static unsigned int kvm_mmu_page_track_acc(struct kvm_vcpu *vcpu, gfn_t gfn, > unsigned int acc) > { > + if (!kvmi_tracked_gfn(vcpu, gfn)) > + return acc; > + > if (kvm_page_track_is_active(vcpu, gfn, KVM_PAGE_TRACK_PREREAD)) > acc &= ~ACC_USER_MASK; > if (kvm_page_track_is_active(vcpu, gfn, KVM_PAGE_TRACK_PREWRITE) || > If this patch is always needed, then the function should be named something like kvm_mmu_apply_introspection_access and kvmi_tracked_gfn should be tested from the moment it is introduced. But the commit message says nothing about _why_ it is needed, so I cannot guess. I would very much avoid it however. Is it just an optimization? Paolo
On Tue, 13 Aug 2019 11:08:39 +0200, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 09/08/19 18:00, Adalbert Lazăr wrote: > > It should complete the commit fd34a9518173 ("kvm: x86: consult the page tracking from kvm_mmu_get_page() and __direct_map()") > > > > Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com> > > --- > > arch/x86/kvm/mmu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > > index 65b6acba82da..fd64cf1115da 100644 > > --- a/arch/x86/kvm/mmu.c > > +++ b/arch/x86/kvm/mmu.c > > @@ -2660,6 +2660,9 @@ static void clear_sp_write_flooding_count(u64 *spte) > > static unsigned int kvm_mmu_page_track_acc(struct kvm_vcpu *vcpu, gfn_t gfn, > > unsigned int acc) > > { > > + if (!kvmi_tracked_gfn(vcpu, gfn)) > > + return acc; > > + > > if (kvm_page_track_is_active(vcpu, gfn, KVM_PAGE_TRACK_PREREAD)) > > acc &= ~ACC_USER_MASK; > > if (kvm_page_track_is_active(vcpu, gfn, KVM_PAGE_TRACK_PREWRITE) || > > > > If this patch is always needed, then the function should be named > something like kvm_mmu_apply_introspection_access and kvmi_tracked_gfn > should be tested from the moment it is introduced. > > But the commit message says nothing about _why_ it is needed, so I > cannot guess. I would very much avoid it however. Is it just an > optimization? > > Paolo We'll retest to see if we still need kvm_mmu_page_track_acc(). The kvmi_tracked_gfn() check was used to keep the KVM code flow "unchanged" as much as possible. Probably, we can get ride of it.
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 65b6acba82da..fd64cf1115da 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -2660,6 +2660,9 @@ static void clear_sp_write_flooding_count(u64 *spte) static unsigned int kvm_mmu_page_track_acc(struct kvm_vcpu *vcpu, gfn_t gfn, unsigned int acc) { + if (!kvmi_tracked_gfn(vcpu, gfn)) + return acc; + if (kvm_page_track_is_active(vcpu, gfn, KVM_PAGE_TRACK_PREREAD)) acc &= ~ACC_USER_MASK; if (kvm_page_track_is_active(vcpu, gfn, KVM_PAGE_TRACK_PREWRITE) ||
It should complete the commit fd34a9518173 ("kvm: x86: consult the page tracking from kvm_mmu_get_page() and __direct_map()") Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com> --- arch/x86/kvm/mmu.c | 3 +++ 1 file changed, 3 insertions(+)