Re: [PATCH 4/6] edac, mce_amd_inj: Add capability to trigger apic interrupts

[Borislav Petkov <bp@alien8.de>]

On Wed, May 27, 2015 at 02:03:36PM -0500, Aravind Gopalakrishnan wrote:
> With this extension to the flags attribute, deferred error interrupts
> and threshold interrupts can be triggered to test the apic interrupt
> handler functionality for these type of errors
> 
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
>  drivers/edac/mce_amd_inj.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/edac/mce_amd_inj.c b/drivers/edac/mce_amd_inj.c
> index daec4af..f1c1433 100644
> --- a/drivers/edac/mce_amd_inj.c
> +++ b/drivers/edac/mce_amd_inj.c
> @@ -34,12 +34,16 @@ static u8 n_banks;
>  enum injection_type {
>  	SW_INJ = 0,	/* SW injection, simply decode the error */
>  	HW_INJ,		/* Trigger a #MC */
> +	DFR_INT_INJ,	/* Trigger Deferred error interrupt */
> +	THR_INT_INJ,	/* Trigger threshold interrupt */
>  	N_INJ_TYPES,
>  };
>  
>  static const char * const flags_options[] = {
>  	[SW_INJ] = "sw",
>  	[HW_INJ] = "hw",
> +	[DFR_INT_INJ] = "dfr",
> +	[THR_INT_INJ] = "thr",
>  	NULL
>  };
>  
> @@ -191,6 +195,16 @@ static void trigger_mce(void *info)
>  	asm volatile("int $18");
>  }
>  
> +static void trigger_dfr_int(void *info)
> +{
> +	asm volatile("int $244");
> +}
> +
> +static void trigger_thr_int(void *info)
> +{
> +	asm volatile("int $249");
> +}

Hardcoded naked numbers huh?

Guess what happens when someone changes DEFERRED_ERROR_VECTOR and
THRESHOLD_APIC_VECTOR.

> +
>  static void do_inject(void)
>  {
>  	u64 mcg_status = 0;
> @@ -202,6 +216,20 @@ static void do_inject(void)
>  		return;
>  	}
>  
> +	if (inj_type == DFR_INT_INJ) {
> +		/*
> +		 * Ensure necessary status bits for deferred errors:
> +		 * a. MCx_STATUS[Deferred] is set -
> +		 *	This is to ensure the error will be handled by the
> +		 *	interrupt handler
> +		 * b. unset MCx_STATUS[UC]
> +		 *	As deferred errors are _not_ UC
> +		 */
> +
> +		i_mce.status |= MCI_STATUS_DEFERRED;
> +		i_mce.status |= (i_mce.status & ~MCI_STATUS_UC);

		i_mce.status &= ~MCI_STATUS_UC;

> +	}
> +
>  	get_online_cpus();
>  	if (!cpu_online(cpu))
>  		goto err;
> @@ -228,7 +256,12 @@ static void do_inject(void)
>  
>  	toggle_hw_mce_inject(cpu, false);
>  
> -	smp_call_function_single(cpu, trigger_mce, NULL, 0);
> +	if (inj_type == DFR_INT_INJ)
> +		smp_call_function_single(cpu, trigger_dfr_int, NULL, 0);
> +	else if (inj_type == THR_INT_INJ)
> +		smp_call_function_single(cpu, trigger_thr_int, NULL, 0);
> +	else
> +		smp_call_function_single(cpu, trigger_mce, NULL, 0);

I guess a switch-case is kinda offering itself here...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--