linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] jump-label: export jump_label_inc/jump_label_dec
@ 2011-11-28 12:39 Xiao Guangrong
  2011-11-28 12:41 ` [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable Xiao Guangrong
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Xiao Guangrong @ 2011-11-28 12:39 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, Jason Baron, LKML, KVM

Export these two symbols, they will be used by mmu audit

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 kernel/jump_label.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index bbdfe2a..7d3d452 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -70,6 +70,7 @@ void jump_label_inc(struct jump_label_key *key)
 		jump_label_update(key, JUMP_LABEL_ENABLE);
 	jump_label_unlock();
 }
+EXPORT_SYMBOL_GPL(jump_label_inc);

 void jump_label_dec(struct jump_label_key *key)
 {
@@ -79,6 +80,7 @@ void jump_label_dec(struct jump_label_key *key)
 	jump_label_update(key, JUMP_LABEL_DISABLE);
 	jump_label_unlock();
 }
+EXPORT_SYMBOL_GPL(jump_label_dec);

 static int addr_conflict(struct jump_entry *entry, void *start, void *end)
 {
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
  2011-11-28 12:39 [PATCH 1/5] jump-label: export jump_label_inc/jump_label_dec Xiao Guangrong
@ 2011-11-28 12:41 ` Xiao Guangrong
  2011-11-28 22:33   ` Jason Baron
  2011-11-28 12:41 ` [PATCH 3/5] KVM: x86: remove the dead code of KVM_EXIT_HYPERCALL Xiao Guangrong
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Xiao Guangrong @ 2011-11-28 12:41 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, Jason Baron, LKML, KVM

The tracepoint is only used to audit mmu code, it should not be exposed to
user, let us replace it with jump-lable

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/kvm/mmu.c         |   16 +++++++++++-----
 arch/x86/kvm/mmu_audit.c   |   28 +++++++++++++---------------
 arch/x86/kvm/mmutrace.h    |   19 -------------------
 arch/x86/kvm/paging_tmpl.h |    4 ++--
 4 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index d737443..62f69db 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -68,6 +68,12 @@ char *audit_point_name[] = {
 	"post sync"
 };

+#ifdef CONFIG_KVM_MMU_AUDIT
+static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point);
+#else
+static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point) { }
+#endif
+
 #undef MMU_DEBUG

 #ifdef MMU_DEBUG
@@ -2852,12 +2858,12 @@ static void mmu_sync_roots(struct kvm_vcpu *vcpu)
 		return;

 	vcpu_clear_mmio_info(vcpu, ~0ul);
-	trace_kvm_mmu_audit(vcpu, AUDIT_PRE_SYNC);
+	kvm_mmu_audit(vcpu, AUDIT_PRE_SYNC);
 	if (vcpu->arch.mmu.root_level == PT64_ROOT_LEVEL) {
 		hpa_t root = vcpu->arch.mmu.root_hpa;
 		sp = page_header(root);
 		mmu_sync_children(vcpu, sp);
-		trace_kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
+		kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
 		return;
 	}
 	for (i = 0; i < 4; ++i) {
@@ -2869,7 +2875,7 @@ static void mmu_sync_roots(struct kvm_vcpu *vcpu)
 			mmu_sync_children(vcpu, sp);
 		}
 	}
-	trace_kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
+	kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
 }

 void kvm_mmu_sync_roots(struct kvm_vcpu *vcpu)
@@ -3667,7 +3673,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,

 	spin_lock(&vcpu->kvm->mmu_lock);
 	++vcpu->kvm->stat.mmu_pte_write;
-	trace_kvm_mmu_audit(vcpu, AUDIT_PRE_PTE_WRITE);
+	kvm_mmu_audit(vcpu, AUDIT_PRE_PTE_WRITE);

 	mask.cr0_wp = mask.cr4_pae = mask.nxe = 1;
 	for_each_gfn_indirect_valid_sp(vcpu->kvm, sp, gfn, node) {
@@ -3700,7 +3706,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 	}
 	mmu_pte_write_flush_tlb(vcpu, zap_page, remote_flush, local_flush);
 	kvm_mmu_commit_zap_page(vcpu->kvm, &invalid_list);
-	trace_kvm_mmu_audit(vcpu, AUDIT_POST_PTE_WRITE);
+	kvm_mmu_audit(vcpu, AUDIT_POST_PTE_WRITE);
 	spin_unlock(&vcpu->kvm->mmu_lock);
 }

diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 746ec25..5df6736 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -224,30 +224,29 @@ static void audit_vcpu_spte(struct kvm_vcpu *vcpu)
 	mmu_spte_walk(vcpu, audit_spte);
 }

-static void kvm_mmu_audit(void *ignore, struct kvm_vcpu *vcpu, int point)
+static bool mmu_audit;
+static struct jump_label_key mmu_audit_key;
+
+static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 {
 	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);

-	if (!__ratelimit(&ratelimit_state))
-		return;
+	if (static_branch((&mmu_audit_key))) {
+		if (!__ratelimit(&ratelimit_state))
+			return;

-	vcpu->kvm->arch.audit_point = point;
-	audit_all_active_sps(vcpu->kvm);
-	audit_vcpu_spte(vcpu);
+		vcpu->kvm->arch.audit_point = point;
+		audit_all_active_sps(vcpu->kvm);
+		audit_vcpu_spte(vcpu);
+	}
 }

-static bool mmu_audit;
-
 static void mmu_audit_enable(void)
 {
-	int ret;
-
 	if (mmu_audit)
 		return;

-	ret = register_trace_kvm_mmu_audit(kvm_mmu_audit, NULL);
-	WARN_ON(ret);
-
+	jump_label_inc(&mmu_audit_key);
 	mmu_audit = true;
 }

@@ -256,8 +255,7 @@ static void mmu_audit_disable(void)
 	if (!mmu_audit)
 		return;

-	unregister_trace_kvm_mmu_audit(kvm_mmu_audit, NULL);
-	tracepoint_synchronize_unregister();
+	jump_label_dec(&mmu_audit_key);
 	mmu_audit = false;
 }

diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index eed67f3..89fb0e8 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -243,25 +243,6 @@ TRACE_EVENT(
 	TP_printk("addr:%llx gfn %llx access %x", __entry->addr, __entry->gfn,
 		  __entry->access)
 );
-
-TRACE_EVENT(
-	kvm_mmu_audit,
-	TP_PROTO(struct kvm_vcpu *vcpu, int audit_point),
-	TP_ARGS(vcpu, audit_point),
-
-	TP_STRUCT__entry(
-		__field(struct kvm_vcpu *, vcpu)
-		__field(int, audit_point)
-	),
-
-	TP_fast_assign(
-		__entry->vcpu = vcpu;
-		__entry->audit_point = audit_point;
-	),
-
-	TP_printk("vcpu:%d %s", __entry->vcpu->cpu,
-		  audit_point_name[__entry->audit_point])
-);
 #endif /* _TRACE_KVMMMU_H */

 #undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 52e9d58..1561028 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -632,7 +632,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
 	if (mmu_notifier_retry(vcpu, mmu_seq))
 		goto out_unlock;

-	trace_kvm_mmu_audit(vcpu, AUDIT_PRE_PAGE_FAULT);
+	kvm_mmu_audit(vcpu, AUDIT_PRE_PAGE_FAULT);
 	kvm_mmu_free_some_pages(vcpu);
 	if (!force_pt_level)
 		transparent_hugepage_adjust(vcpu, &walker.gfn, &pfn, &level);
@@ -643,7 +643,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
 		 sptep, *sptep, emulate);

 	++vcpu->stat.pf_fixed;
