Re: SCSI QLA not working on latest *-mm SN2

Re: SCSI QLA not working on latest *-mm SN2

[Paul Jackson]

Andrew Vasquez has been looking at this, via private email with just
me (no progress yet).  Figured I update the larger list with this much ...

I am still seeing the SN2 SCSI QLA failure as I reported yesterday, but
now against 2.6.9-rc2-mm1.

For the benefit of those with limited memories, such as myself, I will
repeat the symptoms from the top.

Those with good memories can probably stop reading here - nothing much
new to report other than retesting on 2.6.9-rc2-mm1.

===

This is now testing against 2.6.9-rc2-mm1, plus a one line workaround
for some ACPI problem, from Jesse Barnes.  The symptoms resemble what I
first saw of this failure on 2.6.9-rc1-mm5.  I did not see any such
problem on 2.6.9-rc1-mm4.

I have to make the following change to sn2_defconfig in order to boot:

< CONFIG_SCSI_QLA22XX=y
< CONFIG_SCSI_QLA2300=y
< CONFIG_SCSI_QLA2322=y
---
> # CONFIG_SCSI_QLA22XX is not set
> # CONFIG_SCSI_QLA2300 is not set
> # CONFIG_SCSI_QLA2322 is not set

Once I comment out these QLA lines, I can boot.

Otherwise the boot fails, ending with the following lines of output. 
There are a couple of 5 or 10 second pauses somewhere in the last dozen
lines of this output.

=======================================================

VFS: Disk quotas dquot_6.5.1
Dquot-cache hash table entries: 2048 (order 0, 16384 bytes)
SGI XFS with ACLs, realtime, large block/inode numbers, no debug enabled
SGI XFS Quota Management subsystem
Initializing Cryptographic API
pci_hotplug: PCI Hot Plug PCI Core version: 0.5
IA-PC Multimedia Timer: v1.0, 25 MHz
EFI Time Services Driver v0.4
sn_console: Console driver init
ttySG0 at I/O 0x0 (irq = 0) is a SGI SN L1
RAMDISK driver initialized: 16 RAM disks of 4096K size 1024 blocksize
loop: loaded (max 8 devices)
tg3.c:v3.9 (August 30, 2004)
eth0: Tigon3 [partno(030-1771-000) rev 0105 PHY(5701)] (PCI:66MHz:64-bit) 10/100/1000BaseT Ethernet 08:00:69:13:dc:f6
eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] Split[0] WireSpeed[1] TSOcap[0] 
netconsole: not configured, aborting
Uniform Multi-Platform E-IDE driver Revision: 7.00alpha2
ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx
SGIIOC4: IDE controller at PCI slot 0000:01:01.0, revision 79
ide0: BM-DMA at 0xc00002080c200140-0xc00002080c200163
hda: MATSHITADVD-ROM SR-8588, ATAPI CD/DVD-ROM drive
Using anticipatory io scheduler
ide0 at 0xc00002080c200100-0xc00002080c200107,0xc00002080c200120 on irq 55
ide1: I/O resource 0x376-0x376 not free.
ide1: ports already in use, skipping probe
ide2: I/O resource 0x3EE-0x3EE not free.
ide2: ports already in use, skipping probe
ide3: I/O resource 0x36E-0x36E not free.
ide3: ports already in use, skipping probe
ide4: I/O resource 0x3E6-0x3E6 not free.
ide4: ports already in use, skipping probe
ide5: I/O resource 0x366-0x366 not free.
ide5: ports already in use, skipping probe
hda: ATAPI 48X DVD-ROM drive, 256kB Cache, UDMA(16)
Uniform CD-ROM driver Revision: 3.20
qla1280: QLA12160 found on PCI bus 1, dev 3
scsi(0): Enabling SN2 PCI DMA dual channel lockup workaround
scsi(0): Enabling SN2 PCI DMA workaround
scsi(0:0): Resetting SCSI BUS
scsi(0:1): Resetting SCSI BUS
scsi0 : QLogic QLA12160 PCI to SCSI Host Adapter
       Firmware version: 10.04.32, Driver version 3.24.4
  Vendor: SGI       Model: ST336753LC        Rev: 2741
  Type:   Direct-Access                      ANSI SCSI revision: 03
scsi(0:0:1:0): Sync: period 9, offset 14, Wide, DT, Tagged queuing: depth 255
  Vendor: SGI       Model: ST336753LC        Rev: 2741
  Type:   Direct-Access                      ANSI SCSI revision: 03
scsi(0:0:2:0): Sync: period 9, offset 14, Wide, DT, Tagged queuing: depth 255
QLogic Fibre Channel HBA Driver (a0000001007b29c0)
qla2200 0000:03:01.0: Found an ISP2200, irq 58, iobase 0xc00002080f400000
qla2200 0000:03:01.0: Configuring PCI space...
PCI: slot 0000:03:01.0 has incorrect PCI cache line size of 0 bytes, correcting to 128
POD entered via OS requested halt, using Cac mode
2 008: POD SysCt Cac> 

=======================================================

By way of comparison, the lines just before and after the above
point of death, on a successful boot w/o the QLA2 config, looks like:

=======================================================

scsi0 : QLogic QLA12160 PCI to SCSI Host Adapter
       Firmware version: 10.04.32, Driver version 3.24.4
  Vendor: SGI       Model: ST336753LC        Rev: 2741
  Type:   Direct-Access                      ANSI SCSI revision: 03
scsi(0:0:1:0): Sync: period 9, offset 14, Wide, DT, Tagged queuing: depth 255
  Vendor: SGI       Model: ST336753LC        Rev: 2741
  Type:   Direct-Access                      ANSI SCSI revision: 03
scsi(0:0:2:0): Sync: period 9, offset 14, Wide, DT, Tagged queuing: depth 255
SCSI device sda: 71687372 512-byte hdwr sectors (36704 MB)
SCSI device sda: drive cache: write through
 sda: sda1 sda2 sda3
Attached scsi disk sda at scsi0, channel 0, id 1, lun 0
SCSI device sdb: 71687372 512-byte hdwr sectors (36704 MB)
SCSI device sdb: drive cache: write through
 sdb: sdb1 sdb2 sdb3 sdb4 sdb5 sdb6 sdb7 sdb8
Attached scsi disk sdb at scsi0, channel 0, id 2, lun 0
Fusion MPT base driver 3.01.16
Copyright (c) 1999-2004 LSI Logic Corporation
Fusion MPT SCSI Host driver 3.01.16
mice: PS/2 mouse device common for all mice

=======================================================

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Thursday, September 16, 2004 12:56 pm, Paul Jackson wrote:
> Andrew Vasquez has been looking at this, via private email with just
> me (no progress yet).  Figured I update the larger list with this much ...

It seems to be failing on one of the accesses to PCI_COMMAND in config space 
in qla2x00_reset_chip().  I'm checking now to see if we're accessing the card 
right after a reset but before the card has finished.  That would cause a 
master abort, the symptom I'm seeing at least.

> I am still seeing the SN2 SCSI QLA failure as I reported yesterday, but
> now against 2.6.9-rc2-mm1.

Me too.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Andrew Vasquez]

[-- Attachment #1: Type: text/plain, Size: 669 bytes --]

On Thu, 2004-09-16 at 13:05, Jesse Barnes wrote:
> On Thursday, September 16, 2004 12:56 pm, Paul Jackson wrote:
> > Andrew Vasquez has been looking at this, via private email with just
> > me (no progress yet).  Figured I update the larger list with this much ...
> 
> It seems to be failing on one of the accesses to PCI_COMMAND in config space 
> in qla2x00_reset_chip().  I'm checking now to see if we're accessing the card 
> right after a reset but before the card has finished.  That would cause a 
> master abort, the symptom I'm seeing at least.
> 

Interesting, the only changes in reset_chip() are for PCI posting
issues.  Relevant diff attached.

--
Andrew

[-- Attachment #2: posting.diff --]
[-- Type: text/x-patch, Size: 4177 bytes --]

diff -Nurdp -X dontdiff 80000b14/qla_init.c 80000b21/qla_init.c
--- 80000b14/qla_init.c	2004-06-23 17:12:33.000000000 -0700
+++ 80000b21/qla_init.c	2004-09-02 13:11:35.000000000 -0700
@@ -315,6 +317,7 @@ qla2x00_pci_config(scsi_qla_host_t *ha)
 
 			/* Select FPM registers. */
 			WRT_REG_WORD(&ha->iobase->ctrl_status, 0x20);
+			RD_REG_WORD(&ha->iobase->ctrl_status);
 
 			/* Get the fb rev level */
 			ha->fb_rev = RD_FB_CMD_REG(ha, ha->iobase);
@@ -324,6 +327,7 @@ qla2x00_pci_config(scsi_qla_host_t *ha)
 
 			/* Deselect FPM registers. */
 			WRT_REG_WORD(&ha->iobase->ctrl_status, 0x0);
+			RD_REG_WORD(&ha->iobase->ctrl_status);
 
 			/* Release RISC module. */
 			WRT_REG_WORD(&ha->iobase->hccr, HCCR_RELEASE_RISC);
@@ -417,25 +421,32 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 				udelay(100);
 			}
 		} else {
+			RD_REG_WORD(&reg->hccr);	/* PCI Posting. */
 			udelay(10);
 		}
 
 		/* Select FPM registers. */
 		WRT_REG_WORD(&reg->ctrl_status, 0x20);
+		RD_REG_WORD(&reg->ctrl_status);		/* PCI Posting. */
 
 		/* FPM Soft Reset. */
 		WRT_REG_WORD(&reg->fpm_diag_config, 0x100);
+		RD_REG_WORD(&reg->fpm_diag_config);	/* PCI Posting. */
 
 		/* Toggle Fpm Reset. */
-		if (!IS_QLA2200(ha))
+		if (!IS_QLA2200(ha)) {
 			WRT_REG_WORD(&reg->fpm_diag_config, 0x0);
+			RD_REG_WORD(&reg->fpm_diag_config); /* PCI Posting. */
+		}
 
 		/* Select frame buffer registers. */
 		WRT_REG_WORD(&reg->ctrl_status, 0x10);
+		RD_REG_WORD(&reg->ctrl_status);		/* PCI Posting. */
 
 		/* Reset frame buffer FIFOs. */
 		if (IS_QLA2200(ha)) {
 			WRT_FB_CMD_REG(ha, reg, 0xa000);
+			RD_FB_CMD_REG(ha, reg);		/* PCI Posting. */
 		} else {
 			WRT_FB_CMD_REG(ha, reg, 0x00fc);
 
@@ -449,19 +460,25 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 
 		/* Select RISC module registers. */
 		WRT_REG_WORD(&reg->ctrl_status, 0);
+		RD_REG_WORD(&reg->ctrl_status);		/* PCI Posting. */
 
 		/* Reset RISC processor. */
 		WRT_REG_WORD(&reg->hccr, HCCR_RESET_RISC);
+		RD_REG_WORD(&reg->hccr);		/* PCI Posting. */
 
 		/* Release RISC processor. */
 		WRT_REG_WORD(&reg->hccr, HCCR_RELEASE_RISC);
+		RD_REG_WORD(&reg->hccr);		/* PCI Posting. */
 	}
 
 	WRT_REG_WORD(&reg->hccr, HCCR_CLR_RISC_INT);
+	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 	WRT_REG_WORD(&reg->hccr, HCCR_CLR_HOST_INT);
+	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	/* Reset ISP chip. */
 	WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
