Received: by 2002:a05:6a10:6744:0:0:0:0 with SMTP id w4csp5702556pxu; Thu, 22 Oct 2020 09:00:14 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx5YzENvanobc1MvMZOdYqIe3rtZMazZ3DFRHi9faxW31OuxeMO6Fg24O52/lel8+d7m7sQ X-Received: by 2002:a17:906:bc50:: with SMTP id s16mr2792996ejv.275.1603382414049; Thu, 22 Oct 2020 09:00:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1603382414; cv=none; d=google.com; s=arc-20160816; b=YQlRmdVcm/wnBAaQdVygkYkpEDuud1e0+aPGrMYAOlyj/OBFWdOKoQEu3Px+Z7OfJg DUGfMEkTaEV5cYa6ze2q66AECoClfNDPI1afRxRHKSsr+f8wrImVFgvQ3iSqKk0HB2Lv duOwNaoEUDHUm6qWHthbNHyrS6D10njh+8DGXAtCQwEp8tiicywSQlczaAdu5QNMxDTW qDr11QFK1WZUMf52sCZgm5vY6jAEeqr2HX9xXd2T6/2/shIH+cJ3C8sNUaDPexDf1tr6 MoGmBmNZwNs6yKIrQM/UBSK3n56HhMxLGc6/dxkuhk6f5bF+yV0OyYT4eF/sAoBX1TXn Tz4g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:date:cc:to:from:subject:message-id; bh=ClywlnkYN/lXk13GMdixk5lq2ImPgNlf/4OKUUtsUAk=; b=xrGLZ0Bq78mAqhEPdNj40QdaQJ+BOqwtTFp0yceT1jXLVedkFvDGIolqG9VyWDI+mH ieuDkajr9dML/qawZnuaw6hvenJAoRdcMOxk7C9ktD3/B2na2HJYeyOEM6P5HgubzeCv xpW9nGeCuf00MzB7pm4HYkOtfoewqStA0COPUu8APqHXdrv0TlpBzm1zD7abGKpvi9c1 u2GH5cPc7fu1awtF2zxCgUNiGhs0/7UHJW3pJ3orr6NMkzYheYfTxt4UJhgnEs6zexLs 50qeCe1Ypub5vyBL+sBXZf+osOwyd1LMIcQKBgRBFiyFtai1EV4rKA023lHisWul3E24 Efwg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id lh7si1093605ejb.309.2020.10.22.08.59.51; Thu, 22 Oct 2020 09:00:14 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2508755AbgJVHlI (ORCPT + 99 others); Thu, 22 Oct 2020 03:41:08 -0400 Received: from kernel.crashing.org ([76.164.61.194]:45150 "EHLO kernel.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2506725AbgJVHlH (ORCPT ); Thu, 22 Oct 2020 03:41:07 -0400 Received: from localhost (gate.crashing.org [63.228.1.57]) (authenticated bits=0) by kernel.crashing.org (8.14.7/8.14.7) with ESMTP id 09M7eafG027618 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 22 Oct 2020 02:40:40 -0500 Message-ID: Subject: Re: [PATCH] net: ftgmac100: Fix missing TX-poll issue From: Benjamin Herrenschmidt To: Arnd Bergmann Cc: Dylan Hung , Jakub Kicinski , Joel Stanley , "David S . Miller" , "netdev@vger.kernel.org" , Linux Kernel Mailing List , Po-Yu Chuang , linux-aspeed , OpenBMC Maillist , BMC-SW Date: Thu, 22 Oct 2020 18:40:35 +1100 In-Reply-To: References: <20201019073908.32262-1-dylan_hung@aspeedtech.com> <20201019120040.3152ea0b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> <32bfb619bbb3cd6f52f9e5da205673702fed228f.camel@kernel.crashing.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2020-10-21 at 14:11 +0200, Arnd Bergmann wrote: > (replying to my own mail from a different address to deal with the > regular one being blacklisted somewhere, sorry for any duplicates) > > On Wed, Oct 21, 2020 at 9:16 AM Arnd Bergmann wrote: > > > > On Wed, Oct 21, 2020 at 12:10 AM Benjamin Herrenschmidt > > wrote: > > > On Tue, 2020-10-20 at 21:49 +0200, Arnd Bergmann wrote: > > > > On Tue, Oct 20, 2020 at 11:37 AM Dylan Hung wrote: > > > > > > +1 @first is system memory from dma_alloc_coherent(), right? > > > > > > > > > > > > You shouldn't have to do this. Is coherent DMA memory broken on your > > > > > > platform? > > > > > > > > > > It is about the arbitration on the DRAM controller. There are two queues in the dram controller, one is for the CPU access and the other is for the HW engines. > > > > > When CPU issues a store command, the dram controller just acknowledges cpu's request and pushes the request into the queue. Then CPU triggers the HW MAC engine, the HW engine starts to fetch the DMA memory. > > > > > But since the cpu's request may still stay in the queue, the HW engine may fetch the wrong data. > > > > > > Actually, I take back what I said earlier, the above seems to imply > > > this is more generic. > > > > > > Dylan, please confirm, does this affect *all* DMA capable devices ? If > > > yes, then it's a really really bad design bug in your chips > > > unfortunately and the proper fix is indeed to make dma_wmb() do a dummy > > > read of some sort (what address though ? would any dummy non-cachable > > > page do ?) to force the data out as *all* drivers will potentially be > > > affected. > > > > > > I was under the impression that it was a specific timing issue in the > > > vhub and ethernet parts, but if it's more generic then it needs to be > > > fixed globally. > > > > We have CONFIG_ARM_HEAVY_MB for SoCs with similar problems, > > it turns mb() and wmb() into a platform specific function call, though it > > doesn't do that for dma_wmb() and smp_wmb(), which should not > > be affected if the problem is only missing serialization between DMA > > and CPU writes. Right. I got mixed up by David mention of dma_wmb, it's not the issue here, it's indeed the ordering of writes to "coherent" memory vs iowrite. > > > > If either of the two is the case, then the READ_ONCE() would just > > > > introduce a long delay before the iowrite32() that makes it more likely > > > > that the data is there, but the inconsistent state would still be observable > > > > by the device if it is still working on previous frames. > > > > > > I think it just get stuck until we try another packet, ie, it doesn't > > > see the new descriptor valid bit. But Dylan can elaborate. > > > > Ok, that would point to an insufficient barrier in iowrite32() as well, > > not in dma_wmb(). Correct. > > At the moment, the only chips that need the heavy barrier are > > omap4 and mstar_v7, and early l2 cache controllers (not the one > > on Cortex-A7) have another synchronization callback that IIRC > > is used for streaming mappings. .../... > > Obviously, adding one of these for ast2600 would slow down every > > mb() and writel() a lot, but if it is a chip-wide problem rather than > > one isolated to the network device, it would be the correct solution, > > provided that a correct code sequence can be found. I'm surprised that problem doesn't already exist on the ast2400 and 2500 and I thus worry about the performance impact of such a workaround applied generally to every MMIO writes.... But we did kill mmiowb so ... ;-) Cheers, Ben.