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

Xu, Anthony
2008-06-10T07:57:55Z

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?
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)
{ 5, 10, 11 }
})
Method (_STA, 0, NotSerialized)
@@ -1019,7 +1019,7 @@ DefinitionBlock (
Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link
Name(_UID, 2)
Name(_PRS, ResourceTemplate(){
- Interrupt (, Level, ActiveHigh, Shared)
+ Interrupt (, Level, ActiveLow, Shared)
{ 5, 10, 11 }
})
Method (_STA, 0, NotSerialized)
@@ -1064,7 +1064,7 @@ DefinitionBlock (
Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link
Name(_UID, 3)
Name(_PRS, ResourceTemplate(){
- Interrupt (, Level, ActiveHigh, Shared)
+ Interrupt (, Level, ActiveLow, Shared)
{ 5, 10, 11 }
})
Method (_STA, 0, NotSerialized)
@@ -1109,7 +1109,7 @@ DefinitionBlock (
Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link
Name(_UID, 4)
Name(_PRS, ResourceTemplate(){
- Interrupt (, Level, ActiveHigh, Shared)
+ Interrupt (, Level, ActiveLow, Shared)
{ 5, 10, 11 }
})
Method (_STA, 0, NotSerialized)
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));
}

/***********************************************************/

Alexander Graf wrote:
> On Jun 10, 2008, at 8:33 AM, Xu, Anthony wrote:
>
>> Avi Kivity wrote:
>>>
>>> I suggest modifying the firmware to report the interrupts as active
>>> high. Since Xen does not emulate polarity, the change will not
>>> affect it and the firmware can continue to be shared. I'd also
>>> recommend fixing Xen to emulate the polarity correctly, if possible.
>>
>> Thanks for your comments
>> I agree modifying common code is not a good method.
>>
>> While your suggestion seems be infeasible too.
>> According to acpi spec, only irq <=15 can be configured, such as
>> trigger level, polarity.
>> For irq >15 , means connect to IOAPIC directly, it can't be
>> configured, it must be level triger, active low.
>>
>> I can't find any mechanism in firmware to configure irqs (> 15).
>> Please enlighten me if you have.
>
>
> You can change the defaults on that IOAPIC-wise. IIRC this was in the
> MADT, but I'd have to check.
>
>> From some experimental, Firmware both in IA64 and IA32 doesn't
>> program IOAPIC, It's Guest OS to program.
>> Guest OS gets PRT first,
>> If the PCI device connected to IOAPIC pin directly, looks like below
>> /* Device 1, INTA - INTD */
>> Package(){0x0001ffff, 0, 0, 20},
>> Package(){0x0001ffff, 1, 0, 21},
>> Package(){0x0001ffff, 2, 0, 22},
>> Package(){0x0001ffff, 3, 0, 23},
>> Guest OS configures this IOAPIC pin with level triger, active low
>> unconditionally.
>
> Yes, the Guest OS reads the PRT and configures the IOAPIC and LAPIC
> accordingly.
>
>> If the PCI device connected to IOAPIC pin through interrupt link, the
>> irq attribute(level, polarity) is decided by interrupt link
>> attribute, Below is the interrupt link attribute in kvm/ia32
>> Device(LNKA){
>> Name(_HID, EISAID("PNP0C0F")) // PCI interrupt
>> link Name(_UID, 1) Name(_PRS,
>> ResourceTemplate(){ Interrupt (, Level,
>> ActiveHigh, Shared) { 5, 10, 11 }
>> })
>> It's defined as level trigger, activehigh, that's the reason why pci
>> device worked well in kvm/ia32.
>
> This is the what I call "Legacy" way of handling interrupt lanes.
> Usually nowadays you simply connect devices magically to IOAPIC pins,
> which is exactly what you showed in the previous section. This "new"
> behavior is usually activated when the OS calls the _PIC function in
> the DSDT with a parameter != 0.
>
>> I think below scheme is feasible,
>> 1. all PCI devices in Qemu uses level trigger, active low interrupt.
>> (not include ide, even though it is a PCI device, it uses legacy
>> interrupt mechanism)
>
> This is what the PCI spec requires anyway. The way it's done right now
> looks wrong to me.
>
>> 2. in Guest Firmware, all PCI devices' interrupts are configured as
>> level trigger, active low for KVM/IA32 Guest firmware, just a
>> little modifications Name(_PRS, ResourceTemplate(){
>> Interrupt (, Level, ActiveHigh, Shared)-->
>> Interrupt (, Level, ActiveLow, Shared)
>
> While at it it might be a good idea to switch to the direct IOAPIC-
> mapping approach. I sent a patch that did this on the kvm list some
> time ago. Please consider reading that and tell me what you think of
> it. It apparently broke Windows, but that might be simply an issue of
> the wrong ActiveHigh/ActiveLow configuration.
>
>> There are some modifications in Qemu, But I think it's a worthwhile,
>> it's a thoroghly solution both for KVM/IA32 and KVM/IA64.
>
> In essence this sounds great to me! I would love if we could talk
> about this on the KVM Forum.
>
> Thank you,
>
> Alex