Xen devel - Aug 2013 - [PATCH] VMX, apicv: add boot parameter to enable/disable APIC-v dynamically

From: Yang Zhang <yang.z.zhang@Intel.com>

Add a boot parameter to enable/disable the APIC-v dynamically. APIC-v is
enabled by default. User can use apicv=0 to disable it.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vmx/vmcs.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index de9f592..b52408d 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -46,6 +46,9 @@ boolean_param("vpid", opt_vpid_enabled);
 static bool_t __read_mostly opt_unrestricted_guest_enabled = 1;
 boolean_param("unrestricted_guest", opt_unrestricted_guest_enabled);
 
+static bool_t __read_mostly opt_apicv_enabled = 1;
+boolean_param("apicv", opt_apicv_enabled);
+
 /*
  * These two parameters are used to config the controls for Pause-Loop Exiting:
  * ple_gap:    upper bound on the amount of time between two successive
@@ -196,12 +199,12 @@ static int vmx_init_vmcs_config(void)
          * "APIC Register Virtualization" and "Virtual Interrupt
Delivery"
          * can be set only when "use TPR shadow" is set
          */
-        if ( _vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW )
+        if ( _vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW &&
+                opt_apicv_enabled )
             opt |= SECONDARY_EXEC_APIC_REGISTER_VIRT |
                    SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
                    SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
 
-
         _vmx_secondary_exec_control = adjust_vmx_controls(
             "Secondary Exec Control", min, opt,
             MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch);
-- 
1.7.1

On 09/08/13 10:01, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
>
> Add a boot parameter to enable/disable the APIC-v dynamically. APIC-v is
> enabled by default. User can use apicv=0 to disable it.
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
Please patch docs/misc/xen-command-line.markdown as well.
>  xen/arch/x86/hvm/vmx/vmcs.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index de9f592..b52408d 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -46,6 +46,9 @@ boolean_param("vpid", opt_vpid_enabled);
>  static bool_t __read_mostly opt_unrestricted_guest_enabled = 1;
>  boolean_param("unrestricted_guest",
opt_unrestricted_guest_enabled);
>  
> +static bool_t __read_mostly opt_apicv_enabled = 1;
> +boolean_param("apicv", opt_apicv_enabled);
> +
>  /*
>   * These two parameters are used to config the controls for Pause-Loop
Exiting:
>   * ple_gap:    upper bound on the amount of time between two successive
> @@ -196,12 +199,12 @@ static int vmx_init_vmcs_config(void)
>           * "APIC Register Virtualization" and "Virtual
Interrupt Delivery"
>           * can be set only when "use TPR shadow" is set
>           */
> -        if ( _vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW )
> +        if ( _vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW
&&
> +                opt_apicv_enabled )
>              opt |= SECONDARY_EXEC_APIC_REGISTER_VIRT |
>                     SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>                     SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
>  
> -
Unrelated whitespace alteration.

~Andrew
>          _vmx_secondary_exec_control = adjust_vmx_controls(
>              "Secondary Exec Control", min, opt,
>              MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch);

Andrew Cooper wrote on 2013-08-09:
> On 09/08/13 10:01, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> Add a boot parameter to enable/disable the APIC-v dynamically.
>> APIC-v is enabled by default. User can use apicv=0 to disable it.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
> 
> Please patch docs/misc/xen-command-line.markdown as well.
Sure.
> 
>>  xen/arch/x86/hvm/vmx/vmcs.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c
>> b/xen/arch/x86/hvm/vmx/vmcs.c index de9f592..b52408d 100644
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -46,6 +46,9 @@ boolean_param("vpid", opt_vpid_enabled); 
static
>> bool_t __read_mostly opt_unrestricted_guest_enabled = 1;
>> boolean_param("unrestricted_guest",
opt_unrestricted_guest_enabled);
>> 
>> +static bool_t __read_mostly opt_apicv_enabled = 1;
>> +boolean_param("apicv", opt_apicv_enabled);
>> +
>>  /*
>>   * These two parameters are used to config the controls for
>> Pause-Loop
> Exiting:
>>   * ple_gap:    upper bound on the amount of time between two
> successive
>> @@ -196,12 +199,12 @@ static int vmx_init_vmcs_config(void)
>>           * "APIC Register Virtualization" and "Virtual
Interrupt Delivery"
>>           * can be set only when "use TPR shadow" is set
>>           */
>> -        if ( _vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW )
+
>>     if ( _vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW
&& +
>>         opt_apicv_enabled )
>>              opt |= SECONDARY_EXEC_APIC_REGISTER_VIRT |
>>                     SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>>                     SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
>> -
> 
> Unrelated whitespace alteration.
The whitespace is redundant. It is introduced by my previous patch of enabling
APIC-v, so I remove it here.
> ~Andrew
> 
>>          _vmx_secondary_exec_control = adjust_vmx_controls(
>>              "Secondary Exec Control", min, opt,
>>              MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch);

Best regards,
Yang

On 09/08/13 11:00, Zhang, Yang Z wrote:
> -
>> Unrelated whitespace alteration.
> The whitespace is redundant. It is introduced by my previous patch of
enabling APIC-v, so I remove it here.
Then please correct your previous patch not to introduce it in the first
place.

~Andrew

Andrew Cooper wrote on 2013-08-09:
> On 09/08/13 11:00, Zhang, Yang Z wrote:
>> -
>>> Unrelated whitespace alteration.
>> The whitespace is redundant. It is introduced by my previous patch
>> of
> enabling APIC-v, so I remove it here.
> 
> Then please correct your previous patch not to introduce it in the first
place.
No, those patches already got applied in last year. Anyway, I can send out a
separate patch to do this.

Best regards,
Yang

On 09/08/13 11:19, Zhang, Yang Z wrote:
> Andrew Cooper wrote on 2013-08-09:
>> On 09/08/13 11:00, Zhang, Yang Z wrote:
>>> -
>>>> Unrelated whitespace alteration.
>>> The whitespace is redundant. It is introduced by my previous patch
>>> of
>> enabling APIC-v, so I remove it here.
>>
>> Then please correct your previous patch not to introduce it in the
first place.
> No, those patches already got applied in last year. Anyway, I can send out
a separate patch to do this.
>
> Best regards,
> Yang
>
>
Ah ok.

~Andrew

>>> On 09.08.13 at 12:19, "Zhang, Yang Z"
<yang.z.zhang@intel.com> wrote:
> Andrew Cooper wrote on 2013-08-09:
>> On 09/08/13 11:00, Zhang, Yang Z wrote:
>>> -
>>>> Unrelated whitespace alteration.
>>> The whitespace is redundant. It is introduced by my previous patch
>>> of
>> enabling APIC-v, so I remove it here.
>> 
>> Then please correct your previous patch not to introduce it in the
first
> place.
> No, those patches already got applied in last year. Anyway, I can send out
a
> separate patch to do this.
No, let''s keep it here. The patch itself is simple enough to warrant
that cleanup being included.

Jan