From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 1/2] kvm: libkvm: support for irq routing Date: Thu, 15 Jan 2009 12:10:09 +0200 Message-ID: <496F0B81.30202@redhat.com> References: <1231941191-966-1-git-send-email-avi@redhat.com> <200901151420.39324.sheng@linux.intel.com> <496F090F.4050805@redhat.com> <200901151805.03447.sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Sheng Yang Return-path: Received: from mx2.redhat.com ([66.187.237.31]:59497 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756116AbZAOKKM (ORCPT ); Thu, 15 Jan 2009 05:10:12 -0500 In-Reply-To: <200901151805.03447.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: Sheng Yang wrote: > On Thursday 15 January 2009 17:59:43 Avi Kivity wrote: > >> Sheng Yang wrote: >> >>> On Thursday 15 January 2009 14:17:16 Sheng Yang wrote: >>> >>>> On Wednesday 14 January 2009 21:53:10 Avi Kivity wrote: >>>> >>>>> Signed-off-by: Avi Kivity >>>>> >>>> Do we need a lock for the table? >>>> >>> And kvm_add_irq_route/kvm_del_irq_route should be generic used, how about >>> transfer a kvm_irq_routing_entry as parameter? >>> >> These structures + selectors + unions are clumsy. For libkvm, I'd >> prefer adding kvm_add_msi() and kvm_del_msi(). >> > > But... > >> +int kvm_add_irq_route(kvm_context_t kvm, int gsi, int irqchip, int pin) >> +{ >> +#ifdef KVM_CAP_IRQ_ROUTING >> + struct kvm_irq_routing *z; >> + struct kvm_irq_routing_entry *e; >> + int n, size; >> + >> + if (kvm->irq_routes->nr == kvm->nr_allocated_irq_routes) { >> + n = kvm->nr_allocated_irq_routes * 2; >> + if (n < 64) >> + n = 64; >> + size = sizeof(struct kvm_irq_routing); >> + size += n * sizeof(*e); >> + z = realloc(kvm->irq_routes, size); >> + if (!z) >> + return -ENOMEM; >> + kvm->nr_allocated_irq_routes = n; >> + kvm->irq_routes = z; >> + } >> + n = kvm->irq_routes->nr++; >> + e = &kvm->irq_routes->entries[n]; >> + memset(e, 0, sizeof(*e)); >> + e->gsi = gsi; >> + e->type = KVM_IRQ_ROUTING_IRQCHIP; >> + e->flags = 0; >> + e->u.irqchip.irqchip = irqchip; >> + e->u.irqchip.pin = pin; >> + return 0; >> +#else >> + return -ENOSYS; >> +#endif >> +} >> > > Besides three lines, all can be reused... I don't see the reason for another > function... > > And the name here is irq_route, I suppose it should be generic used. Can be > core function which can be wrapped. > Yes, I guess we can rename it irqchip_route. When we add msis, both this function and the new msi function can share code. -- error compiling committee.c: too many arguments to function