alpha I/O access and mb()

Here you are two patches:

alpha-mb-2.2.diff add the missing mb() to the cores that still lack it (against 2.2.18pre25)

alpha-mb-2.4.diff add missing defines from core_t2.h for non generic kernel (against 2.4.0test11)

Please apply on your trees.

I've also noted that 2.4 uses mb() after read[bwlq] while 2.2 don't. Who's right?

Abramo Bagnara                       mailto:[email protected]

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is            http://www.alsa-project.org
sponsored by SuSE Linux    http://www.suse.com

It sounds good!

@@ -437,11 +437,13 @@
 __EXTERN_INLINE void pyxis_bw_writel(unsigned int b, unsigned long addr)
 {
 	*(vuip)(addr+PYXIS_BW_MEM) = b;
+	mb();
 }
 
 __EXTERN_INLINE void pyxis_bw_writeq(unsigned long b, unsigned long addr)
 {
 	*(vulp)(addr+PYXIS_BW_MEM) = b;
+	mb();
 }
 
 __EXTERN_INLINE unsigned long pyxis_srm_base(unsigned long addr)

@@ -504,6 +506,7 @@
 	if (work) {
 		work += 0x00;	/* add transfer length */
 		*(vuip) work = b * 0x01010101;
+		mb();
 	}
 }
 

@@ -513,6 +516,7 @@
 	if (work) {
 		work += 0x08;	/* add transfer length */
 		*(vuip) work = b * 0x00010001;
+		mb();
 	}
 }
 

@@ -551,6 +555,7 @@
 	set_hae(msb);
 
 	*(vuip) ((addr << 5) + PYXIS_SPARSE_MEM + 0x00) = b * 0x01010101;
+	mb();
 }
 
 __EXTERN_INLINE void pyxis_writew(unsigned short b, unsigned long addr)

@@ -562,6 +567,7 @@
 	set_hae(msb);
 
 	*(vuip) ((addr << 5) + PYXIS_SPARSE_MEM + 0x08) = b * 0x00010001;
+	mb();
 }
 
 __EXTERN_INLINE unsigned long pyxis_readl(unsigned long addr)

@@ -577,11 +583,13 @@
 __EXTERN_INLINE void pyxis_writel(unsigned int b, unsigned long addr)
 {
 	*(vuip) (addr + PYXIS_DENSE_MEM) = b;
+	mb();
 }
 
 __EXTERN_INLINE void pyxis_writeq(unsigned long b, unsigned long addr)
 {
 	*(vulp) (addr + PYXIS_DENSE_MEM) = b;
+	mb();
 }
 
 /* Find the DENSE memory area for a given bus address.  */

@@ -543,6 +543,7 @@
 		set_hae(msb);
 	}
 	*(vuip) ((addr << 5) + APECS_SPARSE_MEM + 0x00) = b * 0x01010101;
+	mb();
 }
 
 __EXTERN_INLINE void apecs_writew(unsigned short b, unsigned long addr)

@@ -555,16 +556,19 @@
 		set_hae(msb);
 	}
 	*(vuip) ((addr << 5) + APECS_SPARSE_MEM + 0x08) = b * 0x00010001;
+	mb();
 }
 
 __EXTERN_INLINE void apecs_writel(unsigned int b, unsigned long addr)
 {
 	*(vuip) (addr + APECS_DENSE_MEM) = b;
+	mb();
 }
 
 __EXTERN_INLINE void apecs_writeq(unsigned long b, unsigned long addr)
 {
 	*(vulp) (addr + APECS_DENSE_MEM) = b;
+	mb();
 }
 
 /* Find the DENSE memory area for a given bus address.  */

@@ -460,6 +460,7 @@
 		work += 0x00;	/* add transfer length */
 		w = __kernel_insbl(b, addr & 3);
 		*(vuip) work = w;
+		mb();
 	}
 }
 

@@ -470,6 +471,7 @@
 		work += 0x08;	/* add transfer length */
 		w = __kernel_inswl(b, addr & 3);
 		*(vuip) work = w;
+		mb();
 	}
 }
 

@@ -507,6 +509,7 @@
 
 	w = __kernel_insbl(b, addr & 3);
 	*(vuip) ((addr << 5) + CIA_SPARSE_MEM + 0x00) = w;
