Re: [PATCH 1/2] kvm: libkvm: support for irq routing

Avi Kivity
2009-01-15T10:10:12Z

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 <avi@redhat.com>
>>>>>
>>>> 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