Andrew Cooper
2011-Sep-26 14:48 UTC
[Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries
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. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> 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@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Sep-26 14:57 UTC
Re: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries
>>> On 26.09.11 at 16:48, Andrew Cooper <andrew.cooper3@citrix.com> 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> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > 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@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andrew Cooper
2011-Sep-26 15:15 UTC
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@citrix.com> 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). > > JanI 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@citrix.com> >> >> 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@lists.xensource.com >> 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@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Sep-26 15:40 UTC
Re: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries
>>> On 26.09.11 at 17:15, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > On 26/09/11 15:57, Jan Beulich wrote: >>>>> On 26.09.11 at 16:48, Andrew Cooper <andrew.cooper3@citrix.com> 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.There''s one more alternative: You could propose the same renaming on Linux. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Sep-26 15:45 UTC
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@citrix.com> 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 divergedStay. 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@citrix.com> > >> > >> 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@lists.xensource.com > >> 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@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Sep-26 16:25 UTC
Re: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries
On 26/09/2011 08:40, "Jan Beulich" <JBeulich@suse.com> wrote:>> 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. > > There''s one more alternative: You could propose the same renaming > on Linux.Not a bad idea. But I don''t think it should stop us making these kinds of changes in Xen now. We stayed in sync with upstream Linux on these files for a long while, but we''re well diverged now. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Sep-26 16:27 UTC
Re: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries
On 26/09/2011 08:45, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:>> 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.You might also say it gives a false sense of similarity. Andrew''s now having to go fix the per-cpu vector stuff for example. It got dropped in; it was complex and opaque; it doesn''t really work properly yet. I''d prefer the people doing the porting to have to think about each line a bit more. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2011-Sep-26 17:04 UTC
RE: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries
> From: Keir Fraser [mailto:keir.xen@gmail.com] > Sent: Monday, September 26, 2011 10:27 AM > To: Konrad Rzeszutek Wilk; Andrew Cooper > Cc: xen-devel@lists.xensource.com; Jan Beulich > Subject: Re: [Xen-devel] [PATCH] IRQ Cleanup: rename nr_ioapic_registers to nr_ioapic_entries > > On 26/09/2011 08:45, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote: > > >> 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. > > You might also say it gives a false sense of similarity. Andrew''s now having > to go fix the per-cpu vector stuff for example. It got dropped in; it was > complex and opaque; it doesn''t really work properly yet. I''d prefer the > people doing the porting to have to think about each line a bit more.You also don''t want the people doing the porting thinking about each line so hard that they decide to rewrite all of them to meet their own personal preference. Remember that the number of people familiar with "code X" on Linux probably outnumbers the number of people familiar with code X on Xen by a factor of 10-100. The point of stealing the Linux code to begin with is to leverage developer knowledge and (historical) maintenance knowledge. A suggested compromise: Any divergence of this sort from Linux should be explicitly noted in a comment, e.g. "Linux calls this foo, but Xen calls it bar, though they otherwise should function very similarly". IMHO, Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel