All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] xenoprof: fix up ability to disable it
@ 2016-02-08 16:04 Doug Goldstein
  2016-02-08 16:04 ` [PATCH 2/2] build: convert xenoprof to Kconfig Doug Goldstein
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Doug Goldstein @ 2016-02-08 16:04 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Keir Fraser, Jan Beulich, Jun Nakajima,
	Andrew Cooper, Doug Goldstein, Aravind Gopalakrishnan,
	Suravee Suthikulpanit, Boris Ostrovsky

Allow Xenoprof to be fully disabled when toggling the option off.

CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 xen/arch/x86/Makefile              |  2 +-
 xen/arch/x86/Rules.mk              |  2 ++
 xen/arch/x86/cpu/vpmu_amd.c        |  9 ++++++---
 xen/arch/x86/cpu/vpmu_intel.c      |  9 ++++++---
 xen/arch/x86/hvm/svm/svm.c         |  4 +++-
 xen/arch/x86/hvm/vmx/vmx.c         | 12 ++++++++----
 xen/arch/x86/x86_64/compat/entry.S |  4 ++++
 xen/arch/x86/x86_64/entry.S        |  4 ++++
 xen/include/asm-x86/config.h       |  1 -
 9 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 8e6e901..434d985 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -3,7 +3,7 @@ subdir-y += cpu
 subdir-y += genapic
 subdir-y += hvm
 subdir-y += mm
-subdir-y += oprofile
+subdir-$(xenoprof) += oprofile
 subdir-y += x86_64
 
 obj-bin-y += alternative.init.o
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index a1cdae0..94e4efd 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -10,6 +10,8 @@ CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
 CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
 
+CFLAGS-$(xenoprof) += -DCONFIG_XENOPROF
+
 # Prevent floating-point variables from creeping into Xen.
 CFLAGS += -msoft-float
 
diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index 990e6f3..c7b5bd5 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -22,6 +22,7 @@
  */
 
 #include <xen/config.h>
+#include <xen/kconfig.h>
 #include <xen/xenoprof.h>
 #include <xen/hvm/save.h>
 #include <xen/sched.h>
@@ -378,7 +379,8 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
     if ( (type == MSR_TYPE_CTRL) &&
         is_pmu_enabled(msr_content) && !vpmu_is_set(vpmu, VPMU_RUNNING) )
     {
-        if ( !acquire_pmu_ownership(PMU_OWNER_HVM) )
+        if ( IS_ENABLED(CONFIG_XENOPROF) &&
+             !acquire_pmu_ownership(PMU_OWNER_HVM) )
             return 0;
         vpmu_set(vpmu, VPMU_RUNNING);
 
@@ -393,7 +395,8 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
         vpmu_reset(vpmu, VPMU_RUNNING);
         if ( has_hvm_container_vcpu(v) && is_msr_bitmap_on(vpmu) )
              amd_vpmu_unset_msr_bitmap(v);
-        release_pmu_ownship(PMU_OWNER_HVM);
+        if ( IS_ENABLED(CONFIG_XENOPROF) )
+            release_pmu_ownship(PMU_OWNER_HVM);
     }
 
     if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED)
@@ -441,7 +444,7 @@ static void amd_vpmu_destroy(struct vcpu *v)
     vpmu->context = NULL;
     vpmu->priv_context = NULL;
 
-    if ( vpmu_is_set(vpmu, VPMU_RUNNING) )
+    if ( IS_ENABLED(CONFIG_XENOPROF) && vpmu_is_set(vpmu, VPMU_RUNNING) )
         release_pmu_ownship(PMU_OWNER_HVM);
 
     vpmu_clear(vpmu);
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 50b5831..226f5e7 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -19,6 +19,7 @@
  */
 
 #include <xen/config.h>
+#include <xen/kconfig.h>
 #include <xen/sched.h>
 #include <xen/xenoprof.h>
 #include <xen/irq.h>
@@ -476,7 +477,7 @@ static int core2_vpmu_alloc_resource(struct vcpu *v)
     struct xen_pmu_intel_ctxt *core2_vpmu_cxt = NULL;
     uint64_t *p = NULL;
 
-    if ( !acquire_pmu_ownership(PMU_OWNER_HVM) )
+    if ( IS_ENABLED(CONFIG_XENOPROF) && !acquire_pmu_ownership(PMU_OWNER_HVM) )
         return 0;
 
     if ( has_hvm_container_vcpu(v) )
@@ -517,7 +518,8 @@ static int core2_vpmu_alloc_resource(struct vcpu *v)
     return 1;
 
 out_err:
-    release_pmu_ownship(PMU_OWNER_HVM);
+    if ( IS_ENABLED(CONFIG_XENOPROF) )
+        release_pmu_ownship(PMU_OWNER_HVM);
 
     xfree(core2_vpmu_cxt);
     xfree(p);
@@ -892,7 +894,8 @@ static void core2_vpmu_destroy(struct vcpu *v)
     vpmu->priv_context = NULL;
     if ( has_hvm_container_vcpu(v) && cpu_has_vmx_msr_bitmap )
         core2_vpmu_unset_msr_bitmap(v->arch.hvm_vmx.msr_bitmap);
-    release_pmu_ownship(PMU_OWNER_HVM);
+    if ( IS_ENABLED(CONFIG_XENOPROF) )
+        release_pmu_ownship(PMU_OWNER_HVM);
     vpmu_clear(vpmu);
 }
 
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index e62dfa1..473c781 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -17,6 +17,7 @@
  */
 
 #include <xen/config.h>
+#include <xen/kconfig.h>
 #include <xen/init.h>
 #include <xen/lib.h>
 #include <xen/trace.h>
@@ -1214,7 +1215,8 @@ static void svm_vcpu_destroy(struct vcpu *v)
 {
     vpmu_destroy(v);
     svm_destroy_vmcb(v);
-    passive_domain_destroy(v);
+    if ( IS_ENABLED(CONFIG_XENOPROF) )
+        passive_domain_destroy(v);
 }
 
 static void svm_inject_trap(const struct hvm_trap *trap)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b9f340c..669bc2d 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -16,6 +16,7 @@
  */
 
 #include <xen/config.h>
+#include <xen/kconfig.h>
 #include <xen/init.h>
 #include <xen/lib.h>
 #include <xen/trace.h>
@@ -173,7 +174,8 @@ static void vmx_vcpu_destroy(struct vcpu *v)
     vmx_vcpu_disable_pml(v);
     vmx_destroy_vmcs(v);
     vpmu_destroy(v);
-    passive_domain_destroy(v);
+    if ( IS_ENABLED(CONFIG_XENOPROF) )
+        passive_domain_destroy(v);
 }
 
 static DEFINE_PER_CPU(struct vmx_msr_state, host_msr_state);
@@ -2447,7 +2449,8 @@ static int vmx_msr_read_intercept(unsigned int msr, uint64_t *msr_content)
             goto gp_fault;
         break;
     default:
-        if ( passive_domain_do_rdmsr(msr, msr_content) )
+        if ( IS_ENABLED(CONFIG_XENOPROF) &&
+             passive_domain_do_rdmsr(msr, msr_content) )
             goto done;
         switch ( long_mode_do_msr_read(msr, msr_content) )
         {
@@ -2666,10 +2669,11 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
             goto gp_fault;
         break;
     default:
-        if ( passive_domain_do_wrmsr(msr, msr_content) )
+        if ( IS_ENABLED(CONFIG_XENOPROF) &&
+             passive_domain_do_wrmsr(msr, msr_content) )
             return X86EMUL_OKAY;
 
-        if ( wrmsr_viridian_regs(msr, msr_content) ) 
+        if ( wrmsr_viridian_regs(msr, msr_content) )
             break;
 
         switch ( long_mode_do_msr_write(msr, msr_content) )
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index 3088aa7..6424ed0 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -394,6 +394,10 @@ compat_crash_page_fault:
 #define compat_kexec_op do_ni_hypercall
 #endif
 
+#ifndef CONFIG_XENOPROF
+#define compat_xenoprof_op do_ni_hypercall
+#endif
+
 ENTRY(compat_hypercall_table)
         .quad compat_set_trap_table     /*  0 */
         .quad do_mmu_update
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 94a54aa..0a73878 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -727,6 +727,10 @@ ENTRY(exception_table)
 #define do_kexec_op do_ni_hypercall
 #endif
 
+#ifndef CONFIG_XENOPROF
+#define do_xenoprof_op do_ni_hypercall
+#endif
+
 ENTRY(hypercall_table)
         .quad do_set_trap_table     /*  0 */
         .quad do_mmu_update
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index d97877d..a45d3ee 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -47,7 +47,6 @@
 #define CONFIG_VGA 1
 #define CONFIG_VIDEO 1
 
-#define CONFIG_XENOPROF 1
 #define CONFIG_WATCHDOG 1
 
 #define CONFIG_MULTIBOOT 1
-- 
2.4.10

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

* [PATCH 2/2] build: convert xenoprof to Kconfig
  2016-02-08 16:04 [PATCH 1/2] xenoprof: fix up ability to disable it Doug Goldstein
@ 2016-02-08 16:04 ` Doug Goldstein
  2016-02-08 16:14   ` Andrew Cooper
  2016-02-08 16:24 ` [PATCH 1/2] xenoprof: fix up ability to disable it Boris Ostrovsky
  2016-02-08 16:25 ` Andrew Cooper
  2 siblings, 1 reply; 8+ messages in thread
From: Doug Goldstein @ 2016-02-08 16:04 UTC (permalink / raw)
  To: xen-devel; +Cc: Keir Fraser, Doug Goldstein, Jan Beulich, Andrew Cooper

Convert the xenoprof x86 build time option to Kconfig.

CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 xen/arch/x86/Makefile |  2 +-
 xen/arch/x86/Rules.mk |  3 ---
 xen/common/Kconfig    | 13 +++++++++++++
 xen/common/Makefile   |  2 +-
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 434d985..1bcb08b 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -3,7 +3,7 @@ subdir-y += cpu
 subdir-y += genapic
 subdir-y += hvm
 subdir-y += mm
-subdir-$(xenoprof) += oprofile
+subdir-$(CONFIG_XENOPROF) += oprofile
 subdir-y += x86_64
 
 obj-bin-y += alternative.init.o
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 94e4efd..14519e3 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -3,15 +3,12 @@
 
 HAS_NUMA := y
 HAS_CORE_PARKING := y
-xenoprof := y
 
 CFLAGS += -I$(BASEDIR)/include 
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
 CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
 
-CFLAGS-$(xenoprof) += -DCONFIG_XENOPROF
-
 # Prevent floating-point variables from creeping into Xen.
 CFLAGS += -msoft-float
 
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 6f404b4..fbb64a7 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -84,6 +84,19 @@ config LATE_HWDOM
 
 	  If unsure, say N.
 
+# Adds support for Xenoprof
+config XENOPROF
+	bool "Xenoprof support"
+	default y
+	depends on X86
+	---help---
+	  Xenoprof is a system-wide profiler for Xen virtual machine
+	  environments, capable of profiling the Xen virtual machine monitor,
+	  multiple Linux guest operating systems, and applications running on
+	  them.
+
+	  If unsure, say Y.
+
 # Enable/Disable XSM support
 config XSM
 	bool "Xen Security Modules support"
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 6e82b33..126d373 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -62,7 +62,7 @@ obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo unlz4
 
 obj-$(perfc)       += perfc.o
 obj-$(crash_debug) += gdbstub.o
-obj-$(xenoprof)    += xenoprof.o
+obj-$(CONFIG_XENOPROF)    += xenoprof.o
 
 obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall.o tmem_xen.o xlat.o)
 
-- 
2.4.10

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

* Re: [PATCH 2/2] build: convert xenoprof to Kconfig
  2016-02-08 16:04 ` [PATCH 2/2] build: convert xenoprof to Kconfig Doug Goldstein