-	trace_kvm_mmu_audit(vcpu, AUDIT_POST_PAGE_FAULT);
+	kvm_mmu_audit(vcpu, AUDIT_POST_PAGE_FAULT);
 	spin_unlock(&vcpu->kvm->mmu_lock);

 	return emulate;
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] KVM: x86: remove the dead code of KVM_EXIT_HYPERCALL
  2011-11-28 12:39 [PATCH 1/5] jump-label: export jump_label_inc/jump_label_dec Xiao Guangrong
  2011-11-28 12:41 ` [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable Xiao Guangrong
@ 2011-11-28 12:41 ` Xiao Guangrong
  2011-11-28 12:56   ` Gleb Natapov
  2011-11-28 12:42 ` [PATCH 4/5] KVM: MMU: move the relevant mmu code to mmu.c Xiao Guangrong
  2011-11-28 12:43 ` [PATCH 5/5] KVM: MMU: remove oos_shadow parameter Xiao Guangrong
  3 siblings, 1 reply; 13+ messages in thread
From: Xiao Guangrong @ 2011-11-28 12:41 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

KVM_EXIT_HYPERCALL is not used anymore, so remove the code

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/kvm/x86.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d54746c..7b6fd72 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5386,10 +5386,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	if (r <= 0)
 		goto out;

-	if (kvm_run->exit_reason == KVM_EXIT_HYPERCALL)
-		kvm_register_write(vcpu, VCPU_REGS_RAX,
-				     kvm_run->hypercall.ret);
-
 	r = __vcpu_run(vcpu);

 out:
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] KVM: MMU: move the relevant mmu code to mmu.c
  2011-11-28 12:39 [PATCH 1/5] jump-label: export jump_label_inc/jump_label_dec Xiao Guangrong
  2011-11-28 12:41 ` [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable Xiao Guangrong
  2011-11-28 12:41 ` [PATCH 3/5] KVM: x86: remove the dead code of KVM_EXIT_HYPERCALL Xiao Guangrong
@ 2011-11-28 12:42 ` Xiao Guangrong
  2011-11-28 12:43 ` [PATCH 5/5] KVM: MMU: remove oos_shadow parameter Xiao Guangrong
  3 siblings, 0 replies; 13+ messages in thread
From: Xiao Guangrong @ 2011-11-28 12:42 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

Move the mmu code in kvm_arch_vcpu_init() to kvm_mmu_create()

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/include/asm/kvm_host.h |    6 ++++++
 arch/x86/kvm/mmu.c              |    6 +++++-
 arch/x86/kvm/x86.c              |   11 +----------
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1769f3d..020413a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -752,6 +752,7 @@ void __kvm_mmu_free_some_pages(struct kvm_vcpu *vcpu);
 int kvm_mmu_load(struct kvm_vcpu *vcpu);
 void kvm_mmu_unload(struct kvm_vcpu *vcpu);
 void kvm_mmu_sync_roots(struct kvm_vcpu *vcpu);
+gpa_t translate_nested_gpa(struct kvm_vcpu *vcpu, gpa_t gpa, u32 access);
 gpa_t kvm_mmu_gva_to_gpa_read(struct kvm_vcpu *vcpu, gva_t gva,
 			      struct x86_exception *exception);
 gpa_t kvm_mmu_gva_to_gpa_fetch(struct kvm_vcpu *vcpu, gva_t gva,
@@ -773,6 +774,11 @@ void kvm_disable_tdp(void);
 int complete_pio(struct kvm_vcpu *vcpu);
 bool kvm_check_iopl(struct kvm_vcpu *vcpu);

+static inline gpa_t translate_gpa(struct kvm_vcpu *vcpu, gpa_t gpa, u32 access)
+{
+	return gpa;
+}
+
 static inline struct kvm_mmu_page *page_header(hpa_t shadow_page)
 {
 	struct page *page = pfn_to_page(shadow_page >> PAGE_SHIFT);
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 62f69db..262a3af 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3839,7 +3839,11 @@ static int alloc_mmu_pages(struct kvm_vcpu *vcpu)
 int kvm_mmu_create(struct kvm_vcpu *vcpu)
 {
 	ASSERT(vcpu);
-	ASSERT(!VALID_PAGE(vcpu->arch.mmu.root_hpa));
+
+	vcpu->arch.walk_mmu = &vcpu->arch.mmu;
+	vcpu->arch.mmu.root_hpa = INVALID_PAGE;
+	vcpu->arch.mmu.translate_gpa = translate_gpa;
+	vcpu->arch.nested_mmu.translate_gpa = translate_nested_gpa;

 	return alloc_mmu_pages(vcpu);
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7b6fd72..45c8640 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3427,12 +3427,7 @@ void kvm_get_segment(struct kvm_vcpu *vcpu,
 	kvm_x86_ops->get_segment(vcpu, var, seg);
 }

-static gpa_t translate_gpa(struct kvm_vcpu *vcpu, gpa_t gpa, u32 access)
-{
-	return gpa;
-}
-
-static gpa_t translate_nested_gpa(struct kvm_vcpu *vcpu, gpa_t gpa, u32 access)
+gpa_t translate_nested_gpa(struct kvm_vcpu *vcpu, gpa_t gpa, u32 access)
 {
 	gpa_t t_gpa;
 	struct x86_exception exception;
@@ -5912,10 +5907,6 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	kvm = vcpu->kvm;

 	vcpu->arch.emulate_ctxt.ops = &emulate_ops;
-	vcpu->arch.walk_mmu = &vcpu->arch.mmu;
-	vcpu->arch.mmu.root_hpa = INVALID_PAGE;
-	vcpu->arch.mmu.translate_gpa = translate_gpa;
-	vcpu->arch.nested_mmu.translate_gpa = translate_nested_gpa;
 	if (!irqchip_in_kernel(kvm) || kvm_vcpu_is_bsp(vcpu))
 		vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
 	else
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] KVM: MMU: remove oos_shadow parameter
  2011-11-28 12:39 [PATCH 1/5] jump-label: export jump_label_inc/jump_label_dec Xiao Guangrong
                   ` (2 preceding siblings ...)
  2011-11-28 12:42 ` [PATCH 4/5] KVM: MMU: move the relevant mmu code to mmu.c Xiao Guangrong
@ 2011-11-28 12:43 ` Xiao Guangrong
  2011-11-28 15:09   ` Avi Kivity
  3 siblings, 1 reply; 13+ messages in thread
From: Xiao Guangrong @ 2011-11-28 12:43 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

The unsync code should be stable now, maybe it is the time to remove this
parameter to cleanup the code a little bit

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 Documentation/kernel-parameters.txt |    3 ---
 arch/x86/kvm/mmu.c                  |    5 -----
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a0c5c5f..7402a24 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1178,9 +1178,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs.
 			Default is 0 (don't ignore, but inject #GP)

-	kvm.oos_shadow=	[KVM] Disable out-of-sync shadow paging.
-			Default is 1 (enabled)
-
 	kvm.mmu_audit=	[KVM] This is a R/W parameter which allows audit
 			KVM MMU at runtime.
 			Default is 0 (off)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 262a3af..b1178d1 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -93,9 +93,6 @@ static int dbg = 0;
 module_param(dbg, bool, 0644);
 #endif

-static int oos_shadow = 1;
-module_param(oos_shadow, bool, 0644);
-
 #ifndef MMU_DEBUG
 #define ASSERT(x) do { } while (0)
 #else
@@ -2196,8 +2193,6 @@ static int mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn,
 			return 1;

 		if (!need_unsync && !s->unsync) {
-			if (!oos_shadow)
-				return 1;
 			need_unsync = true;
 		}
 	}
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] KVM: x86: remove the dead code of KVM_EXIT_HYPERCALL
  2011-11-28 12:41 ` [PATCH 3/5] KVM: x86: remove the dead code of KVM_EXIT_HYPERCALL Xiao Guangrong
@ 2011-11-28 12:56   ` Gleb Natapov
  2011-11-28 14:39     ` Avi Kivity
  0 siblings, 1 reply; 13+ messages in thread
From: Gleb Natapov @ 2011-11-28 12:56 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

On Mon, Nov 28, 2011 at 08:41:38PM +0800, Xiao Guangrong wrote:
> KVM_EXIT_HYPERCALL is not used anymore, so remove the code
> 
Why not remove the define as well?

> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  arch/x86/kvm/x86.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index d54746c..7b6fd72 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5386,10 +5386,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  	if (r <= 0)
>  		goto out;
> 
> -	if (kvm_run->exit_reason == KVM_EXIT_HYPERCALL)
> -		kvm_register_write(vcpu, VCPU_REGS_RAX,
> -				     kvm_run->hypercall.ret);
> -
>  	r = __vcpu_run(vcpu);
> 
>  out:
> -- 
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
			Gleb.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] KVM: x86: remove the dead code of KVM_EXIT_HYPERCALL
  2011-11-28 12:56   ` Gleb Natapov
@ 2011-11-28 14:39     ` Avi Kivity
  0 siblings, 0 replies; 13+ messages in thread
