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

Alexander Graf
2013-04-17T11:54:03Z

On 17.04.2013, at 13:50, Paolo Bonzini wrote:

> 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.

I'm actually reworking this bit right now. Every time we use __KVM in non-uapi headers or C files we're basically doing something seriously wrong. KVM code should only depend on CONFIG_HAVE_KVM_....

Alex