Re: [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled

[Liran Alon <LIRAN.ALON@ORACLE.COM>]

On 13/11/17 10:36, Paolo Bonzini wrote:
> On 12/11/2017 17:31, Arbel Moshe wrote:
>>   		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>>   		SECONDARY_EXEC_DESC |
>>   		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
>> -		SECONDARY_EXEC_APIC_REGISTER_VIRT |
>> -		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>>   		SECONDARY_EXEC_WBINVD_EXITING;
>>
>> +	if (kvm_vcpu_apicv_active(&vmx->vcpu)) {
>> +		vmx->nested.nested_vmx_secondary_ctls_high |=
>> +			(SECONDARY_EXEC_APIC_REGISTER_VIRT |
>> +			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY);
>> +	}
>> +
>
> I think kvm_vcpu_apicv_active may change after
> nested_vmx_setup_ctls_msrs is called.  You need to clear the bits in
> refresh_apicv_exec_ctrl.

Agreed. Seems this is called from kvm_vcpu_deactivate_apicv() which is 
only called from kvm_hv_activate_synic() which enables Hyper-V SynIC.

However, in case Hyper-V SynIC is not enabled, QEMU will never issue 
ioctl that invokes kvm_vcpu_deactivate_apicv() and therefore 
refresh_apicv_exec_ctrl() won't be called.

Therefore, we suggest the following:
1. Keeping the code Arbel added to nested_vmx_setup_ctls_msrs().
2. Adding clearing of relevant bits also to refresh_apicv_exec_ctrl().
3. Fix bug of not also clearing PIN_BASED_POSTED_INTR from the VMCS & 
nested_vmx_pinbased_ctls_high in refresh_apicv_exec_ctrl().

Arbel will fix these in v2 of this series.

Thanks.
-Liran

>
> Thanks,
>
> Paolo
>