Re: [XEN PATCH V2] x86/MCE: move intel mcheck init code to separate file

2024-04-18 Thread Jan Beulich
On 09.04.2024 14:06, Sergiy Kibrik wrote:
> Separate Intel nonfatal MCE initialization code from generic MCE code, the 
> same
> way it is done for AMD code. This is to be able to later make intel/amd MCE
> code optional in the build.
> 
> Convert to Xen coding style. Clean up unused includes. Remove seemingly
> outdated comment about MCE check period.
> 
> No functional change intended.
> 
> Signed-off-by: Sergiy Kibrik 

Reviewed-by: Jan Beulich 
with ...

> --- /dev/null
> +++ b/xen/arch/x86/cpu/mcheck/intel-nonfatal.c
> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Non Fatal Machine Check Exception Reporting
> + * (C) Copyright 2002 Dave Jones. 
> + */
> +
> +#include 
> +
> +#include "mce.h"
> +#include "vmce.h"
> +
> +static struct timer mce_timer;
> +
> +#define MCE_PERIOD MILLISECS(8000)
> +#define MCE_PERIOD_MIN MILLISECS(2000)
> +#define MCE_PERIOD_MAX MILLISECS(16000)
> +
> +static uint64_t period = MCE_PERIOD;
> +static int adjust = 0;
> +static int variable_period = 1;
> +
> +static void cf_check mce_checkregs(void *info)
> +{
> +mctelem_cookie_t mctc;
> +struct mca_summary bs;
> +static uint64_t dumpcount = 0;
> +
> +mctc = mcheck_mca_logout(MCA_POLLER, this_cpu( poll_bankmask),
> + , NULL);
> +
> +if ( bs.errcnt && mctc != NULL )
> +{
> +adjust++;
> +
> +/* If Dom0 enabled the VIRQ_MCA event, then notify it.

... comment style here corrected, too (which I'll try t to remember to take
care of while committing).

Jan



Re: [XEN PATCH V2] x86/MCE: move intel mcheck init code to separate file

2024-04-09 Thread Stefano Stabellini
On Tue, 9 Apr 2024, Sergiy Kibrik wrote:
> Separate Intel nonfatal MCE initialization code from generic MCE code, the 
> same
> way it is done for AMD code. This is to be able to later make intel/amd MCE
> code optional in the build.
> 
> Convert to Xen coding style. Clean up unused includes. Remove seemingly
> outdated comment about MCE check period.
> 
> No functional change intended.
> 
> Signed-off-by: Sergiy Kibrik 
> ---
>  xen/arch/x86/cpu/mcheck/Makefile |  1 +
>  xen/arch/x86/cpu/mcheck/intel-nonfatal.c | 85 
>  xen/arch/x86/cpu/mcheck/mce.h|  1 +
>  xen/arch/x86/cpu/mcheck/non-fatal.c  | 82 +--
>  4 files changed, 88 insertions(+), 81 deletions(-)
>  create mode 100644 xen/arch/x86/cpu/mcheck/intel-nonfatal.c
> 
> V2:
>  * convert to Xen coding style
>  * file naming
>  * drop outdated comment

I find code movement together with coding style changes harder to
review, but I know that Jan asked for it. I reviewed the code and
everything checks out.

Reviewed-by: Stefano Stabellini 


> diff --git a/xen/arch/x86/cpu/mcheck/Makefile 
> b/xen/arch/x86/cpu/mcheck/Makefile
> index 0d63ff9096..f927f10b4d 100644
> --- a/xen/arch/x86/cpu/mcheck/Makefile
> +++ b/xen/arch/x86/cpu/mcheck/Makefile
> @@ -2,6 +2,7 @@ obj-y += amd_nonfatal.o
>  obj-y += mce_amd.o
>  obj-y += mcaction.o
>  obj-y += barrier.o
> +obj-y += intel-nonfatal.o
>  obj-y += mctelem.o
>  obj-y += mce.o
>  obj-y += mce-apei.o
> diff --git a/xen/arch/x86/cpu/mcheck/intel-nonfatal.c 
> b/xen/arch/x86/cpu/mcheck/intel-nonfatal.c
> new file mode 100644
> index 00..e18e8a4030
> --- /dev/null
> +++ b/xen/arch/x86/cpu/mcheck/intel-nonfatal.c
> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Non Fatal Machine Check Exception Reporting
> + * (C) Copyright 2002 Dave Jones. 
> + */
> +
> +#include 
> +
> +#include "mce.h"
> +#include "vmce.h"
> +
> +static struct timer mce_timer;
> +
> +#define MCE_PERIOD MILLISECS(8000)
> +#define MCE_PERIOD_MIN MILLISECS(2000)
> +#define MCE_PERIOD_MAX MILLISECS(16000)
> +
> +static uint64_t period = MCE_PERIOD;
> +static int adjust = 0;
> +static int variable_period = 1;
> +
> +static void cf_check mce_checkregs(void *info)
> +{
> +mctelem_cookie_t mctc;
> +struct mca_summary bs;
> +static uint64_t dumpcount = 0;
> +
> +mctc = mcheck_mca_logout(MCA_POLLER, this_cpu( poll_bankmask),
> + , NULL);
> +
> +if ( bs.errcnt && mctc != NULL )
> +{
> +adjust++;
> +
> +/* If Dom0 enabled the VIRQ_MCA event, then notify it.
> + * Otherwise, if dom0 has had plenty of time to register
> + * the virq handler but still hasn't then dump telemetry
> + * to the Xen console.  The call count may be incremented
> + * on multiple cpus at once and is indicative only - just
> + * a simple-minded attempt to avoid spamming the console
> + * for corrected errors in early startup.
> + */
> +
> +if ( dom0_vmce_enabled() )
> +{
> +mctelem_commit(mctc);
> +send_global_virq(VIRQ_MCA);
> +}
> +else if ( ++dumpcount >= 10 )
> +{
> +x86_mcinfo_dump((struct mc_info *)mctelem_dataptr(mctc));
> +mctelem_dismiss(mctc);
> +}
> +else
> +mctelem_dismiss(mctc);
> +}
> +else if ( mctc != NULL )
> +mctelem_dismiss(mctc);
> +}
> +
> +static void cf_check mce_work_fn(void *data)
> +{
> +on_each_cpu(mce_checkregs, NULL, 1);
> +
> +if ( variable_period )
> +{
> +if ( adjust )
> +period /= (adjust + 1);
> +else
> +period *= 2;
> +if ( period > MCE_PERIOD_MAX )
> +period = MCE_PERIOD_MAX;
> +if ( period < MCE_PERIOD_MIN )
> +period = MCE_PERIOD_MIN;
> +}
> +
> +set_timer(_timer, NOW() + period);
> +adjust = 0;
> +}
> +
> +void __init intel_nonfatal_mcheck_init(struct cpuinfo_x86 *unused)
> +{
> +init_timer(_timer, mce_work_fn, NULL, 0);
> +set_timer(_timer, NOW() + MCE_PERIOD);
> +}
> diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
> index 7f26afae23..4806405f96 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.h
> +++ b/xen/arch/x86/cpu/mcheck/mce.h
> @@ -47,6 +47,7 @@ enum mcheck_type amd_mcheck_init(const struct cpuinfo_x86 
> *c, bool bsp);
>  enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, bool bsp);
>  
>  void amd_nonfatal_mcheck_init(struct cpuinfo_x86 *c);
> +void intel_nonfatal_mcheck_init(struct cpuinfo_x86 *c);
>  
>  extern unsigned int firstbank;
>  extern unsigned int ppin_msr;
> diff --git a/xen/arch/x86/cpu/mcheck/non-fatal.c 
> b/xen/arch/x86/cpu/mcheck/non-fatal.c
> index 1c0c32ba08..33cacd15c2 100644
> --- a/xen/arch/x86/cpu/mcheck/non-fatal.c
> +++ b/xen/arch/x86/cpu/mcheck/non-fatal.c
> @@ -7,84 +7,7 @@
>   *
>   */
>  
> -#include 
> -#include 
> -#include