Jimi Xenidis
2006-Apr-19 20:58 UTC
[Xen-devel] [rfc][patch] use of IRQ_LEVEL to remove strcmp()s
Ok this is blind, but it builds, so please take it more as a suggestion, There are a sequence of strcmp()s in irq.c to detect if an IO-APIC is level or edge triggered. Perhaps (borrowing form Linux) with the use of IRQ_LEVEL, we can detect this more efficiently? Side Note: I believe that "ioapic_ack_new" is the same as IRQ_PER_CPU. Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com> -- diff -r 72f9c751d3ea xen/include/xen/irq.h --- a/xen/include/xen/irq.h Wed Apr 19 18:32:20 2006 +0100 +++ b/xen/include/xen/irq.h Wed Apr 19 15:44:50 2006 -0400 @@ -22,6 +22,7 @@ struct irqaction #define IRQ_PENDING 4 /* IRQ pending - replay on enable */ #define IRQ_REPLAY 8 /* IRQ has been replayed but not acked yet */ #define IRQ_GUEST 16 /* IRQ is handled by guest OS(es) */ +#define IRQ_LEVEL 64 /* IRQ level triggered */ #define IRQ_PER_CPU 256 /* IRQ is per CPU */ /* diff -r 72f9c751d3ea xen/arch/x86/io_apic.c --- a/xen/arch/x86/io_apic.c Wed Apr 19 18:32:20 2006 +0100 +++ b/xen/arch/x86/io_apic.c Wed Apr 19 15:44:50 2006 -0400 @@ -701,9 +701,10 @@ static inline void ioapic_register_intr( static inline void ioapic_register_intr(int irq, int vector, unsigned long trigger) { if ((trigger == IOAPIC_AUTO && IO_APIC_irq_trigger(irq)) || - trigger == IOAPIC_LEVEL) - irq_desc[vector].handler = &ioapic_level_type; - else + trigger == IOAPIC_LEVEL) { + irq_desc[vector].handler = &ioapic_level_type; + irq_desc[vector].status = IRQ_LEVEL; + } else irq_desc[vector].handler = &ioapic_edge_type; } @@ -2044,8 +2045,11 @@ int ioapic_guest_write(unsigned long phy } /* Set the correct irq-handling type. */ - irq_desc[IO_APIC_VECTOR(new_irq)].handler = new_rte.trigger ? - &ioapic_level_type: &ioapic_edge_type; + if (new_rte.trigger) { + irq_desc[IO_APIC_VECTOR(new_irq)].handler = &ioapic_level_type; + irq_desc[IO_APIC_VECTOR(new_irq)].status = IRQ_LEVEL; + } else + irq_desc[IO_APIC_VECTOR(new_irq)].handler = &ioapic_edge_type; if ( old_irq != new_irq ) add_pin_to_irq(new_irq, apic, pin); diff -r 72f9c751d3ea xen/arch/x86/irq.c --- a/xen/arch/x86/irq.c Wed Apr 19 18:32:20 2006 +0100 +++ b/xen/arch/x86/irq.c Wed Apr 19 15:44:50 2006 -0400 @@ -356,26 +356,13 @@ int pirq_acktype(int irq) desc = &irq_desc[vector]; - /* - * Edge-triggered IO-APIC interrupts need no final acknowledgement: - * we ACK early during interrupt processing. - */ - if ( !strcmp(desc->handler->typename, "IO-APIC-edge") ) - return ACKTYPE_NONE; - - /* Legacy PIC interrupts can be acknowledged from any CPU. */ - if ( !strcmp(desc->handler->typename, "XT-PIC") ) - return ACKTYPE_UNMASK; - - /* - * Level-triggered IO-APIC interrupts need to be acknowledged on the CPU - * on which they were received. This is because we tickle the LAPIC to EOI. - */ - if ( !strcmp(desc->handler->typename, "IO-APIC-level") ) - return ioapic_ack_new ? ACKTYPE_EOI : ACKTYPE_UNMASK; - - BUG(); - return 0; + if (desc->status & IRQ_LEVEL) { + if (desc->status & IRQ_PER_CPU || ioapic_ack_new) + return ACKTYPE_EOI; + else + return ACKTYPE_UNMASK; + } + return ACKTYPE_NONE; } int pirq_guest_bind(struct vcpu *v, int irq, int will_share) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-Apr-19 21:11 UTC
Re: [Xen-devel] [rfc][patch] use of IRQ_LEVEL to remove strcmp()s
On 19 Apr 2006, at 21:58, Jimi Xenidis wrote:> Ok this is blind, but it builds, so please take it more as a > suggestion, > There are a sequence of strcmp()s in irq.c to detect if an IO-APIC is > level or edge triggered. > Perhaps (borrowing form Linux) with the use of IRQ_LEVEL, we can > detect this more efficiently?Given the set of hw_interrupt_types we support right now this generalised logic appears to work, but in fact we really are testing for a specific PIC configuration here. For example, if we added a new hw_interrupt_type that was also level-triggered but which could be masked quickly and cleanly then it would have ACKTYPE_UNMASK not ACKTYPE_EOI. So the generalisation you are suggesting (to check for IRQ_LEVEL) doesn''t really work. The strcmps aren''t really a problem -- they''re not on a critical path, and we explicitly bug if we do not match on any of them so we should be resilient to addition of new PIC types or renamings of existing PIC types. I think the current code has the additional benefit of ''obviousness''.> Side Note: I believe that "ioapic_ack_new" is the same as IRQ_PER_CPU.Xen/x86 does not use IRQ_PER_CPU at all. ioapic_ack_new simply determines whether we handle level-triggered IO-APIC interrupts by mask-eoi-process-unmask or by process-eoi. The new method is the latter because it turns out that many IO-APICs do not mask cleanly (they create spurious aliased interrupts). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel