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

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

[Xu, Anthony]

In kvm-ia64, we use the same guest firmware (GFW)as in Xen, 
GFW uses PRT to present PCI interrupt routing, all PCI devices'
interrupt pins
connect to IOAPIC, which doesn't match with kvm-ia64 Qemu PCI interrupt
routing.

This patch modify Qemu PCI interupt routing code to match with GFW, 
Then PCI devices in qemu can work in kvm-ia64, for exmaple, NIC


Signed-off-by: Anthony Xu < anthony.xu@intel.com >


diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 99a1736..80c116c 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -272,7 +272,11 @@ void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic,
int irq, int level)

        if (irq >= 0 && irq < IOAPIC_NUM_PINS) {
                entry = ioapic->redirtbl[irq];
-               level ^= entry.fields.polarity;
+// polarity of all devices in qemu is active high
+//  regardless of ioapic setting
+
+//             level ^= entry.fields.polarity;
+
                if (!level)
                        ioapic->irr &= ~mask;
                else {

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

[Avi Kivity]

Xu, Anthony wrote:
> In kvm-ia64, we use the same guest firmware (GFW)as in Xen, 
> GFW uses PRT to present PCI interrupt routing, all PCI devices'
> interrupt pins
> connect to IOAPIC, which doesn't match with kvm-ia64 Qemu PCI interrupt
> routing.
>
> This patch modify Qemu PCI interupt routing code to match with GFW, 
> Then PCI devices in qemu can work in kvm-ia64, for exmaple, NIC
>
>
> Signed-off-by: Anthony Xu < anthony.xu@intel.com >
>
>
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 99a1736..80c116c 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -272,7 +272,11 @@ void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic,
> int irq, int level)
>
>         if (irq >= 0 && irq < IOAPIC_NUM_PINS) {
>                 entry = ioapic->redirtbl[irq];
> -               level ^= entry.fields.polarity;
> +// polarity of all devices in qemu is active high
> +//  regardless of ioapic setting
> +
> +//             level ^= entry.fields.polarity;
> +
>   

There are two errors in this patch:

- kvm is not there just for qemu; it should be possible to use kvm with 
some other userspace, which would assume that kvm correctly emulates 
ioapic polarity
- you are modifying shared code and so affect x86 as well

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.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

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

[Alexander Graf]

Avi Kivity wrote:
> Xu, Anthony wrote:
>> In kvm-ia64, we use the same guest firmware (GFW)as in Xen, GFW uses 
>> PRT to present PCI interrupt routing, all PCI devices'
>> interrupt pins
>> connect to IOAPIC, which doesn't match with kvm-ia64 Qemu PCI interrupt
>> routing.
>>
>> This patch modify Qemu PCI interupt routing code to match with GFW, 
>> Then PCI devices in qemu can work in kvm-ia64, for exmaple, NIC
>>
>>
>> Signed-off-by: Anthony Xu < anthony.xu@intel.com >
>>
>>
>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>> index 99a1736..80c116c 100644
>> --- a/virt/kvm/ioapic.c
>> +++ b/virt/kvm/ioapic.c
>> @@ -272,7 +272,11 @@ void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic,
>> int irq, int level)
>>
>>         if (irq >= 0 && irq < IOAPIC_NUM_PINS) {
>>                 entry = ioapic->redirtbl[irq];
>> -               level ^= entry.fields.polarity;
>> +// polarity of all devices in qemu is active high
>> +//  regardless of ioapic setting
>> +
>> +//             level ^= entry.fields.polarity;
>> +
>>   
>
> There are two errors in this patch:
>
> - kvm is not there just for qemu; it should be possible to use kvm 
> with some other userspace, which would assume that kvm correctly 
> emulates ioapic polarity
> - you are modifying shared code and so affect x86 as well

Apparently this is broken on x86 too. I was just trying this patch with 
Mac OS X as target and magically the in-kernel APIC starts working, so I 
guess something is going wrong already here.
Btw, according to the ACPI tables, all PCI interrupts are currently 
defined Active-Low.

Alex

>
> 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.
>

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

[Alexander Graf]

Alexander Graf wrote:
> Avi Kivity wrote:
>> Xu, Anthony wrote:
>>> In kvm-ia64, we use the same guest firmware (GFW)as in Xen, GFW uses 
>>> PRT to present PCI interrupt routing, all PCI devices'
>>> interrupt pins
>>> connect to IOAPIC, which doesn't match with kvm-ia64 Qemu PCI interrupt
>>> routing.
>>>
>>> This patch modify Qemu PCI interupt routing code to match with GFW, 
>>> Then PCI devices in qemu can work in kvm-ia64, for exmaple, NIC
>>>
>>>
>>> Signed-off-by: Anthony Xu < anthony.xu@intel.com >
>>>
>>>
>>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
>>> index 99a1736..80c116c 100644
>>> --- a/virt/kvm/ioapic.c
>>> +++ b/virt/kvm/ioapic.c
>>> @@ -272,7 +272,11 @@ void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic,
>>> int irq, int level)
>>>
>>>         if (irq >= 0 && irq < IOAPIC_NUM_PINS) {
>>>                 entry = ioapic->redirtbl[irq];
>>> -               level ^= entry.fields.polarity;
>>> +// polarity of all devices in qemu is active high
>>> +//  regardless of ioapic setting
>>> +
>>> +//             level ^= entry.fields.polarity;
>>> +
>>>   
>>
>> There are two errors in this patch:
>>
>> - kvm is not there just for qemu; it should be possible to use kvm 
>> with some other userspace, which would assume that kvm correctly 
>> emulates ioapic polarity
>> - you are modifying shared code and so affect x86 as well
>
> Apparently this is broken on x86 too. I was just trying this patch 
> with Mac OS X as target and magically the in-kernel APIC starts 
> working, so I guess something is going wrong already here.
> Btw, according to the ACPI tables, all PCI interrupts are currently 
> defined Active-Low.

Sorry, ActiveHigh that is. Nevertheless I am having trouble with this 
since the very first time I used osx inside KVM. Does PCI allow Active

 > Interrupt (, Level, ActiveHigh, Shared)

According to the PCI 3.0 Spec, "Interrupts on PCI are optional and 
defined as 'level sensitive,' asserted low (negative true)".

Alex

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

[Xu, Anthony]

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 <\x15 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.

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.

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.

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)
	
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)


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.