@ 2016-02-08 16:14   ` Andrew Cooper
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2016-02-08 16:14 UTC (permalink / raw)
  To: Doug Goldstein, xen-devel; +Cc: Keir Fraser, Jan Beulich

On 08/02/16 16:04, Doug Goldstein wrote:
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 6f404b4..fbb64a7 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -84,6 +84,19 @@ config LATE_HWDOM
>  
>  	  If unsure, say N.
>  
> +# Adds support for Xenoprof
> +config XENOPROF
> +	bool "Xenoprof support"

Despite the awkward CONFIG name, this is Xen Oprofile Support

With the bits of text updated appropriately, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>

> +	default y
> +	depends on X86
> +	---help---
> +	  Xenoprof is a system-wide profiler for Xen virtual machine
> +	  environments, capable of profiling the Xen virtual machine monitor,
> +	  multiple Linux guest operating systems, and applications running on
> +	  them.
> +
> +	  If unsure, say Y.
> +
>  # Enable/Disable XSM support
>  config XSM
>  	bool "Xen Security Modules support"
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 6e82b33..126d373 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -62,7 +62,7 @@ obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo unlz4
>  
>  obj-$(perfc)       += perfc.o
>  obj-$(crash_debug) += gdbstub.o
> -obj-$(xenoprof)    += xenoprof.o
> +obj-$(CONFIG_XENOPROF)    += xenoprof.o
>  
>  obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall.o tmem_xen.o xlat.o)
>  

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

* Re: [PATCH 1/2] xenoprof: fix up ability to disable it
  2016-02-08 16:04 [PATCH 1/2] xenoprof: fix up ability to disable it Doug Goldstein
  2016-02-08 16:04 ` [PATCH 2/2] build: convert xenoprof to Kconfig Doug Goldstein
