Re: [RFC PATCH v6 70/92] kvm: x86: filter out access rights only when tracked by the introspection tool

2019-08-13 Thread Adalbert Lazăr 
On Tue, 13 Aug 2019 11:08:39 +0200, Paolo Bonzini  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 
> > ---
> >  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.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH v6 70/92] kvm: x86: filter out access rights only when tracked by the introspection tool

2019-08-13 Thread Paolo Bonzini 
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 
> ---
>  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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[RFC PATCH v6 70/92] kvm: x86: filter out access rights only when tracked by the introspection tool

2019-08-09 Thread Adalbert Lazăr 
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 
---
 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) ||
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization