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
|
Home