NetBSD Problem Report #38932

From bouyer@antioche.lip6.fr  Tue Jun 10 14:51:16 2008
Return-Path: <bouyer@antioche.lip6.fr>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id C377A63B8BC
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 10 Jun 2008 14:51:15 +0000 (UTC)
Message-Id: <200806101451.m5AEp9XM015664@armandeche.lip6.fr>
Date: Tue, 10 Jun 2008 16:51:09 +0200 (MEST)
From: Manuel Bouyer <bouyer@antioche.lip6.fr>
Reply-To: bouyer@antioche.lip6.fr
To: gnats-bugs@gnats.NetBSD.org
Subject: sgimips bus_dmamap_sync() is incorrect
X-Send-Pr-Version: 3.95

>Number:         38932
>Category:       port-sgimips
>Synopsis:       sgimips bus_dmamap_sync() is incorrect
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    port-sgimips-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 10 14:55:00 +0000 2008
>Closed-Date:    Wed Jun 11 15:41:47 +0000 2008
>Last-Modified:  Wed Jun 11 15:41:47 +0000 2008
>Originator:     bouyer@antioche.lip6.fr
>Release:        current
>Organization:
>Environment:
System:
Architecture: sgimips
Machine: sgimips
>Description:
	The sgimips bus_dmamap_sync() do nothing for POSTREAD/POSTWRITE
	operations, assuming the PREREAD/PREWRITE operations have properly
	flushed/invalidated the cache. But cache invalidation should be done
	on POSTREAD operations too:
	if a driver loops waiting for a device to update a memory word
	by DMA (say, a status completion word of a DMA descriptor),
	the first read will properly fetch the value from memory, but
	subsequent ones will just get again the cached value, and the
	driver will never notice the updated word. If
	bus_dmamap_sync(POSTREAD) does a cache invalidation, the driver will
	see the updated value when the device writes it to memory.

>How-To-Repeat:
	code inspection.
>Fix:
	bus_dmamap_sync(POSTREAD) should do a cache invalidation.
	bus_dmamap_sync(PREREAD) should probably be turned to a NOP
	(we may still to cache invalidation in addition to cache flush
	in the case PREREAD|PREWRITE).

>Release-Note:

>Audit-Trail:
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: gnats-bugs@NetBSD.org
Cc: port-sgimips-maintainer@NetBSD.org, gnats-admin@NetBSD.org,
        netbsd-bugs@NetBSD.org, tsutsui@ceres.dti.ne.jp
Subject: Re: port-sgimips/38932: sgimips bus_dmamap_sync() is incorrect
Date: Wed, 11 Jun 2008 04:31:35 +0900

 > 	bus_dmamap_sync(POSTREAD) should do a cache invalidation.
 > 	bus_dmamap_sync(PREREAD) should probably be turned to a NOP

 No, it won't work if specified region isn't cacheline aligned.

 We can't invalidate cache if specified region is not cacheline aligned
 (otherwise some necessary data in the same cacheline could be lost) and
 we have to use writeback and invalidate in that case, but
 such writeback should be done _before_ starting DMA otherwise
 transfered data via DMA will be lost by writeback.
 That's the reason why all POSTWRITE and POSTREAD are no-op.

 Drivers should call PREREAD after each polling instead.
 (see sys/dev/ic/rtl8169.c:re_rxeof() etc.)
 ---
 Izumi Tsutsui

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@NetBSD.org
Cc: port-sgimips-maintainer@NetBSD.org, gnats-admin@NetBSD.org,
        netbsd-bugs@NetBSD.org
