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

Borislav Petkov
2015-05-29T15:37:17Z

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.
--