WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Jan Beulich <JBeulich@xxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries
From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Date: Mon, 26 Sep 2011 16:15:21 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 26 Sep 2011 08:16:23 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E80AF140200007800057DA4@xxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <6896ad0a17986a06a470.1317048538@xxxxxxxxxxxxxxxxxxxxxxxxxxx> <4E80AF140200007800057DA4@xxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.21) Gecko/20110831 Lightning/1.0b2 Thunderbird/3.1.13
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