@ 2016-02-08 16:24 ` Boris Ostrovsky
  2016-02-09  3:01   ` Doug Goldstein
  2016-02-09  4:05   ` Doug Goldstein
  2016-02-08 16:25 ` Andrew Cooper
  2 siblings, 2 replies; 8+ messages in thread
From: Boris Ostrovsky @ 2016-02-08 16:24 UTC (permalink / raw)
  To: Doug Goldstein, xen-devel
  Cc: Kevin Tian, Keir Fraser, Jan Beulich, Jun Nakajima,
	Andrew Cooper, Aravind Gopalakrishnan, Suravee Suthikulpanit



On 02/08/2016 11:04 AM, Doug Goldstein wrote:
> Allow Xenoprof to be fully disabled when toggling the option off.
>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> CC: Jun Nakajima <jun.nakajima@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>


> @@ -2666,10 +2669,11 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
>               goto gp_fault;
>           break;
>       default:
> -        if ( passive_domain_do_wrmsr(msr, msr_content) )
> +        if ( IS_ENABLED(CONFIG_XENOPROF) &&
> +             passive_domain_do_wrmsr(msr, msr_content) )
>               return X86EMUL_OKAY;
>   
> -        if ( wrmsr_viridian_regs(msr, msr_content) )
> +        if ( wrmsr_viridian_regs(msr, msr_content) )
>               break;

