From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86: clean up __io_apic_eoi() Date: Fri, 11 Nov 2011 16:49:23 +0000 Message-ID: <4EBD5213.3030407@citrix.com> References: <4EBD5639020000780006077C@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: <4EBD5639020000780006077C@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 11/11/11 16:07, Jan Beulich wrote: > Irrespective of the IO-APIC vector sharing suppression patch just sent > the logic in this function needs to iterate over all RTEs, since > multiple pins within an IO-APIC may still use the same vector. Why? The whole point of preventing vector sharing for IO-APICs is to prevent two or more RTEs referencing the same vector. ~Andrew > This is > due to the irq_2_pin[] mapping not necessarily being 1:1. > > Consequently we should remove the commented out code as well as the > respective comments provisioned for the point in time when vector > sharing between unrelated RTEs would be disabled. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/io_apic.c > +++ b/xen/arch/x86/io_apic.c > @@ -281,36 +281,15 @@ static void __io_apic_eoi(unsigned int a > /* If pin is unknown, search for it */ > if ( pin == -1 ) > { > - unsigned int p; > - for ( p = 0; p < nr_ioapic_entries[apic]; ++p ) > + for ( pin = 0; pin < nr_ioapic_entries[apic]; ++pin ) > { > - entry = __ioapic_read_entry(apic, p, TRUE); > + entry = __ioapic_read_entry(apic, pin, 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); > - } > + __io_apic_eoi(apic, vector, pin); > } > > /* 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; > } > > > > -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com