+	RD_REG_WORD(&reg->ctrl_status);			/* PCI Posting. */
 
 	/* Wait for RISC to recover from reset. */
 	if (IS_QLA2100(ha) || IS_QLA2200(ha) || IS_QLA2300(ha)) {
@@ -482,12 +499,13 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 
 	/* Reset RISC processor. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RESET_RISC);
+	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	WRT_REG_WORD(&reg->semaphore, 0);
 
 	/* Release RISC processor. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RELEASE_RISC);
-	RD_REG_WORD(&reg->hccr);		/* PCI Posting. */
+	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	if (IS_QLA2100(ha) || IS_QLA2200(ha) || IS_QLA2300(ha)) {
 		for (cnt = 0; cnt < 30000; cnt++) {
@@ -516,8 +534,10 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 	pci_write_config_word(ha->pdev, PCI_COMMAND, cmd);
 
 	/* Disable RISC pause on FPM parity error. */
-	if (!IS_QLA2100(ha))
+	if (!IS_QLA2100(ha)) {
 		WRT_REG_WORD(&reg->hccr, HCCR_DISABLE_PARITY_PAUSE);
+		RD_REG_WORD(&reg->hccr);		/* PCI Posting. */
+	}
 
 	spin_unlock_irqrestore(&ha->hardware_lock, flags);
 }
@@ -548,6 +568,8 @@ qla2x00_chip_diag(scsi_qla_host_t *ha)
 
 	/* Reset ISP chip. */
 	WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
+	RD_REG_WORD(&reg->ctrl_status);			/* PCI Posting. */
+
 	/*
 	 * We need to have a delay here since the card will not respond while
 	 * in reset causing an MCA on some architectures.
@@ -568,7 +590,9 @@ qla2x00_chip_diag(scsi_qla_host_t *ha)
 
 	/* Reset RISC processor. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RESET_RISC);
+	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RELEASE_RISC);
+	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	/* Workaround for QLA2312 PCI parity error */
 	if (IS_QLA2100(ha) || IS_QLA2200(ha) || IS_QLA2300(ha)) {

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]

On Thursday, September 16, 2004 1:56 pm, Andrew Vasquez wrote:
> On Thu, 2004-09-16 at 13:05, Jesse Barnes wrote:
> > On Thursday, September 16, 2004 12:56 pm, Paul Jackson wrote:
> > > Andrew Vasquez has been looking at this, via private email with just
> > > me (no progress yet).  Figured I update the larger list with this much
> > > ...
> >
> > It seems to be failing on one of the accesses to PCI_COMMAND in config
> > space in qla2x00_reset_chip().  I'm checking now to see if we're
> > accessing the card right after a reset but before the card has finished. 
> > That would cause a master abort, the symptom I'm seeing at least.
>
> Interesting, the only changes in reset_chip() are for PCI posting
> issues.  Relevant diff attached.

Yeah, I think one of these is the culprit.  Before I got your message, I fixed 
some of them in my tree already (see attached) and things seem to work.

        WRT_REG_WORD(&reg->hccr, HCCR_CLR_RISC_INT);
+       RD_REG_WORD(&reg->hccr);                        /* PCI Posting. */
        WRT_REG_WORD(&reg->hccr, HCCR_CLR_HOST_INT);
+       RD_REG_WORD(&reg->hccr);                        /* PCI Posting. */
 
        /* Reset ISP chip. */
        WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
+       RD_REG_WORD(&reg->ctrl_status);                 /* PCI Posting. */
 
In particular, are the above ok?  If the chip is resetting, won't doing a read 
cause a machine check (or at the very least, a device select timeout, which 
will return all ones on friendlier platforms).

        WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
+       RD_REG_WORD(&reg->ctrl_status);                 /* PCI Posting. */

Same here?

Thanks,
Jesse

[-- Attachment #2: qla2xxx-less-posting.patch --]
[-- Type: text/plain, Size: 1727 bytes --]

diff -Napur -X /home/jbarnes/dontdiff linux-2.6.9-rc2-mm1.orig/drivers/scsi/qla2xxx/qla_init.c linux-2.6.9-rc2-mm1/drivers/scsi/qla2xxx/qla_init.c
--- linux-2.6.9-rc2-mm1.orig/drivers/scsi/qla2xxx/qla_init.c	2004-09-16 09:35:50.000000000 -0700
+++ linux-2.6.9-rc2-mm1/drivers/scsi/qla2xxx/qla_init.c	2004-09-16 14:00:32.000000000 -0700
@@ -463,13 +463,10 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 	}
 
 	WRT_REG_WORD(&reg->hccr, HCCR_CLR_RISC_INT);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 	WRT_REG_WORD(&reg->hccr, HCCR_CLR_HOST_INT);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	/* Reset ISP chip. */
 	WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
-	RD_REG_WORD(&reg->ctrl_status);			/* PCI Posting. */
 
 	/* Wait for RISC to recover from reset. */
 	if (IS_QLA2100(ha) || IS_QLA2200(ha) || IS_QLA2300(ha)) {
@@ -490,7 +487,6 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 
 	/* Reset RISC processor. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RESET_RISC);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	WRT_REG_WORD(&reg->semaphore, 0);
 
@@ -559,7 +555,6 @@ qla2x00_chip_diag(scsi_qla_host_t *ha)
 
 	/* Reset ISP chip. */
 	WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
-	RD_REG_WORD(&reg->ctrl_status);			/* PCI Posting. */
 
 	/*
 	 * We need to have a delay here since the card will not respond while
@@ -581,9 +576,7 @@ qla2x00_chip_diag(scsi_qla_host_t *ha)
 
 	/* Reset RISC processor. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RESET_RISC);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RELEASE_RISC);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	/* Workaround for QLA2312 PCI parity error */
 	if (IS_QLA2100(ha) || IS_QLA2200(ha) || IS_QLA2300(ha)) {

Re: SCSI QLA not working on latest *-mm SN2

[Andrew Vasquez]

[-- Attachment #1: Type: text/plain, Size: 2160 bytes --]

On Thu, 2004-09-16 at 14:09, Jesse Barnes wrote:
> On Thursday, September 16, 2004 1:56 pm, Andrew Vasquez wrote:
> > On Thu, 2004-09-16 at 13:05, Jesse Barnes wrote:
> > > On Thursday, September 16, 2004 12:56 pm, Paul Jackson wrote:
> > > > Andrew Vasquez has been looking at this, via private email with just
> > > > me (no progress yet).  Figured I update the larger list with this much
> > > > ...
> > >
> > > It seems to be failing on one of the accesses to PCI_COMMAND in config
> > > space in qla2x00_reset_chip().  I'm checking now to see if we're
> > > accessing the card right after a reset but before the card has finished. 
> > > That would cause a master abort, the symptom I'm seeing at least.
> >
> > Interesting, the only changes in reset_chip() are for PCI posting
> > issues.  Relevant diff attached.
> 
> Yeah, I think one of these is the culprit.  Before I got your message, I fixed 
> some of them in my tree already (see attached) and things seem to work.
> 

Hmm, seems we were a bit too over-aggressive in placement of the
readw()s :(

>         WRT_REG_WORD(&reg->hccr, HCCR_CLR_RISC_INT);
> +       RD_REG_WORD(&reg->hccr);                        /* PCI Posting. */
>         WRT_REG_WORD(&reg->hccr, HCCR_CLR_HOST_INT);
> +       RD_REG_WORD(&reg->hccr);                        /* PCI Posting. */
>  
>         /* Reset ISP chip. */
>         WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
> +       RD_REG_WORD(&reg->ctrl_status);                 /* PCI Posting. */
>  
> In particular, are the above ok?  If the chip is resetting, won't doing a read 
> cause a machine check (or at the very least, a device select timeout, which 
> will return all ones on friendlier platforms).
> 

There are several more which deltas in qla_dbg.c which are suspect
also.  

>         WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
> +       RD_REG_WORD(&reg->ctrl_status);                 /* PCI Posting. */
> 
> Same here?
> 

Andrew, please add Jesse's patch along with the patch I'm attaching to
your tree.  I'll be sure to add the ia64 machine back into our test
ring.


Signed-off-by: Andrew Vasquez <andrew.vasquez@qlogic.com>

[-- Attachment #2: qla_dbg_less_posting.diff --]
[-- Type: text/x-patch, Size: 727 bytes --]

diff -Nurd -X /home/praka/Work/QLogic/Drivers/8.x/dontdiff linux-2.6.9-rc2-mm1/drivers/scsi/qla2xxx/qla_dbg.c linux-2.6.9-rc2-mm1_praka/drivers/scsi/qla2xxx/qla_dbg.c
--- linux-2.6.9-rc2-mm1/drivers/scsi/qla2xxx/qla_dbg.c	2004-09-16 14:30:38.000000000 -0700
+++ linux-2.6.9-rc2-mm1_praka/drivers/scsi/qla2xxx/qla_dbg.c	2004-09-16 14:36:18.907767776 -0700
@@ -712,7 +712,6 @@
 
 		/* Reset the ISP. */
 		WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
-		RD_REG_WORD(&reg->ctrl_status);		/* PCI Posting. */
 	}
 
 	for (cnt = 30000; RD_MAILBOX_REG(ha, reg, 0) != 0 &&
@@ -746,7 +745,6 @@
 
 			/* Release RISC. */
 			WRT_REG_WORD(&reg->hccr, HCCR_RELEASE_RISC);
-			RD_REG_WORD(&reg->hccr);	/* PCI Posting. */
 		}
 	}
 

Re: SCSI QLA not working on latest *-mm SN2

[Andrew Morton]

Andrew Vasquez <andrew.vasquez@qlogic.com> wrote:
>
> Andrew, please add Jesse's patch along with the patch I'm attaching to
> your tree.  I'll be sure to add the ia64 machine back into our test
> ring.

Could someone send me Jesse's patch?

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On Thursday, September 16, 2004 3:25 pm, Andrew Morton wrote:
> Andrew Vasquez <andrew.vasquez@qlogic.com> wrote:
> > Andrew, please add Jesse's patch along with the patch I'm attaching to
> > your tree.  I'll be sure to add the ia64 machine back into our test
> > ring.
>
> Could someone send me Jesse's patch?

Here it is.

Reduce some overaggressive compensation for PCI write posting.  In some cases 
we don't actually want to read a value back, since the card could be 
resetting.

Signed-off-by: Jesse Barnes <jbarnes@sgi.com>

Thanks,
Jesse

[-- Attachment #2: qla2xxx-less-posting.patch --]
[-- Type: text/plain, Size: 1727 bytes --]

diff -Napur -X /home/jbarnes/dontdiff linux-2.6.9-rc2-mm1.orig/drivers/scsi/qla2xxx/qla_init.c linux-2.6.9-rc2-mm1/drivers/scsi/qla2xxx/qla_init.c
--- linux-2.6.9-rc2-mm1.orig/drivers/scsi/qla2xxx/qla_init.c	2004-09-16 09:35:50.000000000 -0700
+++ linux-2.6.9-rc2-mm1/drivers/scsi/qla2xxx/qla_init.c	2004-09-16 14:00:32.000000000 -0700
@@ -463,13 +463,10 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 	}
 
 	WRT_REG_WORD(&reg->hccr, HCCR_CLR_RISC_INT);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 	WRT_REG_WORD(&reg->hccr, HCCR_CLR_HOST_INT);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	/* Reset ISP chip. */
 	WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
-	RD_REG_WORD(&reg->ctrl_status);			/* PCI Posting. */
 
 	/* Wait for RISC to recover from reset. */
 	if (IS_QLA2100(ha) || IS_QLA2200(ha) || IS_QLA2300(ha)) {
@@ -490,7 +487,6 @@ qla2x00_reset_chip(scsi_qla_host_t *ha) 
 
 	/* Reset RISC processor. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RESET_RISC);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	WRT_REG_WORD(&reg->semaphore, 0);
 
@@ -559,7 +555,6 @@ qla2x00_chip_diag(scsi_qla_host_t *ha)
 
 	/* Reset ISP chip. */
 	WRT_REG_WORD(&reg->ctrl_status, CSR_ISP_SOFT_RESET);
-	RD_REG_WORD(&reg->ctrl_status);			/* PCI Posting. */
 
 	/*
 	 * We need to have a delay here since the card will not respond while
@@ -581,9 +576,7 @@ qla2x00_chip_diag(scsi_qla_host_t *ha)
 
 	/* Reset RISC processor. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RESET_RISC);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 	WRT_REG_WORD(&reg->hccr, HCCR_RELEASE_RISC);
-	RD_REG_WORD(&reg->hccr);			/* PCI Posting. */
 
 	/* Workaround for QLA2312 PCI parity error */
 	if (IS_QLA2100(ha) || IS_QLA2200(ha) || IS_QLA2300(ha)) {

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Thursday, September 16, 2004 3:29 pm, Jesse Barnes wrote:
> On Thursday, September 16, 2004 3:25 pm, Andrew Morton wrote:
> > Andrew Vasquez <andrew.vasquez@qlogic.com> wrote:
> > > Andrew, please add Jesse's patch along with the patch I'm attaching to
> > > your tree.  I'll be sure to add the ia64 machine back into our test
> > > ring.
> >
> > Could someone send me Jesse's patch?
>
> Here it is.
>
> Reduce some overaggressive compensation for PCI write posting.  In some
> cases we don't actually want to read a value back, since the card could be
> resetting.
>
> Signed-off-by: Jesse Barnes <jbarnes@sgi.com>

Btw Andrew (Vasquez), there's a small doc I put together that should describe 
when you have to worry about PCI posting.  It's in the tree:  
Documentation/io_ordering.txt.  If it's incomplete or confusing, just let me 
know and I'll update it.

Thanks,
Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Grant Grundler]

Jesse Barnes wrote:
...
> Btw Andrew (Vasquez), there's a small doc I put together that should describe 
> when you have to worry about PCI posting.  It's in the tree:  
> Documentation/io_ordering.txt.  If it's incomplete or confusing, just let me 
> know and I'll update it.

Jesse,
Both. incomplete and confusing.
"concrete example of a hypothetical driver" wasn't my first warning
this document needed work. :^)

I've hacked up the 2.6.9 version and even what I did still needs more work.
Have time to correct my mistakes and answer the questions I ask?

I'd be happy to review it again after you've done another round on it.

[]'s should all go away - used those to mark editorial notes.

hth,
grant


--------------------- cut here ------------------

Weakly Ordered Memory Mapped IO
-------------------------------

SGI Altix chipset implements weakly ordered Memory-Mapped I/O writes.
On this platform, driver writers are responsible for ensuring I/O writes
to memory-mapped addresses arrive in the order intended.

Like for PCI write posting problems, this is done by reading
a 'safe' device or bridge register, causing the I/O chipset to
flush pending writes to the device before any reads are issued.
A driver would issue the "safe" read immediately prior to the exit
of a critical section of code protected by spinlocks.  This would
ensure subsequent writes to I/O space arrived only after all prior
writes (much like a memory barrier op, mb(), only with respect to I/O).

Note: MMIO reads are expensive! Don't add MMIO reads after *every* MMIO
      write unless the device programming model absolutely requires it.


An example from a hypothetical device driver might help:

        ...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  val = readl(my_status);
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  val = readl(my_status);
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
        ...

In the case above, the device may receive newval2 before it receives newval,
which could cause problems.

[ Is this example broken or am I just staying up too late?
  The example is doing a readl() in the second critical section.
  Shouldn't that enforce the write ordering?
]

Fixing it is easy enough though:

        ...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  val = readl(my_status);
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  (void)readl(safe_register); /* maybe a config register? */
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  val = readl(my_status);
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  (void)readl(safe_register); /* maybe a config register? */
CPU B:  spin_unlock_irqrestore(&dev_lock, flags)

The reads from safe_register will cause the I/O chipset to flush any
pending writes before actually posting the read to the chipset, preventing
possible data corruption.

[ How about interactions with:
  o read_relaxed()?
  o DMA?
  o IO Port space reads?
  o IO Port space writes?
]

Documentation/io_ordering.txt is wrong

[Matthew Wilcox]

On Sat, Sep 18, 2004 at 12:10:01AM -0600, Grant Grundler wrote:
> Jesse Barnes wrote:
> ...
> > Btw Andrew (Vasquez), there's a small doc I put together that should describe 
> > when you have to worry about PCI posting.  It's in the tree:  
> > Documentation/io_ordering.txt.  If it's incomplete or confusing, just let me 
> > know and I'll update it.
> 
> Jesse,
> Both. incomplete and confusing.
> "concrete example of a hypothetical driver" wasn't my first warning
> this document needed work. :^)

Not just incomplete and confusing, but actively wrong.  spin_lock/
spin_unlock should imply ordering of readb().  What you're describing
there is __readb().  See Documentation/DocBook/deviceiobook.tmpl.  Also,
rmb() should ensure ordering of io reads; there should be no need to
touch the device.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: Documentation/io_ordering.txt is wrong

[Jesse Barnes]

On Saturday, September 18, 2004 10:57 am, Matthew Wilcox wrote:
> On Sat, Sep 18, 2004 at 12:10:01AM -0600, Grant Grundler wrote:
> > Jesse Barnes wrote:
> > ...
> >
> > > Btw Andrew (Vasquez), there's a small doc I put together that should
> > > describe when you have to worry about PCI posting.  It's in the tree:
> > > Documentation/io_ordering.txt.  If it's incomplete or confusing, just
> > > let me know and I'll update it.
> >
> > Jesse,
> > Both. incomplete and confusing.
> > "concrete example of a hypothetical driver" wasn't my first warning
> > this document needed work. :^)
>
> Not just incomplete and confusing, but actively wrong.  spin_lock/
> spin_unlock should imply ordering of readb().  What you're describing
> there is __readb().  See Documentation/DocBook/deviceiobook.tmpl.  Also,
> rmb() should ensure ordering of io reads; there should be no need to
> touch the device.

I already sent a reply to this, so you now know that I was wrong in describing 
the ordering issue as one of weak ordering, rather it's just supposed to be 
describing simple write posting.  Also, the first sample code is incorrect, 
since the second spinlock protected region has a read before a write and 
incorrectly states that the second write may arrive before the first.  I 
think Grant has some updates, if not I'll post a patch to fixup the doc.

Thanks,
Jesse

Re: Documentation/io_ordering.txt is wrong

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

On Saturday, September 18, 2004 10:57 am, Matthew Wilcox wrote:
> On Sat, Sep 18, 2004 at 12:10:01AM -0600, Grant Grundler wrote:
> > Jesse Barnes wrote:
> > ...
> >
> > > Btw Andrew (Vasquez), there's a small doc I put together that should
> > > describe when you have to worry about PCI posting.  It's in the tree:
> > > Documentation/io_ordering.txt.  If it's incomplete or confusing, just
> > > let me know and I'll update it.
> >
> > Jesse,
> > Both. incomplete and confusing.
> > "concrete example of a hypothetical driver" wasn't my first warning
> > this document needed work. :^)
>
> Not just incomplete and confusing, but actively wrong.  spin_lock/
> spin_unlock should imply ordering of readb().  What you're describing
> there is __readb().  See Documentation/DocBook/deviceiobook.tmpl.  Also,
> rmb() should ensure ordering of io reads; there should be no need to
> touch the device.

Is this any better?  I just sent it off to Grant too.

Thanks,
Jesse

[-- Attachment #2: io_ordering.txt --]
[-- Type: text/plain, Size: 2098 bytes --]

Dealing with posted writes
--------------------------

On some platforms platforms, driver writers are responsible for
ensuring that I/O writes to memory-mapped addresses on their device
arrive in the order intended.  This is typically done by reading a
'safe' device or bridge register, causing the I/O chipset to flush
pending writes to the device before any reads are posted.  A driver
would usually use this technique immediately prior to the exit of a
critical section of code protected by spinlocks.  This would ensure
that subsequent writes to I/O space arrived only after all prior
writes (much like a memory barrier op, mb(), only with respect to
I/O).

Some pseudocode to illustrate the problem:

        ...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
        ...

In the case above, the device may receive newval2 before it receives newval,
which could cause problems.  Fixing it is easy enough though:

        ...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  (void)readl(safe_register); /* maybe a config register? */
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  (void)readl(safe_register); /* or read_relaxed() */
CPU B:  spin_unlock_irqrestore(&dev_lock, flags)

Here, the reads from safe_register will cause the I/O chipset to flush any
pending writes before actually posting the read to the chipset, preventing
possible data corruption.

This sort of synchronization is only necessary for read/write calls,
not in/out calls, since they're by definition strongly ordered.

We should probably add a writeflush call or something to deal with the
above in an easier to read way.  Some platforms could even implement
such a routine more efficiently than a regular read.

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Friday, September 17, 2004 11:10 pm, Grant Grundler wrote:
> Jesse Barnes wrote:
> ...
>
> > Btw Andrew (Vasquez), there's a small doc I put together that should
> > describe when you have to worry about PCI posting.  It's in the tree:
> > Documentation/io_ordering.txt.  If it's incomplete or confusing, just let
> > me know and I'll update it.
>
> Jesse,
> Both. incomplete and confusing.
> "concrete example of a hypothetical driver" wasn't my first warning
> this document needed work. :^)

Heh, yeah I noticed that on re-reading too :)

> I've hacked up the 2.6.9 version and even what I did still needs more work.
> Have time to correct my mistakes and answer the questions I ask?
>
> I'd be happy to review it again after you've done another round on it.
>
> []'s should all go away - used those to mark editorial notes.

Sure, thanks for reading it.

> Weakly Ordered Memory Mapped IO
> -------------------------------
>
> SGI Altix chipset implements weakly ordered Memory-Mapped I/O writes.
> On this platform, driver writers are responsible for ensuring I/O writes
> to memory-mapped addresses arrive in the order intended.

This is incorrect.  I wrote this before I understood that 'pci write posting' 
was the common term for describing the fact that writes from different CPUs 
could arrive out of order.  A s/weakly ordered/write posting would make the 
existing document much more accurate.

> Like for PCI write posting problems, this is done by reading
> a 'safe' device or bridge register, causing the I/O chipset to
> flush pending writes to the device before any reads are issued.
> A driver would issue the "safe" read immediately prior to the exit
> of a critical section of code protected by spinlocks.  This would
> ensure subsequent writes to I/O space arrived only after all prior
> writes (much like a memory barrier op, mb(), only with respect to I/O).
>
> Note: MMIO reads are expensive! Don't add MMIO reads after *every* MMIO
>       write unless the device programming model absolutely requires it.

Yes!

>
>
> An example from a hypothetical device driver might help:
>
>         ...
> CPU A:  spin_lock_irqsave(&dev_lock, flags)
> CPU A:  val = readl(my_status);
> CPU A:  ...
> CPU A:  writel(newval, ring_ptr);
> CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
>         ...
> CPU B:  spin_lock_irqsave(&dev_lock, flags)
> CPU B:  val = readl(my_status);
> CPU B:  ...
> CPU B:  writel(newval2, ring_ptr);
> CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
>         ...
>
> In the case above, the device may receive newval2 before it receives
> newval, which could cause problems.
>
> [ Is this example broken or am I just staying up too late?
>   The example is doing a readl() in the second critical section.
>   Shouldn't that enforce the write ordering?
> ]

Yep, that's a bug.  It should just be writes.

> The reads from safe_register will cause the I/O chipset to flush any
> pending writes before actually posting the read to the chipset, preventing
> possible data corruption.
>
> [ How about interactions with:
>   o read_relaxed()?
>   o DMA?
>   o IO Port space reads?
>   o IO Port space writes?
> ]

None that I know of.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Grant Grundler]

