From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Re: 4.0/4.1 requests - IO-APIC EOI v4 [RFC] Date: Fri, 9 Sep 2011 17:50:50 +0100 Message-ID: <4E6A43EA.8060700@citrix.com> References: <4E689D9A.1020405@citrix.com> <4E68C6C10200007800055485@nat28.tlf.novell.com> <4E68C09B.8050409@citrix.com> <4E68E1D30200007800055501@nat28.tlf.novell.com> <4E68C9A9.8090707@citrix.com> <4E6A2B7F.6060006@citrix.com> <4E6A4F66020000780005589B@nat28.tlf.novell.com> <4E6A36EB.6000802@citrix.com> <4E6A54DB02000078000558B6@nat28.tlf.novell.com> <4E6A3D5B.5010104@citrix.com> <4E6A430B.30308@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------080404060101050305050700" Return-path: In-Reply-To: <4E6A430B.30308@citrix.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 List-Id: xen-devel@lists.xenproject.org --------------080404060101050305050700 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 09/09/11 17:47, Andrew Cooper wrote: > On 09/09/11 17:22, Andrew Cooper wrote: >> ---SNIP--- >> >> Version 3. Applied Jan's suggestions, and extended some of the comments. >> > V4 - get the BUG_ON logic correct (oops). > > I have now done a few reboot tests and a kexec test with this patch. > > Are there any other tests you would suggest, or shall I formally submit > the patch for unstable and backporting? > Remember to qrefresh this time. -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com --------------080404060101050305050700 Content-Type: text/x-patch; name="IO-APIC-eoi.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="IO-APIC-eoi.patch" diff -r 0268e7380953 xen/arch/x86/io_apic.c --- a/xen/arch/x86/io_apic.c Mon Sep 05 15:10:28 2011 +0100 +++ b/xen/arch/x86/io_apic.c Fri Sep 09 17:48:33 2011 +0100 @@ -68,6 +68,16 @@ int __read_mostly nr_ioapics; #define MAX_PLUS_SHARED_IRQS nr_irqs_gsi #define PIN_MAP_SIZE (MAX_PLUS_SHARED_IRQS + nr_irqs_gsi) + +#define ioapic_has_eoi_reg(apic) (mp_ioapics[(apic)].mpc_apicver >= 0x20) + +static void __io_apic_eoi(unsigned int apic, unsigned int vector, unsigned int pin); +static void io_apic_eoi(unsigned int apic, unsigned int vector, unsigned int pin); + +#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) * @@ -357,7 +367,7 @@ static void __eoi_IO_APIC_irq(unsigned i pin = entry->pin; if (pin == -1) break; - io_apic_eoi(entry->apic, vector); + __io_apic_eoi(entry->apic, vector, pin); if (!entry->next) break; entry = irq_2_pin + entry->next; @@ -397,18 +407,7 @@ static void clear_IO_APIC_pin(unsigned i entry.trigger = 1; __ioapic_write_entry(apic, pin, TRUE, entry); } - if (mp_ioapics[apic].mpc_apicver >= 0x20) - io_apic_eoi(apic, entry.vector); - else { - /* - * Mechanism by which we clear remoteIRR in this case is by - * changing the trigger mode to edge and back to level. - */ - entry.trigger = 0; - __ioapic_write_entry(apic, pin, TRUE, entry); - entry.trigger = 1; - __ioapic_write_entry(apic, pin, TRUE, entry); - } + __io_apic_eoi(apic, entry.vector, pin); } /* @@ -1750,7 +1749,7 @@ static void end_level_ioapic_irq (unsign { int ioapic; for (ioapic = 0; ioapic < nr_ioapics; ioapic++) - io_apic_eoi(ioapic, i); + io_apic_eoi_vector(ioapic, i); } v = apic_read(APIC_TMR + ((i & ~0x1f) >> 1)); @@ -2622,3 +2621,124 @@ void __init init_ioapic_mappings(void) printk(XENLOG_INFO "IRQ limits: %u GSI, %u MSI/MSI-X\n", nr_irqs_gsi, nr_irqs - nr_irqs_gsi); } + +/* 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); +} + +/* 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 + * 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 ) + vector = __ioapic_read_entry(apic, pin, TRUE).vector; + + *(IO_APIC_BASE(apic)+16) = vector; + } + else + { + /* Else fake an EOI by switching to edge triggered mode + * and back */ + 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_registers[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. */ + + if ( ! entry.mask ) + { + /* If entry is not currently masked, mask it and make + * a note to unmask it later */ + entry.mask = 1; + __ioapic_write_entry(apic, pin, TRUE, entry); + need_to_unmask = 1; + } + + /* Flip the trigger mode to edge and back */ + entry.trigger = 0; + __ioapic_write_entry(apic, pin, TRUE, entry); + entry.trigger = 1; + __ioapic_write_entry(apic, pin, TRUE, entry); + + if ( need_to_unmask ) + { + /* Unmask if neccesary */ + entry.mask = 0; + __ioapic_write_entry(apic, pin, TRUE, entry); + } + + } + } + + /* 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 ) + { + /* If entry is not currently masked, mask it and make + * a note to unmask it later */ + entry.mask = 1; + __ioapic_write_entry(apic, pin, TRUE, entry); + need_to_unmask = 1; + } + + /* Flip the trigger mode to edge and back */ + entry.trigger = 0; + __ioapic_write_entry(apic, pin, TRUE, entry); + entry.trigger = 1; + __ioapic_write_entry(apic, pin, TRUE, entry); + + if ( need_to_unmask ) + { + /* Unmask if neccesary */ + entry.mask = 0; + __ioapic_write_entry(apic, pin, TRUE, entry); + } + } +} diff -r 0268e7380953 xen/include/asm-x86/io_apic.h --- a/xen/include/asm-x86/io_apic.h Mon Sep 05 15:10:28 2011 +0100 +++ b/xen/include/asm-x86/io_apic.h Fri Sep 09 17:48:33 2011 +0100 @@ -157,11 +157,6 @@ static inline void io_apic_write(unsigne __io_apic_write(apic, reg, value); } -static inline void io_apic_eoi(unsigned int apic, unsigned int vector) -{ - *(IO_APIC_BASE(apic)+16) = vector; -} - /* * Re-write a value: to be used for read-modify-write * cycles where the read already set up the index register. --------------080404060101050305050700 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------080404060101050305050700--