Re: [PATCH 5/7] KVM: Extract generic irqchip logic into irqchip.c

Paolo Bonzini
2013-04-17T11:50:33Z

Il 16/04/2013 19:26, Alexander Graf ha scritto:
> The current irq_comm.c file contains pieces of code that are generic
> across different irqchip implementations, as well as code that is
> fully IOAPIC specific.
>
> Split the generic bits out into irqchip.c.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
> arch/x86/kvm/Makefile | 2 +-
> include/trace/events/kvm.h | 12 +++-
> virt/kvm/irq_comm.c | 117 ----------------------------------
> virt/kvm/irqchip.c | 152 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 163 insertions(+), 120 deletions(-)
> create mode 100644 virt/kvm/irqchip.c
>
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 04d3040..a797b8e 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -7,7 +7,7 @@ CFLAGS_vmx.o := -I.
>
> kvm-y += $(addprefix ../../../virt/kvm/, kvm_main.o ioapic.o \
> coalesced_mmio.o irq_comm.o eventfd.o \
> - assigned-dev.o)
> + assigned-dev.o irqchip.o)
> kvm-$(CONFIG_IOMMU_API) += $(addprefix ../../../virt/kvm/, iommu.o)
> kvm-$(CONFIG_KVM_ASYNC_PF) += $(addprefix ../../../virt/kvm/, async_pf.o)
>
> diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
> index 19911dd..2fe2d53 100644
> --- a/include/trace/events/kvm.h
> +++ b/include/trace/events/kvm.h
> @@ -37,7 +37,7 @@ TRACE_EVENT(kvm_userspace_exit,
> __entry->errno < 0 ? -__entry->errno : __entry->reason)
> );
>
> -#if defined(__KVM_HAVE_IRQ_LINE)
> +#if defined(__KVM_HAVE_IRQ_LINE) || defined(__KVM_HAVE_IRQCHIP)

I think it makes more sense to replace the #if altogether with
__KVM_HAVE_IRQCHIP. The only arches that have __KVM_HAVE_IRQ_LINE
already have __KVM_HAVE_IOAPIC, so it is ok to do that.

Paolo

