From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Egger Subject: Re: [PATCH 6] X86/MCE: update vMCE injection for AMD Date: Thu, 27 Sep 2012 11:56:57 +0200 Message-ID: <506422E9.3060100@amd.com> References: <506173FB.8070603@amd.com> <5061A833020000780009DA1A@nat28.tlf.novell.com> <50619B61.2030609@amd.com> <5062CC25020000780009DE44@nat28.tlf.novell.com> <5064143A.6070203@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Liu, Jinsong" Cc: "xen-devel@lists.xensource.com" , "keir@xen.org" , "Ian.Campbell@citrix.com" , Jan Beulich List-Id: xen-devel@lists.xenproject.org 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 >>> >>> 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 > Suggested_by: Christoph Egger Acked-by: Christoph Egger > > 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