Looks like a stray change.

Other than that,

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

While at it --- I think we should put VPMU code under CONFIG option too, 
especially given its support status. I can do that (unless Doug wants to).

-boris

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

* Re: [PATCH 1/2] xenoprof: fix up ability to disable it
  2016-02-08 16:04 [PATCH 1/2] xenoprof: fix up ability to disable it Doug Goldstein
  2016-02-08 16:04 ` [PATCH 2/2] build: convert xenoprof to Kconfig Doug Goldstein
  2016-02-08 16:24 ` [PATCH 1/2] xenoprof: fix up ability to disable it Boris Ostrovsky
@ 2016-02-08 16:25 ` Andrew Cooper
  2 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2016-02-08 16:25 UTC (permalink / raw)
  To: Doug Goldstein, xen-devel
  Cc: Kevin Tian, Keir Fraser, Suravee Suthikulpanit, Jan Beulich,
	Aravind Gopalakrishnan, Jun Nakajima, Boris Ostrovsky

On 08/02/16 16:04, Doug Goldstein wrote:
> diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
> index 990e6f3..c7b5bd5 100644
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -22,6 +22,7 @@
>   */
>  
>  #include <xen/config.h>
> +#include <xen/kconfig.h>
>  #include <xen/xenoprof.h>
>  #include <xen/hvm/save.h>
>  #include <xen/sched.h>
> @@ -378,7 +379,8 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
>      if ( (type == MSR_TYPE_CTRL) &&
>          is_pmu_enabled(msr_content) && !vpmu_is_set(vpmu, VPMU_RUNNING) )
>      {
> -        if ( !acquire_pmu_ownership(PMU_OWNER_HVM) )
> +        if ( IS_ENABLED(CONFIG_XENOPROF) &&
> +             !acquire_pmu_ownership(PMU_OWNER_HVM) )
>              return 0;
>          vpmu_set(vpmu, VPMU_RUNNING);
>  

Instead of modifying each caller into the oprofile code like this, I
wonder whether it would be neater to produce an empty set of static
inlines in xenoprof.h when !CONFIG_XENOPROF, similar to
CONFIG_SHADOW_PAGING in xen/include/asm-x86/shadow.h

~Andrew

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

* Re: [PATCH 1/2] xenoprof: fix up ability to disable it
  2016-02-08 16:24 ` [PATCH 1/2] xenoprof: fix up ability to disable it Boris Ostrovsky
