RE: [RFC]RE: [PATCH] kvm-ia64 irq assignment 1/2 kernel

Xu, Anthony
2008-06-12T16:08:32Z

Alexander Graf wrote:
> On Jun 10, 2008, at 12:57 AM, Xu, Anthony wrote:
>
>> Thanks for comments
>>
>> Basically we are on the same page, while I didn't find your patch
>> about irq assignment, can you post it in this thread again, thx?
>
> I'll attach it to this mail.
This patch is stilling use legacy LNKA~LNKD for ioapic,
As you said irq-pins > 15 are not used.

>
>> Below patch makes all PCI devices use level-trigger , active low
>> interrupt, it worked well when running linux guest, I didn't try
>> windows guest yet.
>> (didn't have windows image in hand)
>>
>> Please comment!
>>
>> If this is acceptabled, we can figure out how to use IOAPIC in kvm/
>> ia32 based on this. Which will reduce irq sharing dramatically.
>>
>>
>> Thanks,
>> Anthony
>>
>>
>>
>> diff --git a/bios/acpi-dsdt.dsl b/bios/acpi-dsdt.dsl
>> index 21fc76a..4b5e824 100755
>> --- a/bios/acpi-dsdt.dsl
>> +++ b/bios/acpi-dsdt.dsl
>> @@ -974,7 +974,7 @@ DefinitionBlock (
>> Name(_HID, EISAID("PNP0C0F")) // PCI interrupt
>> link Name(_UID, 1)
>> Name(_PRS, ResourceTemplate(){
>> - Interrupt (, Level, ActiveHigh, Shared)
>> + Interrupt (, Level, ActiveLow, Shared)
>
> This looks pretty much correct to me ;-). You might also want to add
> the GSI functionality I have in my patch. The only thing we have not
> discussed so far is, how do interrupts get routed when _PIC is not set
> to 1, aka the "boot case"?

Here Avi is correct,
PIC only support activehigh level-triggered interrupt. From spec, PCI
device uses activelow level-triggered interrupt.
I guess it is interrupt Link to reverse it.

>
>>
>> { 5, 10, 11 }
>> })
>> Method (_STA, 0, NotSerialized)
>
> [...snip...]
>
>>
>> diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
>> index a23a466..df0ea33 100644
>> --- a/qemu/hw/pci.c
>> +++ b/qemu/hw/pci.c
>> @@ -548,7 +548,7 @@ static void pci_set_irq(void *opaque, int
>> irq_num, int level) pci_dev = bus->parent_dev;
>> }
>> bus->irq_count[irq_num] += change;
>> - bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num]
>> != 0); + bus->set_irq(bus->irq_opaque, irq_num, !(bus-
>>> irq_count[irq_num] !=
>> 0));
>> }
>
> I don't think this is the right place to do it. Probably it would be a
> better idea to have either the APIC emulation know that the levels are
> reversed or make every device trigger ActiveLow interrupts.

Maybe it not a right place:-)
Making every device trigger activelow interrupt introduce a lot of
modifications, it's last resort.
APIC emulation is inside kernel now, it is not a good idea to introduce
qemu-special piece of code into kernel as Avi mentioned.

Thanks.