From: Avi Kivity @ 2011-11-28 14:39 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Xiao Guangrong, Marcelo Tosatti, LKML, KVM

On 11/28/2011 02:56 PM, Gleb Natapov wrote:
> On Mon, Nov 28, 2011 at 08:41:38PM +0800, Xiao Guangrong wrote:
> > KVM_EXIT_HYPERCALL is not used anymore, so remove the code
> > 
> Why not remove the define as well?
>
>

It's in kvm.h, best not to touch it in case someone has

  case KVM_EXIT_HYPERCALL:
       abort();

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] KVM: MMU: remove oos_shadow parameter
  2011-11-28 12:43 ` [PATCH 5/5] KVM: MMU: remove oos_shadow parameter Xiao Guangrong
@ 2011-11-28 15:09   ` Avi Kivity
  0 siblings, 0 replies; 13+ messages in thread
From: Avi Kivity @ 2011-11-28 15:09 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Marcelo Tosatti, LKML, KVM

On 11/28/2011 02:43 PM, Xiao Guangrong wrote:
> The unsync code should be stable now, maybe it is the time to remove this
> parameter to cleanup the code a little bit
>

Thanks, all applied.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
  2011-11-28 12:41 ` [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable Xiao Guangrong
@ 2011-11-28 22:33   ` Jason Baron
  2011-11-29  3:56     ` Xiao Guangrong
  0 siblings, 1 reply; 13+ messages in thread
From: Jason Baron @ 2011-11-28 22:33 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

On Mon, Nov 28, 2011 at 08:41:00PM +0800, Xiao Guangrong wrote:
> +static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
>  {
>  	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);
> 
> -	if (!__ratelimit(&ratelimit_state))
> -		return;
> +	if (static_branch((&mmu_audit_key))) {
> +		if (!__ratelimit(&ratelimit_state))
> +			return;
> 
> -	vcpu->kvm->arch.audit_point = point;
> -	audit_all_active_sps(vcpu->kvm);
> -	audit_vcpu_spte(vcpu);
> +		vcpu->kvm->arch.audit_point = point;
> +		audit_all_active_sps(vcpu->kvm);
> +		audit_vcpu_spte(vcpu);
> +	}
>  }

hmmm..this always going to do a call to 'kvm_mmu_audit' and then return.
I think you want to avoid the function call altogether. You could do
something like:

#define kvm_mmu_audit()
	if (static_branch((&mmu_audit_key))) {
		__kvm_mmu_audit();
	}

and s/kvm_mmu_audit/__kvm_mmu_audit

That should give you a single nop for the case where kvm_mmu_audit is
disabled instead of a function call.

Thanks,

-Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
  2011-11-28 22:33   ` Jason Baron
