[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries



On Mon, Sep 26, 2011 at 04:15:21PM +0100, Andrew Cooper wrote:
> On 26/09/11 15:57, Jan Beulich wrote:
> >>>> On 26.09.11 at 16:48, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> >> The name "nr_ioapic_registers" is wrong and actively misleading.  The
> >> variable holds the number of redirection entries for each apic, which
> >> is two registers fewer than the total number of registers.
> > To be honest, this is the kind of renaming that I wouldn't want to do
> > as long as Linux, from where the code originates, continues to use
> > the prior name (no matter whether you consider it misleading).
> >
> > Jan
> 
> I guess it depends on whether you consider that we should stay in line
> with Linux or not.  While the code did start in Linux, it has diverged

Stay. Intel does a lot of work of "lift from Linux" and drop in Xen
code. There is similarity.

> so far that it really is its own file now.
> 
> Either we should un-diverge from Linux (unlikely to happen), or consider
> it properly forked and not worry; staying in this intermediate state is
> not sustainable and leads to keeping misleading bits about while an
> overhaul is needed.

So maybe a solution is to provide a patch to the Linux code that would
do the rename? And then you are in line?

> 
> ~Andrew
> 
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >>
> >> diff -r a422e2a4451e -r 6896ad0a1798 xen/arch/x86/io_apic.c
> >> --- a/xen/arch/x86/io_apic.c       Sun Sep 18 00:26:52 2011 +0100
> >> +++ b/xen/arch/x86/io_apic.c       Mon Sep 26 15:47:58 2011 +0100
> >> @@ -58,7 +58,7 @@ s8 __read_mostly sis_apic_bug = -1;
> >>  /*
> >>   * # of IRQ routing registers
> >>   */
> >> -int __read_mostly nr_ioapic_registers[MAX_IO_APICS];
> >> +int __read_mostly nr_ioapic_entries[MAX_IO_APICS];
> >>  int __read_mostly nr_ioapics;
> >>  
> >>  /*
> >> @@ -149,7 +149,7 @@ struct IO_APIC_route_entry **alloc_ioapi
> >>      for (apic = 0; apic < nr_ioapics; apic++) {
> >>          ioapic_entries[apic] =
> >>              xmalloc_array(struct IO_APIC_route_entry,
> >> -                          nr_ioapic_registers[apic]);
> >> +                          nr_ioapic_entries[apic]);
> >>          if (!ioapic_entries[apic])
> >>              goto nomem;
> >>      }
> >> @@ -245,7 +245,7 @@ static void __io_apic_eoi(unsigned int a
> >>          if ( pin == -1 )
> >>          {
> >>              unsigned int p;
> >> -            for ( p = 0; p < nr_ioapic_registers[apic]; ++p )
> >> +            for ( p = 0; p < nr_ioapic_entries[apic]; ++p )
> >>              {
> >>                  entry = __ioapic_read_entry(apic, p, TRUE);
> >>                  if ( entry.vector == vector )
> >> @@ -328,7 +328,7 @@ int save_IO_APIC_setup(struct IO_APIC_ro
> >>          if (!ioapic_entries[apic])
> >>              return -ENOMEM;
> >>  
> >> -        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
> >> +        for (pin = 0; pin < nr_ioapic_entries[apic]; pin++)
> >>        ioapic_entries[apic][pin] = __ioapic_read_entry(apic, pin, 1);
> >>      }
> >>  
> >> @@ -349,7 +349,7 @@ void mask_IO_APIC_setup(struct IO_APIC_r
> >>          if (!ioapic_entries[apic])
> >>              break;
> >>  
> >> -        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
> >> +        for (pin = 0; pin < nr_ioapic_entries[apic]; pin++) {
> >>              struct IO_APIC_route_entry entry;
> >>  
> >>              entry = ioapic_entries[apic][pin];
> >> @@ -376,7 +376,7 @@ int restore_IO_APIC_setup(struct IO_APIC
> >>          if (!ioapic_entries[apic])
> >>              return -ENOMEM;
> >>  
> >> -        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
> >> +        for (pin = 0; pin < nr_ioapic_entries[apic]; pin++)
> >>        ioapic_write_entry(apic, pin, 1, ioapic_entries[apic][pin]);
> >>      }
> >>  
> >> @@ -524,7 +524,7 @@ static void clear_IO_APIC (void)
> >>      int apic, pin;
> >>  
> >>      for (apic = 0; apic < nr_ioapics; apic++) {
> >> -        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
> >> +        for (pin = 0; pin < nr_ioapic_entries[apic]; pin++)
> >>              clear_IO_APIC_pin(apic, pin);
> >>      }
> >>  }
> >> @@ -785,7 +785,7 @@ void /*__init*/ setup_ioapic_dest(void)
> >>          return;
> >>  
> >>      for (ioapic = 0; ioapic < nr_ioapics; ioapic++) {
> >> -        for (pin = 0; pin < nr_ioapic_registers[ioapic]; pin++) {
> >> +        for (pin = 0; pin < nr_ioapic_entries[ioapic]; pin++) {
> >>              irq_entry = find_irq_entry(ioapic, pin, mp_INT);
> >>              if (irq_entry == -1)
> >>                  continue;
> >> @@ -1031,7 +1031,7 @@ static int pin_2_irq(int idx, int apic, 
> >>           */
> >>          i = irq = 0;
> >>          while (i < apic)
> >> -            irq += nr_ioapic_registers[i++];
> >> +            irq += nr_ioapic_entries[i++];
> >>          irq += pin;
> >>          break;
> >>      }
> >> @@ -1051,7 +1051,7 @@ static inline int IO_APIC_irq_trigger(in
> >>      int apic, idx, pin;
> >>  
> >>      for (apic = 0; apic < nr_ioapics; apic++) {
> >> -        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
> >> +        for (pin = 0; pin < nr_ioapic_entries[apic]; pin++) {
> >>              idx = find_irq_entry(apic,pin,mp_INT);
> >>              if ((idx != -1) && (irq == pin_2_irq(idx,apic,pin)))
> >>                  return irq_trigger(idx);
> >> @@ -1092,7 +1092,7 @@ static void __init setup_IO_APIC_irqs(vo
> >>      apic_printk(APIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");
> >>  
> >>      for (apic = 0; apic < nr_ioapics; apic++) {
> >> -        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
> >> +        for (pin = 0; pin < nr_ioapic_entries[apic]; pin++) {
> >>  
> >>              /*
> >>               * add it to the IO-APIC irq-routing table:
> >> @@ -1218,7 +1218,7 @@ static void /*__init*/ __print_IO_APIC(v
> >>      printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
> >>      for (i = 0; i < nr_ioapics; i++)
> >>          printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
> >> -               mp_ioapics[i].mpc_apicid, nr_ioapic_registers[i]);
> >> +               mp_ioapics[i].mpc_apicid, nr_ioapic_entries[i]);
> >>  
> >>      /*
> >>       * We are a bit conservative about what we expect.  We have to
> >> @@ -1378,7 +1378,7 @@ static void __init enable_IO_APIC(void)
> >>      for(apic = 0; apic < nr_ioapics; apic++) {
> >>          int pin;
> >>          /* See if any of the pins is in ExtINT mode */
> >> -        for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
> >> +        for (pin = 0; pin < nr_ioapic_entries[apic]; pin++) {
> >>              struct IO_APIC_route_entry entry = ioapic_read_entry(apic, 
> >> pin, 
> >> 0);
> >>  
> >>              /* If the interrupt line is enabled and in ExtInt mode
> >> @@ -2116,7 +2116,7 @@ static void __init ioapic_pm_state_alloc
> >>      int i, nr_entry = 0;
> >>  
> >>      for (i = 0; i < nr_ioapics; i++)
> >> -        nr_entry += nr_ioapic_registers[i];
> >> +        nr_entry += nr_ioapic_entries[i];
> >>  
> >>      ioapic_pm_state = _xmalloc(sizeof(struct 
> >> IO_APIC_route_entry)*nr_entry,
> >>                                 sizeof(struct IO_APIC_route_entry));
> >> @@ -2158,7 +2158,7 @@ void ioapic_suspend(void)
> >>  
> >>      spin_lock_irqsave(&ioapic_lock, flags);
> >>      for (apic = 0; apic < nr_ioapics; apic++) {
> >> -        for (i = 0; i < nr_ioapic_registers[apic]; i ++, entry ++ ) {
> >> +        for (i = 0; i < nr_ioapic_entries[apic]; i ++, entry ++ ) {
> >>              *(((int *)entry) + 1) = __io_apic_read(apic, 0x11 + 2 * i);
> >>              *(((int *)entry) + 0) = __io_apic_read(apic, 0x10 + 2 * i);
> >>          }
> >> @@ -2180,7 +2180,7 @@ void ioapic_resume(void)
> >>              reg_00.bits.ID = mp_ioapics[apic].mpc_apicid;
> >>              __io_apic_write(apic, 0, reg_00.raw);
> >>          }
> >> -        for (i = 0; i < nr_ioapic_registers[apic]; i++, entry++) {
> >> +        for (i = 0; i < nr_ioapic_entries[apic]; i++, entry++) {
> >>              __io_apic_write(apic, 0x11+2*i, *(((int *)entry)+1));
> >>              __io_apic_write(apic, 0x10+2*i, *(((int *)entry)+0));
> >>          }
> >> @@ -2609,8 +2609,8 @@ void __init init_ioapic_mappings(void)
> >>          {
> >>              /* The number of IO-APIC IRQ registers (== #pins): */
> >>              reg_01.raw = io_apic_read(i, 1);
> >> -            nr_ioapic_registers[i] = reg_01.bits.entries + 1;
> >> -            nr_irqs_gsi += nr_ioapic_registers[i];
> >> +            nr_ioapic_entries[i] = reg_01.bits.entries + 1;
> >> +            nr_irqs_gsi += nr_ioapic_entries[i];
> >>          }
> >>      }
> >>  
> >> diff -r a422e2a4451e -r 6896ad0a1798 
> >> xen/drivers/passthrough/amd/iommu_intr.c
> >> --- a/xen/drivers/passthrough/amd/iommu_intr.c     Sun Sep 18 00:26:52 
> >> 2011 +0100
> >> +++ b/xen/drivers/passthrough/amd/iommu_intr.c     Mon Sep 26 15:47:58 
> >> 2011 +0100
> >> @@ -165,7 +165,7 @@ int __init amd_iommu_setup_ioapic_remapp
> >>      /* Read ioapic entries and update interrupt remapping table 
> >> accordingly 
> >> */
> >>      for ( apic = 0; apic < nr_ioapics; apic++ )
> >>      {
> >> -        for ( pin = 0; pin < nr_ioapic_registers[apic]; pin++ )
> >> +        for ( pin = 0; pin < nr_ioapic_entries[apic]; pin++ )
> >>          {
> >>              *(((int *)&rte) + 1) = io_apic_read(apic, 0x11 + 2 * pin);
> >>              *(((int *)&rte) + 0) = io_apic_read(apic, 0x10 + 2 * pin);
> >> diff -r a422e2a4451e -r 6896ad0a1798 xen/drivers/passthrough/vtd/intremap.c
> >> --- a/xen/drivers/passthrough/vtd/intremap.c       Sun Sep 18 00:26:52 
> >> 2011 +0100
> >> +++ b/xen/drivers/passthrough/vtd/intremap.c       Mon Sep 26 15:47:58 
> >> 2011 +0100
> >> @@ -33,7 +33,7 @@
> >>  
> >>  #ifdef __ia64__
> >>  #define nr_ioapics              iosapic_get_nr_iosapics()
> >> -#define nr_ioapic_registers(i)  iosapic_get_nr_pins(i)
> >> +#define nr_ioapic_entries(i)  iosapic_get_nr_pins(i)
> >>  #define __io_apic_read(apic, reg) \
> >>      (*IO_APIC_BASE(apic) = reg, *(IO_APIC_BASE(apic)+4))
> >>  #define __io_apic_write(apic, reg, val) \
> >> @@ -53,7 +53,7 @@
> >>  #else
> >>  #include <asm/apic.h>
> >>  #include <asm/io_apic.h>
> >> -#define nr_ioapic_registers(i)  nr_ioapic_registers[i]
> >> +#define nr_ioapic_entries(i)  nr_ioapic_entries[i]
> >>  #endif
> >>  
> >>  /*
> >> @@ -91,7 +91,7 @@ static int init_apic_pin_2_ir_idx(void)
> >>  
> >>      nr_pins = 0;
> >>      for ( i = 0; i < nr_ioapics; i++ )
> >> -        nr_pins += nr_ioapic_registers(i);
> >> +        nr_pins += nr_ioapic_entries(i);
> >>  
> >>      _apic_pin_2_ir_idx = xmalloc_array(int, nr_pins);
> >>      apic_pin_2_ir_idx = xmalloc_array(int *, nr_ioapics);
> >> @@ -109,7 +109,7 @@ static int init_apic_pin_2_ir_idx(void)
> >>      for ( i = 0; i < nr_ioapics; i++ )
> >>      {
> >>          apic_pin_2_ir_idx[i] = &_apic_pin_2_ir_idx[nr_pins];
> >> -        nr_pins += nr_ioapic_registers(i);
> >> +        nr_pins += nr_ioapic_entries(i);
> >>      }
> >>  
> >>      return 0;
> >> diff -r a422e2a4451e -r 6896ad0a1798 xen/include/asm-x86/io_apic.h
> >> --- a/xen/include/asm-x86/io_apic.h        Sun Sep 18 00:26:52 2011 +0100
> >> +++ b/xen/include/asm-x86/io_apic.h        Mon Sep 26 15:47:58 2011 +0100
> >> @@ -77,7 +77,7 @@ union IO_APIC_reg_03 {
> >>   * # of IO-APICs and # of IRQ routing registers
> >>   */
> >>  extern int nr_ioapics;
> >> -extern int nr_ioapic_registers[MAX_IO_APICS];
> >> +extern int nr_ioapic_entries[MAX_IO_APICS];
> >>  
> >>  enum ioapic_irq_destination_types {
> >>    dest_Fixed = 0,
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@xxxxxxxxxxxxxxxxxxx 
> >> http://lists.xensource.com/xen-devel 
> >
> 
> -- 
> Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
> T: +44 (0)1223 225 900, http://www.citrix.com
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.