[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [Patch 2/6] Xen/MCE: remove mcg_ctl and other adjustment for future vMCE



>>> On 05.08.12 at 12:24, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> Jan Beulich wrote:
>>>>> On 23.07.12 at 11:39, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>>> @@ -175,11 +179,16 @@
>>>                     *val);
>>>          break;
>>>      case MSR_IA32_MCG_CTL:
>>> -        /* Always 0 if no CTL support */
>>>          if ( cur->arch.mcg_cap & MCG_CTL_P )
>>> -            *val = vmce->mcg_ctl & h_mcg_ctl;
>>> -        mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL 0x%"PRIx64"\n",
>>> -                   *val);
>>> +        {
>>> +            *val = ~0UL;
>>> +            mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL
>>> 0x%"PRIx64"\n", *val); +        } +        else
>>> +        {
>>> +            mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n");
>>> +            ret = -1;
>> 
>> Is there a particular reason to make this access fault here, when
>> it didn't before? I.e. was there anything wrong with the previous
>> approach of returning zero on reads and ignoring writes when
>> !MCG_CTL_P?
>> 
> 
> Semantically this code is better than previous approach, since !MCG_CTL_P 
> means unimplemented MCG_CTL so access it would generate GP#.

Agreed. But nevertheless I'd like to be a little more conservative
here. After all, "knowing" that this won't break Windows or Linux
isn't covering all possible HVM guests (and the quotes are there
to indicate that (a) unless you have access to Windows sources,
you can't really know, you may at best have empirical data
suggesting so, and (b) makes you/us dependent on all older
Windows/Linux versions you didn't try out/look at behave
correctly here too).

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.