On Mon, Sep 20, 2004 at 03:40:02PM -0700, Jesse Barnes wrote:
> > Weakly Ordered Memory Mapped IO
> > -------------------------------
> >
> > SGI Altix chipset implements weakly ordered Memory-Mapped I/O writes.
> > On this platform, driver writers are responsible for ensuring I/O writes
> > to memory-mapped addresses arrive in the order intended.
> 
> This is incorrect.  I wrote this before I understood that 'pci write posting' 
> was the common term for describing the fact that writes from different CPUs 
> could arrive out of order.  A s/weakly ordered/write posting would make the 
> existing document much more accurate.

"write posting" is orthogonal to PCI ordering rules.
AFAIK, Write posting is not specific to PCI - but any memory mapped IO.

I understand "write posting" as when the CPU posts the write
to the chipset and the chipset says the write is done even though
it hasn't reached the PCI device. It just means the write has reached
the PCI "domain" (which is supposed to be strongly ordered).

Page 304 of "ia-64 linux kernel" book decsribes how writel ordering
(within a CPU) is enforced with "release semantics" (ie .rel in asm ).
IA64 spinlocks also use .rel and thus I'm pretty sure from the CPU
PoV, writel()s will be posted by the releasing the lock.
I think willy pointed at the docs describing this in general terms.

Secondly, I don't recall hearing about problems like this
on Intel or HP ia64 machines. I've only run into PCI posted write
and DMA syncronization problems where the drivers aren't following
all the rules quite right (missing mb() and readl()'s mostly).

So far, I still think this document is misnamed and should
be called something like "SGI Altix porting issues" and moved
under the Documentation/ia64 directory.


> > [ Is this example broken or am I just staying up too late?
> >   The example is doing a readl() in the second critical section.
> >   Shouldn't that enforce the write ordering?
> > ]
> 
> Yep, that's a bug.  It should just be writes.

ok. Can you fix that up and post a new version where I can see it?

> > The reads from safe_register will cause the I/O chipset to flush any
> > pending writes before actually posting the read to the chipset, preventing
> > possible data corruption.
> >
> > [ How about interactions with:
> >   o read_relaxed()?
> >   o DMA?
> >   o IO Port space reads?
> >   o IO Port space writes?
> > ]
> 
> None that I know of.

You mean none that are surprising to you?
ie writes can pass read_relaxed() transactions or vice versa?
DMA read returns can bypass MMIO writes? (parisc chipsets allow this)

IIRC, IO port space writes are NOT posted.
So the rules for ordering must be impacted or different somehow.
ie Are IO Port space writes strongly ordered WRT MMIO space writes?

thanks,
grant

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

On Monday, September 20, 2004 4:27 pm, Grant Grundler wrote:
> "write posting" is orthogonal to PCI ordering rules.
> AFAIK, Write posting is not specific to PCI - but any memory mapped IO.

Right.

> I understand "write posting" as when the CPU posts the write
> to the chipset and the chipset says the write is done even though
> it hasn't reached the PCI device. It just means the write has reached
> the PCI "domain" (which is supposed to be strongly ordered).

That's my understanding too.

> Secondly, I don't recall hearing about problems like this
> on Intel or HP ia64 machines. I've only run into PCI posted write
> and DMA syncronization problems where the drivers aren't following
> all the rules quite right (missing mb() and readl()'s mostly).

Problems like what?  If mmio writes are posted, then the driver has to deal 
with it with reads like you said.  If the example code was fixed to lose the 
read() in the second spinlock protected region, I think it would describe 
mmio write posting accurately, no?

> So far, I still think this document is misnamed and should
> be called something like "SGI Altix porting issues" and moved
> under the Documentation/ia64 directory.

But it has nothing to do with Altix at all...

>
> > > [ Is this example broken or am I just staying up too late?
> > >   The example is doing a readl() in the second critical section.
> > >   Shouldn't that enforce the write ordering?
> > > ]
> >
> > Yep, that's a bug.  It should just be writes.
>
> ok. Can you fix that up and post a new version where I can see it?
>
> > > The reads from safe_register will cause the I/O chipset to flush any
> > > pending writes before actually posting the read to the chipset,
> > > preventing possible data corruption.
> > >
> > > [ How about interactions with:
> > >   o read_relaxed()?
> > >   o DMA?
> > >   o IO Port space reads?
> > >   o IO Port space writes?
> > > ]
> >
> > None that I know of.
>
> You mean none that are surprising to you?
> ie writes can pass read_relaxed() transactions or vice versa?
> DMA read returns can bypass MMIO writes? (parisc chipsets allow this)

No, as far as mmio ordering goes, read_relaxed is exactly the same as read, so 
in the example code, a read_relaxed would be sufficient for write ordering.

> IIRC, IO port space writes are NOT posted.
> So the rules for ordering must be impacted or different somehow.
> ie Are IO Port space writes strongly ordered WRT MMIO space writes?

Right, they're supposed to be strongly ordered, I think arches are supposed to 
guarantee that in their in/out routines.

Here's a new version that should be clearer.

Thanks,
Jesse

[-- Attachment #2: io_ordering.txt --]
[-- Type: text/plain, Size: 2098 bytes --]

Dealing with posted writes
--------------------------

On some platforms platforms, driver writers are responsible for
ensuring that I/O writes to memory-mapped addresses on their device
arrive in the order intended.  This is typically done by reading a
'safe' device or bridge register, causing the I/O chipset to flush
pending writes to the device before any reads are posted.  A driver
would usually use this technique immediately prior to the exit of a
critical section of code protected by spinlocks.  This would ensure
that subsequent writes to I/O space arrived only after all prior
writes (much like a memory barrier op, mb(), only with respect to
I/O).

Some pseudocode to illustrate the problem:

        ...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
        ...

In the case above, the device may receive newval2 before it receives newval,
which could cause problems.  Fixing it is easy enough though:

        ...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  (void)readl(safe_register); /* maybe a config register? */
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  (void)readl(safe_register); /* or read_relaxed() */
CPU B:  spin_unlock_irqrestore(&dev_lock, flags)

Here, the reads from safe_register will cause the I/O chipset to flush any
pending writes before actually posting the read to the chipset, preventing
possible data corruption.

This sort of synchronization is only necessary for read/write calls,
not in/out calls, since they're by definition strongly ordered.

We should probably add a writeflush call or something to deal with the
above in an easier to read way.  Some platforms could even implement
such a routine more efficiently than a regular read.

Re: SCSI QLA not working on latest *-mm SN2

[Grant Grundler]

On Mon, Sep 20, 2004 at 05:09:44PM -0700, Jesse Barnes wrote:
> > Secondly, I don't recall hearing about problems like this
> > on Intel or HP ia64 machines. I've only run into PCI posted write
> > and DMA syncronization problems where the drivers aren't following
> > all the rules quite right (missing mb() and readl()'s mostly).
> 
> Problems like what?

I've never heard of multiple writes from different CPUs going out of order
to the PCI device.

> If mmio writes are posted, then the driver has to deal 
> with it with reads like you said.

No it doesn't. Only if it depends on *when* the write hits the device.
The classic example is:
	writel(x, CMD_RESET);
	udelay(10);
	readl(x+STATUS);	/* parisc will crash if not ready */

>   If the example code was fixed to lose the 
> read() in the second spinlock protected region, I think it would describe 
> mmio write posting accurately, no?

No.  Can you add something to the example that shows they expected
the writes to hit the device at a certain time?

The CPU would continue doing other work before the writes reach
the device but they would reach the device in order.
I'm pretty sure of that on most IA32, parisc, and IA64 platforms.

The only exceptions I'm aware of are some broken ia32 chipsets which
have issues with write ordering - see TG3_FLAG_MBOX_WRITE_REORDER usage
in drivers/net/tg3.*.
Comment says:
        /* If we have an AMD 762 or Intel ICH/ICH0/ICH2 chipset, write
         * reordering to the mailbox registers done by the host
         * controller can cause major troubles.  We read back from
         * every mailbox register write to force the writes to be
         * posted to the chip in order.
         */

I haven't seen any evidence of this happening yet on ia64.
If it is, then I'd really like to know about it and we should fix
tg3 since both HP and SGI ship product that depends on tg3 driver.

> > So far, I still think this document is misnamed and should
> > be called something like "SGI Altix porting issues" and moved
> > under the Documentation/ia64 directory.
> 
> But it has nothing to do with Altix at all...

Ok.
Can you be explicit on which platforms and which drivers
anyone at SGI has seen the ordering problem?
Why did you write this document in the first place?

The first sentence on the new version (below) still introduces
this as an ordering problem and not a write posting problem.

> > You mean none that are surprising to you?
> > ie writes can pass read_relaxed() transactions or vice versa?
> > DMA read returns can bypass MMIO writes? (parisc chipsets allow this)
> 
> No, as far as mmio ordering goes, read_relaxed is exactly the same as read,
> so in the example code, a read_relaxed would be sufficient for write ordering.

Ok - that's surprising to me and should be clearly stated.
I do not expect read_relaxed() to enforce ordering in either direction
of the data path - not for MMIO writes nor DMA writes.

> > IIRC, IO port space writes are NOT posted.
> > So the rules for ordering must be impacted or different somehow.
> > ie Are IO Port space writes strongly ordered WRT MMIO space writes?
> 
> Right, they're supposed to be strongly ordered, I think arches are
> supposed to guarantee that in their in/out routines.

Yes. Again, stating it in this document makes it clear what you
expect from the platform support code.

> Here's a new version that should be clearer.
> 
> Thanks,
> Jesse

> Dealing with posted writes
> --------------------------
> 
> On some platforms platforms, driver writers are responsible for
> ensuring that I/O writes to memory-mapped addresses on their device
> arrive in the order intended.

The writes will arrive in order according to PCI ordering rules.
Wasn't this supposed to be about write posting?

Documentation/DocBook/deviceiobook.tmpl has a paragraph on write posting.
I think a patch to deviceiobook.tmpl would be better than having
write posting discussed in a seperate file if you think it needs
an example.

> This is typically done by reading a
> 'safe' device or bridge register, causing the I/O chipset to flush
> pending writes to the device before any reads are posted.  A driver
> would usually use this technique immediately prior to the exit of a
> critical section of code protected by spinlocks.  This would ensure
> that subsequent writes to I/O space arrived only after all prior
> writes (much like a memory barrier op, mb(), only with respect to
> I/O).
> 
> Some pseudocode to illustrate the problem:
> 
>         ...
> CPU A:  spin_lock_irqsave(&dev_lock, flags)
> CPU A:  ...
> CPU A:  writel(newval, ring_ptr);
> CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
>         ...
> CPU B:  spin_lock_irqsave(&dev_lock, flags)
> CPU B:  ...
> CPU B:  writel(newval2, ring_ptr);
> CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
>         ...

I'm pretty sure spinlocks are supposed to provide memory barriers.
Maybe that's only to gcc so it doesn't re-order loads/stores around
a spinlock. But I thought the ia64 implemention used the same "release"
and "acquire" semantics as readX() and writeX() do.

The alph implementation explicitly enforces it:
(arch/alpha/lib/io.c)

	void _writel(u32 b, unsigned long addr)
	{
		__writel(b, addr);
		mb();
	}


> In the case above, the device may receive newval2 before it receives newval,
> which could cause problems.  Fixing it is easy enough though:
> 
>         ...
> CPU A:  spin_lock_irqsave(&dev_lock, flags)
> CPU A:  ...
> CPU A:  writel(newval, ring_ptr);
> CPU A:  (void)readl(safe_register); /* maybe a config register? */
> CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
>         ...
> CPU B:  spin_lock_irqsave(&dev_lock, flags)
> CPU B:  ...
> CPU B:  writel(newval2, ring_ptr);
> CPU B:  (void)readl(safe_register); /* or read_relaxed() */
> CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
> 
> Here, the reads from safe_register will cause the I/O chipset to flush any
> pending writes before actually posting the read to the chipset, preventing
> possible data corruption.

That should probably be
	 "...flush any posted writes before posting the read return...".


> This sort of synchronization is only necessary for read/write calls,
> not in/out calls, since they're by definition strongly ordered.

Similarly:
	inX and outX calls are strongly ordered and non-postable.
	They do not need special handling. But this is something to watch out
	for when converting drivers to use MMIO space from IO Port space.

> We should probably add a writeflush call or something to deal with the
> above in an easier to read way.  Some platforms could even implement
> such a routine more efficiently than a regular read.

thanks,
grant

Re: SCSI QLA not working on latest *-mm SN2

[Jeremy Higdon]

Lots of issues covered.

I'd like to cover one of them first, since it is an underlying
principle in the discussion.

On Mon, Sep 20, 2004 at 11:46:26PM -0600, Grant Grundler wrote:
> On Mon, Sep 20, 2004 at 05:09:44PM -0700, Jesse Barnes wrote:
> > > Secondly, I don't recall hearing about problems like this
> > > on Intel or HP ia64 machines. I've only run into PCI posted write
> > > and DMA syncronization problems where the drivers aren't following
> > > all the rules quite right (missing mb() and readl()'s mostly).
> > 
> > Problems like what?
> 
> I've never heard of multiple writes from different CPUs going out of order
> to the PCI device.

It was my understanding that this could be a problem on any
MP machine where the CPUs use a write buffer (which is just
about everything today).

The question seems to be whether release semantics (or equivalent
on other chips) in the IA64 apply to MMIO writes.  I believe that
they do not.  It seems that you think that it does.

On Altix, we ran into a problem with the qla1280 driver (see
version 1.56 in the scsi-misc-2.6 bk tree) because the spinunlock
(apparently) did not imply a retirement of a previous mmio write.
In that rev, I added an mmio read to so that the mmio write would
be completed before releasing the spinlock (I believe the host
lock held during the call to queuecommand).

Before making that change, the problem was the two different CPUs
would mmio write to the Request In register, and the ordering
would flip, causing the qla1280 to think that it suddenly had
an entire request queue.  We didn't see this problem on puny
64p machines (at least not under ordinary stress testing); we
needed a 512p machine to see it, though odds are that it would
have occurred very occasionally on smaller machines.

jeremy

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 2:45 am, Jeremy Higdon wrote:
> It was my understanding that this could be a problem on any
> MP machine where the CPUs use a write buffer (which is just
> about everything today).
>
> The question seems to be whether release semantics (or equivalent
> on other chips) in the IA64 apply to MMIO writes.  I believe that
> they do not.  It seems that you think that it does.

I think it's even more subtle than that.  Stores with release semantics will 
be visible on other CPUs if they're written to cacheable space, but in the 
case of MMIO writes, it's the local receiving Hub that can cause the problem.  
It'll try to send the PIO along to its destination Hub as soon as possible, 
but due to congestion, credit counts, etc., the write may be delayed and 
occur *after* a write from another CPU whose Hub is closer.  But then again, 
I could be confused.

> On Altix, we ran into a problem with the qla1280 driver (see
> version 1.56 in the scsi-misc-2.6 bk tree) because the spinunlock
> (apparently) did not imply a retirement of a previous mmio write.
> In that rev, I added an mmio read to so that the mmio write would
> be completed before releasing the spinlock (I believe the host
> lock held during the call to queuecommand).
>
> Before making that change, the problem was the two different CPUs
> would mmio write to the Request In register, and the ordering
> would flip, causing the qla1280 to think that it suddenly had
> an entire request queue.  We didn't see this problem on puny
> 64p machines (at least not under ordinary stress testing); we
> needed a 512p machine to see it, though odds are that it would
> have occurred very occasionally on smaller machines.

Good example, thanks for reminding me.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 1:46 am, Grant Grundler wrote:
> I've never heard of multiple writes from different CPUs going out of order
> to the PCI device.

Are you sure?  Isn't that the definition of write posting?  At any rate, sn2 
has the write posting issue (which seems to call for a pioflush routine), but 
also a *potential* for out of order writes, which is what's described in this 
document.  There was a big discussion on lkml about this awhile back when I 
tried to push in an mmiob() (pioflush) API.  It was lacking arguments, 
however, and so was not really usable on many platforms.

> > If mmio writes are posted, then the driver has to deal
> > with it with reads like you said.
>
> No it doesn't. Only if it depends on *when* the write hits the device.
> The classic example is:
>  writel(x, CMD_RESET);
>  udelay(10);
>  readl(x+STATUS); /* parisc will crash if not ready */
>
> >   If the example code was fixed to lose the
> > read() in the second spinlock protected region, I think it would describe
> > mmio write posting accurately, no?
>
> No.  Can you add something to the example that shows they expected
> the writes to hit the device at a certain time?

Yeah, the qla2xxx driver is a perfect example of this.  But we don't have an 
alternative to udelay at this point, maybe we need pioflush(device *, 
unsigned long addr)?  That would allow arch code to read from the device's 
bridge config space, or at the very least do a delay (a maximal value, 
guaranteed to not be exceeded) and read addr?

> The CPU would continue doing other work before the writes reach
> the device but they would reach the device in order.
> I'm pretty sure of that on most IA32, parisc, and IA64 platforms.

Not on sn2.  The local chipset could receive the write, the process could be 
rescheduled to another cpu closer to the target device, and its write (the 
second one) could get there first.  So on sn2 we need a read following writes 
prior to spinlock release.

> The only exceptions I'm aware of are some broken ia32 chipsets which
> have issues with write ordering - see TG3_FLAG_MBOX_WRITE_REORDER usage
> in drivers/net/tg3.*.
> Comment says:
>         /* If we have an AMD 762 or Intel ICH/ICH0/ICH2 chipset, write
>          * reordering to the mailbox registers done by the host
>          * controller can cause major troubles.  We read back from
>          * every mailbox register write to force the writes to be
>          * posted to the chip in order.
>          */
>
> I haven't seen any evidence of this happening yet on ia64.
> If it is, then I'd really like to know about it and we should fix
> tg3 since both HP and SGI ship product that depends on tg3 driver.

This will only happen if the writes come from different CPUs.  A stream of 
program order writes under a spinlock will *not* be reordered (that's a big 
hw bug IMO).

> > > So far, I still think this document is misnamed and should
> > > be called something like "SGI Altix porting issues" and moved
> > > under the Documentation/ia64 directory.
> >
> > But it has nothing to do with Altix at all...
>
> Ok.
> Can you be explicit on which platforms and which drivers
> anyone at SGI has seen the ordering problem?
> Why did you write this document in the first place?

To help people understand that they need to deal with the issue.

> The first sentence on the new version (below) still introduces
> this as an ordering problem and not a write posting problem.
>
> > > You mean none that are surprising to you?
> > > ie writes can pass read_relaxed() transactions or vice versa?
> > > DMA read returns can bypass MMIO writes? (parisc chipsets allow this)
> >
> > No, as far as mmio ordering goes, read_relaxed is exactly the same as
> > read, so in the example code, a read_relaxed would be sufficient for
> > write ordering.
>
> Ok - that's surprising to me and should be clearly stated.
> I do not expect read_relaxed() to enforce ordering in either direction
> of the data path - not for MMIO writes nor DMA writes.

But it *has* to for mmio writes.  Not only do those transactions occur on the 
same 'channel', but the read may depend on the side effects of prior writes.

> Yes. Again, stating it in this document makes it clear what you
> expect from the platform support code.

Ok.

> The writes will arrive in order according to PCI ordering rules.
> Wasn't this supposed to be about write posting?

More confusion.  It's about write posting with a small, added wrinkle.  The 
issues are related enough that it probably makes sense to include them in the 
same doc (though I had neglected to describe the fact that writes won't get 
to the device right away).

> Documentation/DocBook/deviceiobook.tmpl has a paragraph on write posting.
> I think a patch to deviceiobook.tmpl would be better than having
> write posting discussed in a seperate file if you think it needs
> an example.

Ok, that sounds good.

> I'm pretty sure spinlocks are supposed to provide memory barriers.
> Maybe that's only to gcc so it doesn't re-order loads/stores around
> a spinlock. But I thought the ia64 implemention used the same "release"
> and "acquire" semantics as readX() and writeX() do.

This has nothing to do with weak ordering (I was wrong to state it that way, 
and ended up just making for a confusing analogy).  It has to do with writes 
coming from different nodes on the NUMA fabric.

> > Here, the reads from safe_register will cause the I/O chipset to flush
> > any pending writes before actually posting the read to the chipset,
> > preventing possible data corruption.
>
> That should probably be
>   "...flush any posted writes before posting the read return...".

Ok.

>
> > This sort of synchronization is only necessary for read/write calls,
> > not in/out calls, since they're by definition strongly ordered.
>
> Similarly:
>  inX and outX calls are strongly ordered and non-postable.
>  They do not need special handling. But this is something to watch out
>  for when converting drivers to use MMIO space from IO Port space.

Sounds good.

Thanks again,
Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

On Tuesday, September 21, 2004 1:46 am, Grant Grundler wrote:
> No it doesn't. Only if it depends on *when* the write hits the device.
> The classic example is:
>  writel(x, CMD_RESET);
>  udelay(10);
>  readl(x+STATUS); /* parisc will crash if not ready */

Ok, hopefully I've covered this in this release (patch to deviceiobook will 
come later).

The short of it is that we really need pioflush.  I'll resurrect my mmiob 
patches, change the name and prototype, and resubmit.

Thanks,
Jesse

[-- Attachment #2: io_ordering.txt --]
[-- Type: text/plain, Size: 3466 bytes --]

Dealing with posted writes
--------------------------

On some platforms platforms, driver writers are responsible for
ensuring that I/O writes to memory-mapped addresses on their device
arrive when expected and in the order intended.  This is typically
done by reading a 'safe' device or bridge register, causing the I/O
chipset to flush pending writes to the device before any reads are
posted.  A driver would usually use this technique immediately prior
to the exit of a critical section of code protected by spinlocks.
This would ensure that subsequent writes to I/O space arrived only
after all prior writes (much like a memory barrier op, mb(), only with
respect to I/O).

Some pseudocode to illustrate the problem of write posting:

...
spin_lock_irqsave(&dev_lock, flags)
...
writel(resetval, reset_reg); /* reset the card */
udelay(10); /* wait for reset (also needs pioflush) */
val = readl(ring_ptr); /* read initial value */
spin_unlock_irqrestore(&dev_lock, flags)
...

In this case, the card is reset by the first write.  The driver
attempts to wait for the completion of the reset using udelay.  But
since the write may be delayed and the udelay will probably start
executing right away, it may be that there's not enough time for the
write to actually arrive at the card and for the reset to occur before
the read is executed.  On some platforms, this can result in a machine
check.  Without a pioflush routine, the udelay must account for worst
case behavior.

And an example of reordering of writes between CPUs on a NUMA machine:

	...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  (void)readl(safe_register); /* or read_relaxed() */

In the case above, the device may receive newval2 before it receives newval,
which could cause problems.  Fixing it is easy enough though:

        ...
CPU A:  spin_lock_irqsave(&dev_lock, flags)
CPU A:  ...
CPU A:  writel(newval, ring_ptr);
CPU A:  (void)readl(safe_register); /* maybe a config register? */
CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
        ...
CPU B:  spin_lock_irqsave(&dev_lock, flags)
CPU B:  ...
CPU B:  writel(newval2, ring_ptr);
CPU B:  (void)readl(safe_register); /* or read_relaxed() */
CPU B:  spin_unlock_irqrestore(&dev_lock, flags)

Here, the reads from safe_register will cause the I/O chipset to flush any
posted writes before actually sending the read to the chipset, preventing
possible data corruption.

inX and outX calls, on the other hand, are strongly ordered and
non-postable.  They do not need special handling. But this is
something to watch out for when converting drivers to use MMIO space
from IO Port space.

A new pioflush routine could address both of the above problems
(though drivers would still have to know how long to wait for card
resets).  It would ensure that pio writes had arrived at their
destination device before allowing executing in the current context to
continue.  Since some platforms would only be able to achieve this
through a read of a bridge config register, I think a prototype like:
  pioflush(struct device *dev, unsigned long addr);
would be necessary.  The dev argument would correspond to the device
in question, and the addr argument would be a safe register to read on
the device.  Either could be zero, but not both.

Re: SCSI QLA not working on latest *-mm SN2

[James Bottomley]

On Tue, 2004-09-21 at 11:13, Jesse Barnes wrote:
> On Tuesday, September 21, 2004 1:46 am, Grant Grundler wrote:
> > No it doesn't. Only if it depends on *when* the write hits the device.
> > The classic example is:
> >  writel(x, CMD_RESET);
> >  udelay(10);
> >  readl(x+STATUS); /* parisc will crash if not ready */
> 
> Ok, hopefully I've covered this in this release (patch to deviceiobook will 
> come later).
> 
> The short of it is that we really need pioflush.  I'll resurrect my mmiob 
> patches, change the name and prototype, and resubmit.

Just to get back to the actual qla2xxx problem.

Do we agree that there are two possible solutions:

1) Find a safe mmio read to trigger the flush of the posted reset write.

2) Use a pio write to trigger the reset because we know the reset is
active as soon as the write returns.

and if so, which one are we going to implement?

James

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 11:41 am, James Bottomley wrote:
> Just to get back to the actual qla2xxx problem.
>
> Do we agree that there are two possible solutions:
>
> 1) Find a safe mmio read to trigger the flush of the posted reset write.
>
> 2) Use a pio write to trigger the reset because we know the reset is
> active as soon as the write returns.
>
> and if so, which one are we going to implement?