- Thanks
Anthony

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

[Alexander Graf]

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 <\x15 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

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

[Avi Kivity]

Alexander Graf wrote:
>>
>> Apparently this is broken on x86 too. I was just trying this patch 
>> with Mac OS X as target and magically the in-kernel APIC starts 
>> working, so I guess something is going wrong already here.
>> Btw, according to the ACPI tables, all PCI interrupts are currently 
>> defined Active-Low.
>

So there's something else wrong.

> Sorry, ActiveHigh that is. Nevertheless I am having trouble with this 
> since the very first time I used osx inside KVM. Does PCI allow Active
>
> > Interrupt (, Level, ActiveHigh, Shared)
>
> According to the PCI 3.0 Spec, "Interrupts on PCI are optional and 
> defined as 'level sensitive,' asserted low (negative true)".

The pci interrupts are active low, but they are converted to active high 
by the chipset qemu emulates, so active high is correct.  Does OS X boot 
from the qemu bios or something else?  If the latter, it may need 
adjustment.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

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

[Avi Kivity]

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 <\x15 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.
>   

Yes.

> I can't find any mechanism in firmware to configure irqs (> 15). Please
> enlighten me if you have.
>
>   

Okay. In any case we should emulate hardware as closely as possible to 
reality, so mu suggestion wasn't a good one.

> 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)
> 	
> 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)
>
>
> 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.
>
>   

I agree. Note that the piix chipset used on x86 inverts the pci 
interrupts again so they become active high. But for ioapic mode we may 
be able to use active low interrupts.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.