I don't actually condone the current situation. I'd really like to see
a changeset that makes vmx->secondary_exec_control an actual cache of
the secondary processor-based VM-execution controls from the vmcs01.
On Wed, Nov 22, 2017 at 9:17 AM, Jim Mattson <jmattson@xxxxxxxxxx> wrote:
> Does this change play well with the other functions that toggle
> secondary control bits without updating vmx->secondary_exec_control?
> e.g. vmx_disable_shadow_vmcs, set_current_vmptr,
> vmx_set_virtual_x2apic_mode, vmcs_set_secondary_exec_control, and
> possibly others?
>
> Perhaps the proposed call to vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
> vmx->secondary_exec_control) should instead be a call to
> vmcs_set_secondary_exec_control(vmx->secondary_exec_control)?
>
> On Wed, Nov 22, 2017 at 2:23 AM, Arbel Moshe <arbel.moshe@xxxxxxxxxx> wrote:
>> Handle apicv secondary exec ctrls correctly, without duplicating
>> logic.
>> This commit doesn't change semantics.
>>
>> Signed-off-by: Arbel Moshe <arbel.moshe@xxxxxxxxxx>
>> Reviewed-by: Liran Alon <liran.alon@xxxxxxxxxx>
>> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
>> ---
>> arch/x86/kvm/vmx.c | 14 ++++++--------
>> 1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 7c3522a989d0..84ccd3b2762c 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -5237,20 +5237,18 @@ static u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx)
>> return pin_based_exec_ctrl;
>> }
>>
>> +static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx);
>> +
>> static void vmx_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
>> {
>> struct vcpu_vmx *vmx = to_vmx(vcpu);
>>
>> vmcs_write32(PIN_BASED_VM_EXEC_CONTROL, vmx_pin_based_exec_ctrl(vmx));
>> +
>> if (cpu_has_secondary_exec_ctrls()) {
>> - if (kvm_vcpu_apicv_active(vcpu))
>> - vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
>> - SECONDARY_EXEC_APIC_REGISTER_VIRT |
>> - SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>> - else
>> - vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
>> - SECONDARY_EXEC_APIC_REGISTER_VIRT |
>> - SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>> + vmx_compute_secondary_exec_control(vmx);
>> + vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
>> + vmx->secondary_exec_control);
>> }
>>
>> if (cpu_has_vmx_msr_bitmap())
>> --
>> 2.14.1
>>