or
 3) use a regular write() with an associated pioflush()

For now, (1) is a good option if the driver can do it (I still haven't heard 
if qla2xxx supports this).  I suspect that neither (1) nor (2) is available 
to some device, leaving only udelay() or (3), which I'm working on.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Matthew Wilcox]

On Tue, Sep 21, 2004 at 11:58:24AM -0400, Jesse Barnes wrote:
> On Tuesday, September 21, 2004 11:41 am, James Bottomley wrote:
> > Just to get back to the actual qla2xxx problem.
> >
> > Do we agree that there are two possible solutions:
> >
> > 1) Find a safe mmio read to trigger the flush of the posted reset write.
> >
> > 2) Use a pio write to trigger the reset because we know the reset is
> > active as soon as the write returns.
> >
> > and if so, which one are we going to implement?
> 
> or
>  3) use a regular write() with an associated pioflush()
> 
> For now, (1) is a good option if the driver can do it (I still haven't heard 
> if qla2xxx supports this).  I suspect that neither (1) nor (2) is available 
> to some device, leaving only udelay() or (3), which I'm working on.

How can we do pioflush()?  My understanding of PCI ordering rules is that
we need to go all the way down to the device to prevent some intermediate
bridge from delaying the write arbitrarily.  So we'd actually need the
pci_dev and read its device ID back from config space, or something.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 12:01 pm, Matthew Wilcox wrote:
> How can we do pioflush()?  My understanding of PCI ordering rules is that
> we need to go all the way down to the device to prevent some intermediate
> bridge from delaying the write arbitrarily.  So we'd actually need the
> pci_dev and read its device ID back from config space, or something.

Reading from the closest bridge won't be enough?  If not, then dealing with 
posting in a nice way is simply impossible for some devices.  We'd be stuck 
with udelay().

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[James Bottomley]

On Tue, 2004-09-21 at 12:05, Jesse Barnes wrote:
> Reading from the closest bridge won't be enough?  If not, then dealing with 
> posting in a nice way is simply impossible for some devices.  We'd be stuck 
> with udelay().

That's correct.  The posted write is held somewhere in one of the
bridges, but the PCI ordering rules only require it to be ordered with
other MMIO reads to the *device*; so you only guarantee that the posted
write is flushed by doing a device read....obviously, there exist
bridges with looser ideas than this, but we need to follow the PCI
specs.

udelay() isn't a viable solution because the PCI specs have no upper
bound on the length of time a write may remain posted.

James

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 12:11 pm, James Bottomley wrote:
> On Tue, 2004-09-21 at 12:05, Jesse Barnes wrote:
> > Reading from the closest bridge won't be enough?  If not, then dealing
> > with posting in a nice way is simply impossible for some devices.  We'd
> > be stuck with udelay().
>
> That's correct.  The posted write is held somewhere in one of the
> bridges, but the PCI ordering rules only require it to be ordered with
> other MMIO reads to the *device*; so you only guarantee that the posted
> write is flushed by doing a device read....obviously, there exist
> bridges with looser ideas than this, but we need to follow the PCI
> specs.
>
> udelay() isn't a viable solution because the PCI specs have no upper
> bound on the length of time a write may remain posted.

Well then.  Sounds like we're hosed for the qla2xxx case at least.  I think we 
still need pioflush for the case of writes from different CPUs (described in 
the document).  Using it *should* make the window pretty small for qla2xxx 
type problems too, but we have no guarantee.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[James Bottomley]

On Tue, 2004-09-21 at 12:18, Jesse Barnes wrote:
> Well then.  Sounds like we're hosed for the qla2xxx case at least.  I think we 
> still need pioflush for the case of writes from different CPUs (described in 
> the document).  Using it *should* make the window pretty small for qla2xxx 
> type problems too, but we have no guarantee.

Not if we can do a PIO write ... PIO writes aren't posted.

James

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

On Tuesday, September 21, 2004 12:11 pm, James Bottomley wrote:
> On Tue, 2004-09-21 at 12:05, Jesse Barnes wrote:
> > Reading from the closest bridge won't be enough?  If not, then dealing
> > with posting in a nice way is simply impossible for some devices.  We'd
> > be stuck with udelay().
>
> That's correct.  The posted write is held somewhere in one of the
> bridges, but the PCI ordering rules only require it to be ordered with
> other MMIO reads to the *device*; so you only guarantee that the posted
> write is flushed by doing a device read....obviously, there exist
> bridges with looser ideas than this, but we need to follow the PCI
> specs.
>
> udelay() isn't a viable solution because the PCI specs have no upper
> bound on the length of time a write may remain posted.

Does this patch describe and correctly implement what we've discussed?  I've 
added information to io_ordering.txt to describe the conclusion about posting 
we seem to have agreed on.  Obviously, this is just a first cut.  It 
compiles, but I haven't made prototypes for any arch other than ia64.

Thanks,
Jesse

