Re: [Xen-devel] [PATCH 6] X86/MCE: update vMCE injection for AMD

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.09.12 at 11:06, Christoph Egger <Christoph.Egger@xxxxxxx> wrote:
> On 09/25/12 07:00, Liu, Jinsong wrote:
> 
>> X86/MCE: update vMCE injection for AMD
>> 
>> For Intel MCE, it broadcasts vMCE to all vcpus. For AMD MCE, it injects
>> vMCE only to vcpu0. This patch update inject_vmce for AMD.
>> 
>> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
>> Suggested_by: Christoph Egger <Christoph.Egger@xxxxxxx>
> 
> 
> Acked-by: Christoph Egger <Christoph.Egger@xxxxxxx>

Are you sure (see below)?

>> diff -r a6d12a1bc758 xen/arch/x86/cpu/mcheck/mce.h
>> --- a/xen/arch/x86/cpu/mcheck/mce.h  Thu Sep 20 00:03:25 2012 +0800
>> +++ b/xen/arch/x86/cpu/mcheck/mce.h  Tue Sep 25 19:52:20 2012 +0800
>> @@ -168,7 +168,7 @@
>>  
>>  int fill_vmsr_data(struct mcinfo_bank *mc_bank, struct domain *d,
>>      uint64_t gstatus);
>> -int inject_vmce(struct domain *d);
>> +int inject_vmce(struct domain *d, bool_t vmce_broadcast);
>>  
>>  static inline int mce_vendor_bank_msr(const struct vcpu *v, uint32_t msr)
>>  {
>> diff -r a6d12a1bc758 xen/arch/x86/cpu/mcheck/mce_intel.c
>> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c    Thu Sep 20 00:03:25 2012 +0800
>> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c    Tue Sep 25 19:52:20 2012 +0800
>> @@ -365,7 +365,7 @@
>>                  }
>>  
>>                  /* We will inject vMCE to DOMU*/
>> -                if ( inject_vmce(d) < 0 )
>> +                if ( inject_vmce(d, 1) < 0 )
>>                  {
>>                      mce_printk(MCE_QUIET, "inject vMCE to DOM%d"
>>                        " failed\n", d->domain_id);
>> diff -r a6d12a1bc758 xen/arch/x86/cpu/mcheck/vmce.c
>> --- a/xen/arch/x86/cpu/mcheck/vmce.c Thu Sep 20 00:03:25 2012 +0800
>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c Tue Sep 25 19:52:20 2012 +0800
>> @@ -344,11 +344,14 @@
>>  HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
>>                            vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
>>  
>> -int inject_vmce(struct domain *d)
>> +/* 
>> + * for Intel MCE, broadcast vMCE to all vcpus
>> + * for AMD MCE, only inject vMCE to vcpu0
>> + */
>> +int inject_vmce(struct domain *d, bool_t vmce_broadcast)
>>  {
>>      struct vcpu *v;
>>  
>> -    /* inject vMCE to all vcpus */
>>      for_each_vcpu(d, v)
>>      {
>>          if ( !test_and_set_bool(v->mce_pending) &&
>> @@ -365,6 +368,9 @@
>>                         d->domain_id, v->vcpu_id);
>>              return -1;
>>          }
>> +
>> +        if ( !vmce_broadcast )
>> +            break;

That'll allow (non-broadcast) injection to vCPU 0 only - is that
really the right thing to do? I.e. shouldn't the caller rather be
given flexibility to specify which vCPU this is to go to (with a
negative value meaning broadcast)?

And I'm intending to fold this into patch 2 anyway before
committing.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel