Re: [PATCH 1/2] kvm tools: Fix IRQ assignments

[Cyrill Gorcunov <gorcunov@gmail.com>]

On 05/06/2011 12:04 PM, Pekka Enberg wrote:
> On Fri, May 6, 2011 at 11:01 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
>> On Fri, 2011-05-06 at 10:43 +0300, Pekka Enberg wrote:
>>> On Fri, May 6, 2011 at 10:36 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
>>>> afaik, we need to have same IRQ pins for devices which may have
>>>> different IRQ lines (All virtio-blk share same IRQ pin but different
>>>> line, I assume it'll be same with different virtio devices).
>>>>
>>>> So mptable has to manage association between device type and the
>>>> corresponding IRQ pin/line assignment, So dynamic assignment will be
>>>> more of a call with device type and return IRQ pin + line - which makes
>>>> it more complex than just allocating next free IRQ line.
>>>
>>> Well then make kvm__request_irq() return a pointer to struct irq and
>>> have a hard-coded array of IRQ pin + line pairs in irq.c, for example.
>>> That'll reduce complexity in the virtio drivers and it'll serve as a
>>> starting point for proper IRQ allocator.
>>
>> The big problem with dynamic allocations is having mptable register IRQ
>> sources properly, So I went ahead and deleted IRQ source definitions
>> from mptable.c and noticed that everything still works.
>>
>> I've spoke with Cyrill and neither him or myself were too sure if it's
>> ok to do that now and if so, would it still be correct once SMP gets
>> added.
>>
>> So here is my question: Would it be ok to drop IRQ source definitions
>> from mptable?
> 
> Now why do we need to do that? Shouldn't we pass the "device tree" to
> mptable.c and build the IRQ mappings based on that? Isn't that exactly
> what we do right now with the hard-coded enums?
> 
>                        Pekka

  Yes, that is what we do, and it seems to be the "right" way how sane bios
should behave, same time (i might be wrong since I didn't checked this
part of kernel sources) the kernel might scan memory for pci configs and
read pis/irq directly from there, I think this way kernel workaround broken
bioses but now sure if we should rely on this feature.

-- 
Thanks,
  Cyrill