Subject: Re: port-sgimips/38932: sgimips bus_dmamap_sync() is incorrect
Date: Tue, 10 Jun 2008 21:59:36 +0200

 On Tue, Jun 10, 2008 at 07:35:02PM +0000, Izumi Tsutsui wrote:
 > The following reply was made to PR port-sgimips/38932; it has been noted by GNATS.
 > 
 > From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
 > To: gnats-bugs@NetBSD.org
 > Cc: port-sgimips-maintainer@NetBSD.org, gnats-admin@NetBSD.org,
 >         netbsd-bugs@NetBSD.org, tsutsui@ceres.dti.ne.jp
 > Subject: Re: port-sgimips/38932: sgimips bus_dmamap_sync() is incorrect
 > Date: Wed, 11 Jun 2008 04:31:35 +0900
 > 
 >  > 	bus_dmamap_sync(POSTREAD) should do a cache invalidation.
 >  > 	bus_dmamap_sync(PREREAD) should probably be turned to a NOP
 >  
 >  No, it won't work if specified region isn't cacheline aligned.
 >  
 >  We can't invalidate cache if specified region is not cacheline aligned
 >  (otherwise some necessary data in the same cacheline could be lost) and
 >  we have to use writeback and invalidate in that case, but
 >  such writeback should be done _before_ starting DMA otherwise
 >  transfered data via DMA will be lost by writeback.
 >  That's the reason why all POSTWRITE and POSTREAD are no-op.
 >  
 >  Drivers should call PREREAD after each polling instead.
 >  (see sys/dev/ic/rtl8169.c:re_rxeof() etc.)

 I see your point. But I'm not sure such devices can work reliably
 with a write-back cache if regions are not cacheline-aligned.
 For example, I'm almost sure an uchi/ehci can't work reliably on hosts
 with write-back caches (unless the descriptors are mappped uncached).
 I suspect re(4) has the same issue with re_desc->re_cmdstat (the updated
 re_cmdstat from a descriptor may be lost if the host writes another
 descriptor in the same cache line).

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Cc: gnats-bugs@NetBSD.org, port-sgimips-maintainer@NetBSD.org,
        gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: port-sgimips/38932: sgimips bus_dmamap_sync() is incorrect
Date: Tue, 10 Jun 2008 22:11:14 +0200

 On Wed, Jun 11, 2008 at 05:05:50AM +0900, Izumi Tsutsui wrote:
 > > I see your point. But I'm not sure such devices can work reliably
 > > with a write-back cache if regions are not cacheline-aligned.
 > > For example, I'm almost sure an uchi/ehci can't work reliably on hosts
 > > with write-back caches (unless the descriptors are mappped uncached).
 > > I suspect re(4) has the same issue with re_desc->re_cmdstat (the updated
 > > re_cmdstat from a descriptor may be lost if the host writes another
 > > descriptor in the same cache line).
 > 
 > Yes, ideally we can allocate only one descriptor per each cacheline
 > to avoid race, but not all devices (for PC) assume such hardware.
 > In such case, BUS_DMA_COHERENT is mandatory.
 > 
 > But anyway we can't invalidate cache in POSTREAD/POSTWRITE
 > because they have the same problems even on data xfers.

 On second though I think you're right on this. Please close the PR.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: bouyer@antioche.eu.org
Cc: gnats-bugs@NetBSD.org, port-sgimips-maintainer@NetBSD.org,
        gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org,
        tsutsui@ceres.dti.ne.jp
Subject: Re: port-sgimips/38932: sgimips bus_dmamap_sync() is incorrect
Date: Wed, 11 Jun 2008 05:05:50 +0900

 > I see your point. But I'm not sure such devices can work reliably
 > with a write-back cache if regions are not cacheline-aligned.
 > For example, I'm almost sure an uchi/ehci can't work reliably on hosts
 > with write-back caches (unless the descriptors are mappped uncached).
 > I suspect re(4) has the same issue with re_desc->re_cmdstat (the updated
 > re_cmdstat from a descriptor may be lost if the host writes another
 > descriptor in the same cache line).

 Yes, ideally we can allocate only one descriptor per each cacheline
 to avoid race, but not all devices (for PC) assume such hardware.
 In such case, BUS_DMA_COHERENT is mandatory.

 But anyway we can't invalidate cache in POSTREAD/POSTWRITE
 because they have the same problems even on data xfers.
 ---
 Izumi Tsutsui

State-Changed-From-To: open->closed
State-Changed-By: tsutsui@NetBSD.org
State-Changed-When: Thu, 12 Jun 2008 00:41:47 +0900
State-Changed-Why:
Not a bug.


>Unformatted:

Home	PR Database Search