Re: [Xen-devel] [PATCH 3/4 V2] X86: MPX IA32_BNDCFGS msr handle

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.11.13 at 03:28, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> Jan Beulich wrote:
>>>>> "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> 11/26/13 3:12 PM >>>
>>> Jan Beulich wrote:
>>>>>>> On 26.11.13 at 11:11, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
>>>>>>> wrote: 
>>>>> Jan Beulich wrote:
>>>>>>>>> On 25.11.13 at 09:25, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
>>>>>>>>> wrote:
>>>>>>> @@ -3010,6 +3025,12 @@ int hvm_msr_read_intercept(unsigned int
>>>>>>>          msr, uint64_t *msr_content) hvm_get_guest_pat(v,
>>>>>>> msr_content); break; 
>>>>>>> 
>>>>>>> +    case MSR_IA32_BNDCFGS:
>>>>>>> +        if ( !cpu_has_mpx )
>>>>>> 
>>>>>> Wasn't it you who started to properly use hvm_cpuid() for cases
>>>>>> like this? We're not after host capabilities here, but after what
>>>>>> the guest is supposed to (not) use.
>>>>> 
>>>>> When malicious or flawed guest access reserved or unimplemented
>>>>> msr, I think inject #GP is better than silently ignore?
>>>> 
>>>> I didn't say "silently ignore"; in fact, I asked you to _tighten_
>>>> the check (hvm_cpuid() shouldn't return the respective flag set when
>>>> !cpu_has_mpx).
>>> 
>>> Not quite clear your meaning here, hvm_cpuid will not return flag
>>> when !cpu_has_mpx. 
>> 
>> In your patch you check the hardware capability, but you ought to
>> check whether the guest knows the feature to be available. The
>> feature will obviously not show as available if the hardware
>> capability is not there. 
> 
> But the check !cpu_has_mpx has implicitly meant guest will not know this 
> feature -- if h/w has not mpx cability, xen will not expose it to guest. 
> Anything I'm missing here?

Yes - you continue to ignore the inverse case: Hardware _has_ MPX,
but the guest config disables the feature's visibility to the guest (via
overriding of CPUID flags).

One other note: Rather than "manually" handling the MSR if the
optional VMX support for it is unavailable, we should perhaps not
expose the feature to guests in the first place. Otherwise your
patch is, I'm afraid, still incomplete in that it lacks migration support
(i.e. saving and restoring of the MSR value).

Jan


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