+	mb();
 }
 
 __EXTERN_INLINE void cia_writew(unsigned short b, unsigned long addr)

@@ -519,6 +522,7 @@
 
 	w = __kernel_inswl(b, addr & 3);
 	*(vuip) ((addr << 5) + CIA_SPARSE_MEM + 0x08) = w;
+	mb();
 }
 
 __EXTERN_INLINE unsigned long cia_readl(unsigned long addr)

@@ -534,11 +538,13 @@
 __EXTERN_INLINE void cia_writel(unsigned int b, unsigned long addr)
 {
 	*(vuip) (addr + CIA_DENSE_MEM) = b;
+	mb();
 }
 
 __EXTERN_INLINE void cia_writeq(unsigned long b, unsigned long addr)
 {
 	*(vulp) (addr + CIA_DENSE_MEM) = b;
+	mb();
 }
 
 /* Find the DENSE memory area for a given bus address.  */

@@ -499,6 +499,7 @@
 	if (work) {
 		work += 0x00;	/* add transfer length */
 		*(vuip) work = b * 0x01010101;
+		mb();
 	}
 }
 

@@ -508,6 +509,7 @@
 	if (work) {
 		work += 0x08;	/* add transfer length */
 		*(vuip) work = b * 0x00010001;
+		mb();
 	}
 }
 

@@ -518,6 +520,7 @@
 	if (work) {
 		work += 0x18;	/* add transfer length */
 		*(vuip) work = b;
+		mb();
 	}
 }
 

@@ -530,6 +533,7 @@
 		work += 0x18;	/* add transfer length */
 		*(vuip) work = b;
 		*(vuip) (work + (4 << 5)) = b >> 32;
+		mb();
 	}
 }
 

@@ -592,6 +596,7 @@
 	set_hae(msb);
 
 	*(vuip) ((addr << 5) + T2_SPARSE_MEM + 0x00) = b * 0x01010101;
+	mb();
 }
 
 __EXTERN_INLINE void t2_writew(unsigned short b, unsigned long addr)

@@ -603,6 +608,7 @@
 	set_hae(msb);
 
 	*(vuip) ((addr << 5) + T2_SPARSE_MEM + 0x08) = b * 0x00010001;
+	mb();
 }
 
 /* On SABLE with T2, we must use SPARSE memory even for 32-bit access. */

@@ -615,6 +621,7 @@
 	set_hae(msb);
 
 	*(vuip) ((addr << 5) + T2_SPARSE_MEM + 0x18) = b;
+	mb();
 }
 
 __EXTERN_INLINE void t2_writeq(unsigned long b, unsigned long addr)

@@ -628,6 +635,7 @@
 	work = (addr << 5) + T2_SPARSE_MEM + 0x18;
 	*(vuip)work = b;
 	*(vuip)(work + (4 << 5)) = b >> 32;
+	mb();
 }
 
 /* Find the DENSE memory area for a given bus address.  */

@@ -349,6 +349,7 @@
 	}
 	w = __kernel_insbl(b, addr & 3);
 	*(vuip) ((addr << 5) + LCA_SPARSE_MEM + 0x00) = w;
+	mb();
 }
 
 __EXTERN_INLINE void lca_writew(unsigned short b, unsigned long addr)

@@ -363,16 +364,19 @@
 	}
 	w = __kernel_inswl(b, addr & 3);
 	*(vuip) ((addr << 5) + LCA_SPARSE_MEM + 0x08) = w;
+	mb();
 }
 
 __EXTERN_INLINE void lca_writel(unsigned int b, unsigned long addr)
 {
 	*(vuip) (addr + LCA_DENSE_MEM) = b;
+	mb();
 }
 
 __EXTERN_INLINE void lca_writeq(unsigned long b, unsigned long addr)
 {
 	*(vulp) (addr + LCA_DENSE_MEM) = b;
+	mb();
 }
 
 /* Find the DENSE memory area for a given bus address.  */

@@ -352,6 +352,7 @@
 	if (work) {
 		work += 0x00;	/* add transfer length */
 		*(vuip) work = b * 0x01010101;
+		mb();
 	}
 }
 

@@ -361,6 +362,7 @@
 	if (work) {
 		work += 0x08;	/* add transfer length */
 		*(vuip) work = b * 0x00010001;
+		mb();
 	}
 }
 

