From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Move irq sharing information to irqchip level. Date: Sun, 23 Aug 2009 14:28:24 +0300 Message-ID: <4A9127D8.7060709@redhat.com> References: <20090813171910.GA4636@redhat.com> <4A892473.9010000@redhat.com> <20090821181638.GA30248@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:7992 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933496AbZHWL2Y (ORCPT ); Sun, 23 Aug 2009 07:28:24 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n7NBSQJr014459 for ; Sun, 23 Aug 2009 07:28:26 -0400 In-Reply-To: <20090821181638.GA30248@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 08/21/2009 09:16 PM, Gleb Natapov wrote: > On Mon, Aug 17, 2009 at 12:35:47PM +0300, Avi Kivity wrote: > >> On 08/13/2009 08:19 PM, Gleb Natapov wrote: >> >>> This removes assumptions that max GSIs is smaller than number of pins. >>> Sharing is tracked on pin level not GSI level. >>> >>> Signed-off-by: Gleb Natapov >>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >>> index b17d845..4c15bdd 100644 >>> --- a/arch/x86/include/asm/kvm_host.h >>> +++ b/arch/x86/include/asm/kvm_host.h >>> @@ -413,7 +413,6 @@ struct kvm_arch{ >>> gpa_t ept_identity_map_addr; >>> >>> unsigned long irq_sources_bitmap; >>> - unsigned long irq_states[KVM_IOAPIC_NUM_PINS]; >>> u64 vm_init_tsc; >>> }; >>> >>> diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h >>> index 7d6058a..c025a23 100644 >>> --- a/arch/x86/kvm/irq.h >>> +++ b/arch/x86/kvm/irq.h >>> @@ -71,6 +71,7 @@ struct kvm_pic { >>> int output; /* intr from master PIC */ >>> struct kvm_io_device dev; >>> void (*ack_notifier)(void *opaque, int irq); >>> + unsigned long irq_states[16]; >>> }; >>> >>> >> I think it's cleaner to move this into the routing table to avoid >> duplication. Currently there is no object which is unique to a gsi, >> but after your array patch, it can be placed next to the hlist_head. >> >> > The problem is that at this level it is not known if GSI is MSI or not. > Current code dials with this by arbitrary assuming that GSI is MSI if it > is greater then KVM_IOAPIC_NUM_PINS, but this is not enforced when > routing table is installed and userspace is free to use any GSI as MSI. > Of cause this problem would not exist if MSIs were not linked to GSIs > in the first place. > Does msi actually care about the states? I don't think it does. > The code duplication is very small (one line of code) and logically it > is more correct to track duplications at irqchip level. Theoretically > GSI may be shared when PIC is used and not shared when IOAPIC is used. > > No, it's more correct at the GSI level, since all interrupt lines connected to one GSI are shared. -- error compiling committee.c: too many arguments to function