Re: [Xen-devel] [PATCH v4 1/2] xenoprof: fix up ability to disable it
On 17/02/16 18:02, Doug Goldstein wrote: > Allow Xenoprof to be fully disabled when toggling the option off. > > Signed-off-by: Doug Goldstein > --- > CC: Keir Fraser > CC: Jan Beulich > CC: Andrew Cooper > > change since v3: > - drop (void)var; from static inlines > - fix typo that broke build (must have forgotten to do XEN_CONFIG_EXPERT=y > make > change since v2: > - move all functions in xenoprof.h inside CONFIG_XENOPROF as suggested by > Andrew Cooper > change since v1: > - switch to #define empty 'functions' as suggested by Andrew Cooper > --- > xen/arch/x86/Makefile | 2 +- > xen/arch/x86/Rules.mk | 2 ++ > xen/arch/x86/x86_64/compat/entry.S | 4 > xen/arch/x86/x86_64/entry.S| 4 > xen/include/asm-x86/config.h | 1 - > xen/include/asm-x86/xenoprof.h | 19 +++ > xen/include/xen/xenoprof.h | 25 +++-- > 7 files changed, 49 insertions(+), 8 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/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 > diff --git a/xen/include/asm-x86/xenoprof.h b/xen/include/asm-x86/xenoprof.h > index b006ddc..7044084 100644 > --- a/xen/include/asm-x86/xenoprof.h > +++ b/xen/include/asm-x86/xenoprof.h > @@ -67,9 +67,28 @@ void xenoprof_backtrace(struct vcpu *, const struct > cpu_user_regs *, > "xenoprof/x86 with autotranslated mode enabled"\ > "isn't supported yet\n"); \ > } while (0) > + > +#ifdef CONFIG_XENOPROF Sorry for not commenting before you resubmitted. I would suggest having this #ifdef immediately under this files inclusion guard. That way you hide all symbols and declarations in the !CONFIG_XENOPROF case. Given that this patch compiles, I don't expect you will encounter any issues from moving it. > int passive_domain_do_rdmsr(unsigned int msr, uint64_t *msr_content); > int passive_domain_do_wrmsr(unsigned int msr, uint64_t msr_content); > void passive_domain_destroy(struct vcpu *v); > +#else As a matter of style, please put newlines around here to space it out a bit, and /* CONFIG_XENOPROF */ after the else, as the original #if is a long way away. > +static inline int passive_domain_do_rdmsr(unsigned int msr, > + uint64_t *msr_content) > +{ > +return 0; > +} > + > +static inline int passive_domain_do_wrmsr(unsigned int msr, > + uint64_t msr_content) > +{ > +return 0; > +} > + > +static inline void passive_domain_destroy(struct vcpu *v) For brevity, "static inline void passive_domain_destroy(struct vcpu *v) {}" is perfectly fine here. > +{ > +} > +#endif > > #endif /* __ASM_X86_XENOPROF_H__ */ > > diff --git a/xen/include/xen/xenoprof.h b/xen/include/xen/xenoprof.h > index 9b9ef56..8148c01 100644
[Xen-devel] [PATCH v4 1/2] xenoprof: fix up ability to disable it
Allow Xenoprof to be fully disabled when toggling the option off. Signed-off-by: Doug Goldstein --- CC: Keir Fraser CC: Jan Beulich CC: Andrew Cooper change since v3: - drop (void)var; from static inlines - fix typo that broke build (must have forgotten to do XEN_CONFIG_EXPERT=y make change since v2: - move all functions in xenoprof.h inside CONFIG_XENOPROF as suggested by Andrew Cooper change since v1: - switch to #define empty 'functions' as suggested by Andrew Cooper --- xen/arch/x86/Makefile | 2 +- xen/arch/x86/Rules.mk | 2 ++ xen/arch/x86/x86_64/compat/entry.S | 4 xen/arch/x86/x86_64/entry.S| 4 xen/include/asm-x86/config.h | 1 - xen/include/asm-x86/xenoprof.h | 19 +++ xen/include/xen/xenoprof.h | 25 +++-- 7 files changed, 49 insertions(+), 8 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/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 diff --git a/xen/include/asm-x86/xenoprof.h b/xen/include/asm-x86/xenoprof.h index b006ddc..7044084 100644 --- a/xen/include/asm-x86/xenoprof.h +++ b/xen/include/asm-x86/xenoprof.h @@ -67,9 +67,28 @@ void xenoprof_backtrace(struct vcpu *, const struct cpu_user_regs *, "xenoprof/x86 with autotranslated mode enabled"\ "isn't supported yet\n"); \ } while (0) + +#ifdef CONFIG_XENOPROF int passive_domain_do_rdmsr(unsigned int msr, uint64_t *msr_content); int passive_domain_do_wrmsr(unsigned int msr, uint64_t msr_content); void passive_domain_destroy(struct vcpu *v); +#else +static inline int passive_domain_do_rdmsr(unsigned int msr, + uint64_t *msr_content) +{ +return 0; +} + +static inline int passive_domain_do_wrmsr(unsigned int msr, + uint64_t msr_content) +{ +return 0; +} + +static inline void passive_domain_destroy(struct vcpu *v) +{ +} +#endif #endif /* __ASM_X86_XENOPROF_H__ */ diff --git a/xen/include/xen/xenoprof.h b/xen/include/xen/xenoprof.h index 9b9ef56..8148c01 100644 --- a/xen/include/xen/xenoprof.h +++ b/xen/include/xen/xenoprof.h @@ -64,19 +64,32 @@ struct xenoprof { #endif struct domain; -int is_active(struct domain *d); -int is_passive(struct domain *d); -void free_xenoprof_pages(struct domain *d); - -int xenoprof_add_trace(struct vcpu *, uint64_t pc, int mode); - #define PMU_OWNER_NONE 0 #define PMU_OWNER_XENOPROF 1 #define PMU_OWNER_HVM 2 + +#ifdef CONFIG_XENOPROF int acquire_pmu_ownership(int pmu_ownership); void release_pmu_ownership(int pmu_ownership); +int is_active(struct domain *d); +int is_passive(struct domain *d); +void free_xenoprof_pages(struct domain *d); + +int xenoprof_add_trace(struct vcpu *, uint64_t pc, int mode); + void xenoprof_log_event(struct vcpu *, const struct cpu_user_regs *, uint64_t pc, int mode, int event); +#else +static inline