[-- Attachment #2: ioflush-ia64.patch --]
[-- Type: text/x-diff, Size: 11706 bytes --]

===== Documentation/io_ordering.txt 1.1 vs edited =====
--- 1.1/Documentation/io_ordering.txt	2003-03-18 02:02:11 -08:00
+++ edited/Documentation/io_ordering.txt	2004-09-21 09:59:09 -07:00
@@ -1,47 +1,75 @@
-On some platforms, so-called memory-mapped I/O is weakly ordered.  On such
-platforms, driver writers are responsible for ensuring that I/O writes to
-memory-mapped addresses on their device arrive in the order intended.  This is
-typically done by reading a 'safe' device or bridge register, causing the I/O
-chipset to flush pending writes to the device before any reads are posted.  A
-driver would usually use this technique immediately prior to the exit of a
-critical section of code protected by spinlocks.  This would ensure that
-subsequent writes to I/O space arrived only after all prior writes (much like a
-memory barrier op, mb(), only with respect to I/O).
+Dealing with posted writes
+--------------------------
 
-A more concrete example from a hypothetical device driver:
+Driver writers are responsible for ensuring that I/O writes to memory-mapped
+addresses on their device arrive when expected and in the order intended.
+This is typically done by reading a 'safe' device or bridge register, causing
+the I/O chipset to flush pending writes to the device before any reads are
+sent to the target device.  A driver would usually use this technique
+immediately prior to a read after a card reset or the exit of a critical
+section of code protected by spinlocks.  This would ensure that subsequent I/O
+space accesses arrived only after all prior writes.  There are really two
+issues at play here, one is 'posting', i.e. memory-mapped I/O writes not sent
+to the device immediately, and ordering, where on a large system writes from
+different CPUs may arrive out of order.
 
-        ...
+Some pseudocode to illustrate the problem of write posting:
+
+...
+spin_lock_irqsave(&dev_lock, flags)
+...
+writel(resetval, reset_reg); /* reset the card */
+udelay(10); /* wait for reset (also needs pioflush) */
+val = readl(ring_ptr); /* read initial value */
+spin_unlock_irqrestore(&dev_lock, flags)
+...
+
+In this case, the card is reset by the first write.  The driver attempts to
+wait for the completion of the reset using udelay.  But since the write may be
+delayed and the udelay will probably start executing right away, it may be
+that there's not enough time for the write to actually arrive at the card and
+for the reset to occur before the read is executed.  On some platforms, this
+can result in a machine check.  Unfortunately, there's no way to guarantee
+that a write has arrived at a device short of a read from the same address
+space, so in some cases, udelay() is the only option.  In any case, the driver
+should issue an ioflush() call prior to the udelay(), passing in 0 for the
+addr argument if no safe register exists.  This will allow the platform to
+make an effort to get the write as close to the device as possible before
+allowing the udelay to begin.
+
+And an example of reordering of writes between CPUs on a NUMA machine:
+
+	...
 CPU A:  spin_lock_irqsave(&dev_lock, flags)
-CPU A:  val = readl(my_status);
 CPU A:  ...
 CPU A:  writel(newval, ring_ptr);
 CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
         ...
 CPU B:  spin_lock_irqsave(&dev_lock, flags)
-CPU B:  val = readl(my_status);
 CPU B:  ...
 CPU B:  writel(newval2, ring_ptr);
-CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
-        ...
+CPU B:  (void)readl(safe_register); /* or read_relaxed() */
 
 In the case above, the device may receive newval2 before it receives newval,
 which could cause problems.  Fixing it is easy enough though:
 
         ...
 CPU A:  spin_lock_irqsave(&dev_lock, flags)
-CPU A:  val = readl(my_status);
 CPU A:  ...
 CPU A:  writel(newval, ring_ptr);
-CPU A:  (void)readl(safe_register); /* maybe a config register? */
+CPU A:  ioflush(dev, safe_register);
 CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
         ...
 CPU B:  spin_lock_irqsave(&dev_lock, flags)
-CPU B:  val = readl(my_status);
 CPU B:  ...
 CPU B:  writel(newval2, ring_ptr);
-CPU B:  (void)readl(safe_register); /* maybe a config register? */
+CPU B:  ioflush(dev, safe_register);
 CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
 
-Here, the reads from safe_register will cause the I/O chipset to flush any
-pending writes before actually posting the read to the chipset, preventing
-possible data corruption.
+Here, the ioflush() call will cause the I/O chipset to flush any outstanding
+writes before actually sending the read to the chipset, preventing possible
+data corruption.
+
+inX and outX calls, on the other hand, are strongly ordered and non-postable.
+They do not need special handling.  But this is something to watch out for
+when converting drivers to use MMIO space from IO Port space.
===== arch/ia64/sn/io/machvec/iomv.c 1.9 vs edited =====
--- 1.9/arch/ia64/sn/io/machvec/iomv.c	2004-05-26 06:49:19 -07:00
+++ edited/arch/ia64/sn/io/machvec/iomv.c	2004-09-21 09:34:19 -07:00
@@ -54,23 +54,18 @@
 EXPORT_SYMBOL(sn_io_addr);
 
 /**
- * sn_mmiob - I/O space memory barrier
+ * __sn_ioflush - I/O space write flush
  *
- * Acts as a memory mapped I/O barrier for platforms that queue writes to 
- * I/O space.  This ensures that subsequent writes to I/O space arrive after
- * all previous writes.  For most ia64 platforms, this is a simple
- * 'mf.a' instruction.  For other platforms, mmiob() may have to read
- * a chipset register to ensure ordering.
+ * See include/asm-ia64/io.h and Documentation/io_ordering.txt for details.
  *
  * On SN2, we wait for the PIO_WRITE_STATUS SHub register to clear.
  * See PV 871084 for details about the WAR about zero value.
  *
  */
-void
-sn_mmiob (void)
+void __sn_ioflush(struct device *dev, unsigned long addr)
 {
 	while ((((volatile unsigned long) (*pda->pio_write_status_addr)) & SH_PIO_WRITE_STATUS_0_PENDING_WRITE_COUNT_MASK) != 
 				SH_PIO_WRITE_STATUS_0_PENDING_WRITE_COUNT_MASK)
 		cpu_relax();
 }
-EXPORT_SYMBOL(sn_mmiob);
+EXPORT_SYMBOL(__sn_ioflush);
===== include/asm-ia64/io.h 1.19 vs edited =====
--- 1.19/include/asm-ia64/io.h	2004-02-03 21:31:10 -08:00
+++ edited/include/asm-ia64/io.h	2004-09-21 09:46:08 -07:00
@@ -91,6 +91,26 @@
  */
 #define __ia64_mf_a()	ia64_mfa()
 
+/**
+ * __ia64_ioflush - I/O write flush
+ * @dev: device we're flushing
+ * @addr: safe register to read
+ *
+ * Flush I/O space writes out to their target device to ensure ordering.
+ * all previous writes.  For most ia64 platforms, this is a simple
+ * 'mf.a' instruction, so the address is ignored.  For other platforms,
+ * the address may be required to ensure proper ordering of writes to I/O space
+ * since a 'dummy' read might be necessary to barrier the write operation.
+ *
+ * If either @dev or @addr is 0, don't use it.
+ *
+ * See Documentation/io_ordering.txt for more information.
+ */
+static inline void __ia64_ioflush (struct device *dev, unsigned long addr)
+{
+	ia64_mfa();
+}
+
 static inline const unsigned long
 __ia64_get_io_port_base (void)
 {
@@ -267,6 +287,7 @@
 #define __outb		platform_outb
 #define __outw		platform_outw
 #define __outl		platform_outl
+#define __ioflush	platform_ioflush
 
 #define inb(p)		__inb(p)
 #define inw(p)		__inw(p)
@@ -280,6 +301,7 @@
 #define outsb(p,s,c)	__outsb(p,s,c)
 #define outsw(p,s,c)	__outsw(p,s,c)
 #define outsl(p,s,c)	__outsl(p,s,c)
+#define ioflush(d,a)	__ioflush(d,a)
 
 /*
  * The address passed to these functions are ioremap()ped already.
===== include/asm-ia64/machvec.h 1.26 vs edited =====
--- 1.26/include/asm-ia64/machvec.h	2004-08-03 16:05:22 -07:00
+++ edited/include/asm-ia64/machvec.h	2004-09-21 09:35:32 -07:00
@@ -62,6 +62,7 @@
 typedef void ia64_mv_outb_t (unsigned char, unsigned long);
 typedef void ia64_mv_outw_t (unsigned short, unsigned long);
 typedef void ia64_mv_outl_t (unsigned int, unsigned long);
+typedef void ia64_mv_ioflush_t (struct device *, unsigned long);
 typedef unsigned char ia64_mv_readb_t (void *);
 typedef unsigned short ia64_mv_readw_t (void *);
 typedef unsigned int ia64_mv_readl_t (void *);
@@ -130,6 +131,7 @@
 #  define platform_outb		ia64_mv.outb
 #  define platform_outw		ia64_mv.outw
 #  define platform_outl		ia64_mv.outl
+#  define platform_ioflush	ia64_mv.ioflush
 #  define platform_readb        ia64_mv.readb
 #  define platform_readw        ia64_mv.readw
 #  define platform_readl        ia64_mv.readl
@@ -176,6 +178,7 @@
 	ia64_mv_outb_t *outb;
 	ia64_mv_outw_t *outw;
 	ia64_mv_outl_t *outl;
+	ia64_mv_ioflush_t *ioflush;
 	ia64_mv_readb_t *readb;
 	ia64_mv_readw_t *readw;
 	ia64_mv_readl_t *readl;
@@ -218,6 +221,7 @@
 	platform_outb,				\
 	platform_outw,				\
 	platform_outl,				\
+	platform_ioflush,			\
 	platform_readb,				\
 	platform_readw,				\
 	platform_readl,				\
@@ -343,6 +347,9 @@
 #endif
 #ifndef platform_outl
 # define platform_outl		__ia64_outl
+#endif
+#ifndef platform_ioflush
+# define platform_ioflush	__ia64_ioflush
 #endif
 #ifndef platform_readb
 # define platform_readb		__ia64_readb
===== include/asm-ia64/machvec_init.h 1.7 vs edited =====
--- 1.7/include/asm-ia64/machvec_init.h	2004-02-06 00:30:24 -08:00
+++ edited/include/asm-ia64/machvec_init.h	2004-09-21 09:35:47 -07:00
@@ -12,6 +12,7 @@
 extern ia64_mv_outb_t __ia64_outb;
 extern ia64_mv_outw_t __ia64_outw;
 extern ia64_mv_outl_t __ia64_outl;
+extern ia64_mv_ioflush_t __ia64_ioflush;
 extern ia64_mv_readb_t __ia64_readb;
 extern ia64_mv_readw_t __ia64_readw;
 extern ia64_mv_readl_t __ia64_readl;
===== include/asm-ia64/machvec_sn2.h 1.14 vs edited =====
--- 1.14/include/asm-ia64/machvec_sn2.h	2004-07-10 17:14:00 -07:00
+++ edited/include/asm-ia64/machvec_sn2.h	2004-09-21 09:36:14 -07:00
@@ -49,6 +49,7 @@
 extern ia64_mv_outb_t __sn_outb;
 extern ia64_mv_outw_t __sn_outw;
 extern ia64_mv_outl_t __sn_outl;
+extern ia64_mv_ioflush_t __sn_ioflush;
 extern ia64_mv_readb_t __sn_readb;
 extern ia64_mv_readw_t __sn_readw;
 extern ia64_mv_readl_t __sn_readl;
@@ -92,6 +93,7 @@
 #define platform_outb			__sn_outb
 #define platform_outw			__sn_outw
 #define platform_outl			__sn_outl
+#define platform_ioflush		__sn_ioflush
 #define platform_readb			__sn_readb
 #define platform_readw			__sn_readw
 #define platform_readl			__sn_readl
===== include/asm-ia64/sn/io.h 1.7 vs edited =====
--- 1.7/include/asm-ia64/sn/io.h	2004-02-13 07:00:22 -08:00
+++ edited/include/asm-ia64/sn/io.h	2004-09-21 09:38:27 -07:00
@@ -58,8 +58,8 @@
 #include <asm/sn/sn2/shubio.h>
 
 /*
- * Used to ensure write ordering (like mb(), but for I/O space)
+ * Used to ensure write ordering
  */
-extern void sn_mmiob(void);
+extern void __sn_ioflush(struct device *dev, unsigned long addr);
 
 #endif /* _ASM_IA64_SN_IO_H */
===== include/asm-ia64/sn/sn2/io.h 1.6 vs edited =====
--- 1.6/include/asm-ia64/sn/sn2/io.h	2004-07-22 17:00:00 -07:00
+++ edited/include/asm-ia64/sn/sn2/io.h	2004-09-21 09:31:56 -07:00
@@ -11,8 +11,10 @@
 #include <linux/compiler.h>
 #include <asm/intrinsics.h>
 
-extern void * sn_io_addr(unsigned long port) __attribute_const__; /* Forward definition */
-extern void sn_mmiob(void); /* Forward definition */
+/* Forward declarations */
+struct device;
+extern void *sn_io_addr(unsigned long port) __attribute_const__;
+extern void __sn_ioflush(struct device *dev, unsigned long addr);
 
 #define __sn_mf_a()   ia64_mfa()
 
@@ -91,7 +93,7 @@
 
 	if ((addr = sn_io_addr(port))) {
 		*addr = val;
-		sn_mmiob();
+		__sn_ioflush(0, 0);
 	}
 }
 
@@ -102,7 +104,7 @@
 
 	if ((addr = sn_io_addr(port))) {
 		*addr = val;
-		sn_mmiob();
+		__sn_ioflush(0, 0);
 	}
 }
 
@@ -113,7 +115,7 @@
 
 	if ((addr = sn_io_addr(port))) {
 		*addr = val;
-		sn_mmiob();
+		__sn_ioflush(0, 0);
 	}
 }
 

Re: SCSI QLA not working on latest *-mm SN2

[Matthew Wilcox]

On Tue, Sep 21, 2004 at 01:03:18PM -0400, Jesse Barnes wrote:
> Does this patch describe and correctly implement what we've discussed?  I've 
> added information to io_ordering.txt to describe the conclusion about posting 
> we seem to have agreed on.  Obviously, this is just a first cut.  It 
> compiles, but I haven't made prototypes for any arch other than ia64.

Please kill the Documentation/io_ordering.txt file and work on improving
the information in Documentation/DocBook/deviceiobook instead.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 1:15 pm, Matthew Wilcox wrote:
> On Tue, Sep 21, 2004 at 01:03:18PM -0400, Jesse Barnes wrote:
> > Does this patch describe and correctly implement what we've discussed? 
> > I've added information to io_ordering.txt to describe the conclusion
> > about posting we seem to have agreed on.  Obviously, this is just a first
> > cut.  It compiles, but I haven't made prototypes for any arch other than
> > ia64.
>
> Please kill the Documentation/io_ordering.txt file and work on improving
> the information in Documentation/DocBook/deviceiobook instead.

Sure, I can do that in the next spin.  Please make sure things look ok 
otherwise though.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[James Bottomley]

On Tue, 2004-09-21 at 13:03, Jesse Barnes wrote:
> +Driver writers are responsible for ensuring that I/O writes to memory-mapped
> +addresses on their device arrive when expected and in the order intended.

Really, no.  You're making the document more confusing.  PCI devices
have *two* types of non DMA accesses (well, three, but lets forget
configuration space for the moment).

I/O Space accesses (what we call PIO) and memory accesses (what we call
MMIO)

> +This is typically done by reading a 'safe' device or bridge register, causing
> +the I/O chipset to flush pending writes to the device before any reads are

Not "bridge register" the specs say this must be an access to the
device's space.

> +sent to the target device.  A driver would usually use this technique
> +immediately prior to a read after a card reset or the exit of a critical
> +section of code protected by spinlocks.  This would ensure that subsequent I/O
> +space accesses arrived only after all prior writes.  There are really two
> +issues at play here, one is 'posting', i.e. memory-mapped I/O writes not sent
> +to the device immediately, and ordering, where on a large system writes from
> +different CPUs may arrive out of order.
>  
> -        ...
> +Some pseudocode to illustrate the problem of write posting:
> +
> +...
> +spin_lock_irqsave(&dev_lock, flags)
> +...
> +writel(resetval, reset_reg); /* reset the card */
> +udelay(10); /* wait for reset (also needs pioflush) */
> +val = readl(ring_ptr); /* read initial value */
> +spin_unlock_irqrestore(&dev_lock, flags)
> +...
> +
> +In this case, the card is reset by the first write.  The driver attempts to
> +wait for the completion of the reset using udelay.  But since the write may be
> +delayed and the udelay will probably start executing right away, it may be
> +that there's not enough time for the write to actually arrive at the card and
> +for the reset to occur before the read is executed.  On some platforms, this
> +can result in a machine check.  Unfortunately, there's no way to guarantee
> +that a write has arrived at a device short of a read from the same address

Not same address space, any address space (IO, memory or config) of the
device will do.

> +space, so in some cases, udelay() is the only option.  In any case, the driver
> +should issue an ioflush() call prior to the udelay(), passing in 0 for the

No; using udelay() to try to wait for the flush of posted writes to
occur is always a bug.

> +addr argument if no safe register exists.  This will allow the platform to
> +make an effort to get the write as close to the device as possible before
> +allowing the udelay to begin.

What ioflush() call?  There's no such thing in PCI; this is effectively
our problem.  If there were a nice flush instruction we wouldn't have to
worry about reading from somewhere on the device.  The problem is that
there's no a-priori way of knowing what read is safe to do, so there's
no generic way to extract a posting flush API.

James

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

On Tuesday, September 21, 2004 1:20 pm, James Bottomley wrote:
> On Tue, 2004-09-21 at 13:03, Jesse Barnes wrote:
> > +Driver writers are responsible for ensuring that I/O writes to
> > memory-mapped +addresses on their device arrive when expected and in the
> > order intended.
>
> Really, no.  You're making the document more confusing.  PCI devices
> have *two* types of non DMA accesses (well, three, but lets forget
> configuration space for the moment).
>
> I/O Space accesses (what we call PIO) and memory accesses (what we call
> MMIO)

Right, fixed.

> > +This is typically done by reading a 'safe' device or bridge register,
> > causing +the I/O chipset to flush pending writes to the device before any
> > reads are
>
> Not "bridge register" the specs say this must be an access to the
> device's space.

Fixed.

> > Unfortunately, there's no way to guarantee +that a write has arrived at a
> > device short of a read from the same address
>
> Not same address space, any address space (IO, memory or config) of the
> device will do.

Ok, fixed.

> > +space, so in some cases, udelay() is the only option.  In any case, the
> > driver +should issue an ioflush() call prior to the udelay(), passing in
> > 0 for the
>
> No; using udelay() to try to wait for the flush of posted writes to
> occur is always a bug.

I thought we determined that sometimes there's no other option?  I'll remove 
that sentence anyway.


