Re: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/11/13 14:27, Liu, Jinsong wrote:
> Andrew Cooper wrote:
>> On 27/11/13 13:50, Liu, Jinsong wrote:
>>> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00
>>> 2001 
>>> From: Liu Jinsong <jinsong.liu@intel.com>
>>> Date: Thu, 28 Nov 2013 05:26:06 +0800
>>> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle
>>>
>>> When MPX supported, a new guest-state field for IA32_BNDCFGS
>>> is added to the VMCS. In addition, two new controls are added:
>>>  - a VM-exit control called "clear BNDCFGS"
>>>  - a VM-entry control called "load BNDCFGS."
>>> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS.
>>>
>>> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>>> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com>
>>>
>>> Unlikely, but in case VMX support is not available, not expose
>>> MPX to hvm guest.
>> You are still missing the point.
>>
>> I as the administrator choose to prevent an HVM guest from using MPX.
>> Perhaps I want to create a heterogeneous pool.
>>
>> Therefore, the bit is disabled in the domains cpuid policy, despite
>> being available on the hardware.
>>
>> ~Andrew
>>
> Could you tell me the reason why choose to prevent HVM from using MPX?
>
> Thanks,
> Jinsong

For exactly the case I gave - a VM in a heterogeneous pool where one
server supports MPX and the other is lacking the MPX feature.

~Andrew