@ 2011-11-29  3:56     ` Xiao Guangrong
  2011-11-29 10:02       ` Avi Kivity
  0 siblings, 1 reply; 13+ messages in thread
From: Xiao Guangrong @ 2011-11-29  3:56 UTC (permalink / raw)
  To: Jason Baron; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM

On 11/29/2011 06:33 AM, Jason Baron wrote:

 
> hmmm..this always going to do a call to 'kvm_mmu_audit' and then return.
> I think you want to avoid the function call altogether. You could do
> something like:
> 
> #define kvm_mmu_audit()
> 	if (static_branch((&mmu_audit_key))) {
> 		__kvm_mmu_audit();
> 	}
> 
> and s/kvm_mmu_audit/__kvm_mmu_audit
> 
> That should give you a single nop for the case where kvm_mmu_audit is
> disabled instead of a function call.


Good point, thanks Jason!

Avi, could you please apply the following patch instead?

Subject: [PATCH v2 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable

The tracepoint is only used to audit mmu code, it should not be exposed to
user, let us replace it with jump-lable

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/kvm/mmu.c         |   23 ++++++++++++-----------
 arch/x86/kvm/mmu_audit.c   |   17 +++++++++--------
 arch/x86/kvm/mmutrace.h    |   19 -------------------
 arch/x86/kvm/paging_tmpl.h |    4 ++--
 4 files changed, 23 insertions(+), 40 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index d737443..34bc3fc 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2840,6 +2840,13 @@ static int mmu_alloc_roots(struct kvm_vcpu *vcpu)
 		return mmu_alloc_shadow_roots(vcpu);
 }

+#ifdef CONFIG_KVM_MMU_AUDIT
+#include "mmu_audit.c"
+#else
+static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point) { }
+static void mmu_audit_disable(void) { }
+#endif
+
 static void mmu_sync_roots(struct kvm_vcpu *vcpu)
 {
 	int i;
@@ -2852,12 +2859,12 @@ static void mmu_sync_roots(struct kvm_vcpu *vcpu)
 		return;

 	vcpu_clear_mmio_info(vcpu, ~0ul);
-	trace_kvm_mmu_audit(vcpu, AUDIT_PRE_SYNC);
+	kvm_mmu_audit(vcpu, AUDIT_PRE_SYNC);
 	if (vcpu->arch.mmu.root_level == PT64_ROOT_LEVEL) {
 		hpa_t root = vcpu->arch.mmu.root_hpa;
 		sp = page_header(root);
 		mmu_sync_children(vcpu, sp);
-		trace_kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
+		kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
 		return;
 	}
 	for (i = 0; i < 4; ++i) {
@@ -2869,7 +2876,7 @@ static void mmu_sync_roots(struct kvm_vcpu *vcpu)
 			mmu_sync_children(vcpu, sp);
 		}
 	}
