[PATCH] [1/2] Move NUMAQ io handling into arch/x86/pci/numa.c

[PATCH] [1/2] Move NUMAQ io handling into arch/x86/pci/numa.c

[Andi Kleen]

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

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
+#define XQUAD_PORTIO_QUAD 0x40000  /* 256k per quad. */
+
 #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 */
+#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);
+}
+
 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));
 
 	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);
 
 	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;
 	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);
 
 	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;
 	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;
 	}
 
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] [2/2] Remove NUMAQ support in io_32.h

[Andi Kleen]

Now that the only user does it on its own remove the NUMAQ support macros in io_32.h

The next step would be to convert the preprocessor mess to actually readable
standard inlines.

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/include/asm-x86/io_32.h
===================================================================
--- linux.orig/include/asm-x86/io_32.h
+++ linux/include/asm-x86/io_32.h
@@ -275,29 +275,6 @@ static inline void slow_down_io(void) {
 
 #endif
 
-#ifdef CONFIG_X86_NUMAQ
-extern void *xquad_portio;    /* Where the IO area was mapped */
-#define XQUAD_PORT_ADDR(port, quad) (xquad_portio + (XQUAD_PORTIO_QUAD*quad) + port)
-#define __BUILDIO(bwl,bw,type) \
-static inline void out##bwl##_quad(unsigned type value, int port, int quad) { \
-	if (xquad_portio) \
-		write##bwl(value, XQUAD_PORT_ADDR(port, quad)); \
-	else \
-		out##bwl##_local(value, port); \
-} \
-static inline void out##bwl(unsigned type value, int port) { \
-	out##bwl##_quad(value, port, 0); \
-} \
-static inline unsigned type in##bwl##_quad(int port, int quad) { \
-	if (xquad_portio) \
-		return read##bwl(XQUAD_PORT_ADDR(port, quad)); \
-	else \
-		return in##bwl##_local(port); \
-} \
-static inline unsigned type in##bwl(int port) { \
-	return in##bwl##_quad(port, 0); \
-}
-#else
 #define __BUILDIO(bwl,bw,type) \
 static inline void out##bwl(unsigned type value, int port) { \
 	out##bwl##_local(value, port); \
@@ -305,8 +282,6 @@ static inline void out##bwl(unsigned typ
 static inline unsigned type in##bwl(int port) { \
 	return in##bwl##_local(port); \
 }
-#endif
-
 
 #define BUILDIO(bwl,bw,type) \
 static inline void out##bwl##_local(unsigned type value, int port) { \

Re: [PATCH] [1/2] Move NUMAQ io handling into arch/x86/pci/numa.c

[Ingo Molnar]

* Andi Kleen <ak@suse.de> wrote:

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

thanks, applied.

	Ingo

Re: [PATCH] [2/2] Remove NUMAQ support in io_32.h

[Ingo Molnar]

* Andi Kleen <ak@suse.de> wrote:

> Now that the only user does it on its own remove the NUMAQ support 
> macros in io_32.h

thanks, applied.

> The next step would be to convert the preprocessor mess to actually 
> readable standard inlines.

agreed.

	Ingo

Re: [PATCH] [1/2] Move NUMAQ io handling into arch/x86/pci/numa.c

[Mel Gorman]

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

Re: [PATCH] [1/2] Move NUMAQ io handling into arch/x86/pci/numa.c

[Andi Kleen]

> > +#define XQUAD_PORTIO_BASE 0xfe400000
>
> There is still a definition in include/asm-x86/io_32.h . Is this
> intentional?

Probably not.

>
> > +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?

The extern was intended to be removed, but somehow that change didn't make 
it into the final patch. Doesn't matter much either way though.

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

Yes looks good; thanks.

-Andi