> > +addr argument if no safe register exists.  This will allow the platform
> > to +make an effort to get the write as close to the device as possible
> > before +allowing the udelay to begin.
>
> What ioflush() call?  There's no such thing in PCI; this is effectively
> our problem.  If there were a nice flush instruction we wouldn't have to
> worry about reading from somewhere on the device.  The problem is that
> there's no a-priori way of knowing what read is safe to do, so there's
> no generic way to extract a posting flush API.

The one I just added with this patch.  It takes a struct device and address 
arguments, the latter is supposed to be a safe register, like config space.

I'm tying together the concepts of write posting and ordering intentionally, 
but I wonder if that's a good idea.  On the one hand, we don't want to 
introduce *two* new driver APIs, but on the other, ensuring ordering and 
actually flushing writes out are two different things.  Maybe we just need to 
tell people 

Jesse

[-- Attachment #2: ioflush-ia64-2.patch --]
[-- Type: text/x-diff, Size: 11817 bytes --]

===== Documentation/io_ordering.txt 1.1 vs edited =====
--- 1.1/Documentation/io_ordering.txt	2003-03-18 02:02:11 -08:00
+++ edited/Documentation/io_ordering.txt	2004-09-21 10:32:06 -07:00
@@ -1,24 +1,60 @@
-On some platforms, so-called memory-mapped I/O is weakly ordered.  On such
-platforms, driver writers are responsible for ensuring that I/O writes to
-memory-mapped addresses on their device arrive in the order intended.  This is
-typically done by reading a 'safe' device or bridge register, causing the I/O
-chipset to flush pending writes to the device before any reads are posted.  A
-driver would usually use this technique immediately prior to the exit of a
-critical section of code protected by spinlocks.  This would ensure that
-subsequent writes to I/O space arrived only after all prior writes (much like a
-memory barrier op, mb(), only with respect to I/O).
+Dealing with posted writes
+--------------------------
 
-A more concrete example from a hypothetical device driver:
+Driver writers are responsible for ensuring that I/O writes to their device
+arrive when expected and in the order intended.  This is typically done by
+reading a 'safe' device register, causing the I/O chipset to flush pending
+writes to the device before any reads are sent to the target device.  A driver
+would usually use this technique immediately prior to a read after a card
+reset or the exit of a critical section of code protected by spinlocks.  This
+would ensure that subsequent I/O space accesses arrived only after all prior
+writes.  There are really two issues at play here, one is 'posting', i.e.
+memory-mapped I/O writes not sent to the device immediately, and ordering,
+where on a large system writes from different CPUs may arrive out of order.
 
-        ...
+Some pseudocode to illustrate the problem of write posting:
+
+...
+spin_lock_irqsave(&dev_lock, flags)
+...
+writel(resetval, reset_reg); /* reset the card */
+udelay(10); /* wait for reset (also needs pioflush) */
+val = readl(ring_ptr); /* read initial value */
+spin_unlock_irqrestore(&dev_lock, flags)
+...
+
+In this case, the card is reset by the first write.  The driver attempts to
+wait for the completion of the reset using udelay.  But since the write may be
+delayed and the udelay will probably start executing right away, it may be
+that there's not enough time for the write to actually arrive at the card and
+for the reset to occur before the read is executed.  On some platforms, this
+can result in a machine check.  Unfortunately, there's no way to guarantee
+that a write has arrived at a device short of a read from a safe register.  In
+any case, the driver should issue an ioflush() call prior to the udelay(),
+passing in 0 for the addr argument if no safe register exists or if the driver
+is merely trying to ensure write ordering.  This will allow the platform to
+make an effort to get the write as close to the device as possible before
+allowing the udelay to begin.  Example:
+
+...
+spin_lock_irqsave(&dev_lock, flags)
+...
+writel(resetval, reset_reg); /* reset the card */
+ioflush(dev, config_reg); /* flush out the write */
+udelay(10);
+val = readl(ring_ptr); /* read initial value */
+spin_unlock_irqrestore(&dev_lock, flags)
+...
+
+Here's an example of reordering of writes between CPUs on a NUMA machine:
+
+	...
 CPU A:  spin_lock_irqsave(&dev_lock, flags)
-CPU A:  val = readl(my_status);
 CPU A:  ...
 CPU A:  writel(newval, ring_ptr);
 CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
         ...
 CPU B:  spin_lock_irqsave(&dev_lock, flags)
-CPU B:  val = readl(my_status);
 CPU B:  ...
 CPU B:  writel(newval2, ring_ptr);
 CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
@@ -29,19 +65,21 @@
 
         ...
 CPU A:  spin_lock_irqsave(&dev_lock, flags)
-CPU A:  val = readl(my_status);
 CPU A:  ...
 CPU A:  writel(newval, ring_ptr);
-CPU A:  (void)readl(safe_register); /* maybe a config register? */
+CPU A:  ioflush(dev, 0);
 CPU A:  spin_unlock_irqrestore(&dev_lock, flags)
         ...
 CPU B:  spin_lock_irqsave(&dev_lock, flags)
-CPU B:  val = readl(my_status);
 CPU B:  ...
 CPU B:  writel(newval2, ring_ptr);
-CPU B:  (void)readl(safe_register); /* maybe a config register? */
+CPU B:  ioflush(dev, 0);
 CPU B:  spin_unlock_irqrestore(&dev_lock, flags)
 
-Here, the reads from safe_register will cause the I/O chipset to flush any
-pending writes before actually posting the read to the chipset, preventing
-possible data corruption.
+Here, the ioflush() call will cause the I/O chipset to flush any outstanding
+writes before actually sending the read to the chipset, preventing possible
+data corruption.
+
+inX and outX calls, on the other hand, are strongly ordered and non-postable.
+They do not need special handling.  But this is something to watch out for
+when converting drivers to use MMIO space from IO Port space.
===== arch/ia64/sn/io/machvec/iomv.c 1.9 vs edited =====
--- 1.9/arch/ia64/sn/io/machvec/iomv.c	2004-05-26 06:49:19 -07:00
+++ edited/arch/ia64/sn/io/machvec/iomv.c	2004-09-21 09:34:19 -07:00
@@ -54,23 +54,18 @@
 EXPORT_SYMBOL(sn_io_addr);
 
 /**
- * sn_mmiob - I/O space memory barrier
+ * __sn_ioflush - I/O space write flush
  *
- * Acts as a memory mapped I/O barrier for platforms that queue writes to 
- * I/O space.  This ensures that subsequent writes to I/O space arrive after
- * all previous writes.  For most ia64 platforms, this is a simple
- * 'mf.a' instruction.  For other platforms, mmiob() may have to read
- * a chipset register to ensure ordering.
+ * See include/asm-ia64/io.h and Documentation/io_ordering.txt for details.
  *
  * On SN2, we wait for the PIO_WRITE_STATUS SHub register to clear.
  * See PV 871084 for details about the WAR about zero value.
  *
  */
-void
-sn_mmiob (void)
+void __sn_ioflush(struct device *dev, unsigned long addr)
 {
 	while ((((volatile unsigned long) (*pda->pio_write_status_addr)) & SH_PIO_WRITE_STATUS_0_PENDING_WRITE_COUNT_MASK) != 
 				SH_PIO_WRITE_STATUS_0_PENDING_WRITE_COUNT_MASK)
 		cpu_relax();
 }
-EXPORT_SYMBOL(sn_mmiob);
+EXPORT_SYMBOL(__sn_ioflush);
===== include/asm-ia64/io.h 1.19 vs edited =====
--- 1.19/include/asm-ia64/io.h	2004-02-03 21:31:10 -08:00
+++ edited/include/asm-ia64/io.h	2004-09-21 10:30:16 -07:00
@@ -91,6 +91,27 @@
  */
 #define __ia64_mf_a()	ia64_mfa()
 
+/**
+ * __ia64_ioflush - I/O write flush
+ * @dev: device we're flushing
+ * @addr: safe register to read
+ *
+ * Flush I/O space writes out to their target device to ensure ordering.
+ * all previous writes.  For most ia64 platforms, this is a simple
+ * 'mf.a' instruction, so the address is ignored.  For other platforms,
+ * the address may be required to ensure proper ordering of writes to I/O space
+ * since a 'dummy' read might be necessary to barrier the write operation.
+ *
+ * If either @dev or @addr is 0, don't use it.  @addr should be 0 if the driver
+ * is just trying to make sure writes arrive in order.
+ *
+ * See Documentation/io_ordering.txt for more information.
+ */
+static inline void __ia64_ioflush (struct device *dev, unsigned long addr)
+{
+	ia64_mfa();
+}
+
 static inline const unsigned long
 __ia64_get_io_port_base (void)
 {
@@ -267,6 +288,7 @@
 #define __outb		platform_outb
 #define __outw		platform_outw
 #define __outl		platform_outl
+#define __ioflush	platform_ioflush
 
 #define inb(p)		__inb(p)
 #define inw(p)		__inw(p)
@@ -280,6 +302,7 @@
 #define outsb(p,s,c)	__outsb(p,s,c)
 #define outsw(p,s,c)	__outsw(p,s,c)
 #define outsl(p,s,c)	__outsl(p,s,c)
+#define ioflush(d,a)	__ioflush(d,a)
 
 /*
  * The address passed to these functions are ioremap()ped already.
===== include/asm-ia64/machvec.h 1.26 vs edited =====
--- 1.26/include/asm-ia64/machvec.h	2004-08-03 16:05:22 -07:00
+++ edited/include/asm-ia64/machvec.h	2004-09-21 09:35:32 -07:00
@@ -62,6 +62,7 @@
 typedef void ia64_mv_outb_t (unsigned char, unsigned long);
 typedef void ia64_mv_outw_t (unsigned short, unsigned long);
 typedef void ia64_mv_outl_t (unsigned int, unsigned long);
+typedef void ia64_mv_ioflush_t (struct device *, unsigned long);
 typedef unsigned char ia64_mv_readb_t (void *);
 typedef unsigned short ia64_mv_readw_t (void *);
 typedef unsigned int ia64_mv_readl_t (void *);
@@ -130,6 +131,7 @@
 #  define platform_outb		ia64_mv.outb
 #  define platform_outw		ia64_mv.outw
 #  define platform_outl		ia64_mv.outl
+#  define platform_ioflush	ia64_mv.ioflush
 #  define platform_readb        ia64_mv.readb
 #  define platform_readw        ia64_mv.readw
 #  define platform_readl        ia64_mv.readl
@@ -176,6 +178,7 @@
 	ia64_mv_outb_t *outb;
 	ia64_mv_outw_t *outw;
 	ia64_mv_outl_t *outl;
+	ia64_mv_ioflush_t *ioflush;
 	ia64_mv_readb_t *readb;
 	ia64_mv_readw_t *readw;
 	ia64_mv_readl_t *readl;
@@ -218,6 +221,7 @@
 	platform_outb,				\
 	platform_outw,				\
 	platform_outl,				\
+	platform_ioflush,			\
 	platform_readb,				\
 	platform_readw,				\
 	platform_readl,				\
@@ -343,6 +347,9 @@
 #endif
 #ifndef platform_outl
 # define platform_outl		__ia64_outl
+#endif
+#ifndef platform_ioflush
+# define platform_ioflush	__ia64_ioflush
 #endif
 #ifndef platform_readb
 # define platform_readb		__ia64_readb
===== include/asm-ia64/machvec_init.h 1.7 vs edited =====
--- 1.7/include/asm-ia64/machvec_init.h	2004-02-06 00:30:24 -08:00
+++ edited/include/asm-ia64/machvec_init.h	2004-09-21 09:35:47 -07:00
@@ -12,6 +12,7 @@
 extern ia64_mv_outb_t __ia64_outb;
 extern ia64_mv_outw_t __ia64_outw;
 extern ia64_mv_outl_t __ia64_outl;
+extern ia64_mv_ioflush_t __ia64_ioflush;
 extern ia64_mv_readb_t __ia64_readb;
 extern ia64_mv_readw_t __ia64_readw;
 extern ia64_mv_readl_t __ia64_readl;
===== include/asm-ia64/machvec_sn2.h 1.14 vs edited =====
--- 1.14/include/asm-ia64/machvec_sn2.h	2004-07-10 17:14:00 -07:00
+++ edited/include/asm-ia64/machvec_sn2.h	2004-09-21 09:36:14 -07:00
@@ -49,6 +49,7 @@
 extern ia64_mv_outb_t __sn_outb;
 extern ia64_mv_outw_t __sn_outw;
 extern ia64_mv_outl_t __sn_outl;
+extern ia64_mv_ioflush_t __sn_ioflush;
 extern ia64_mv_readb_t __sn_readb;
 extern ia64_mv_readw_t __sn_readw;
 extern ia64_mv_readl_t __sn_readl;
@@ -92,6 +93,7 @@
 #define platform_outb			__sn_outb
 #define platform_outw			__sn_outw
 #define platform_outl			__sn_outl
+#define platform_ioflush		__sn_ioflush
 #define platform_readb			__sn_readb
 #define platform_readw			__sn_readw
 #define platform_readl			__sn_readl
===== include/asm-ia64/sn/io.h 1.7 vs edited =====
--- 1.7/include/asm-ia64/sn/io.h	2004-02-13 07:00:22 -08:00
+++ edited/include/asm-ia64/sn/io.h	2004-09-21 09:38:27 -07:00
@@ -58,8 +58,8 @@
 #include <asm/sn/sn2/shubio.h>
 
 /*
- * Used to ensure write ordering (like mb(), but for I/O space)
+ * Used to ensure write ordering
  */
-extern void sn_mmiob(void);
+extern void __sn_ioflush(struct device *dev, unsigned long addr);
 
 #endif /* _ASM_IA64_SN_IO_H */
===== include/asm-ia64/sn/sn2/io.h 1.6 vs edited =====
--- 1.6/include/asm-ia64/sn/sn2/io.h	2004-07-22 17:00:00 -07:00
+++ edited/include/asm-ia64/sn/sn2/io.h	2004-09-21 09:31:56 -07:00
@@ -11,8 +11,10 @@
 #include <linux/compiler.h>
 #include <asm/intrinsics.h>
 
-extern void * sn_io_addr(unsigned long port) __attribute_const__; /* Forward definition */
-extern void sn_mmiob(void); /* Forward definition */
+/* Forward declarations */
+struct device;
+extern void *sn_io_addr(unsigned long port) __attribute_const__;
+extern void __sn_ioflush(struct device *dev, unsigned long addr);
 
 #define __sn_mf_a()   ia64_mfa()
 
@@ -91,7 +93,7 @@
 
 	if ((addr = sn_io_addr(port))) {
 		*addr = val;
-		sn_mmiob();
+		__sn_ioflush(0, 0);
 	}
 }
 
@@ -102,7 +104,7 @@
 
 	if ((addr = sn_io_addr(port))) {
 		*addr = val;
-		sn_mmiob();
+		__sn_ioflush(0, 0);
 	}
 }
 
@@ -113,7 +115,7 @@
 
 	if ((addr = sn_io_addr(port))) {
 		*addr = val;
-		sn_mmiob();
+		__sn_ioflush(0, 0);
 	}
 }
 

Re: SCSI QLA not working on latest *-mm SN2

[James Bottomley]

On Tue, 2004-09-21 at 13:46, Jesse Barnes wrote:
> > What ioflush() call?  There's no such thing in PCI; this is effectively
> > our problem.  If there were a nice flush instruction we wouldn't have to
> > worry about reading from somewhere on the device.  The problem is that
> > there's no a-priori way of knowing what read is safe to do, so there's
> > no generic way to extract a posting flush API.
> 
> The one I just added with this patch.  It takes a struct device and address 
> arguments, the latter is supposed to be a safe register, like config space.
> 
> I'm tying together the concepts of write posting and ordering intentionally, 
> but I wonder if that's a good idea.  On the one hand, we don't want to 
> introduce *two* new driver APIs, but on the other, ensuring ordering and 
> actually flushing writes out are two different things.  Maybe we just need to 
> tell people 

Really, I don't think this is a good idea.  If there were a way to
produce an api that was just ioflush(struct device *) then yes, since it
would reduce confusion.  However, since all your API does is hide the
fact that you're doing a MMIO read then the proposed API provides no
relief from the fact that the driver writer needs to understand
posting...all it does is add one extra function for them to misuse or
get confused about.

James

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 1:56 pm, James Bottomley wrote:
> Really, I don't think this is a good idea.  If there were a way to
> produce an api that was just ioflush(struct device *) then yes, since it
> would reduce confusion.  However, since all your API does is hide the
> fact that you're doing a MMIO read then the proposed API provides no
> relief from the fact that the driver writer needs to understand
> posting...all it does is add one extra function for them to misuse or
> get confused about.

Agreed.  I'll rename it, fix the io_ordering.txt doc to only describe I/O 
ordering issues, and resubmit it solely as a performance improvement API.

Grant, you say that I/O writes can't possibly arrive out of order if issued 
from different CPUs on any machines you're aware of?  If not, it may be that 
the API will only benefit SGI machines.

Thanks,
Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Grant Grundler]

