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

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

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

Thanks,
Jinsong

>> +        }
>>          break;
>>      default:
>>          ret = mce_bank_msr(cur, msr) ? bank_mce_rdmsr(cur, msr,
>>      val) : 0; @@ -287,15 +296,16 @@ struct domain_mca_msrs *vmce =
>> dom_vmce(cur->domain);      int ret = 1; 
>> 
>> -    if ( !g_mcg_cap )
>> -        return 0;
>> -
>>      spin_lock(&vmce->lock);
>> 
>>      switch ( msr )
>>      {
>>      case MSR_IA32_MCG_CTL:
>> -        vmce->mcg_ctl = val;
>> +        if ( !(cur->arch.mcg_cap & MCG_CTL_P) )
>> +        {
>> +            mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n"); +          
>> ret = -1; +        }
>>          break;
>>      case MSR_IA32_MCG_STATUS:
>>          vmce->mcg_status = val;
> 
> Other than that, the patch looks fine to me.
> 
> Jan


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