-	trace_kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
+	kvm_mmu_audit(vcpu, AUDIT_POST_SYNC);
 }

 void kvm_mmu_sync_roots(struct kvm_vcpu *vcpu)
@@ -3667,7 +3674,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,

 	spin_lock(&vcpu->kvm->mmu_lock);
 	++vcpu->kvm->stat.mmu_pte_write;
-	trace_kvm_mmu_audit(vcpu, AUDIT_PRE_PTE_WRITE);
+	kvm_mmu_audit(vcpu, AUDIT_PRE_PTE_WRITE);

 	mask.cr0_wp = mask.cr4_pae = mask.nxe = 1;
 	for_each_gfn_indirect_valid_sp(vcpu->kvm, sp, gfn, node) {
@@ -3700,7 +3707,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 	}
 	mmu_pte_write_flush_tlb(vcpu, zap_page, remote_flush, local_flush);
 	kvm_mmu_commit_zap_page(vcpu->kvm, &invalid_list);
-	trace_kvm_mmu_audit(vcpu, AUDIT_POST_PTE_WRITE);
+	kvm_mmu_audit(vcpu, AUDIT_POST_PTE_WRITE);
 	spin_unlock(&vcpu->kvm->mmu_lock);
 }

@@ -4030,12 +4037,6 @@ void kvm_mmu_destroy(struct kvm_vcpu *vcpu)
 	mmu_free_memory_caches(vcpu);
 }

-#ifdef CONFIG_KVM_MMU_AUDIT
-#include "mmu_audit.c"
-#else
-static void mmu_audit_disable(void) { }
-#endif
-
 void kvm_mmu_module_exit(void)
 {
 	mmu_destroy_caches();
diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 746ec25..967a535 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -224,7 +224,7 @@ static void audit_vcpu_spte(struct kvm_vcpu *vcpu)
 	mmu_spte_walk(vcpu, audit_spte);
 }

-static void kvm_mmu_audit(void *ignore, struct kvm_vcpu *vcpu, int point)
+static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 {
 	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);

@@ -237,17 +237,19 @@ static void kvm_mmu_audit(void *ignore, struct kvm_vcpu *vcpu, int point)
 }

 static bool mmu_audit;
+static struct jump_label_key mmu_audit_key;
+
+#define kvm_mmu_audit(vcpu, point)		\
+	if (static_branch((&mmu_audit_key))) {	\
+		__kvm_mmu_audit(vcpu, point);	\
+	}

 static void mmu_audit_enable(void)
 {
-	int ret;
-
 	if (mmu_audit)
 		return;

-	ret = register_trace_kvm_mmu_audit(kvm_mmu_audit, NULL);
-	WARN_ON(ret);
-
+	jump_label_inc(&mmu_audit_key);
 	mmu_audit = true;
 }

@@ -256,8 +258,7 @@ static void mmu_audit_disable(void)
 	if (!mmu_audit)
 		return;

-	unregister_trace_kvm_mmu_audit(kvm_mmu_audit, NULL);
-	tracepoint_synchronize_unregister();
+	jump_label_dec(&mmu_audit_key);
 	mmu_audit = false;
 }

diff --git a/arch/x86/kvm/mmutrace.h b/arch/x86/kvm/mmutrace.h
index eed67f3..89fb0e8 100644
--- a/arch/x86/kvm/mmutrace.h
+++ b/arch/x86/kvm/mmutrace.h
@@ -243,25 +243,6 @@ TRACE_EVENT(
 	TP_printk("addr:%llx gfn %llx access %x", __entry->addr, __entry->gfn,
 		  __entry->access)
 );