@ 2016-02-09  3:01   ` Doug Goldstein
  2016-02-09  4:05   ` Doug Goldstein
  1 sibling, 0 replies; 8+ messages in thread
From: Doug Goldstein @ 2016-02-09  3:01 UTC (permalink / raw)
  To: Boris Ostrovsky, xen-devel
  Cc: Kevin Tian, Keir Fraser, Jan Beulich, Jun Nakajima,
	Andrew Cooper, Aravind Gopalakrishnan, Suravee Suthikulpanit


[-- Attachment #1.1: Type: text/plain, Size: 1716 bytes --]

On 2/8/16 10:24 AM, Boris Ostrovsky wrote:
> 
> 
> On 02/08/2016 11:04 AM, Doug Goldstein wrote:
>> Allow Xenoprof to be fully disabled when toggling the option off.
>>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
>> CC: Jun Nakajima <jun.nakajima@intel.com>
>> CC: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> 
> 
>> @@ -2666,10 +2669,11 @@ static int vmx_msr_write_intercept(unsigned
>> int msr, uint64_t msr_content)
>>               goto gp_fault;
>>           break;
>>       default:
>> -        if ( passive_domain_do_wrmsr(msr, msr_content) )
>> +        if ( IS_ENABLED(CONFIG_XENOPROF) &&
>> +             passive_domain_do_wrmsr(msr, msr_content) )
>>               return X86EMUL_OKAY;
>>   -        if ( wrmsr_viridian_regs(msr, msr_content) )
>> +        if ( wrmsr_viridian_regs(msr, msr_content) )
>>               break;
> 
> Looks like a stray change.
> 
> Other than that,
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> While at it --- I think we should put VPMU code under CONFIG option too,
> especially given its support status. I can do that (unless Doug wants to).
> 
> -boris

Boris,

Thanks for the review. I didn't add your Reviewed-by because I've
reworked this patch to take into account Andrew Cooper's suggestion to
provide dummy function calls instead of wrapping everything with
IS_ENABLED().

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: [PATCH 1/2] xenoprof: fix up ability to disable it
  2016-02-08 16:24 ` [PATCH 1/2] xenoprof: fix up ability to disable it Boris Ostrovsky
  2016-02-09  3:01   ` Doug Goldstein
@ 2016-02-09  4:05   ` Doug Goldstein
  2016-02-09 13:34     ` Boris Ostrovsky
  1 sibling, 1 reply; 8+ messages in thread
From: Doug Goldstein @ 2016-02-09  4:05 UTC (permalink / raw)
  To: Boris Ostrovsky, xen-devel
  Cc: Kevin Tian, Keir Fraser, Jan Beulich, Jun Nakajima,
	Andrew Cooper, Aravind Gopalakrishnan, Suravee Suthikulpanit


[-- Attachment #1.1: Type: text/plain, Size: 1664 bytes --]

On 2/8/16 10:24 AM, Boris Ostrovsky wrote:
> 
> 
> On 02/08/2016 11:04 AM, Doug Goldstein wrote:
>> Allow Xenoprof to be fully disabled when toggling the option off.
>>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
>> CC: Jun Nakajima <jun.nakajima@intel.com>
>> CC: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> 
> 
>> @@ -2666,10 +2669,11 @@ static int vmx_msr_write_intercept(unsigned
>> int msr, uint64_t msr_content)
>>               goto gp_fault;
>>           break;
>>       default:
>> -        if ( passive_domain_do_wrmsr(msr, msr_content) )
>> +        if ( IS_ENABLED(CONFIG_XENOPROF) &&
>> +             passive_domain_do_wrmsr(msr, msr_content) )
>>               return X86EMUL_OKAY;
>>   -        if ( wrmsr_viridian_regs(msr, msr_content) )
>> +        if ( wrmsr_viridian_regs(msr, msr_content) )
>>               break;
> 
> Looks like a stray change.
> 
> Other than that,
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> While at it --- I think we should put VPMU code under CONFIG option too,
> especially given its support status. I can do that (unless Doug wants to).
> 
> -boris

Boris,

I forgot to answer the VPMU part. If you have the bandwidth to do that
I'll happily test it. Otherwise I'm not sure when I'll have a chance to
wrap it with Kconfig.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: [PATCH 1/2] xenoprof: fix up ability to disable it
  2016-02-09  4:05   ` Doug Goldstein
@ 2016-02-09 13:34     ` Boris Ostrovsky
  0 siblings, 0 replies; 8+ messages in thread
From: Boris Ostrovsky @ 2016-02-09 13:34 UTC (permalink / raw)
  To: Doug Goldstein, xen-devel
  Cc: Kevin Tian, Keir Fraser, Jan Beulich, Jun Nakajima,
	Andrew Cooper, Aravind Gopalakrishnan, Suravee Suthikulpanit

On 02/08/2016 11:05 PM, Doug Goldstein wrote:
> On 2/8/16 10:24 AM, Boris Ostrovsky wrote:
>>
>> While at it --- I think we should put VPMU code under CONFIG option too,
>> especially given its support status. I can do that (unless Doug wants to).
>>
>
>
> I forgot to answer the VPMU part. If you have the bandwidth to do that
> I'll happily test it. Otherwise I'm not sure when I'll have a chance to
> wrap it with Kconfig.
>

Sure, I'll do that once I get a few other things cleared.

-boris

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

end of thread, other threads:[~2016-02-09 13:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-08 16:04 [PATCH 1/2] xenoprof: fix up ability to disable it Doug Goldstein
2016-02-08 16:04 ` [PATCH 2/2] build: convert xenoprof to Kconfig Doug Goldstein
2016-02-08 16:14   ` Andrew Cooper
2016-02-08 16:24 ` [PATCH 1/2] xenoprof: fix up ability to disable it Boris Ostrovsky
2016-02-09  3:01   ` Doug Goldstein
2016-02-09  4:05   ` Doug Goldstein
2016-02-09 13:34     ` Boris Ostrovsky
2016-02-08 16:25 ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.