lkml.org 
[lkml]   [2008]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2008-02-05 23:29    [W:0.046 / U:1.232 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site