-
-TRACE_EVENT(
-	kvm_mmu_audit,
-	TP_PROTO(struct kvm_vcpu *vcpu, int audit_point),
-	TP_ARGS(vcpu, audit_point),
-
-	TP_STRUCT__entry(
-		__field(struct kvm_vcpu *, vcpu)
-		__field(int, audit_point)
-	),
-
-	TP_fast_assign(
-		__entry->vcpu = vcpu;
-		__entry->audit_point = audit_point;
-	),
-
-	TP_printk("vcpu:%d %s", __entry->vcpu->cpu,
-		  audit_point_name[__entry->audit_point])
-);
 #endif /* _TRACE_KVMMMU_H */

 #undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 52e9d58..1561028 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -632,7 +632,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
 	if (mmu_notifier_retry(vcpu, mmu_seq))
 		goto out_unlock;

-	trace_kvm_mmu_audit(vcpu, AUDIT_PRE_PAGE_FAULT);
+	kvm_mmu_audit(vcpu, AUDIT_PRE_PAGE_FAULT);
 	kvm_mmu_free_some_pages(vcpu);
 	if (!force_pt_level)
 		transparent_hugepage_adjust(vcpu, &walker.gfn, &pfn, &level);
@@ -643,7 +643,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr, u32 error_code,
 		 sptep, *sptep, emulate);

 	++vcpu->stat.pf_fixed;
-	trace_kvm_mmu_audit(vcpu, AUDIT_POST_PAGE_FAULT);
+	kvm_mmu_audit(vcpu, AUDIT_POST_PAGE_FAULT);
 	spin_unlock(&vcpu->kvm->mmu_lock);

 	return emulate;
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
  2011-11-29  3:56     ` Xiao Guangrong
@ 2011-11-29 10:02       ` Avi Kivity
  2011-11-30  9:43         ` Xiao Guangrong
  0 siblings, 1 reply; 13+ messages in thread
From: Avi Kivity @ 2011-11-29 10:02 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Jason Baron, Marcelo Tosatti, LKML, KVM

On 11/29/2011 05:56 AM, Xiao Guangrong wrote:
> Subject: [PATCH v2 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
>
> The tracepoint is only used to audit mmu code, it should not be exposed to
> user, let us replace it with jump-lable
>
>
>  static bool mmu_audit;
> +static struct jump_label_key mmu_audit_key;
> +
> +#define kvm_mmu_audit(vcpu, point)		\
> +	if (static_branch((&mmu_audit_key))) {	\
> +		__kvm_mmu_audit(vcpu, point);	\
> +	}
>
>


static inline function, please, and as an incremental against next. I'll
fold it to the parent patch.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
  2011-11-29 10:02       ` Avi Kivity
@ 2011-11-30  9:43         ` Xiao Guangrong
  2011-12-01 10:28           ` Avi Kivity
  0 siblings, 1 reply; 13+ messages in thread
From: Xiao Guangrong @ 2011-11-30  9:43 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Jason Baron, Marcelo Tosatti, LKML, KVM

On 11/29/2011 06:02 PM, Avi Kivity wrote:

> On 11/29/2011 05:56 AM, Xiao Guangrong wrote:
>> Subject: [PATCH v2 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
>>
>> The tracepoint is only used to audit mmu code, it should not be exposed to
>> user, let us replace it with jump-lable
>>
>>
>>  static bool mmu_audit;
>> +static struct jump_label_key mmu_audit_key;
>> +
>> +#define kvm_mmu_audit(vcpu, point)		\
>> +	if (static_branch((&mmu_audit_key))) {	\
>> +		__kvm_mmu_audit(vcpu, point);	\
>> +	}
>>
>>
> 
> 
> static inline function, please, and as an incremental against next. I'll
> fold it to the parent patch.
> 


OK, this is the new one. Thanks!

Subject: [PATCH] KVM: MMU: audit: inline audit function

inline audit function and little cleanup

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/kvm/mmu.c       |   28 +++++++---------------------
 arch/x86/kvm/mmu_audit.c |   29 +++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index b1178d1..7a8e99c 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -59,21 +59,6 @@ enum {
 	AUDIT_POST_SYNC
 };

-char *audit_point_name[] = {
-	"pre page fault",
-	"post page fault",
-	"pre pte write",
-	"post pte write",
-	"pre sync",
-	"post sync"
-};
-
-#ifdef CONFIG_KVM_MMU_AUDIT
-static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point);
-#else
-static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point) { }
-#endif
-
 #undef MMU_DEBUG

 #ifdef MMU_DEBUG
