* [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled
@ 2017-11-12 16:31 Arbel Moshe
2017-11-13 8:36 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Arbel Moshe @ 2017-11-12 16:31 UTC (permalink / raw)
To: pbonzini, rkrcmar, kvm
Cc: idan.brown, liran.alon, Arbel Moshe, Krish Sadhukhan
Implementation of virtual APICv relies on L0 being able to use APICv.
Therefore, if enable_apicv==false, we should not expose APICv to L1.
This commit makes sure to not expose APICv Secondary CPU controls
to L1 when enable_apicv==false.
Signed-off-by: Arbel Moshe <arbel.moshe@oracle.com>
Reviewed-by: Liran Alon <liran.alon@oracle.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
arch/x86/kvm/vmx.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index a6f4f095f8f4..7881533280da 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2809,10 +2809,14 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
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);
+ }
+
if (enable_ept) {
/* nested EPT: emulate EPT also to L1 */
vmx->nested.nested_vmx_secondary_ctls_high |=
--
2.14.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled
2017-11-12 16:31 [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled Arbel Moshe
@ 2017-11-13 8:36 ` Paolo Bonzini
2017-11-13 8:51 ` Liran Alon
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2017-11-13 8:36 UTC (permalink / raw)
To: Arbel Moshe, rkrcmar, kvm; +Cc: idan.brown, liran.alon, Krish Sadhukhan
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.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled
2017-11-13 8:36 ` Paolo Bonzini
@ 2017-11-13 8:51 ` Liran Alon
2017-11-13 10:59 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Liran Alon @ 2017-11-13 8:51 UTC (permalink / raw)
To: Paolo Bonzini, Arbel Moshe, rkrcmar, kvm; +Cc: idan.brown, Krish Sadhukhan
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled
2017-11-13 8:51 ` Liran Alon
@ 2017-11-13 10:59 ` Paolo Bonzini
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2017-11-13 10:59 UTC (permalink / raw)
To: Liran Alon, Arbel Moshe, rkrcmar, kvm; +Cc: idan.brown, Krish Sadhukhan
On 13/11/2017 09:51, Liran Alon wrote:
>
>
> 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().
Yes, I agree with all points. Thanks,
Paolo
> Arbel will fix these in v2 of this series.
>
> Thanks.
> -Liran
>
>>
>> Thanks,
>>
>> Paolo
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-13 10:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-12 16:31 [PATCH] KVM: nVMX: Fix nested APICv Secondary CPU Controls when apicv disabled Arbel Moshe
2017-11-13 8:36 ` Paolo Bonzini
2017-11-13 8:51 ` Liran Alon
2017-11-13 10:59 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.