From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cornelia Huck Subject: Re: [Qemu-devel] [PATCH] KVM: Fix GSI number space limit Date: Fri, 6 Jun 2014 15:41:36 +0200 Message-ID: <20140606154136.60e68c8a.cornelia.huck@de.ibm.com> References: <1402058765-48921-1-git-send-email-agraf@suse.de> <20140606151200.660e3072.cornelia.huck@de.ibm.com> <5391BF0A.3000509@suse.de> <20140606152331.1b72245d.cornelia.huck@de.ibm.com> <5391C1ED.1010405@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, kvm@vger.kernel.org To: Alexander Graf Return-path: Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:47686 "EHLO e06smtp10.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750937AbaFFNls (ORCPT ); Fri, 6 Jun 2014 09:41:48 -0400 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 6 Jun 2014 14:41:46 +0100 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 81CDB1B08078 for ; Fri, 6 Jun 2014 14:42:07 +0100 (BST) Received: from d06av12.portsmouth.uk.ibm.com (d06av12.portsmouth.uk.ibm.com [9.149.37.247]) by b06cxnps4074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s56Dfh5Q26017814 for ; Fri, 6 Jun 2014 13:41:43 GMT Received: from d06av12.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av12.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s56DfeIF030095 for ; Fri, 6 Jun 2014 07:41:42 -0600 In-Reply-To: <5391C1ED.1010405@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, 06 Jun 2014 15:28:13 +0200 Alexander Graf wrote: > > On 06.06.14 15:23, Cornelia Huck wrote: > > On Fri, 06 Jun 2014 15:15:54 +0200 > > Alexander Graf wrote: > > > >> On 06.06.14 15:12, Cornelia Huck wrote: > >>> On Fri, 6 Jun 2014 14:46:05 +0200 > >>> Alexander Graf wrote: > >>> > >>>> KVM tells us the number of GSIs it can handle inside the kernel. That value is > >>>> basically KVM_MAX_IRQ_ROUTES. However when we try to set the GSI mapping table, > >>>> it checks for > >>>> > >>>> r = -EINVAL; > >>>> if (routing.nr >= KVM_MAX_IRQ_ROUTES) > >>>> goto out; > >>>> > >>>> erroring out even when we're only using all of the GSIs. To make sure we never > >>>> hit that limit, let's reduce the number of GSIs we get from KVM by one. > >>>> > >>>> Signed-off-by: Alexander Graf > >>>> --- > >>>> kvm-all.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/kvm-all.c b/kvm-all.c > >>>> index 4e19eff..56a251b 100644 > >>>> --- a/kvm-all.c > >>>> +++ b/kvm-all.c > >>>> @@ -938,7 +938,7 @@ void kvm_init_irq_routing(KVMState *s) > >>>> { > >>>> int gsi_count, i; > >>>> > >>>> - gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING); > >>>> + gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING) - 1; > >>>> if (gsi_count > 0) { > >>>> unsigned int gsi_bits, i; > >>>> > >>> But gsi_count is already marked as used further down in this function, > >>> isn't it? Confused. > >> gsi_bits = ALIGN(gsi_count, 32); > >> [...] > >> for (i = gsi_count; i < gsi_bits; i++) { > >> set_gsi(s, i); > >> } > >> > >> So if you take gsi_count = 1024, what happens? > >> > >> gsi_count = 1024; > >> gsi_bits = 1024; > >> for (i = 1024; i < 1024; i++) { > >> set_gsi(s, i); > >> } > >> > >> At least in my world of C that loop never runs, no? > >> > > But then kvm_irqchip_get_virq() should never return 1024, shouldn't it? > > Right, because it returns the virq number which starts at 0. However, to > describe all virqs from [0..1023] we need 1024 entries which the kernel > errors out on. Ah... that's kvm_irq_routing::nr and not kvm_irq_routing_entry::gsi, so it's basically a kernel misfeature we need to work around. > > > > > And: > > > > void kvm_irqchip_add_irq_route(KVMState *s, int irq, int irqchip, int pin) > > { > > [...] > > assert(pin < s->gsi_count); > > > > would trigger too early with your change, wouldn't it? > > Not really - with my change we only support 1023 virqs. So the biggest > virq number is 1022 which is < 1023 :). > > > Sorry for describing this with actual numbers - I find it easier to > grasp when I think in concrete numbers here - this stuff is just really > spinning my head :). And on top of that, it's Friday :) But yes, makes sense now. Acked-by: Cornelia Huck