Re: [PATCH v2 1/3] KVM: X86: Some cleanups in ioapic.h/lapic.h

[Vitaly Kuznetsov]

Peter Xu <peterx@xxxxxxxxxx> writes:

> Both kvm_apic_match_dest() and kvm_irq_delivery_to_apic() should
> probably suite more to lapic.h comparing to ioapic.h, moving.
>
> kvm_apic_match_dest() is defined twice, once in each of the header.
> Removing the one defined in ioapic.h.
>

kvm_apic_match_dest()'s implementation lives in lapic.c so moving the
declaration to lapic.h makes perfect sense. kvm_irq_delivery_to_apic()'s
body is, however, in irq_comm.c and declarations for it are usually
found in asm/kvm_host.h. I'm not sure but maybe it would make sense to
move kvm_irq_delivery_to_apic()'s body to lapic.c too.

(Personally, I'd also greatly appreciate if functions working with lapic
exclusively would have 'lapic' instead of 'apic' in their names. But
this is unrelated to the patch.)

> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
>  arch/x86/kvm/ioapic.h | 6 ------
>  arch/x86/kvm/lapic.h  | 5 ++++-
>  2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
> index ea1a4e0297da..2fb2e3c80724 100644
> --- a/arch/x86/kvm/ioapic.h
> +++ b/arch/x86/kvm/ioapic.h
> @@ -116,9 +116,6 @@ static inline int ioapic_in_kernel(struct kvm *kvm)
>  }
>  
>  void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu);
> -bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
> -		int short_hand, unsigned int dest, int dest_mode);
> -int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2);
>  void kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu, int vector,
>  			int trigger_mode);
>  int kvm_ioapic_init(struct kvm *kvm);
> @@ -126,9 +123,6 @@ void kvm_ioapic_destroy(struct kvm *kvm);
>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>  		       int level, bool line_status);
>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
> -int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> -			     struct kvm_lapic_irq *irq,
> -			     struct dest_map *dest_map);
>  void kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>  void kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>  void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu,
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 39925afdfcdc..19b36196e2ff 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -83,7 +83,10 @@ int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
>  		       void *data);
>  bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>  			   int short_hand, unsigned int dest, int dest_mode);
> -
> +int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2);
> +int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> +			     struct kvm_lapic_irq *irq,
> +			     struct dest_map *dest_map);
>  bool __kvm_apic_update_irr(u32 *pir, void *regs, int *max_irr);
>  bool kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir, int *max_irr);
>  void kvm_apic_update_ppr(struct kvm_vcpu *vcpu);

-- 
Vitaly