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

[Christoph Egger <Christoph.Egger@xxxxxxx>]

On 09/27/12 11:45, Liu, Jinsong wrote:

>>>
>>> Got it, thanks! It's OK to me, but how about add sanity check and
>>> some comments like 
>>>
>>> ==================
>>> Xen/MCE: add sanity check and comments for vmce injection
>>>
>>> Add sanity check for input vcpu so that malicious value would not
>>> return 0. 
>>> Add comments since vcpu<0 (broadcast) is some implicit to code
>>> reader. 
>>
>>
>> Yeah, a like #define VMCE_INJECT_BROADCAST is also helpful.
>>
>>>
>>> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
>>>
>>> diff -r adc7f057011e xen/arch/x86/cpu/mcheck/vmce.c
>>> --- a/xen/arch/x86/cpu/mcheck/vmce.c        Thu Sep 27 19:52:13 2012 +0800
>>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c        Thu Sep 27 20:56:52 2012 +0800
>>>  @@ -341,10 +341,16 @@ /*
>>>   * for Intel MCE, broadcast vMCE to all vcpus
>>>   * for AMD MCE, only inject vMCE to vcpu0
>>> + *
>>> + * @ d, domain to which would inject vmce
>>> + * @ vcpu,
>>> + *   < 0, broadcast vMCE to all vcpus
>>> + *   >= 0, vcpu who would be injected vMCE
>>
>>
>> Better wording:
>>  >= 0, vcpu, the vMCE is injected to
>>
>> Christoph
>>
> 
> Fine to me, updated accordingly.
> 
> Thanks,
> Jinsong
> 
> ===============
> Xen/MCE: add sanity check and comments for vmce injection
> 
> Add sanity check for input vcpu so that malicious value would not return 0.
> Add comments since vcpu=-1 (broadcast) is some implicit to code reader.
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
> Suggested_by: Christoph Egger <Christoph.Egger@xxxxxxx>


Acked-by: Christoph Egger <Christoph.Egger@xxxxxxx>

> 
> diff -r adc7f057011e xen/arch/x86/cpu/mcheck/mce_intel.c
> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c     Thu Sep 27 19:52:13 2012 +0800
> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c     Fri Sep 28 01:25:19 2012 +0800
> @@ -360,7 +360,7 @@
>                  }
>  
>                  /* We will inject vMCE to DOMU*/
> -                if ( inject_vmce(d, -1) < 0 )
> +                if ( inject_vmce(d, VMCE_INJECT_BROADCAST) < 0 )
>                  {
>                      mce_printk(MCE_QUIET, "inject vMCE to DOM%d"
>                        " failed\n", d->domain_id);
> diff -r adc7f057011e xen/arch/x86/cpu/mcheck/vmce.c
> --- a/xen/arch/x86/cpu/mcheck/vmce.c  Thu Sep 27 19:52:13 2012 +0800
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c  Fri Sep 28 01:25:19 2012 +0800
> @@ -341,14 +341,20 @@
>  /*
>   * for Intel MCE, broadcast vMCE to all vcpus
>   * for AMD MCE, only inject vMCE to vcpu0
> + *
> + * @ d, domain to which would inject vmce
> + * @ vcpu,
> + *   -1 (VMCE_INJECT_BROADCAST), broadcast vMCE to all vcpus
> + *   >= 0, vcpu, the vMCE is injected to
>   */
>  int inject_vmce(struct domain *d, int vcpu)
>  {
>      struct vcpu *v;
> +    int ret = -EINVAL;
>  
>      for_each_vcpu ( d, v )
>      {
> -        if ( vcpu >= 0 && v->vcpu_id != vcpu )
> +        if ( vcpu != VMCE_INJECT_BROADCAST && vcpu != v->vcpu_id )
>              continue;
>  
>          if ( (is_hvm_domain(d) ||
> @@ -358,19 +364,21 @@
>              mce_printk(MCE_VERBOSE, "MCE: inject vMCE to d%d:v%d\n",
>                         d->domain_id, v->vcpu_id);
>              vcpu_kick(v);
> +            ret = 0;
>          }
>          else
>          {
>              mce_printk(MCE_QUIET, "Failed to inject vMCE to d%d:v%d\n",
>                         d->domain_id, v->vcpu_id);
> -            return -EBUSY;
> +            ret = -EBUSY;
> +            break;
>          }
>  
>          if ( vcpu >= 0 )
> -            return 0;
> +            break;
>      }
>  
> -    return v ? -ESRCH : 0;
> +    return ret;
>  }
>  
>  int fill_vmsr_data(struct mcinfo_bank *mc_bank, struct domain *d,
> diff -r adc7f057011e xen/arch/x86/cpu/mcheck/vmce.h
> --- a/xen/arch/x86/cpu/mcheck/vmce.h  Thu Sep 27 19:52:13 2012 +0800
> +++ b/xen/arch/x86/cpu/mcheck/vmce.h  Fri Sep 28 01:25:19 2012 +0800
> @@ -18,6 +18,8 @@
>  
>  int fill_vmsr_data(struct mcinfo_bank *mc_bank, struct domain *d,
>      uint64_t gstatus);
> +
> +#define VMCE_INJECT_BROADCAST -1
>  int inject_vmce(struct domain *d, int vcpu);
>  
>  #endif



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


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