Re: [Xen-devel] [PATCH] x86/MCE: bypass uninitialized vcpu in vMCE injection

[Kai Huang <kai.huang@xxxxxxxxxxxxxxx>]

On 05/08/2014 09:12 AM, Kai Huang wrote:
> On 05/07/2014 09:25 PM, Egger, Christoph wrote:
> > On 07.05.14 09:29, Kai Huang wrote:
> > > Dom0 may bring up less number of vCPUs than xen hypervisor actually 
> > > created for
> > > it, and in this case, on Intel platform, vMCE injection to dom0 will 
> > > fail due to
> > > injecting vMCE to uninitialized vcpu, and cause dom0 crash.
> > >
> > > Signed-off-by: Kai Huang <kai.huang@xxxxxxxxxxxxxxx>
> > > ---
> > >   xen/arch/x86/cpu/mcheck/vmce.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > >
> > > diff --git a/xen/arch/x86/cpu/mcheck/vmce.c 
> > > b/xen/arch/x86/cpu/mcheck/vmce.c
> > > index c83375e..72fe924 100644
> > > --- a/xen/arch/x86/cpu/mcheck/vmce.c
> > > +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> > > @@ -356,6 +356,10 @@ int inject_vmce(struct domain *d, int vcpu)
> > >           if ( vcpu != VMCE_INJECT_BROADCAST && vcpu != v->vcpu_id )
> > >               continue;
> > >   +        /* In case of broadcasting, don't inject to uninitialized 
> > > VCPU */
> > > +        if ( vcpu == VMCE_INJECT_BROADCAST && !v->is_initialised )
> > > +            continue;
> > > +
> > What happens when you inject to exactly one uninitialized VCPU?
> > I think what you want is this:
> >
> >             /* Don't inject to uninitialized VCPU */
> >             if ( !v->is_initialised )
> >                 continue;
> Good suggestion indeed. In inject_vmce, the ret is initialized to 
> -ESRCH at beginning. In case of injecting to one particular vcpu, we 
> will simply bypass all loops of "for_each_vcpu" and don't do the 
> injection at all, while the ret remains -ESRCH. Looks it's the right 
> behaviour for AMD case.
> Thanks Egger for comments (and sorry that I forgot to CC you at 
> beginning:)). I'll follow your suggestion.
Hi Egger,

Forgot to say, the reason I didn't do this at beginning is this code 
will potentially impact vMCE injection behaviour on AMD platform. 
Considering I don't have AMD machine to test, so I can just do a 
theoretically analysis here. If you are OK with this, I'll follow your 
suggestion, Otherwise, I think Jan's suggestion is also good enough.
Thanks,
-Kai
> Thanks,
> -Kai
> > >           if ( (has_hvm_container_domain(d) ||
> > >                 guest_has_trap_callback(d, v->vcpu_id, 
> > > TRAP_machine_check)) &&
> > >                !test_and_set_bool(v->mce_pending) )
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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