kvm: x86: vmx: NULL out hwapic_isr_update() in case of !enable_apicv
PATCH 1/1 kvm: x86: vmx: NULL out hwapic_isr_update() in case of !enable_apicv
In most cases calling hwapic_isr_update(), we always check if kvm_apic_vid_enabled() == 1, but actually, kvm_apic_vid_enabled()
-> kvm_x86_ops->vm_has_apicv()
-> vmx_vm_has_apicv() or '0' in svm case
-> return enable_apicv && irqchip_in_kernel(kvm)So its a little cost to recall vmx_vm_has_apicv() inside hwapic_isr_update(), here just NULL out hwapic_isr_update() in case of !enable_apicv inside hardware_setup() then make all related stuffs follow this. Note we don't check this under that condition of irqchip_in_kernel() since we should make sure definitely any caller don't work without in-kernel irqchip.
--- arch/x86/kvm/lapic.c | 7 ++++--- arch/x86/kvm/vmx.c | 4 +--- 2 files changed, 5 insertions(+), 6 deletions(-)
arch/x86/kvm/lapic.c +4 -3
@@ -402,7 +402,7 @@ static inline void apic_set_isr(int vec, struct kvm_lapic *apic)* because the processor can modify ISR under the hood. Instead * just set SVI. */ -if (unlikely (kvm_apic_vid_enabled (vcpu->kvm))) +if (unlikely (kvm_x86_ops->hwapic_isr_update)) kvm_x86_ops->hwapic_isr_update (vcpu->kvm, vec);else { ++apic->isr_count;
@@ -450,7 +450,7 @@ static inline void apic_clear_isr(int vec, struct kvm_lapic *apic)* on the other hand isr_count and highest_isr_cache are unused * and must be left alone. */ -if (unlikely (kvm_apic_vid_enabled (vcpu->kvm))) +if (unlikely (kvm_x86_ops->hwapic_isr_update)) kvm_x86_ops->hwapic_isr_update (vcpu->kvm,apic_find_highest_isr (apic));else {
@@ -1742,7 +1742,8 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,if (kvm_x86_ops->hwapic_irr_update) kvm_x86_ops->hwapic_irr_update (vcpu,apic_find_highest_irr (apic)); - kvm_x86_ops->hwapic_isr_update (vcpu->kvm,apic_find_highest_isr (apic)); +if (kvm_x86_ops->hwapic_isr_update) + kvm_x86_ops->hwapic_isr_update (vcpu->kvm,apic_find_highest_isr (apic));kvm_make_request (KVM_REQ_EVENT , vcpu);kvm_rtc_eoi_tracking_restore_one (vcpu); }
arch/x86/kvm/vmx.c +1 -3
@@ -5938,6 +5938,7 @@ static __init int hardware_setup(void) kvm_x86_ops->update_cr8_intercept =NULL ; else { kvm_x86_ops->hwapic_irr_update =NULL ; + kvm_x86_ops->hwapic_isr_update =NULL ; kvm_x86_ops->deliver_posted_interrupt =NULL ; kvm_x86_ops->sync_pir_to_irr = vmx_sync_pir_to_irr_dummy; }
@@ -7471,9 +7472,6 @@ static void vmx_hwapic_isr_update(struct kvm *kvm, int isr)u16 status;u8 old; -if (!vmx_vm_has_apicv (kvm)) -return ; -if (isr ==-1 ) isr =0 ;
Signature · 1 line
1.9.1
Paolo Bonzini
maintainerQuoted 66 lines · Tiejun Chen
In most cases calling
hwapic_isr_update(), we always check ifkvm_apic_vid_enabled()== 1, but actually,kvm_apic_vid_enabled()-> kvm_x86_ops->vm_has_apicv() -> vmx_vm_has_apicv() or '0' in svm case -> return enable_apicv && irqchip_in_kernel(kvm)So its a little cost to recall
vmx_vm_has_apicv()insidehwapic_isr_update(), here just NULL outhwapic_isr_update()in case of !enable_apicv insidehardware_setup()then make all related stuffs follow this. Note we don't check this under that condition ofirqchip_in_kernel()since we should make sure definitely any caller don't work without in-kernel irqchip.Signed-off-by Tiejun Chen <[email protected]>--- arch/x86/kvm/lapic.c | 7 ++++--- arch/x86/kvm/vmx.c | 4 +--- 2 files changed, 5 insertions(+), 6 deletions(-)arch/x86/kvm/lapic.c +4
-3@@ -402,7 +402,7 @@ static inline void apic_set_isr(int vec, struct kvm_lapic *apic)* because the processor can modify ISR under the hood. Instead * just set SVI. */ -if (unlikely (kvm_apic_vid_enabled (vcpu->kvm))) +if (unlikely (kvm_x86_ops->hwapic_isr_update)) kvm_x86_ops->hwapic_isr_update (vcpu->kvm, vec);else { ++apic->isr_count;@@ -450,7 +450,7 @@ static inline void apic_clear_isr(int vec, struct kvm_lapic *apic)* on the other hand isr_count and highest_isr_cache are unused * and must be left alone. */ -if (unlikely (kvm_apic_vid_enabled (vcpu->kvm))) +if (unlikely (kvm_x86_ops->hwapic_isr_update)) kvm_x86_ops->hwapic_isr_update (vcpu->kvm,apic_find_highest_isr (apic));else {@@ -1742,7 +1742,8 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,if (kvm_x86_ops->hwapic_irr_update) kvm_x86_ops->hwapic_irr_update (vcpu,apic_find_highest_irr (apic)); - kvm_x86_ops->hwapic_isr_update (vcpu->kvm,apic_find_highest_isr (apic)); +if (kvm_x86_ops->hwapic_isr_update) + kvm_x86_ops->hwapic_isr_update (vcpu->kvm,apic_find_highest_isr (apic));kvm_make_request (KVM_REQ_EVENT , vcpu);kvm_rtc_eoi_tracking_restore_one (vcpu); }arch/x86/kvm/vmx.c +1
-0@@ -5938,6 +5938,7 @@ static __init int hardware_setup(void) kvm_x86_ops->update_cr8_intercept =NULL ; else { kvm_x86_ops->hwapic_irr_update =NULL ; + kvm_x86_ops->hwapic_isr_update =NULL ; kvm_x86_ops->deliver_posted_interrupt =NULL ; kvm_x86_ops->sync_pir_to_irr = vmx_sync_pir_to_irr_dummy; }
arch/x86/kvm/vmx.c static void vmx_hwapic_isr_update(struct kvm *kvm, int isr) · line 7477
Quoted 9 lines
arch/x86/kvm/vmx.c +0
-3@@ -7471,9 +7472,6 @@ static void vmx_hwapic_isr_update(struct kvm *kvm, int isr)u16 status;u8 old; -if (!vmx_vm_has_apicv (kvm)) -return ; -if (isr ==-1 ) isr =0 ;
Thanks, this looks good.
Paolo