NetBSD Problem Report #21665

Received: (qmail 28711 invoked by uid 605); 25 May 2003 01:24:29 -0000
Message-Id: <20030525012427.B862911152@narn.netbsd.org>
Date: Sun, 25 May 2003 01:24:27 +0000 (UTC)
From: ups@stups.com
Sender: gnats-bugs-owner@netbsd.org
Reply-To: ups@stups.com
To: gnats-bugs@gnats.netbsd.org
Subject: bus_dmamap_sync needs a memory barrier
X-Send-Pr-Version: www-1.0

>Number:         21665
>Category:       port-i386
>Synopsis:       bus_dmamap_sync needs a memory barrier
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    port-i386-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 25 01:25:01 +0000 2003
>Closed-Date:    Wed Jan 14 11:49:51 +0000 2004
>Last-Modified:  Wed Jan 14 11:49:51 +0000 2004
>Originator:     Stephan Uphoff
>Release:        1.6T
>Organization:
>Environment:
NetBSD giant 1.6T NetBSD 1.6T (GIANT) #17: Sat May 24 20:58:42 EDT 2003  ups@giant:/usr/home/ups/sources/build/makeobjectdirprefix/usr/home/ups/sources/src/sys/arch/i386/compile/GIANT i386

>Description:

The macro bus_dmamap_sync with operation BUS_DMASYNC_POSTREAD 
for the i386 architecture (all x86 architectures ?) needs a memory
barrier since the P6 family (and others?) performs speculative out
of order reads.

The manual page BUS_DMA(9) states:

      On platforms which implement a weak memory access ordering mod-
      el, bus_dmamap_sync() will always cause the the appropriate mem-
      ory barriers to be issued.

Solution:
      Add an inexpensive locking instruction to the bus_dmamap_sync macro.
>How-To-Repeat:

>Fix:
Patch:
Index: bus.h
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/include/bus.h,v
retrieving revision 1.2
diff -r1.2 bus.h
73a74
> #include <machine/atomic.h>
1126a1128,1133
> do {                                                          \
>         int dummy;                                            \
>         /* Enforce memory order for BUS_DMASYNC_POSTREAD */     \
>         /* Any I/O instruction, locking instruction, */         \
>         /* serializing instruction or the LOCK prefix works */  \
>       (void) x86_atomic_testset_i(&dummy,0);                  \
1128,1129c1135,1138
<           (*(t)->_dmamap_sync)((t), (p), (o), (l), (ops)) : (void)0)
< 
---
>           (*(t)->_dmamap_sync)((t), (p), (o), (l), (ops))     \
>           : (void)0);                                         \
> } while (/* CONSTCOND */ 0)
>       


>Release-Note:
>Audit-Trail:

From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
To: port-i386@netbsd.org, port-amd64@netbsd.org
Cc: gnats-bugs@gnats.netbsd.org, ups@stups.com
Subject: Re: port-i386/21665: bus_dmamap_sync needs a memory barrier
Date: Thu, 08 Jan 2004 19:54:43 +0900

 hi,

 i'll commit the following diff if no one objects.
 (not tested on amd64.)

 YAMAMOTO Takashi


 Index: arch/i386/include/cpufunc.h
 ===================================================================
 --- arch/i386/include/cpufunc.h	(revision 490)
 +++ arch/i386/include/cpufunc.h	(revision 492)
 @@ -54,6 +54,16 @@ x86_pause(void)
  	__asm __volatile("pause");
  }

 +static __inline void
 +x86_lfence(void)
 +{
 +
 +	/*
 +	 * XXX it's better to use real lfence insn if available.
 +	 */
 +	__asm __volatile("lock; addl $0, 0(%%esp)" : : : "memory");
 +}
 +
  #ifdef _KERNEL

  extern unsigned int cpu_feature;
 Index: arch/amd64/include/cpufunc.h
 ===================================================================
 --- arch/amd64/include/cpufunc.h	(revision 490)
 +++ arch/amd64/include/cpufunc.h	(revision 492)
 @@ -54,6 +54,18 @@ x86_pause(void)
  	/* nothing */
  }

 +static __inline void
 +x86_lfence(void)
 +{
 +
 +	/*
 +	 * XXX if lfence isn't available...
 +	 *
 +	 * memory clobber to avoid compiler reordering.
 +	 */
 +	__asm __volatile("lfence" : : : "memory");
 +}
 +
  #ifdef _KERNEL

  extern int cpu_feature;
 Index: arch/x86/include/bus.h
 ===================================================================
 --- arch/x86/include/bus.h	(revision 490)
 +++ arch/x86/include/bus.h	(revision 492)
 @@ -72,6 +72,7 @@
  #define _X86_BUS_H_

  #include <machine/pio.h>
 +#include <machine/cpufunc.h>	/* for x86_lfence */

  #ifdef BUS_SPACE_DEBUG
  #include <sys/systm.h> /* for printf() prototype */
 @@ -1109,6 +1110,9 @@ struct x86_bus_dma_tag {
  		    int, off_t, int, int);
  };

 +static __inline void bus_dmamap_sync(bus_dma_tag_t, bus_dmamap_t,
 +    bus_addr_t, bus_size_t, int) __attribute__((__unused__));
 +
  #define	bus_dmamap_create(t, s, n, m, b, f, p)			\
  	(*(t)->_dmamap_create)((t), (s), (n), (m), (b), (f), (p))
  #define	bus_dmamap_destroy(t, p)				\
 @@ -1123,9 +1127,15 @@ struct x86_bus_dma_tag {
  	(*(t)->_dmamap_load_raw)((t), (m), (sg), (n), (s), (f))
  #define	bus_dmamap_unload(t, p)					\
  	(*(t)->_dmamap_unload)((t), (p))
 -#define	bus_dmamap_sync(t, p, o, l, ops)			\
 -	(void)((t)->_dmamap_sync ?				\
 -	    (*(t)->_dmamap_sync)((t), (p), (o), (l), (ops)) : (void)0)
 +static __inline void
 +bus_dmamap_sync(bus_dma_tag_t t, bus_dmamap_t p, bus_addr_t o, bus_size_t l,
 +    int ops)
 +{
 +	if (ops & BUS_DMASYNC_POSTREAD)
 +		x86_lfence();
 +	if (t->_dmamap_sync)
 +		(*t->_dmamap_sync)(t, p, o, l, ops);
 +}

  #define	bus_dmamem_alloc(t, s, a, b, sg, n, r, f)		\
  	(*(t)->_dmamem_alloc)((t), (s), (a), (b), (sg), (n), (r), (f))

From: YAMAMOTO Takashi <yamt@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:  
Subject: pr/21665 CVS commit: src/sys/arch
Date: Wed, 14 Jan 2004 11:31:55 +0000 (UTC)

 Module Name:	src
 Committed By:	yamt
 Date:		Wed Jan 14 11:31:55 UTC 2004

 Modified Files:
 	src/sys/arch/amd64/include: cpufunc.h
 	src/sys/arch/i386/include: cpufunc.h
 	src/sys/arch/x86/include: bus.h

 Log Message:
 issue memory read barrier for BUS_DMASYNC_POSTREAD operation.
 PR/21665 from Stephan Uphoff.


 To generate a diff of this commit:
 cvs rdiff -r1.3 -r1.4 src/sys/arch/amd64/include/cpufunc.h
 cvs rdiff -r1.27 -r1.28 src/sys/arch/i386/include/cpufunc.h
 cvs rdiff -r1.4 -r1.5 src/sys/arch/x86/include/bus.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed 
State-Changed-By: yamt 
State-Changed-When: Wed Jan 14 11:49:03 UTC 2004 
State-Changed-Why:  
equivalent patch has been committed. 
>Unformatted:

Home	PR Database Search