> TRACE_EVENT(kvm_set_irq,
> TP_PROTO(unsigned int gsi, int level, int irq_source_id),
> TP_ARGS(gsi, level, irq_source_id),
> @@ -122,6 +122,10 @@ TRACE_EVENT(kvm_msi_set_irq,
> {KVM_IRQCHIP_PIC_SLAVE, "PIC slave"}, \
> {KVM_IRQCHIP_IOAPIC, "IOAPIC"}
>
> +#endif /* defined(__KVM_HAVE_IOAPIC) */
> +
> +#if defined(__KVM_HAVE_IRQCHIP)
> +
> TRACE_EVENT(kvm_ack_irq,
> TP_PROTO(unsigned int irqchip, unsigned int pin),
> TP_ARGS(irqchip, pin),
> @@ -136,14 +140,18 @@ TRACE_EVENT(kvm_ack_irq,
> __entry->pin = pin;
> ),
>
> +#ifdef kvm_irqchip
> TP_printk("irqchip %s pin %u",
> __print_symbolic(__entry->irqchip, kvm_irqchips),
> __entry->pin)
> +#else
> + TP_printk("irqchip %d pin %u", __entry->irqchip, __entry->pin)
> +#endif
> );
>
> +#endif /* defined(__KVM_HAVE_IRQCHIP) */
>
>
> -#endif /* defined(__KVM_HAVE_IOAPIC) */
>
> #define KVM_TRACE_MMIO_READ_UNSATISFIED 0
> #define KVM_TRACE_MMIO_READ 1
> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index f02659b..3d900ba 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -151,59 +151,6 @@ static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
> return -EWOULDBLOCK;
> }
>
> -int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
> -{
> - struct kvm_kernel_irq_routing_entry route;
> -
> - if (!irqchip_in_kernel(kvm) || msi->flags != 0)
> - return -EINVAL;
> -
> - route.msi.address_lo = msi->address_lo;
> - route.msi.address_hi = msi->address_hi;
> - route.msi.data = msi->data;
> -
> - return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1, false);
> -}
> -
> -/*
> - * Return value:
> - * < 0 Interrupt was ignored (masked or not delivered for other reasons)
> - * = 0 Interrupt was coalesced (previous irq is still pending)
> - * > 0 Number of CPUs interrupt was delivered to
> - */
> -int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level,
> - bool line_status)
> -{
> - struct kvm_kernel_irq_routing_entry *e, irq_set[KVM_NR_IRQCHIPS];
> - int ret = -1, i = 0;
> - struct kvm_irq_routing_table *irq_rt;
> -
> - trace_kvm_set_irq(irq, level, irq_source_id);
> -
> - /* Not possible to detect if the guest uses the PIC or the
> - * IOAPIC. So set the bit in both. The guest will ignore
> - * writes to the unused one.
> - */
> - rcu_read_lock();
> - irq_rt = rcu_dereference(kvm->irq_routing);
> - if (irq < irq_rt->nr_rt_entries)
> - hlist_for_each_entry(e, &irq_rt->map[irq], link)
> - irq_set[i++] = *e;
> - rcu_read_unlock();
> -
> - while(i--) {
> - int r;
> - r = irq_set[i].set(&irq_set[i], kvm, irq_source_id, level,
> - line_status);
> - if (r < 0)
> - continue;
> -
> - ret = r + ((ret < 0) ? 0 : ret);
> - }
> -
> - return ret;
> -}
> -
> /*
> * Deliver an IRQ in an atomic context if we can, or return a failure,
> * user can retry in a process context.
> @@ -241,62 +188,6 @@ int kvm_set_irq_inatomic(struct kvm *kvm, int irq_source_id, u32 irq, int level)
> return ret;
> }
>
> -bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin)
> -{
> - struct kvm_irq_ack_notifier *kian;
> - int gsi;
> -
> - rcu_read_lock();
> - gsi = rcu_dereference(kvm->irq_routing)->chip[irqchip][pin];
> - if (gsi != -1)
> - hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
> - link)
> - if (kian->gsi = gsi) {
> - rcu_read_unlock();
> - return true;
> - }
> -
> - rcu_read_unlock();
> -
> - return false;
> -}
> -EXPORT_SYMBOL_GPL(kvm_irq_has_notifier);
> -
> -void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin)
> -{
> - struct kvm_irq_ack_notifier *kian;
> - int gsi;
> -
> - trace_kvm_ack_irq(irqchip, pin);
> -
> - rcu_read_lock();
> - gsi = rcu_dereference(kvm->irq_routing)->chip[irqchip][pin];
> - if (gsi != -1)
> - hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
> - link)
> - if (kian->gsi = gsi)
> - kian->irq_acked(kian);
> - rcu_read_unlock();
> -}
> -
> -void kvm_register_irq_ack_notifier(struct kvm *kvm,
> - struct kvm_irq_ack_notifier *kian)
> -{
> - mutex_lock(&kvm->irq_lock);
> - hlist_add_head_rcu(&kian->link, &kvm->irq_ack_notifier_list);
> - mutex_unlock(&kvm->irq_lock);
> - kvm_ioapic_make_eoibitmap_request(kvm);
> -}
> -
> -void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
> - struct kvm_irq_ack_notifier *kian)
> -{
> - mutex_lock(&kvm->irq_lock);
> - hlist_del_init_rcu(&kian->link);
> - mutex_unlock(&kvm->irq_lock);
> - synchronize_rcu();
> - kvm_ioapic_make_eoibitmap_request(kvm);
> -}
>
> int kvm_request_irq_source_id(struct kvm *kvm)
> {
> @@ -381,13 +272,6 @@ void kvm_fire_mask_notifiers(struct kvm *kvm, unsigned irqchip, unsigned pin,
> rcu_read_unlock();
> }
>
> -void kvm_free_irq_routing(struct kvm *kvm)
> -{
> - /* Called only during vm destruction. Nobody can use the pointer
> - at this stage */
> - kfree(kvm->irq_routing);
> -}
> -
> static int setup_routing_entry(struct kvm_irq_routing_table *rt,
> struct kvm_kernel_irq_routing_entry *e,
> const struct kvm_irq_routing_entry *ue)
> @@ -451,7 +335,6 @@ out:
> return r;
> }
>
> -
> int kvm_set_irq_routing(struct kvm *kvm,
> const struct kvm_irq_routing_entry *ue,
> unsigned nr,
> diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
> new file mode 100644
> index 0000000..d8b06ed
> --- /dev/null
> +++ b/virt/kvm/irqchip.c
> @@ -0,0 +1,152 @@
> +/*
> + * irqchip.c: Common API for in kernel interrupt controllers
> + * Copyright (c) 2007, Intel Corporation.
> + * Copyright 2010 Red Hat, Inc. and/or its affiliates.
> + * Copyright (c) 2013, Alexander Graf <agraf@suse.de>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * This file is derived from virt/kvm/irq_comm.c.
> + *
> + * Authors:
> + * Yaozu (Eddie) Dong <Eddie.dong@intel.com>
> + * Alexander Graf <agraf@suse.de>
> + */
> +
> +#include <linux/kvm_host.h>
> +#include <linux/slab.h>
> +#include <linux/export.h>
> +#include <trace/events/kvm.h>
> +#include "irq.h"
> +
> +bool kvm_irq_has_notifier(struct kvm *kvm, unsigned irqchip, unsigned pin)
> +{
> + struct kvm_irq_ack_notifier *kian;
> + int gsi;
> +
> + rcu_read_lock();
> + gsi = rcu_dereference(kvm->irq_routing)->chip[irqchip][pin];
> + if (gsi != -1)
> + hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
> + link)
> + if (kian->gsi = gsi) {
> + rcu_read_unlock();
> + return true;
> + }
> +
> + rcu_read_unlock();
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(kvm_irq_has_notifier);
> +
> +void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin)
> +{
> + struct kvm_irq_ack_notifier *kian;
> + int gsi;
> +
> + trace_kvm_ack_irq(irqchip, pin);
> +
> + rcu_read_lock();
> + gsi = rcu_dereference(kvm->irq_routing)->chip[irqchip][pin];
> + if (gsi != -1)
> + hlist_for_each_entry_rcu(kian, &kvm->irq_ack_notifier_list,
> + link)
> + if (kian->gsi = gsi)
> + kian->irq_acked(kian);
> + rcu_read_unlock();
> +}
> +
> +void kvm_register_irq_ack_notifier(struct kvm *kvm,
> + struct kvm_irq_ack_notifier *kian)
> +{
> + mutex_lock(&kvm->irq_lock);
> + hlist_add_head_rcu(&kian->link, &kvm->irq_ack_notifier_list);
> + mutex_unlock(&kvm->irq_lock);
> +#ifdef __KVM_HAVE_IOAPIC
> + kvm_ioapic_make_eoibitmap_request(kvm);
> +#endif
> +}
> +
> +void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
> + struct kvm_irq_ack_notifier *kian)
> +{
> + mutex_lock(&kvm->irq_lock);
> + hlist_del_init_rcu(&kian->link);
> + mutex_unlock(&kvm->irq_lock);
> + synchronize_rcu();
> +#ifdef __KVM_HAVE_IOAPIC
> + kvm_ioapic_make_eoibitmap_request(kvm);
> +#endif
> +}
> +
> +int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi)
> +{
> + struct kvm_kernel_irq_routing_entry route;
> +
> + if (!irqchip_in_kernel(kvm) || msi->flags != 0)
> + return -EINVAL;
> +
> + route.msi.address_lo = msi->address_lo;
> + route.msi.address_hi = msi->address_hi;
> + route.msi.data = msi->data;
> +
> + return kvm_set_msi(&route, kvm, KVM_USERSPACE_IRQ_SOURCE_ID, 1, false);
> +}
> +
> +/*
> + * Return value:
> + * < 0 Interrupt was ignored (masked or not delivered for other reasons)
> + * = 0 Interrupt was coalesced (previous irq is still pending)
> + * > 0 Number of CPUs interrupt was delivered to
> + */
> +int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level,
> + bool line_status)
> +{
> + struct kvm_kernel_irq_routing_entry *e, irq_set[KVM_NR_IRQCHIPS];
> + int ret = -1, i = 0;
> + struct kvm_irq_routing_table *irq_rt;
> +
> + trace_kvm_set_irq(irq, level, irq_source_id);
> +
> + /* Not possible to detect if the guest uses the PIC or the
> + * IOAPIC. So set the bit in both. The guest will ignore
> + * writes to the unused one.
> + */
> + rcu_read_lock();
> + irq_rt = rcu_dereference(kvm->irq_routing);
> + if (irq < irq_rt->nr_rt_entries)
> + hlist_for_each_entry(e, &irq_rt->map[irq], link)
> + irq_set[i++] = *e;
> + rcu_read_unlock();
> +
> + while(i--) {
> + int r;
> + r = irq_set[i].set(&irq_set[i], kvm, irq_source_id, level,
> + line_status);
> + if (r < 0)
> + continue;
> +
> + ret = r + ((ret < 0) ? 0 : ret);
> + }
> +
> + return ret;
> +}
> +
> +void kvm_free_irq_routing(struct kvm *kvm)
> +{
> + /* Called only during vm destruction. Nobody can use the pointer
> + at this stage */
> + kfree(kvm->irq_routing);
> +}
>