@@ -405,6 +407,7 @@
 	set_hae(msb);
 
 	*(vuip) ((addr << 5) + MCPCIA_SPARSE(hose) + 0x00) = b * 0x01010101;
+	mb();
 }
 
 __EXTERN_INLINE void mcpcia_writew(unsigned short b, unsigned long in_addr)

@@ -418,6 +421,7 @@
 	set_hae(msb);
 
 	*(vuip) ((addr << 5) + MCPCIA_SPARSE(hose) + 0x08) = b * 0x00010001;
+	mb();
 }
 
 __EXTERN_INLINE unsigned long mcpcia_readl(unsigned long in_addr)

@@ -439,6 +443,7 @@
 	unsigned long addr = in_addr & 0xffffffffUL;
 	unsigned long hose = (in_addr >> 32) & 3;
 	*(vuip) (addr + MCPCIA_DENSE(hose)) = b;
+	mb();
 }
 
 __EXTERN_INLINE void mcpcia_writeq(unsigned long b, unsigned long in_addr)

@@ -446,6 +451,7 @@
 	unsigned long addr = in_addr & 0xffffffffUL;
 	unsigned long hose = (in_addr >> 32) & 3;
 	*(vulp) (addr + MCPCIA_DENSE(hose)) = b;
+	mb();
 }
 
 /* Find the DENSE memory area for a given bus address.  */

@@ -533,6 +533,21 @@
 #define __ioremap(a)		t2_ioremap((unsigned long)(a))
 #define __is_ioaddr(a)		t2_is_ioaddr((unsigned long)(a))
 
+#define inb(p)			__inb(p)
+#define inw(p)			__inw(p)
+#define inl(p)			__inl(p)
+#define outb(x,p)		__outb((x),(p))
+#define outw(x,p)		__outw((x),(p))
+#define outl(x,p)		__outl((x),(p))
+#define __raw_readb(a)		__readb(a)
+#define __raw_readw(a)		__readw(a)
+#define __raw_readl(a)		__readl(a)
+#define __raw_readq(a)		__readq(a)
+#define __raw_writeb(v,a)	__writeb((v),(a))
+#define __raw_writew(v,a)	__writew((v),(a))
+#define __raw_writel(v,a)	__writel((v),(a))
+#define __raw_writeq(v,a)	__writeq((v),(a))
+
 #endif /* __WANT_IO_DEF */
 
 #ifdef __IO_EXTERN_INLINE

Let me howl for a minute. :-)

