Messages in this thread Patches in this message | | | Date | Tue, 5 Feb 2008 22:26:04 +0000 | From | Mel Gorman <> | Subject | Re: [PATCH] [1/2] Move NUMAQ io handling into arch/x86/pci/numa.c |
| |
Hi Andi,
On (01/02/08 18:08), Andi Kleen didst pronounce: > > numa.c is the only user of the {in,out}*_quad functions. And it has only a few call > sites. Change them to open code the magic NUMAQ port access. > > Only compile tested >
This failed a boot-test on one of the quad machines. Before the patches I see
qla1280: QLA1040 found on PCI bus 0, dev 11 scsi(0:0): Resetting SCSI BUS scsi0 : QLogic QLA1040 PCI to SCSI Host Adapter Firmware version: 7.65.06, Driver version 3.26 scsi 0:0:0:0: Direct-Access IBM DGHS18X 0360 PQ: 0 ANSI: 3 scsi(0:0:0:0): Sync: period 10, offset 12, Wide scsi 0:0:1:0: Direct-Access IBM DGHS18X 0360 PQ: 0 ANSI: 3 scsi(0:0:1:0): Sync: period 10, offset 12, Wide scsi 0:0:2:0: Direct-Access IBM DGHS18X 0360 PQ: 0 ANSI: 3 scsi(0:0:2:0): Sync: period 10, offset 12, Wide scsi 0:0:3:0: Direct-Access IBM OEM DCHS09X 5454 PQ: 0 ANSI: 2 scsi(0:0:3:0): Sync: period 10, offset 12, Wide scsi 0:0:4:0: Direct-Access IBM OEM DCHS09X 5454 PQ: 0 ANSI: 2 scsi(0:0:4:0): Sync: period 10, offset 12, Wide qla1280: QLA1040 found on PCI bus 2, dev 11 scsi(1:0): Resetting SCSI BUS scsi1 : QLogic QLA1040 PCI to SCSI Host Adapter Firmware version: 7.65.06, Driver version 3.26 qla1280: QLA1040 found on PCI bus 4, dev 11 scsi(2:0): Resetting SCSI BUS scsi2 : QLogic QLA1040 PCI to SCSI Host Adapter Firmware version: 7.65.06, Driver version 3.26 qla1280: QLA1040 found on PCI bus 6, dev 11 scsi(3:0): Resetting SCSI BUS scsi3 : QLogic QLA1040 PCI to SCSI Host Adapter Firmware version: 7.65.06, Driver version 3.26
After both patches are applied, it looks like
qla1280: QLA1040 found on PCI bus 0, dev 11 qla1280: Unable to map I/O memory qla1280: QLA1040 found on PCI bus 2, dev 11 qla1280: Unable to map I/O memory qla1280: QLA1040 found on PCI bus 4, dev 11 qla1280: Unable to map I/O memory qla1280: QLA1040 found on PCI bus 6, dev 11 qla1280: Unable to map I/O memory
and of course the root device is not found. Comments on patch and fix is below.
> Signed-off-by: Andi Kleen <ak@suse.de> > > Index: linux/arch/x86/pci/numa.c > =================================================================== > --- linux.orig/arch/x86/pci/numa.c > +++ linux/arch/x86/pci/numa.c > @@ -5,36 +5,62 @@ > #include <linux/pci.h> > #include <linux/init.h> > #include <linux/nodemask.h> > +#include <mach_apic.h> > #include "pci.h" > > +#define XQUAD_PORTIO_BASE 0xfe400000
There is still a definition in include/asm-x86/io_32.h . Is this intentional?
> +#define XQUAD_PORTIO_QUAD 0x40000 /* 256k per quad. */ > +
Similarly in io_32.h
> #define BUS2QUAD(global) (mp_bus_id_to_node[global]) > #define BUS2LOCAL(global) (mp_bus_id_to_local[global]) > #define QUADLOCAL2BUS(quad,local) (quad_local_to_mp_bus_id[quad][local]) > > +extern void *xquad_portio; /* Where the IO area was mapped */
Does the extern need to be here when you've added it (minus the comment) to mach_apic.h later in the patch?
> +#define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port) > + > #define PCI_CONF1_MQ_ADDRESS(bus, devfn, reg) \ > (0x80000000 | (BUS2LOCAL(bus) << 16) | (devfn << 8) | (reg & ~3)) > > +static void write_cf8(unsigned bus, unsigned devfn, unsigned reg) > +{ > + unsigned val = PCI_CONF1_MQ_ADDRESS(bus, devfn, reg); > + if (xquad_portio) > + writel(val, XQUAD_PORT_ADDR(0xcf8, BUS2QUAD(bus))); > + else > + outl(val, 0xCF8); > +}
Other than a case change for 0xcf8, this looks equivilant. xquad_portio should be set and I see in the dmesg;
Before: xquad_portio vaddr 0xf8800000, len 00100000 After: xquad_portio vaddr 0xf8800000, len 00100000
> + > static int pci_conf1_mq_read(unsigned int seg, unsigned int bus, > unsigned int devfn, int reg, int len, u32 *value) > { > unsigned long flags; > + void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus)); >
It shouldn't matter but for clarity should this value only be calculated when xquad_portio is set?
> if (!value || (bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255)) > return -EINVAL; > > spin_lock_irqsave(&pci_config_lock, flags); > > - outl_quad(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), 0xCF8, BUS2QUAD(bus)); > + write_cf8(bus, devfn, reg);
Before writel(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), XQUAD_PORT_ADDR(0xCF8, BUS2QUAD(bus))) After writel(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), XQUAD_PORT_ADDR(0xcf8, BUS2QUAD(bus)))
No problem there.
> > switch (len) { > case 1: > - *value = inb_quad(0xCFC + (reg & 3), BUS2QUAD(bus)); > + if (xquad_portio) > + *value = readb(adr + (reg & 3)); > + else > + *value = inb(0xCFC + (reg & 3)); > break;
Before readb(XQUAD_PORT_ADDR(0xCFC + (reg & 3), BUS2QUAD(bus))) After readb(XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus)) + (reg & 3))
Not exactly equivilant but does it matter?
#define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port)
Before readb(xquad_portio + (XQUAD_PORTIO_QUAD*BUS2QUAD(bus)) + 0xCFC + (reg & 3)) After readb(xquad_portio + (XQUAD_PORTIO_QUAD*BUS2QUAD(bus)) + 0xcfc + (reg & 3))
Apparently no actual difference.
> case 2: > - *value = inw_quad(0xCFC + (reg & 2), BUS2QUAD(bus)); > + if (xquad_portio) > + *value = readw(adr + (reg & 2)); > + else > + *value = inw(0xCFC + (reg & 2)); > break; > case 4: > - *value = inl_quad(0xCFC, BUS2QUAD(bus)); > + if (xquad_portio) > + *value = readl(adr); > + else > + *value = inl(0xCFC); > break; > } > > @@ -47,23 +73,33 @@ static int pci_conf1_mq_write(unsigned i > unsigned int devfn, int reg, int len, u32 value) > { > unsigned long flags; > + void *adr __iomem = XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus)); > > if ((bus >= MAX_MP_BUSSES) || (devfn > 255) || (reg > 255)) > return -EINVAL; > > spin_lock_irqsave(&pci_config_lock, flags); > > - outl_quad(PCI_CONF1_MQ_ADDRESS(bus, devfn, reg), 0xCF8, BUS2QUAD(bus)); > + write_cf8(bus, devfn, reg); >
Same as above so should be ok.
> switch (len) { > case 1: > - outb_quad((u8)value, 0xCFC + (reg & 3), BUS2QUAD(bus)); > + if (xquad_portio) > + writeb(value, adr + (reg & 3)); > + else > + outb((u8)value, 0xCFC + (reg & 3)); > break;
Before writeb((u8)value, XQUAD_PORT_ADDR(0xCFC + (reg & 3), BUS2QUAD(bus))) After writeb(value, XQUAD_PORT_ADDR(0xcfc, BUS2QUAD(bus)) + (reg & 3))
The only difference is the explicit cast and the signature of writeb() should have forced that cast anyway
> case 2: > - outw_quad((u16)value, 0xCFC + (reg & 2), BUS2QUAD(bus)); > + if (xquad_portio) > + writew(value, adr + (reg & 2)); > + else > + outw((u16)value, 0xCFC + (reg & 2)); > break; > case 4: > - outl_quad((u32)value, 0xCFC, BUS2QUAD(bus)); > + if (xquad_portio) > + writel(value, adr + reg); > + else > + outl((u32)value, 0xCFC); > break;
(adr + reg) is not equivilant to 0xCFC here. This is the source of the boot breakage. Fix is below
> } > > Index: linux/include/asm-x86/mach-numaq/mach_apic.h > =================================================================== > --- linux.orig/include/asm-x86/mach-numaq/mach_apic.h > +++ linux/include/asm-x86/mach-numaq/mach_apic.h > @@ -109,6 +109,8 @@ static inline int mpc_apic_id(struct mpc > return logical_apicid; > } > > +extern void *xquad_portio; > + > static inline void setup_portio_remap(void) > { > int num_quads = num_online_nodes(); >
Patch to fix boot problem as follows ====================================
Fix for "x86: move NUMAQ io handling into arch/x86/pci/numa.c"
The intention of commit c7e844f0415252c7e1a2153a97e7a0c511d61ada was to replace a number of outl_quad() calls with a writel() equivilant. One of the changes was not equivilant. This patch corrects it.
Signed-off-by: Mel Gorman <mel@csn.ul.ie> --- arch/x86/pci/numa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/pci/numa.c b/arch/x86/pci/numa.c index 55270c2..ecbcd51 100644 --- a/arch/x86/pci/numa.c +++ b/arch/x86/pci/numa.c @@ -97,7 +97,7 @@ static int pci_conf1_mq_write(unsigned int seg, unsigned int bus, break; case 4: if (xquad_portio) - writel(value, adr + reg); + writel(value, adr); else outl((u32)value, 0xCFC); break; -- Mel Gorman Part-time Phd Student Linux Technology Center University of Limerick IBM Dublin Software Lab
| |