@@ -1539,6 +1524,13 @@ static int kvm_sync_page_transient(struct kvm_vcpu *vcpu,
 	return ret;
 }

+#ifdef CONFIG_KVM_MMU_AUDIT
+#include "mmu_audit.c"
+#else
+static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point) { }
+static void mmu_audit_disable(void) { }
+#endif
+
 static int kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 			 struct list_head *invalid_list)
 {
@@ -4035,12 +4027,6 @@ void kvm_mmu_destroy(struct kvm_vcpu *vcpu)
 	mmu_free_memory_caches(vcpu);
 }

-#ifdef CONFIG_KVM_MMU_AUDIT
-#include "mmu_audit.c"
-#else
-static void mmu_audit_disable(void) { }
-#endif
-
 void kvm_mmu_module_exit(void)
 {
 	mmu_destroy_caches();
diff --git a/arch/x86/kvm/mmu_audit.c b/arch/x86/kvm/mmu_audit.c
index 5df6736..fe15dcc 100644
--- a/arch/x86/kvm/mmu_audit.c
+++ b/arch/x86/kvm/mmu_audit.c
@@ -19,6 +19,15 @@

 #include <linux/ratelimit.h>

+char const *audit_point_name[] = {
+	"pre page fault",
+	"post page fault",
+	"pre pte write",
+	"post pte write",
+	"pre sync",
+	"post sync"
+};
+
 #define audit_printk(kvm, fmt, args...)		\
 	printk(KERN_ERR "audit: (%s) error: "	\
 		fmt, audit_point_name[kvm->arch.audit_point], ##args)
@@ -227,18 +236,22 @@ static void audit_vcpu_spte(struct kvm_vcpu *vcpu)
 static bool mmu_audit;
 static struct jump_label_key mmu_audit_key;

-static void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
+static void __kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
 {
 	static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 10);

-	if (static_branch((&mmu_audit_key))) {
-		if (!__ratelimit(&ratelimit_state))
-			return;
+	if (!__ratelimit(&ratelimit_state))
+		return;

-		vcpu->kvm->arch.audit_point = point;
-		audit_all_active_sps(vcpu->kvm);
-		audit_vcpu_spte(vcpu);
-	}
+	vcpu->kvm->arch.audit_point = point;
+	audit_all_active_sps(vcpu->kvm);
+	audit_vcpu_spte(vcpu);
+}
+
+static inline void kvm_mmu_audit(struct kvm_vcpu *vcpu, int point)
+{
+	if (static_branch((&mmu_audit_key)))
+		__kvm_mmu_audit(vcpu, point);
 }

 static void mmu_audit_enable(void)
-- 
1.7.7.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable
  2011-11-30  9:43         ` Xiao Guangrong
@ 2011-12-01 10:28           ` Avi Kivity
  0 siblings, 0 replies; 13+ messages in thread
From: Avi Kivity @ 2011-12-01 10:28 UTC (permalink / raw)
  To: Xiao Guangrong; +Cc: Jason Baron, Marcelo Tosatti, LKML, KVM

On 11/30/2011 11:43 AM, Xiao Guangrong wrote:
> Subject: [PATCH] KVM: MMU: audit: inline audit function
>
> inline audit function and little cleanup
>
>

Thanks, applied.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-12-01 10:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-28 12:39 [PATCH 1/5] jump-label: export jump_label_inc/jump_label_dec Xiao Guangrong
2011-11-28 12:41 ` [PATCH 2/5] KVM: MMU: audit: replace mmu audit tracepoint with jump-lable Xiao Guangrong
2011-11-28 22:33   ` Jason Baron
2011-11-29  3:56     ` Xiao Guangrong
2011-11-29 10:02       ` Avi Kivity
2011-11-30  9:43         ` Xiao Guangrong
2011-12-01 10:28           ` Avi Kivity
2011-11-28 12:41 ` [PATCH 3/5] KVM: x86: remove the dead code of KVM_EXIT_HYPERCALL Xiao Guangrong
2011-11-28 12:56   ` Gleb Natapov
2011-11-28 14:39     ` Avi Kivity
2011-11-28 12:42 ` [PATCH 4/5] KVM: MMU: move the relevant mmu code to mmu.c Xiao Guangrong
2011-11-28 12:43 ` [PATCH 5/5] KVM: MMU: remove oos_shadow parameter Xiao Guangrong
2011-11-28 15:09   ` Avi Kivity

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).