The actual issue regarding ordering is generally to ensure something to happen (i.e. to be seen to happen by other agents) _before_ something else. As a result, what we have in mind is to insert a barrier _before_ this `something else'.

However, everything I seem to see about this issue on our planet and that applies to IO subsystems is blindly inserting barriers _after_ the 'something'.

Is software getting sci. fi. ? ;-)

Based on that, let me claim that most of blind barriers inserted this way are useless (thus sob-optimal) and may band-aid useful barriers that are missing. The result is subtle bugs, hidden most of the time, that we will have to suffer for decades.

The only way to do things right regarding ordering it to have device drivers _aware_ of such issues. Now, if we are happy with broken portable or platform-independant drivers that rely on broken hidden ordering alchemy rather than on correctness, then it is another story.

Gérard.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I see perfectly your point and this is the reason why we have __raw_write[bwlq] in 2.4, but write[bwlq] expected semantic is to ensure that write *happens* and are visible by other agents.

You can tell me that almost nobody uses __raw_write now and this is bad and I agree with you, but sometime this is not a perfect world ;-)

Abramo Bagnara                       mailto:[email protected]

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is            http://www.alsa-project.org
sponsored by SuSE Linux    http://www.suse.com

It sounds good!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Ordering and flushing are different issues. Hardware may flush in order to guarantee some order, but if nothing is to order it may not. A memory barrier does not guarantee you that a write will go faster to the system BUS. Basically, if no other agent does need the data and the data is cacheable, the flushing is just useless. Confusing ordering and flushing is a serious mistake in my humble opinion.

Speaking about MMIO which is not cacheable, indeed we want the data targetting the MMIO area to be flushed quickly. But we also want the device to have a consistent vue of the data in memory for all IOs it is provided with. Usually, we have to deal with the following:

1) Prepare some DATA in cachable memory (DMA related)
2) Ensure ordering: i.e. may insert a MEMORY BARRIER
3) Tell the device about the IO to perform: write to MMIO

Drivers that are unaware of (2) _are_ broken.

Or:

1) Read device status register to know about IO that completed.
2) Ensure ordering of speculative reads against DMA from the device. i.e. may insert a MEMORY (READ) BARRIER.
3) Look into memory for IOs that have completed.

Drivers that are unaware of (2) _are_ broken.

Since the hidden BUS stuff just puts its implicit barriers at the wrong place regarding the above, any device driver that does DMA and that does not use explicit barriers is likely to be broken even if it uses normal IOs. Reason is that the PCI specifications also allow host bridges to post IO transactions and thus assuming 15 years old ISA-like behaviour is plain wrong.

The various BUS abstractions I have to suffer of are indeed a great demonstration of our world being not perfect. ;-)

Hehe... I read so often that most drivers are broken that shoe-horning bunches or barriers, bus things and other band-aidings is probably the only way to have some of them mostly usable. ;-)
Or could it be that current O/S guys are still ISA-bussed. ;-)

By the way, given our real world, your patch is probably quite reasonnable. My point was not to disagree with it, in particular.

Gérard.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

These are not "missing". They are intentionally not present so that stuff will be done out of line.

r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

And this would be the only core_*.h files where this intention is expressed?

It's hard to believe, without you explain why ;-)

Abramo Bagnara                       mailto:[email protected]

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is            http://www.alsa-project.org
sponsored by SuSE Linux    http://www.suse.com

It sounds good!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Not at all. See core_lca.h, jensen.h, core_cia.h, core_mcpcia.h.

r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I think you're confusing the issues: the only missing things from core_t2.h are some defines to permit to it to work when CONFIG_ALPHA_T2 is defined.

The with or without mb() issue is handled in io.h and it's common for all cores.

Abramo Bagnara                       mailto:[email protected]

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is            http://www.alsa-project.org
sponsored by SuSE Linux    http://www.suse.com

It sounds good!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Have you tried it? I did. It works fine.

What _is_ defined in asm/core_t2.h is enough for alpha/lib/io.c to define out of line functions that asm/io.h then uses.

This same mechanism is used on cia, mcpcia, lca, and jensen. It has been in place in just this fashion since 2.1.something.

r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

asm/io.h uses out of line function only when CONFIG_ALPHA_GENERIC is defined, otherwise it uses (take writel as an example) __raw_writel that IMHO need to be defined in core_t2.h.

core_t2.h is the only core_*.h file that does not define it and this is why I've proposed that patch.

Perhaps there is a reason I don't understand and in this case I want to apologize and I'd like to ask you to explain me what I'm missing.

Abramo Bagnara                       mailto:[email protected]

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is            http://www.alsa-project.org
sponsored by SuSE Linux    http://www.suse.com

It sounds good!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Perhaps you should _show_ an actual failure rather than just guessing.

You are wildly incorrect asserting that out of line functions are used only with CONFIG_ALPHA_GENERIC. You should examine

#ifndef __raw_writel

# define __raw_writel(v,a)  ___raw_writel((v),(unsigned long)(a))
#endif

and suchlike definitions.

FOR THE NTH TIME, NO IT IS NOT!

How often do I have to point you at the other files that do not define (all of) these macros before you will believe me?

Jesus H Christ! Look at some preprocessor output why don't you? Better yet, compile the kernel with CONFIG_ALPHA_T2. Notice how it does not fail with undefined symbol errors. Notice how there are non-trivial bit fiddling insns implementing the functions in alpha/lib/io.o. That's the quickest way to see that I'm right and you aren't.

End of discussion.

r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Before of all I want to publicly apologize with Richard as never my intention was to exacerbate him.

I've not access to this specific hardware but I was trying to fix the alpha case wrt write[bwlq] function as I've had a lot of trouble with ALSA drivers and 2.2 (that still now is broken wrt mb() and I've sent a patch to Alpha Processor guys). This is the reason why I've given a look to 2.4.

Now I understand, thanks.

I see that some core*.h leaves out of line some functions because they are more complex than a choosen threshold.

Abramo Bagnara                       mailto:[email protected]

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy

ALSA project is            http://www.alsa-project.org
sponsored by SuSE Linux    http://www.suse.com

It sounds good!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/