>
>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Suggested-by: Jan Beulich <jbeulich@suse.com>
>>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>>> ---
>>>  xen/arch/x86/hvm/hvm.c             |    6 ++++++
>>>  xen/arch/x86/hvm/vmx/vmcs.c        |    8 ++++++--
>>>  xen/include/asm-x86/cpufeature.h   |    2 ++
>>>  xen/include/asm-x86/hvm/vmx/vmcs.h |    2 ++
>>>  xen/include/asm-x86/msr-index.h    |    2 ++
>>>  5 files changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index 9c88c73..0f7178b 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2905,6 +2905,12 @@ void hvm_cpuid(unsigned int input, unsigned
>>>          int *eax, unsigned int *ebx, if ( (count == 0) &&
>>>              !cpu_has_smep ) *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>>>
>>> +        /* Don't expose MPX to hvm when VMX support is not
>>> available */ +        if ( (count == 0) && +            
>>> (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) || +             
>>> !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) ) +            *ebx
>>>          &= ~cpufeat_mask(X86_FEATURE_MPX); + /* Don't expose
>>>          INVPCID to non-hap hvm. */ if ( (count == 0) &&
>>>              !hap_enabled(d) ) *ebx &=
>>> ~cpufeat_mask(X86_FEATURE_INVPCID); 
>>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c
>>> b/xen/arch/x86/hvm/vmx/vmcs.c 
>>> index 290b42f..4a1f168 100644
>>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>>> @@ -270,7 +270,8 @@ static int vmx_init_vmcs_config(void)      }
>>>
>>>      min = VM_EXIT_ACK_INTR_ON_EXIT;
>>> -    opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT;
>>> +    opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT |
>>> +          VM_EXIT_CLEAR_BNDCFGS;
>>>      min |= VM_EXIT_IA32E_MODE;
>>>      _vmx_vmexit_control = adjust_vmx_controls(
>>>          "VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS,
>>> &mismatch); @@ -284,7 +285,7 @@ static int vmx_init_vmcs_config(void)
>>>          _vmx_pin_based_exec_control  &= ~
>>> PIN_BASED_POSTED_INTERRUPT; 
>>>
>>>      min = 0;
>>> -    opt = VM_ENTRY_LOAD_GUEST_PAT;
>>> +    opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS;
>>>      _vmx_vmentry_control = adjust_vmx_controls(
>>>          "VMEntry Control", min, opt, MSR_IA32_VMX_ENTRY_CTLS,
>>> &mismatch); 
>>>
>>> @@ -955,6 +956,9 @@ static int construct_vmcs(struct vcpu *v)
>>>          vmx_disable_intercept_for_msr(v, MSR_IA32_SYSENTER_EIP,
>>>          MSR_TYPE_R | MSR_TYPE_W); if ( paging_mode_hap(d) &&
>>>              (!iommu_enabled || iommu_snoop) )
>>> vmx_disable_intercept_for_msr(v, MSR_IA32_CR_PAT, MSR_TYPE_R |
>>> MSR_TYPE_W); +        if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) && +
>>> (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) ) +           
>>> vmx_disable_intercept_for_msr(v, MSR_IA32_BNDCFGS, MSR_TYPE_R |
>>> MSR_TYPE_W);      }  
>>>
>>>      /* I/O access bitmap. */
>>> diff --git a/xen/include/asm-x86/cpufeature.h
>>> b/xen/include/asm-x86/cpufeature.h 
>>> index 1cfaf94..930dc9b 100644
>>> --- a/xen/include/asm-x86/cpufeature.h
>>> +++ b/xen/include/asm-x86/cpufeature.h
>>> @@ -148,6 +148,7 @@
>>>  #define X86_FEATURE_INVPCID	(7*32+10) /* Invalidate Process Context
>>>  ID */ #define X86_FEATURE_RTM 	(7*32+11) /* Restricted
>>>  Transactional Memory */ #define X86_FEATURE_NO_FPU_SEL 	(7*32+13)
>>> /* FPU CS/DS stored as zero */ +#define X86_FEATURE_MPX		(7*32+14)
>>>  /* Memory Protection Extensions */ #define
>>> X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */ 
>>>
>>>  #define cpu_has(c, bit)		test_bit(bit, (c)->x86_capability) @@
>>>  -197,6 +198,7 @@ #define cpu_has_xsave          
>>>  boot_cpu_has(X86_FEATURE_XSAVE) #define cpu_has_avx            
>>>  boot_cpu_has(X86_FEATURE_AVX) #define cpu_has_lwp            
>>> boot_cpu_has(X86_FEATURE_LWP) +#define cpu_has_mpx            
>>> boot_cpu_has(X86_FEATURE_MPX) 
>>>
>>>  #define cpu_has_arch_perfmon   
>>> boot_cpu_has(X86_FEATURE_ARCH_PERFMON) 
>>>
>>> diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h
>>> b/xen/include/asm-x86/hvm/vmx/vmcs.h 
>>> index ebaba5c..75cd653 100644
>>> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h
>>> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
>>> @@ -186,6 +186,7 @@ extern u32 vmx_pin_based_exec_control;
>>>  #define VM_EXIT_SAVE_GUEST_EFER         0x00100000
>>>  #define VM_EXIT_LOAD_HOST_EFER          0x00200000
>>>  #define VM_EXIT_SAVE_PREEMPT_TIMER      0x00400000
>>> +#define VM_EXIT_CLEAR_BNDCFGS           0x00800000
>>>  extern u32 vmx_vmexit_control;
>>>
>>>  #define VM_ENTRY_IA32E_MODE             0x00000200
>>> @@ -194,6 +195,7 @@ extern u32 vmx_vmexit_control;
>>>  #define VM_ENTRY_LOAD_PERF_GLOBAL_CTRL  0x00002000
>>>  #define VM_ENTRY_LOAD_GUEST_PAT         0x00004000
>>>  #define VM_ENTRY_LOAD_GUEST_EFER        0x00008000
>>> +#define VM_ENTRY_LOAD_BNDCFGS           0x00010000
>>>  extern u32 vmx_vmentry_control;
>>>
>>>  #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
>>> diff --git a/xen/include/asm-x86/msr-index.h
>>> b/xen/include/asm-x86/msr-index.h 
>>> index e597a28..ccad1ab 100644
>>> --- a/xen/include/asm-x86/msr-index.h
>>> +++ b/xen/include/asm-x86/msr-index.h
>>> @@ -56,6 +56,8 @@
>>>  #define MSR_IA32_DS_AREA		0x00000600
>>>  #define MSR_IA32_PERF_CAPABILITIES	0x00000345
>>>
>>> +#define MSR_IA32_BNDCFGS		0x00000D90
>>> +
>>>  #define MSR_MTRRfix64K_00000		0x00000250
>>>  #define MSR_MTRRfix16K_80000		0x00000258
>>>  #define MSR_MTRRfix16K_A0000		0x00000259