| Message ID | 1558119524-318-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] xen/lib: Introduce printk_once() and replace some opencoded examples | expand |
>>> On 17.05.19 at 20:58, <andrew.cooper3@citrix.com> wrote: > Reflow the ZynqMP message for grepability, and fix the omission of a newline. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with two remarks: > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -105,6 +105,17 @@ debugtrace_printk(const char *fmt, ...) {} > #define _p(_x) ((void *)(unsigned long)(_x)) > extern void printk(const char *format, ...) > __attribute__ ((format (printf, 1, 2))); > + > +#define printk_once(fmt, args...) \ > +({ \ > + static bool __read_mostly once_; \ > + if ( unlikely(!once_) ) \ > + { \ > + once_ = true; \ > + printk(fmt, ## args); \ > + } \ > +}) Just like Linux we accept this having a small race window, i.e. it not truly being "once" in unlikely cases. I think it would be worthwhile to make this intention explicit in the commit message. Unlike Linux'es the macro doesn't have a "return value". Is this intentional? I view this as particularly useful for the WARN_ONCE() sort-of counterpart, but I have to admit I can't immediately see a good use here, so I'm largely curious. (The 2 uses I could find in x86-specific code in Linux look to me like they'd better be WARN_ONCE().) Jan
Hi Andrew, On 17/05/2019 19:58, Andrew Cooper wrote: > Reflow the ZynqMP message for grepability, and fix the omission of a newline. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Regardless Jan's comment: Reviewed-by: Julien Grall <julien.grall@arm.com> I will let Jan/You to commit the patch. Cheers, > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/cpuerrata.c | 18 ++---------------- > xen/arch/arm/platforms/xilinx-zynqmp.c | 9 ++------- > xen/include/xen/lib.h | 11 +++++++++++ > 3 files changed, 15 insertions(+), 23 deletions(-) > > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c > index 4431b24..8904939 100644 > --- a/xen/arch/arm/cpuerrata.c > +++ b/xen/arch/arm/cpuerrata.c > @@ -336,18 +336,11 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) > switch ( ssbd_state ) > { > case ARM_SSBD_FORCE_DISABLE: > - { > - static bool once = true; > - > - if ( once ) > - printk("%s disabled from command-line\n", entry->desc); > - once = false; > + printk_once("%s disabled from command-line\n", entry->desc); > > arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL); > required = false; > - > break; > - } > > case ARM_SSBD_RUNTIME: > if ( required ) > @@ -359,18 +352,11 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) > break; > > case ARM_SSBD_FORCE_ENABLE: > - { > - static bool once = true; > - > - if ( once ) > - printk("%s forced from command-line\n", entry->desc); > - once = false; > + printk_once("%s forced from command-line\n", entry->desc); > > arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL); > required = true; > - > break; > - } > > default: > ASSERT_UNREACHABLE(); > diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c b/xen/arch/arm/platforms/xilinx-zynqmp.c > index 08e3e11..3060d79 100644 > --- a/xen/arch/arm/platforms/xilinx-zynqmp.c > +++ b/xen/arch/arm/platforms/xilinx-zynqmp.c > @@ -35,14 +35,9 @@ static bool zynqmp_smc(struct cpu_user_regs *regs) > */ > if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) > { > - static bool once = true; > + printk_once(XENLOG_WARNING > + "ZynqMP firmware Error: no SMCCC 1.1 support. Disabling firmware calls\n"); > > - if ( once ) > - { > - printk(XENLOG_WARNING "ZynqMP firmware Error: no SMCCC 1.1 " > - "support. Disabling firmware calls."); > - once = false; > - } > return false; > } > return zynqmp_eemi(regs); > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h > index 91ed56c..ce231c5 100644 > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -105,6 +105,17 @@ debugtrace_printk(const char *fmt, ...) {} > #define _p(_x) ((void *)(unsigned long)(_x)) > extern void printk(const char *format, ...) > __attribute__ ((format (printf, 1, 2))); > + > +#define printk_once(fmt, args...) \ > +({ \ > + static bool __read_mostly once_; \ > + if ( unlikely(!once_) ) \ > + { \ > + once_ = true; \ > + printk(fmt, ## args); \ > + } \ > +}) > + > extern void guest_printk(const struct domain *d, const char *format, ...) > __attribute__ ((format (printf, 2, 3))); > extern void noreturn panic(const char *format, ...) >
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index 4431b24..8904939 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -336,18 +336,11 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) switch ( ssbd_state ) { case ARM_SSBD_FORCE_DISABLE: - { - static bool once = true; - - if ( once ) - printk("%s disabled from command-line\n", entry->desc); - once = false; + printk_once("%s disabled from command-line\n", entry->desc); arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL); required = false; - break; - } case ARM_SSBD_RUNTIME: if ( required ) @@ -359,18 +352,11 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) break; case ARM_SSBD_FORCE_ENABLE: - { - static bool once = true; - - if ( once ) - printk("%s forced from command-line\n", entry->desc); - once = false; + printk_once("%s forced from command-line\n", entry->desc); arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL); required = true; - break; - } default: ASSERT_UNREACHABLE(); diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c b/xen/arch/arm/platforms/xilinx-zynqmp.c index 08e3e11..3060d79 100644 --- a/xen/arch/arm/platforms/xilinx-zynqmp.c +++ b/xen/arch/arm/platforms/xilinx-zynqmp.c @@ -35,14 +35,9 @@ static bool zynqmp_smc(struct cpu_user_regs *regs) */ if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) { - static bool once = true; + printk_once(XENLOG_WARNING + "ZynqMP firmware Error: no SMCCC 1.1 support. Disabling firmware calls\n"); - if ( once ) - { - printk(XENLOG_WARNING "ZynqMP firmware Error: no SMCCC 1.1 " - "support. Disabling firmware calls."); - once = false; - } return false; } return zynqmp_eemi(regs); diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h index 91ed56c..ce231c5 100644 --- a/xen/include/xen/lib.h +++ b/xen/include/xen/lib.h @@ -105,6 +105,17 @@ debugtrace_printk(const char *fmt, ...) {} #define _p(_x) ((void *)(unsigned long)(_x)) extern void printk(const char *format, ...) __attribute__ ((format (printf, 1, 2))); + +#define printk_once(fmt, args...) \ +({ \ + static bool __read_mostly once_; \ + if ( unlikely(!once_) ) \ + { \ + once_ = true; \ + printk(fmt, ## args); \ + } \ +}) + extern void guest_printk(const struct domain *d, const char *format, ...) __attribute__ ((format (printf, 2, 3))); extern void noreturn panic(const char *format, ...)
Reflow the ZynqMP message for grepability, and fix the omission of a newline. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/cpuerrata.c | 18 ++---------------- xen/arch/arm/platforms/xilinx-zynqmp.c | 9 ++------- xen/include/xen/lib.h | 11 +++++++++++ 3 files changed, 15 insertions(+), 23 deletions(-)