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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.12.13 at 13:04, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.
> 
> 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.

???

Jan


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