Re: [Xen-devel] [PATCH 2/3] x86/HVM: support (emulate) UMIP

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.12.16 at 16:31, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 12/07/2016 10:14 AM, Jan Beulich wrote:
>>>>> On 07.12.16 at 16:10, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> On 12/07/2016 06:29 AM, Jan Beulich wrote:
>>>>>>> On 06.12.16 at 17:23, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>>>> On 12/06/2016 06:44 AM, Jan Beulich wrote:
>>>>>> --- a/xen/arch/x86/cpuid.c
>>>>>> +++ b/xen/arch/x86/cpuid.c
>>>>>> @@ -154,6 +154,13 @@ static void __init calculate_hvm_feature
>>>>>>      __set_bit(X86_FEATURE_APIC, hvm_featureset);
>>>>>>  
>>>>>>      /*
>>>>>> +     * Xen can often provide UMIP emulation to HVM guests even if the 
>>>>>> host
>>>>>> +     * doesn't have such functionality.
>>>>>> +     */
>>>>>> +    if ( cpu_has_vmx_dt_exiting || cpu_has_svm )
>>>>>> +        __set_bit(X86_FEATURE_UMIP, hvm_featureset);
>>>>> I don't think I understand how this is going to work for processors that
>>>>> don't support UMIP.
>>>>>
>>>>> How, for example, can guest_cr[4] have X86_CR4_UMIP set on these
>>>>> processors when CPUID will not show the feature being there?
>>>> What we allow the guest to see and what we store into hardware
>>>> registers are two different things: Note how svm_update_guest_cr()
>>>> masks off X86_CR4_UMIP from the vale to be put into the VMCB.
>>> So that was kind of my question --- why would a guest ever try to set
>>> this bit? As far as it is concerned, UMIP is not available and the guest
>>> is then trying to set an unsupported bit in cr4. And that should result
>>> in a #GP.
>> But the code fragment above adds the respective CPUID bit to the
>> permitted features. Believe me, I've tried this with a UMIP-enabled
>> Linux (including proper CPUID based detection).
> 
> Are you referring to these patches: https://lkml.org/lkml/2016/11/8/68 ?
> If yes then they look to be Intel-specific.

No, I had written my own before these had been posted. I did
actually post mine too, but that was a week or so after theirs,
and theirs appears to be more complete.

> If AMD decides to use CPUID0x7.ecx[2] for something else --- won't this
> be a problem for this patch?

I think the two vendors meanwhile do a good job not interfering with
one another's CPUID bits. We'd have ugly problems elsewhere if any
new dual purpose CPUID bit appeared.

Jan


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