Re: [PATCH 2/5] Xen/MCE: vMCE injection

[Christoph Egger <Christoph.Egger@amd.com>]

On 09/20/12 21:15, Liu, Jinsong wrote:

> Christoph Egger wrote:
>> On 09/19/12 10:03, Liu, Jinsong wrote:
>>
>>> Xen/MCE: vMCE injection
>>>
>>> In our test for win8 guest mce, we find a bug that no matter what
>>> SRAO/SRAR error xen inject to win8 guest, it always reboot.
>>>
>>> The root cause is, current Xen vMCE logic inject vMCE# only to
>>> vcpu0, this is not correct for Intel MCE (Under Intel arch, h/w
>>> generate MCE# to all CPUs). 
>>>
>>> This patch fix vMCE injection bug, injecting vMCE# to all vcpus.
>>
>>
>> This breaks the AMD way. The AMD way is to only inject it to vcpu0.
>> I suggest to add a flag argument to inject_vmce() that says whether
>> to inject to all vcpus or just vcpu0.
>> Then set/clear that flag from the caller side depending on whether you
>> run on Intel or AMD.
>>
>> Christoph
>>
> 
> No, it didn't breaks AMD since it only called by intel_memerr_dhandler().

But it will with the mce patches I still have in my queue.

Christoph


> Thanks,
> Jinsong
> 
>>
>>>
>>> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
>>>
>>> diff -r 133664c6bfb4 xen/arch/x86/cpu/mcheck/vmce.c
>>> --- a/xen/arch/x86/cpu/mcheck/vmce.c	Tue Sep 18 22:39:11 2012 +0800
>>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c	Tue Sep 18 23:46:38 2012 +0800
>>> @@ -340,48 +340,27 @@ 
>>>
>>>  int inject_vmce(struct domain *d)
>>>  {
>>> -    int cpu = smp_processor_id();
>>> +    struct vcpu *v;
>>>
>>> -    /* PV guest and HVM guest have different vMCE# injection
>>> methods. */ 
>>> -    if ( !test_and_set_bool(d->vcpu[0]->mce_pending) )
>>> +    /* inject vMCE to all vcpus */
>>> +    for_each_vcpu(d, v)
>>>      {
>>> -        if ( d->is_hvm )
>>> +        if ( !test_and_set_bool(v->mce_pending) && +           
>>> ((d->is_hvm) || +                guest_has_trap_callback(d,
>>> v->vcpu_id, TRAP_machine_check)) )          { 
>>> -            mce_printk(MCE_VERBOSE, "MCE: inject vMCE to HVM DOM
>>> %d\n", 
>>> -                       d->domain_id);
>>> -            vcpu_kick(d->vcpu[0]);
>>> +            mce_printk(MCE_VERBOSE, "MCE: inject vMCE to dom%d
>>> vcpu%d\n", +                       d->domain_id, v->vcpu_id);
>>> +            vcpu_kick(v);
>>>          }
>>>          else
>>>          {
>>> -            mce_printk(MCE_VERBOSE, "MCE: inject vMCE to PV
>>> DOM%d\n", 
>>> -                       d->domain_id);
>>> -            if ( guest_has_trap_callback(d, 0, TRAP_machine_check) )
>>> -            {
>>> -                cpumask_copy(d->vcpu[0]->cpu_affinity_tmp,
>>> -                             d->vcpu[0]->cpu_affinity);
>>> -                mce_printk(MCE_VERBOSE, "MCE: CPU%d set affinity,
>>> old %d\n", 
>>> -                           cpu, d->vcpu[0]->processor);
>>> -                vcpu_set_affinity(d->vcpu[0], cpumask_of(cpu));
>>> -                vcpu_kick(d->vcpu[0]);
>>> -            }
>>> -            else
>>> -            {
>>> -                mce_printk(MCE_VERBOSE,
>>> -                           "MCE: Kill PV guest with No MCE
>>> handler\n"); 
>>> -                domain_crash(d);
>>> -            }
>>> +            mce_printk(MCE_QUIET, "Fail to inject vMCE to dom%d
>>> vcpu%d\n", +                       d->domain_id, v->vcpu_id);
>>> +            return -1;
>>>          }
>>>      }
>>> -    else
>>> -    {
>>> -        /* new vMCE comes while first one has not been injected yet,
>>> -         * in this case, inject fail. [We can't lose this vMCE for
>>> -         * the mce node's consistency].
>>> -         */
>>> -        mce_printk(MCE_QUIET, "There's a pending vMCE waiting to be
>>> injected " 
>>> -                   " to this DOM%d!\n", d->domain_id);
>>> -        return -1;
>>> -    }
>>> +
>>>      return 0;
>>>  }
> 
> 



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632