Messages in this thread | | | Date | Wed, 15 Jun 2022 15:15:56 +0200 | Subject | Re: [PATCH 2/5] KVM: x86: Drop @vcpu parameter from kvm_x86_ops.hwapic_isr_update() | From | Paolo Bonzini <> |
| |
On 6/15/22 01:05, Sean Christopherson wrote: > Drop the unused @vcpu parameter from hwapic_isr_update(). AMD/AVIC is > unlikely to implement the helper, and VMX/APICv doesn't need the vCPU as > it operates on the current VMCS. The result is somewhat odd, but allows > for a decent amount of (future) cleanup in the APIC code. > > No functional change intended.
Yeah, that's a bit odd; what it saves is essentially the apic->vcpu dereference. I don't really like it, so if you want to have a v2 that passes the struct kvm_lapic* instead (which is free and keeps irr/isr functions consistent), I'll gladly switch. But I _have_ queued the series in the meanwhile, so that's a good reason to ignore me.
Paolo
> Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/include/asm/kvm_host.h | 2 +- > arch/x86/kvm/lapic.c | 8 ++++---- > arch/x86/kvm/vmx/vmx.c | 2 +- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 7e98b2876380..16acc54d49a7 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1517,7 +1517,7 @@ struct kvm_x86_ops { > bool (*check_apicv_inhibit_reasons)(enum kvm_apicv_inhibit reason); > void (*refresh_apicv_exec_ctrl)(struct kvm_vcpu *vcpu); > void (*hwapic_irr_update)(struct kvm_vcpu *vcpu, int max_irr); > - void (*hwapic_isr_update)(struct kvm_vcpu *vcpu, int isr); > + void (*hwapic_isr_update)(int isr); > bool (*guest_apic_has_interrupt)(struct kvm_vcpu *vcpu); > void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap); > void (*set_virtual_apic_mode)(struct kvm_vcpu *vcpu); > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index a413a1d8df4c..cc0da5671eb9 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -556,7 +556,7 @@ static inline void apic_set_isr(int vec, struct kvm_lapic *apic) > * just set SVI. > */ > if (unlikely(vcpu->arch.apicv_active)) > - static_call_cond(kvm_x86_hwapic_isr_update)(vcpu, vec); > + static_call_cond(kvm_x86_hwapic_isr_update)(vec); > else { > ++apic->isr_count; > BUG_ON(apic->isr_count > MAX_APIC_VECTOR); > @@ -604,7 +604,7 @@ static inline void apic_clear_isr(int vec, struct kvm_lapic *apic) > * and must be left alone. > */ > if (unlikely(vcpu->arch.apicv_active)) > - static_call_cond(kvm_x86_hwapic_isr_update)(vcpu, apic_find_highest_isr(apic)); > + static_call_cond(kvm_x86_hwapic_isr_update)(apic_find_highest_isr(apic)); > else { > --apic->isr_count; > BUG_ON(apic->isr_count < 0); > @@ -2457,7 +2457,7 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event) > if (vcpu->arch.apicv_active) { > static_call_cond(kvm_x86_apicv_post_state_restore)(vcpu); > static_call_cond(kvm_x86_hwapic_irr_update)(vcpu, -1); > - static_call_cond(kvm_x86_hwapic_isr_update)(vcpu, -1); > + static_call_cond(kvm_x86_hwapic_isr_update)(-1); > } > > vcpu->arch.apic_arb_prio = 0; > @@ -2737,7 +2737,7 @@ int kvm_apic_set_state(struct kvm_vcpu *vcpu, struct kvm_lapic_state *s) > if (vcpu->arch.apicv_active) { > static_call_cond(kvm_x86_apicv_post_state_restore)(vcpu); > static_call_cond(kvm_x86_hwapic_irr_update)(vcpu, apic_find_highest_irr(apic)); > - static_call_cond(kvm_x86_hwapic_isr_update)(vcpu, apic_find_highest_isr(apic)); > + static_call_cond(kvm_x86_hwapic_isr_update)(apic_find_highest_isr(apic)); > } > kvm_make_request(KVM_REQ_EVENT, vcpu); > if (ioapic_in_kernel(vcpu->kvm)) > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 5e14e4c40007..42f8924a90f4 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -6556,7 +6556,7 @@ static void vmx_set_apic_access_page_addr(struct kvm_vcpu *vcpu) > put_page(page); > } > > -static void vmx_hwapic_isr_update(struct kvm_vcpu *vcpu, int max_isr) > +static void vmx_hwapic_isr_update(int max_isr) > { > u16 status; > u8 old;
| |