Re: [PATCH] x86: fix CMCI injection

[Egger Christoph <chegger@amazon.de>]

On 26.02.13 07:04, Keir Fraser wrote:
> On 25/02/2013 16:49, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>> This fixes the wrong use of literal vector 0xF7 with an "int"
>> instruction (invalidated by 25113:14609be41f36) and the fact that doing
>> the injection via a software interrupt was never valid anyway (because
>> cmci_interrupt() acks the LAPIC, which does the wrong thing if the
>> interrupt didn't get delivered though it).
>>
>> In order to do latter, the patch introduces send_IPI_self(), at once
>> removing two opend coded uses of "genapic" in the IRQ handling code.
>>
>> Reported-by: Yongjie Ren <yongjie.ren@intel.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Acked-by: Keir Fraser <keir@xen.org>

One nit inline.


>
>> --- a/xen/arch/x86/cpu/mcheck/mce.c
>> +++ b/xen/arch/x86/cpu/mcheck/mce.c
>> @@ -34,6 +34,7 @@ bool_t __read_mostly mce_broadcast = 0;
>>   bool_t is_mc_panic;
>>   unsigned int __read_mostly nr_mce_banks;
>>   unsigned int __read_mostly firstbank;
>> +uint8_t __read_mostly cmci_apic_vector;
>>
>>   DEFINE_PER_CPU_READ_MOSTLY(struct mca_banks *, poll_bankmask);
>>   DEFINE_PER_CPU_READ_MOSTLY(struct mca_banks *, no_cmci_banks);
>> @@ -1198,12 +1199,6 @@ static void x86_mc_mceinject(void *data)
>>       __asm__ __volatile__("int $0x12");
>>   }
>>
>> -static void x86_cmci_inject(void *data)
>> -{
>> -    printk("Simulating CMCI on cpu %d\n", smp_processor_id());
>> -    __asm__ __volatile__("int $0xf7");
>> -}
>> -
>>   #if BITS_PER_LONG == 64
>>
>>   #define ID2COOKIE(id) ((mctelem_cookie_t)(id))
>> @@ -1479,11 +1474,15 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_m
>>               on_selected_cpus(cpumap, x86_mc_mceinject, NULL, 1);
>>               break;
>>           case XEN_MC_INJECT_TYPE_CMCI:
>> -            if ( !cmci_support )
>> +            if ( !cmci_apic_vector )

cmci_apic_vector is uninitialized here when the platform does not 
support CMCI.

Christoph

>>                   ret = x86_mcerr(
>>                       "No CMCI supported in platform\n", -EINVAL);
>>               else
>> -                on_selected_cpus(cpumap, x86_cmci_inject, NULL, 1);
>> +            {
>> +                if ( cpumask_test_cpu(smp_processor_id(), cpumap) )
>> +                    send_IPI_self(cmci_apic_vector);
>> +                send_IPI_mask(cpumap, cmci_apic_vector);
>> +            }
>>               break;
>>           default:
>>               ret = x86_mcerr("Wrong mca type\n", -EINVAL);
>> --- a/xen/arch/x86/cpu/mcheck/mce.h
>> +++ b/xen/arch/x86/cpu/mcheck/mce.h
>> @@ -37,6 +37,8 @@ enum mcheck_type {
>> mcheck_intel
>>   };
>>
>> +extern uint8_t cmci_apic_vector;
>> +
>>   /* Init functions */
>>   enum mcheck_type amd_mcheck_init(struct cpuinfo_x86 *c);
>>   enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
>> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c
>> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
>> @@ -644,7 +644,6 @@ static void intel_init_cmci(struct cpuin
>>   {
>>       u32 l, apic;
>>       int cpu = smp_processor_id();
>> -    static uint8_t cmci_apic_vector;
>>
>>       if (!mce_available(c) || !cmci_support) {
>>           if (opt_cpu_info)
>> --- a/xen/arch/x86/irq.c
>> +++ b/xen/arch/x86/irq.c
>> @@ -646,7 +646,7 @@ void irq_move_cleanup_interrupt(struct c
>>            * to myself.
>>            */
>>           if (irr  & (1 << (vector % 32))) {
>> -            genapic->send_IPI_self(IRQ_MOVE_CLEANUP_VECTOR);
>> +            send_IPI_self(IRQ_MOVE_CLEANUP_VECTOR);
>>               TRACE_3D(TRC_HW_IRQ_MOVE_CLEANUP_DELAY,
>>                        irq, vector, smp_processor_id());
>>               goto unlock;
>> @@ -692,7 +692,7 @@ static void send_cleanup_vector(struct i
>>
>>       cpumask_and(&cleanup_mask, desc->arch.old_cpu_mask, &cpu_online_map);
>>       desc->arch.move_cleanup_count = cpumask_weight(&cleanup_mask);
>> -    genapic->send_IPI_mask(&cleanup_mask, IRQ_MOVE_CLEANUP_VECTOR);
>> +    send_IPI_mask(&cleanup_mask, IRQ_MOVE_CLEANUP_VECTOR);
>>
>>       desc->arch.move_in_progress = 0;
>>   }
>> --- a/xen/arch/x86/smp.c
>> +++ b/xen/arch/x86/smp.c
>> @@ -38,6 +38,11 @@ void send_IPI_mask(const cpumask_t *mask
>>       genapic->send_IPI_mask(mask, vector);
>>   }
>>
>> +void send_IPI_self(int vector)
>> +{
>> +    genapic->send_IPI_self(vector);
>> +}
>> +
>>   /*
>>    * Some notes on x86 processor bugs affecting SMP operation:
>>    *
>> --- a/xen/include/asm-x86/smp.h
>> +++ b/xen/include/asm-x86/smp.h
>> @@ -29,7 +29,8 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_
>>
>>   void smp_send_nmi_allbutself(void);
>>
>> -void  send_IPI_mask(const cpumask_t *mask, int vector);
>> +void send_IPI_mask(const cpumask_t *, int vector);
>> +void send_IPI_self(int vector);
>>
>>   extern void (*mtrr_hook) (void);
>>
>>
>>