Re: [Xen-devel] [PATCH 16/19] x86/vmce: enable injecting LMCE to guest on Intel host

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.02.17 at 05:48, <haozhong.zhang@xxxxxxxxx> wrote:
> On 02/22/17 08:48 -0700, Jan Beulich wrote:
>> >>> On 17.02.17 at 07:39, <haozhong.zhang@xxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/cpu/mcheck/mcaction.c
>> > +++ b/xen/arch/x86/cpu/mcheck/mcaction.c
>> > @@ -88,17 +88,19 @@ mc_memerr_dhandler(struct mca_binfo *binfo,
>> >                      goto vmce_failed;
>> >                  }
>> >  
>> > -                if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
>> > +                vmce_vcpuid = global->mc_vcpuid;
>> > +                if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
>> > +                     (vmce_vcpuid == -1 ||
>> 
>> What is this -1 matching up with? Or to say it differently, this needs a
>> #define used at both producing and consuming sides.
> 
> It comes from mca_init_global

Now that's why I did add the second sentence: I understand that,
but prior to this patch there was (in the hypervisor) only a
producing side. That's bad enough, because it leaves the consumer
(outside of the hypervisor) at the mercy of this value remaining at
that hard coded -1, and doesn't allow for making the connection.
The latest with you adding a consumer in the hypervisor, the
values should become connectable to one another by using a
suitable #define on both sides. Quite likely that #define should
actually become part of the public interface, so that users of the
value have a proper item to compare against. Having to explain
this made me look at the type of the field, btw: It's uint16_t, so
the comparison above is always false.

The situation looks to be even worse for mc_domid, so I think I'll
prepare a patch (where I then may include the mc_vcpuid aspect
right away).

Jan


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