On Tue, Sep 21, 2004 at 02:09:10PM -0400, Jesse Barnes wrote:
> Grant, you say that I/O writes can't possibly arrive out of order if issued 
> from different CPUs on any machines you're aware of?

Well, that's what I believe right now based on my experience.
Re-reading different parts of the PCI local bus spec doesn't give
me the warm fuzzies - especially in regard to retries.

The qla example Jeremy gave earlier wasn't clear on where the
mmio write reordering was taking place.  Based on my understanding
(and I've been wrong before) of ia64 .rel/.acq semantics the reordering
didn't happen in the CPU coherency - ie writes are ordered WRT
to each other out to the Mckinley bus. That really only leaves
the chipset suspect - ie anything between Mckinley bus and PCI bus.

And this chipset is already known to violate DMA writes and
MMIO read return ordering rules...which led to read_relaxed().
We've agreed DMA reads bypassing MMIO writes violates the spec
but is "mostly harmless" (I haven't seen a case in real life where
it matters). HP parisc and ia64 platforms also implement
this "optimization" (bug) and I've not seen a problem with it yet.

> If not, it may be that the API will only benefit SGI machines.

Or hurt them.
We haven't talked about the flip side of this workaround much.

Normally, I expect the chipset is responsible for maintaining
order of MMIO writes - though that sounds near impossible on
a large fabric where the spinlock transactions may take a different
path than the IO transactions. But allowing out of order MMIO
write transactions is a big deal if we want high performance
devices that can operate correctly by only consuming MMIO
writes and doing DMA for everything else. Adding the MMIO
reads to enforcing MMIO write ordering will set us all back
a few years in terms of performance.

This behavior (bug?) essentially means drivers that only do MMIO
writes during normal operation (and MMIO reads during set up) will
not operate correctly on Altix (or similar if they exist) boxes.

hth,
grant

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 3:06 pm, Grant Grundler wrote:
> The qla example Jeremy gave earlier wasn't clear on where the
> mmio write reordering was taking place.  Based on my understanding
> (and I've been wrong before) of ia64 .rel/.acq semantics the reordering
> didn't happen in the CPU coherency - ie writes are ordered WRT
> to each other out to the Mckinley bus. That really only leaves
> the chipset suspect - ie anything between Mckinley bus and PCI bus.

Yep, that's where the reordering occurs for us.

> Or hurt them.
> We haven't talked about the flip side of this workaround much.
>
> Normally, I expect the chipset is responsible for maintaining
> order of MMIO writes - though that sounds near impossible on
> a large fabric where the spinlock transactions may take a different
> path than the IO transactions.

I think it is.  I wouldn't be surprised if your hw guys told you the same 
thing for your large machines.

> But allowing out of order MMIO 
> write transactions is a big deal if we want high performance
> devices that can operate correctly by only consuming MMIO
> writes and doing DMA for everything else. Adding the MMIO
> reads to enforcing MMIO write ordering will set us all back
> a few years in terms of performance.

But *only* prior to releasing a lock.  Writes in program order will arrive in 
order, it's just that writes from some other CPU may beat them to the device.  
So you'll only have one read for every so many writes.  And if your chipset 
supports it, you don't have to do a full read out to the target bus, but just 
to the local chipset.

> This behavior (bug?) essentially means drivers that only do MMIO
> writes during normal operation (and MMIO reads during set up) will
> not operate correctly on Altix (or similar if they exist) boxes.

It's a pretty hard bug to hit, as Jeremy mentioned.  You'll only see it on 
large boxes.  The performance gain for us comes from not doing a full read 
from the device, but just making sure our local hub has sent the write to its 
destination hub, where ordering is guaranteed.  Of course, it's only a gain 
if one assumes that the necessary full reads are already in place.  If not, 
then the hardware is already imposing I/O space write penalties anyway, 
except for all writes.  I'd think that's worse than just flushing the ones 
you care about, and only when you need to.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Grant Grundler]

On Tue, Sep 21, 2004 at 03:40:32PM -0400, Jesse Barnes wrote:
> > Normally, I expect the chipset is responsible for maintaining
> > order of MMIO writes - though that sounds near impossible on
> > a large fabric where the spinlock transactions may take a different
> > path than the IO transactions.
> 
> I think it is.  I wouldn't be surprised if your hw guys told you the same 
> thing for your large machines.

I was told Superdome chipsets (SX1000) do NOT have this problem.
AFAIK, it only scales to 16 nodes (4 sockets/node) and the fabric
may not have the multiple paths SGI Altix (or other interconnects)
may have. (And I'd like the "other chipsets" better defined if anyone
knows of other chipsets).


I was also told likely *all* larger PCI-E systems will have this problem.
Ie any time the fabric allows multiple pathes to the same device.

And as usual, I was wrong. Someone educated me on HP V-class systems (PARISC)
having the same problem when running in NUMA config (4 node cluster).
Of course parisc-linux doesn't run on V-class...and HP didn't sell that
many V-class clusters...but here's the story anyway.

Despite strongly ordered CPU accesses, the chipset couldn't preserve
ordering across the NUMA links.  The NIC drivers exposed this problem
when writing descriptors to remote shared memory.  This shared memory
is implemented on each Host PCI bus controller for that bus segment.
ie some MMIO writes had to cross both a NUMA Link and X-bar compared
to local nodes only crossing the X-bar.
Result was some of the descriptors picked up by NICs would contain garbage.
The workaround was adding MMIO Reads after each descriptor was
updated - exactly what SGI wants to do for qla driver.

...
> So you'll only have one read for every so many writes.  And if your chipset 
> supports it, you don't have to do a full read out to the target bus, but just 
> to the local chipset.

Yes - agreed - not every MMIO write and we really only need to guarantee
the writes have reach the targeted PCI segment.
But it's still alot more reads and will measureably affect performance
on smaller boxes if it's done unconditionally.

Large scale NUMA is going to suffer under RDMA.
RDMA using smaller boxes will be much faster with at
least 10000-2000 cycles less overhead and latency per packet.

> It's a pretty hard bug to hit, as Jeremy mentioned.  You'll only see it on 
> large boxes.

Yes - a fabric that can't preserve ordering is the key bit here.

> If not, 
> then the hardware is already imposing I/O space write penalties anyway, 
> except for all writes.  I'd think that's worse than just flushing the ones 
> you care about, and only when you need to.

I have the impression it's not feasible for HW to enforce ordering on large
fabrics.  And the "standard" PCI programming model clearly can't deal
with out of order MMIO writes. You guys just have the misfortune of
pushing the "envelope" right now.

I don't want to overload an interface that deals with write posting
with MMIO write ordering workarounds. The cases we need to enforce
write posting are different from the cases which need to enforce
MMIO write ordering. I think I understand both well enough now
and hope you do too. :^) 

hth,
grant

Re: SCSI QLA not working on latest *-mm SN2

[Jeremy Higdon]

On Tue, Sep 21, 2004 at 01:06:25PM -0600, Grant Grundler wrote:
> On Tue, Sep 21, 2004 at 02:09:10PM -0400, Jesse Barnes wrote:
> > Grant, you say that I/O writes can't possibly arrive out of order if issued 
> > from different CPUs on any machines you're aware of?
> 
> Well, that's what I believe right now based on my experience.
> Re-reading different parts of the PCI local bus spec doesn't give
> me the warm fuzzies - especially in regard to retries.
> 
> The qla example Jeremy gave earlier wasn't clear on where the
> mmio write reordering was taking place.  Based on my understanding
> (and I've been wrong before) of ia64 .rel/.acq semantics the reordering
> didn't happen in the CPU coherency - ie writes are ordered WRT
> to each other out to the Mckinley bus. That really only leaves
> the chipset suspect - ie anything between Mckinley bus and PCI bus.

Do IO space writes follow the memory coherency rules?

> And this chipset is already known to violate DMA writes and
> MMIO read return ordering rules...which led to read_relaxed().
> We've agreed DMA reads bypassing MMIO writes violates the spec
> but is "mostly harmless" (I haven't seen a case in real life where
> it matters). HP parisc and ia64 platforms also implement
> this "optimization" (bug) and I've not seen a problem with it yet.
> 
> > If not, it may be that the API will only benefit SGI machines.
> 
> Or hurt them.
> We haven't talked about the flip side of this workaround much.
> 
> Normally, I expect the chipset is responsible for maintaining
> order of MMIO writes - though that sounds near impossible on
> a large fabric where the spinlock transactions may take a different
> path than the IO transactions. But allowing out of order MMIO
> write transactions is a big deal if we want high performance
> devices that can operate correctly by only consuming MMIO
> writes and doing DMA for everything else. Adding the MMIO
> reads to enforcing MMIO write ordering will set us all back
> a few years in terms of performance.

On Altix, we do have the sn_mmiob() option.  I don't think that
we want to Linux API to require that

CPUA:	writel(value_X, common_register)
	spin_unlock(common_lock)

CPUB:	spin_lock(common_lock)
	writel(value_Y, common_register)

be strongly ordered, because it places a performance penalty
on all writes.  On Altix, we have the "sn_mmiob()" function to do
that.  I.e.

CPUA:	writel(value_X, common_register)
	sn_mmiob()
	spin_unlock(common_lock)

CPUB:	spin_lock(common_lock)
	writel(value_Y, common_register)

would strongly order the writes.

I think the flush that Jesse was talking about was an architecture
independent abstraction of the sn_mmiob().

Note that the sn_mmiob() does not guarantee that the preceding writel is
completed -- just that there will be no other writeX issued from other
CPUs ahead of it.

However, if we lose this argument, the simple answer is to add an
sn_mmiob to the Altix platform's writeX functions.  See ___sn_outb(),
by the way.

jeremy

Re: SCSI QLA not working on latest *-mm SN2

[Matthew Wilcox]

On Tue, Sep 21, 2004 at 02:03:42PM -0700, Jeremy Higdon wrote:
> On Altix, we do have the sn_mmiob() option.  I don't think that
> we want to Linux API to require that
> 
> CPUA:	writel(value_X, common_register)
> 	spin_unlock(common_lock)
> 
> CPUB:	spin_lock(common_lock)
> 	writel(value_Y, common_register)
> 
> be strongly ordered, because it places a performance penalty
> on all writes.

I disagree with you.  If this were memory, then you would expect
common_register to be set to value_Y after this sequence.  Why should
IO be different?

> On Altix, we have the "sn_mmiob()" function to do
> that.  I.e.
> 
> CPUA:	writel(value_X, common_register)
> 	sn_mmiob()
> 	spin_unlock(common_lock)
> 
> CPUB:	spin_lock(common_lock)
> 	writel(value_Y, common_register)
> 
> would strongly order the writes.

I think your _raw_spin_unlock() should include an sn_mmiob().

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: SCSI QLA not working on latest *-mm SN2

[Jeremy Higdon]

On Tue, Sep 21, 2004 at 10:11:08PM +0100, Matthew Wilcox wrote:
> On Tue, Sep 21, 2004 at 02:03:42PM -0700, Jeremy Higdon wrote:
> > On Altix, we do have the sn_mmiob() option.  I don't think that
> > we want to Linux API to require that
> > 
> > CPUA:	writel(value_X, common_register)
> > 	spin_unlock(common_lock)
> > 
> > CPUB:	spin_lock(common_lock)
> > 	writel(value_Y, common_register)
> > 
> > be strongly ordered, because it places a performance penalty
> > on all writes.
> 
> I disagree with you.  If this were memory, then you would expect
> common_register to be set to value_Y after this sequence.  Why should
> IO be different?

I/O is partially outside of the memory coherency domain.  So it is
different from memory, even though we might wish that it weren't.

> > On Altix, we have the "sn_mmiob()" function to do
> > that.  I.e.
> > 
> > CPUA:	writel(value_X, common_register)
> > 	sn_mmiob()
> > 	spin_unlock(common_lock)
> > 
> > CPUB:	spin_lock(common_lock)
> > 	writel(value_Y, common_register)
> > 
> > would strongly order the writes.
> 
> I think your _raw_spin_unlock() should include an sn_mmiob().

That could be very painful.
On Irix, we actually had a separate spinunlock (io_spin_unlock) that
added the MIPS equivalent.  I'm assuming we don't want to entertain
that here :-)  (Though tell me if I'm wrong)

jeremy

Re: SCSI QLA not working on latest *-mm SN2

[Jesse Barnes]

On Tuesday, September 21, 2004 5:43 pm, Jeremy Higdon wrote:
> > > CPUA: writel(value_X, common_register)
> > >  sn_mmiob()
> > >  spin_unlock(common_lock)
> > >
> > > CPUB: spin_lock(common_lock)
> > >  writel(value_Y, common_register)
> > >
> > > would strongly order the writes.
> >
> > I think your _raw_spin_unlock() should include an sn_mmiob().
>
> That could be very painful.
> On Irix, we actually had a separate spinunlock (io_spin_unlock) that
> added the MIPS equivalent.  I'm assuming we don't want to entertain
> that here :-)  (Though tell me if I'm wrong)

It's worth noting that this affects some MIPS systems as well, so that's at 
least two Linux arches that would benefit from a nice solution to this 
problem.

Jesse

Re: SCSI QLA not working on latest *-mm SN2

[Matthew Wilcox]

On Tue, Sep 21, 2004 at 02:43:02PM -0700, Jeremy Higdon wrote:
> That could be very painful.
> On Irix, we actually had a separate spinunlock (io_spin_unlock) that
> added the MIPS equivalent.  I'm assuming we don't want to entertain
> that here :-)  (Though tell me if I'm wrong)

That seems like a reasonable solution to me.  Want to float it to Linus
and see how he takes it?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: SCSI QLA not working on latest *-mm SN2

[Jeremy Higdon]

On Wed, Sep 22, 2004 at 01:02:11AM +0100, Matthew Wilcox wrote:
> On Tue, Sep 21, 2004 at 02:43:02PM -0700, Jeremy Higdon wrote:
> > That could be very painful.
> > On Irix, we actually had a separate spinunlock (io_spin_unlock) that
> > added the MIPS equivalent.  I'm assuming we don't want to entertain
> > that here :-)  (Though tell me if I'm wrong)
> 
> That seems like a reasonable solution to me.  Want to float it to Linus
> and see how he takes it?


First let's make sure that we like it best.

The other alternative is an explicit I/O barrier.

	writel(high-water, request-in)
	mmiob()  /* memory-mapped I/O barrier */
	spin_unlock(hostlock)

versus

	writel(high-water, request-in)
	spin_unlock_iob(hostlock)

I think I like the former better (even though we had the latter in Irix).
But I don't have a strong preference.

One of us will offer a patch.

jeremy

Re: SCSI QLA not working on latest *-mm SN2

[Grant Grundler]

On Tue, Sep 21, 2004 at 06:16:52PM -0700, Jeremy Higdon wrote:
> First let's make sure that we like it best.
> 
> The other alternative is an explicit I/O barrier.
> 
> 	writel(high-water, request-in)
> 	mmiob()  /* memory-mapped I/O barrier */
> 	spin_unlock(hostlock)

I strongly prefer a seperate function call.

I'm wondering if one for write posting and a different for write ordering
would be called for. These are really distinct uses but I'm not sure
it's a distinction that's clear to most folks and they will get misused.
If not, then io_mwb() would be my preference.

> versus
> 
> 	writel(high-water, request-in)
> 	spin_unlock_iob(hostlock)

More variants of spinlocks?
When to use and how to implement it become even less clear.

thanks,
grant

Re: SCSI QLA not working on latest *-mm SN2

[Jeremy Higdon]

On Tue, Sep 21, 2004 at 07:44:28PM -0600, Grant Grundler wrote:
> On Tue, Sep 21, 2004 at 06:16:52PM -0700, Jeremy Higdon wrote:
> > First let's make sure that we like it best.
> > 
> > The other alternative is an explicit I/O barrier.
> > 
> > 	writel(high-water, request-in)
> > 	mmiob()  /* memory-mapped I/O barrier */
> > 	spin_unlock(hostlock)
> 
> I strongly prefer a seperate function call.
> 
> I'm wondering if one for write posting and a different for write ordering
> would be called for. These are really distinct uses but I'm not sure
> it's a distinction that's clear to most folks and they will get misused.
> If not, then io_mwb() would be my preference.

I risk opening yet another can of worms, but here goes . . .

I think I'm convinced that we want separate solutions for
write ordering and (I'll call it) write flushing.

Both issues are derived from write posting.

I'd define write posting as: a store instruction completing
before the write is completed.  Write ordering is a problem
if posted writes from two CPUs can complete to the device
out of order.  Write flushing would be the process of
assuring that a posted write has indeed been completed.
A write flush takes care of write ordering.

I think the only way to perform a write flush is to issue
a register read to the card in question.  This makes it
very heavy weight when all you need is ordering, especially
since ordering is apparently not a problem for many (most?)
platforms that do write posting.

So I think we need one solution for write ordering, and
a separate heavier-weight solution for write flushing.  For
platforms in which write posting does not create ordering problems
the write ordering primitive would be a no-op.

The solution for write ordering on the two arches I know about
is processor/CPU chipset specific (SGI MIPS is a "sync" instruction,
Altix is a read of a SHub register).

Whether we call it io_mwb() or mmiob() is not important to me.
Does io_mwb stand for I/O Memory-mapped Write Barrier?

Sorry to be long-winded.

jeremy

I/O write ordering

[Matthew Wilcox]

On Tue, Sep 21, 2004 at 07:58:05PM -0700, Jeremy Higdon wrote:
> I think I'm convinced that we want separate solutions for
> write ordering and (I'll call it) write flushing.

Yes, I think we have consensus on that.

> I'd define write posting as: a store instruction completing
> before the write is completed.  Write ordering is a problem
> if posted writes from two CPUs can complete to the device
> out of order.  Write flushing would be the process of
> assuring that a posted write has indeed been completed.
> A write flush takes care of write ordering.

Clear, succinct, understandable and correct.

> I think the only way to perform a write flush is to issue
> a register read to the card in question.  This makes it
> very heavy weight when all you need is ordering, especially
> since ordering is apparently not a problem for many (most?)
> platforms that do write posting.

Yes, the PCI specs make it clear that bridges can hang on to the
posted write until a read request goes through the bridge, not
just to the bridge.

> So I think we need one solution for write ordering, and
> a separate heavier-weight solution for write flushing.  For
> platforms in which write posting does not create ordering problems
> the write ordering primitive would be a no-op.

Yes.

> The solution for write ordering on the two arches I know about
> is processor/CPU chipset specific (SGI MIPS is a "sync" instruction,
> Altix is a read of a SHub register).
> 
> Whether we call it io_mwb() or mmiob() is not important to me.
> Does io_mwb stand for I/O Memory-mapped Write Barrier?

I think the only disagreement is whether we have it implicit or explicit.
I see three potential solutions:

1. iomb() [we have mb(), rmb(), wmb() already.  Does mb() imply iomb()?
   Vice versa?]
2. io_spin_unlock() which is an io barrier as well as a memory barrier.
3. Make spin_unlock an io barrier.  Expensive.

I'd quite like to hear opinions from the PPC and Alpha people, they seem to
be the most out-of-order architectures.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: I/O write ordering

[Benjamin Herrenschmidt]

On Thu, 2004-09-23 at 00:32, Matthew Wilcox wrote:

> I'd quite like to hear opinions from the PPC and Alpha people, they seem to
> be the most out-of-order architectures.

The more specific the barrier is, the happier we are on ppc so we can
select the right one in our collection :)

Write ordering is usually just a matter of eieio. _however_ the main
problem we have is write ordering between two different storage classes,
that is between cacheable space and non cacheable space. Our normal
write ordering barrier will not order between those, thus causing all
sorts of issues with devices mixing MMIO & DMA. We have to use a more
heavy-weight barrier in those cases, but currently, linux lacks the
proper abstraction for that, thus we had to clutter then normal MMIO
writes to do the big barrier each time to be safe ...

Ben.

Re: I/O write ordering

[Jesse Barnes]

On Wednesday, September 22, 2004 10:40 am, Benjamin Herrenschmidt wrote:
> On Thu, 2004-09-23 at 00:32, Matthew Wilcox wrote:
> > I'd quite like to hear opinions from the PPC and Alpha people, they seem
> > to be the most out-of-order architectures.
>
> The more specific the barrier is, the happier we are on ppc so we can
> select the right one in our collection :)
>
> Write ordering is usually just a matter of eieio. _however_ the main
> problem we have is write ordering between two different storage classes,
> that is between cacheable space and non cacheable space. Our normal
> write ordering barrier will not order between those, thus causing all
> sorts of issues with devices mixing MMIO & DMA. We have to use a more
> heavy-weight barrier in those cases, but currently, linux lacks the
> proper abstraction for that, thus we had to clutter then normal MMIO
> writes to do the big barrier each time to be safe ...

Ok, mmiob() it is then.  Patch coming shortly (I'll post it to lkml).

As for write flushing, I guess we need to document that people should read a 
safe register (or config space if there isn't one, since that's guaranteed to 
soft fail).

Thanks,
Jesse

Re: I/O write ordering

[James Bottomley]

On Wed, 2004-09-22 at 10:32, Matthew Wilcox wrote:
> I think the only disagreement is whether we have it implicit or explicit.
> I see three potential solutions:
> 
> 1. iomb() [we have mb(), rmb(), wmb() already.  Does mb() imply iomb()?
>    Vice versa?]
> 2. io_spin_unlock() which is an io barrier as well as a memory barrier.
> 3. Make spin_unlock an io barrier.  Expensive.
> 
> I'd quite like to hear opinions from the PPC and Alpha people, they seem to
> be the most out-of-order architectures.

I favour 1. iomb; I think, since I/O barriers can be either heavyweight
or non-existent depending on the architecture, iomb() should imply mb()
but not vice versa.

James

Re: I/O write ordering

[Benjamin Herrenschmidt]

> I favour 1. iomb; I think, since I/O barriers can be either heavyweight
> or non-existent depending on the architecture, iomb() should imply mb()
> but not vice versa.

Read & write barriers are different for IO. Actually, as I stated earlier,
the real issue isn't much with IO barriers proper (which are mostly dealt
with within the implementation of the IO accessors), but in IO vs. memory
accesses, at least on PPC. Cacheable and non-cacheable accesses go through
completely different path on the CPU and can be completely re-ordered one
to each other unless more heavy barriers are used. That has been a pain
for ages for us since Linux doesn't provide an abstraction to order those,
it would be nice to get that in place now ;)

