Re: [PATCH] x86: clean up __io_apic_eoi()

[Andrew Cooper <andrew.cooper3@citrix.com>]

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 <jbeulich@suse.com>
>
> --- 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