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

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

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

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

Attachment: vmce_injection_update.patch
Description: vmce_injection_update.patch

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