Ben.

Re: I/O write ordering

[James Bottomley]

On Wed, 2004-09-22 at 10:51, Benjamin Herrenschmidt wrote:
> Read & write barriers are different for IO. Actually, as I stated earlier,
> the real issue isn't much with IO barriers proper (which are mostly dealt
> with within the implementation of the IO accessors), but in IO vs. memory
> accesses, at least on PPC. Cacheable and non-cacheable accesses go through
> completely different path on the CPU and can be completely re-ordered one
> to each other unless more heavy barriers are used. That has been a pain
> for ages for us since Linux doesn't provide an abstraction to order those,
> it would be nice to get that in place now ;)

Just to be sure we're talking about the same thing; by cacheable and
non-cacheable are you thinking about what PCI calls consistent and
streaming (because your consistent memory is implemented as
non-cacheable) or is this really the fact that PCI memory space as seen
by the CPU is all uncacheable?

James

Re: I/O write ordering

[Benjamin Herrenschmidt]

> Just to be sure we're talking about the same thing; by cacheable and
> non-cacheable are you thinking about what PCI calls consistent and
> streaming (because your consistent memory is implemented as
> non-cacheable) or is this really the fact that PCI memory space as seen
> by the CPU is all uncacheable?

Set by the CPU as non-cacheable.

Ben.

Re: I/O write ordering

[James Bottomley]

On Wed, 2004-09-22 at 11:11, Benjamin Herrenschmidt wrote:
> Set by the CPU as non-cacheable.

I know that ... but that definition is architecture specific and not
helpful.

I mean when for I/O transactions does the CPU mark memory as
uncacheable.  Is it for the PCI spaces only or for consistent PCI memory
as well?  We need to ascertain in generic I/O terms exactly what it is
that you want synchronising.

James

Re: I/O write ordering

[Benjamin Herrenschmidt]

On Thu, 2004-09-23 at 01:22, James Bottomley wrote:
> On Wed, 2004-09-22 at 11:11, Benjamin Herrenschmidt wrote:
> > Set by the CPU as non-cacheable.
> 
> I know that ... but that definition is architecture specific and not
> helpful.
> 
> I mean when for I/O transactions does the CPU mark memory as
> uncacheable.  Is it for the PCI spaces only or for consistent PCI memory
> as well?  We need to ascertain in generic I/O terms exactly what it is
> that you want synchronising.

Ok, so as far as we are concerned, synchronization issue is between cacheable
vs. non-cacheable as mapped by the CPU MMU. (synchronisation between various
accesses of the same kind is already correctly handled either by the existing
barriers for cacheable space or by the MMIO accessors themselves).

This includes:

 - MMIO space is always non-cacheable (wether it is PCI or other sorts
of IOs)

 - consistent memory is either cacheable or non-cacheable depending on
the CPU type (some embedded CPUs who don't implement cache coherency
will map consistent memory as non-cacheable, most desktop CPUs and all
64 bits CPU don't mind and will map it cacheable).

Ben.

Re: I/O write ordering

[James Bottomley]

On Wed, 2004-09-22 at 11:28, Benjamin Herrenschmidt wrote:
>  - consistent memory is either cacheable or non-cacheable depending on
> the CPU type (some embedded CPUs who don't implement cache coherency
> will map consistent memory as non-cacheable, most desktop CPUs and all
> 64 bits CPU don't mind and will map it cacheable).

Yes, this was what I feared, since streaming memory has exact ownership
and handoff paradigms for handling it.

Your problem case is consistent memory access vs readX/writeX then.

Could you not just make consistent memory always uncacheable ... then
you'd look identical to everyone else...

The alternative in I/O terms is to implement an ownership model for
consistent PCI memory itself...which is sort of what I had to do in the
dma_noncoherent_alloc() API, but to make that more general would be
quite a bit of work.

James

Re: I/O write ordering

[Benjamin Herrenschmidt]

On Thu, 2004-09-23 at 01:43, James Bottomley wrote:
> On Wed, 2004-09-22 at 11:28, Benjamin Herrenschmidt wrote:
> >  - consistent memory is either cacheable or non-cacheable depending on
> > the CPU type (some embedded CPUs who don't implement cache coherency
> > will map consistent memory as non-cacheable, most desktop CPUs and all
> > 64 bits CPU don't mind and will map it cacheable).
> 
> Yes, this was what I feared, since streaming memory has exact ownership
> and handoff paradigms for handling it.
> 
> Your problem case is consistent memory access vs readX/writeX then.
> 
> Could you not just make consistent memory always uncacheable ... then
> you'd look identical to everyone else...

Nope :) That would be a big waste, and it's not simple as that would
also prevent us from using large pages on the linear mapping unless
we start playing horrible tricks with where consistent memory is
allocated from etc... we have a fully coherent DMA architecture, we'd
rather use it !

And that wouldn't help vs. spinlocks which are in real RAM
(we have a similar issue with IO leaking potentially out of locks, 
if we don't use some heavy barriers there too).

> The alternative in I/O terms is to implement an ownership model for
> consistent PCI memory itself...which is sort of what I had to do in the
> dma_noncoherent_alloc() API, but to make that more general would be
> quite a bit of work.

The alternative is to have proper barrier abstractions to use between
MMIO read & consistent memory read, and between consistent memory write
& MMIO write ... or get the architecture designers at IBM change the
way our barriers operate in future chips ;) (which is less likely ? :)

Ben.
 

Re: I/O write ordering

[Matthew Wilcox]

On Thu, Sep 23, 2004 at 10:19:57AM +1000, Benjamin Herrenschmidt wrote:
> And that wouldn't help vs. spinlocks which are in real RAM
> (we have a similar issue with IO leaking potentially out of locks, 
> if we don't use some heavy barriers there too).

Yep, that was the problem that SGI wanted to solve.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: I/O write ordering

[James Bottomley]

On Wed, 2004-09-22 at 21:58, Matthew Wilcox wrote:
> On Thu, Sep 23, 2004 at 10:19:57AM +1000, Benjamin Herrenschmidt wrote:
> > And that wouldn't help vs. spinlocks which are in real RAM
> > (we have a similar issue with IO leaking potentially out of locks, 
> > if we don't use some heavy barriers there too).
> 
> Yep, that was the problem that SGI wanted to solve.

Yes, but the ppc introduces an additional issue with the consistent
memory problem.

The io memory barrier is used to prevent writes to PCI space leaking
around synchronisation primitives.  However, there's also the ordering
of writes to consistent memory vs writes to PCI space.

The ownership model for consistent memory solves this.

Think of it this way: writes to consistent memory are normal memory
writes (they don't trigger actions in the device), it's the writes to
the device space that trigger actions.  All you want to ensure is that
all the consistent memory writes are issued before the device is
triggered into action.

So, in the ownership model you transfer the ownership of the consistent
memory area to the device (which would be the heavyweight barrier on
ppc) then write to the device space to trigger it.

synchronisation of consistent memory accesses can be done via normal
spin locks; synchronisation of device space needs the the iomb() to
prevent reordering leaks around the spinlock.

James

Re: I/O write ordering

[Benjamin Herrenschmidt]

On Thu, 2004-09-23 at 13:01, James Bottomley wrote:

> The ownership model for consistent memory solves this.
> 
> Think of it this way: writes to consistent memory are normal memory
> writes (they don't trigger actions in the device), it's the writes to
> the device space that trigger actions.  All you want to ensure is that
> all the consistent memory writes are issued before the device is
> triggered into action.
> 
> So, in the ownership model you transfer the ownership of the consistent
> memory area to the device (which would be the heavyweight barrier on
> ppc) then write to the device space to trigger it.
> 
> synchronisation of consistent memory accesses can be done via normal
> spin locks; synchronisation of device space needs the the iomb() to
> prevent reordering leaks around the spinlock.

Sounds good, though may be a bit tricky to deal with for things where
both the kernel and the device are fiddling at the same time, like
the USB OHCI shared area, or ethernet descriptors... But it makes
sense. Could probably be built on top of the dma_consistent_sync... &
friends actually if driver used the properly (which they don't as
nobody really understand their semantics).

Ben.

Re: I/O write ordering

[Grant Grundler]

On Thu, Sep 23, 2004 at 01:40:02PM +1000, Benjamin Herrenschmidt wrote:
> Sounds good, though may be a bit tricky to deal with for things where
> both the kernel and the device are fiddling at the same time, like
> the USB OHCI shared area, or ethernet descriptors...

Normally, "ownership" determines access to particular chunks
of shared data.  Ie the programming model makes it clear when the
CPU can modify data and when the CPU is done modifying data.
At a minimum, the host and/or device has to know when it
lost a race to modify or access data.
If there are race conditions in the programming model, iomb()
will not fix them.

> But it makes
> sense. Could probably be built on top of the dma_consistent_sync... &
> friends actually if driver used the properly (which they don't as
> nobody really understand their semantics).

Or maybe the other way around?
ie find a better replacement for dma_consistent_sync()?
But that's a different rat hole that needs to be hashed out
on linux-ppc list...or some other place far away from me. :^)

thanks,
grant

Re: SCSI QLA not working on latest *-mm SN2

[Guennadi Liakhovetski]

On Mon, 20 Sep 2004, Jesse Barnes wrote:

> On Monday, September 20, 2004 4:27 pm, Grant Grundler wrote:
>> I understand "write posting" as when the CPU posts the write
>> to the chipset and the chipset says the write is done even though
>> it hasn't reached the PCI device. It just means the write has reached
>> the PCI "domain" (which is supposed to be strongly ordered).
>
> That's my understanding too.

It might be OT for this specific discussion - but ("for completeness") - I 
think, there are PCI host-controllers that implement _only_ PCI-to-host 
post-write and read FIFOs (buffers)... Example - IT8152.

Regards
Guennadi
---
Guennadi Liakhovetski

Re: SCSI QLA not working on latest *-mm SN2

[Jeremy Higdon]

On Thu, Sep 16, 2004 at 01:56:50PM -0700, Andrew Vasquez wrote:
> On Thu, 2004-09-16 at 13:05, Jesse Barnes wrote:
> > On Thursday, September 16, 2004 12:56 pm, Paul Jackson wrote:
> > > Andrew Vasquez has been looking at this, via private email with just
> > > me (no progress yet).  Figured I update the larger list with this much ...
> > 
> > It seems to be failing on one of the accesses to PCI_COMMAND in config space 
> > in qla2x00_reset_chip().  I'm checking now to see if we're accessing the card 
> > right after a reset but before the card has finished.  That would cause a 
> > master abort, the symptom I'm seeing at least.
> > 
> 
> Interesting, the only changes in reset_chip() are for PCI posting
> issues.  Relevant diff attached.
> 
> --
> Andrew

Are all of those reads really necessary?  Generally the only reason
for doing a read to flush a posted write is for timing issues (in
which the read may not be good enough, according to a thread I saw
from Grant Grundler), or to enforce ordering before releasing a
lock (sleeping or spinning).

Have you run into platforms in which two I/O writes from one CPU are
retired out of order?

jeremy

Re: SCSI QLA not working on latest *-mm SN2

[Andrew Morton]

Paul Jackson <pj@sgi.com> wrote:
>
>  I am still seeing the SN2 SCSI QLA failure as I reported yesterday, but
>  now against 2.6.9-rc2-mm1.

Is it an acpi thing?  Try reverting the three
incorrect-pci-interrupt-assignment* patches?