| Message ID | 1454947487-8588-1-git-send-email-cardoe@cardoe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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().
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.
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
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
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(-)