[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 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
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.

~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


 


Rackspace

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