From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/IO-APIC: refine EOI-ing of migrating level interrupts Date: Tue, 15 Nov 2011 13:19:31 +0000 Message-ID: <4EC266E3.50706@citrix.com> References: <4EC273B40200007800061145@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4EC273B40200007800061145@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com, Jan Beulich List-Id: xen-devel@lists.xenproject.org On 15/11/11 13:14, Jan Beulich wrote: > Rather than going through all IO-APICs and calling io_apic_eoi_vector() > for the vector in question, just use eoi_IO_APIC_irq(). > > This in turn allows to eliminate quite a bit of other code. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/io_apic.c > +++ b/xen/arch/x86/io_apic.c > @@ -69,10 +69,6 @@ int __read_mostly nr_ioapics; > > #define ioapic_has_eoi_reg(apic) (mp_ioapics[(apic)].mpc_apicver >= 0x20) > > -#define io_apic_eoi_vector(apic, vector) io_apic_eoi((apic), (vector), -1) > -#define io_apic_eoi_pin(apic, pin) io_apic_eoi((apic), -1, (pin)) > - > - > /* > * This is performance-critical, we want to do it O(1) > * > @@ -213,21 +209,18 @@ static void ioapic_write_entry( > spin_unlock_irqrestore(&ioapic_lock, flags); > } > > -/* EOI an IO-APIC entry. One of vector or pin may be -1, indicating that > - * it should be worked out using the other. This function expect that the > - * ioapic_lock is taken, and interrupts are disabled (or there is a good reason > - * not to), and that if both pin and vector are passed, that they refer to the > +/* EOI an IO-APIC entry. Vector may be -1, indicating that it should be > + * worked out using the pin. This function expects that the ioapic_lock is > + * being held, and interrupts are disabled (or there is a good reason not > + * to), and that if both pin and vector are passed, that they refer to the > * same redirection entry in the IO-APIC. */ > static void __io_apic_eoi(unsigned int apic, unsigned int vector, unsigned int pin) > { > - /* Ensure some useful information is passed in */ > - BUG_ON( (vector == -1 && pin == -1) ); > - > /* Prefer the use of the EOI register if available */ > if ( ioapic_has_eoi_reg(apic) ) > { > /* If vector is unknown, read it from the IO-APIC */ > - if ( vector == -1 ) > + if ( vector == IRQ_VECTOR_UNASSIGNED ) Quick style query: I consider IRQ_VECTOR_UNASSIGNED logically different from passing -1 in as a value for vector, even though they are the are the same value. Is it sensible to mix them? ~Andrew > vector = __ioapic_read_entry(apic, pin, TRUE).vector; > > *(IO_APIC_BASE(apic)+16) = vector; > @@ -239,42 +232,6 @@ static void __io_apic_eoi(unsigned int a > struct IO_APIC_route_entry entry; > bool_t need_to_unmask = 0; > > - /* If pin is unknown, search for it */ > - if ( pin == -1 ) > - { > - unsigned int p; > - for ( p = 0; p < nr_ioapic_entries[apic]; ++p ) > - { > - entry = __ioapic_read_entry(apic, p, TRUE); > - if ( entry.vector == vector ) > - { > - pin = p; > - /* break; */ > - > - /* Here should be a break out of the loop, but at the > - * Xen code doesn't actually prevent multiple IO-APIC > - * entries being assigned the same vector, so EOI all > - * pins which have the correct vector. > - * > - * Remove the following code when the above assertion > - * is fulfilled. */ > - __io_apic_eoi(apic, vector, p); > - } > - } > - > - /* If search fails, nothing to do */ > - > - /* if ( pin == -1 ) */ > - > - /* Because the loop wasn't broken out of (see comment above), > - * all relevant pins have been EOI, so we can always return. > - * > - * Re-instate the if statement above when the Xen logic has been > - * fixed.*/ > - > - return; > - } > - > entry = __ioapic_read_entry(apic, pin, TRUE); > > if ( ! entry.mask ) > @@ -301,17 +258,6 @@ static void __io_apic_eoi(unsigned int a > } > } > > -/* EOI an IO-APIC entry. One of vector or pin may be -1, indicating that > - * it should be worked out using the other. This function disables interrupts > - * and takes the ioapic_lock */ > -static void io_apic_eoi(unsigned int apic, unsigned int vector, unsigned int pin) > -{ > - unsigned int flags; > - spin_lock_irqsave(&ioapic_lock, flags); > - __io_apic_eoi(apic, vector, pin); > - spin_unlock_irqrestore(&ioapic_lock, flags); > -} > - > /* > * Saves all the IO-APIC RTE's > */ > @@ -1693,11 +1639,7 @@ static void end_level_ioapic_irq(struct > > /* Manually EOI the old vector if we are moving to the new */ > if ( vector && i != vector ) > - { > - int ioapic; > - for (ioapic = 0; ioapic < nr_ioapics; ioapic++) > - io_apic_eoi_vector(ioapic, i); > - } > + eoi_IO_APIC_irq(desc); > > v = apic_read(APIC_TMR + ((i & ~0x1f) >> 1)); > > > -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com