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

Re: [Xen-devel] X86: generic MSRs save/restore



On 13/12/2013 09:47, Jan Beulich wrote:
>>>> On 13.12.13 at 08:57, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -580,6 +580,50 @@ static int vmx_load_vmcs_ctxt(struct vcp
>>      return 0;
>>  }
>>  
>> +static unsigned int __init vmx_init_msr(void)
>> +{
>> +    return !!cpu_has_mpx;
>> +}
>> +
>> +static void vmx_save_msr(struct vcpu *v, struct hvm_msr *ctxt)
>> +{
>> +    vmx_vmcs_enter(v);
>> +
>> +    if ( cpu_has_mpx )
>> +    {
>> +        __vmread(GUEST_BNDCFGS, &ctxt->msr[ctxt->count].val);
>> +        if ( ctxt->msr[ctxt->count].val )
>>
>>
>> Isn't it better to remove if()?
> Definitely not: That way, if the hardware support MPX but the
> guest never used it, restoring the guest on an MPX-incapable
> host will be possible. And when the MSR is zero, restoring on an
> MPX-capable host will be correct too, as the respective VMCS
> field starts out zeroed.
>
> Jan
>

Furthermore, this looks as if is heading straight for an ABI breakage.

What guarantee is that that the MSRs shall aways be saved and restored
in this specific order forever more in the future?

I think ctxt->count needs to be replaced with an ABI constant.

~Andrew

_______________________________________________
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®.