From owner-svn-src-head@freebsd.org Mon Jan 25 17:28:54 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E368BA46FE3 for ; Mon, 25 Jan 2016 17:28:54 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from pmta2.delivery6.ore.mailhop.org (pmta2.delivery6.ore.mailhop.org [54.200.129.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id ACE531C3 for ; Mon, 25 Jan 2016 17:28:54 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from ilsoft.org (unknown [73.34.117.227]) by outbound2.ore.mailhop.org (Halon Mail Gateway) with ESMTPSA; Mon, 25 Jan 2016 17:29:44 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.14.9) with ESMTP id u0PHSkbc025993; Mon, 25 Jan 2016 10:28:46 -0700 (MST) (envelope-from ian@freebsd.org) Message-ID: <1453742926.42081.8.camel@freebsd.org> Subject: Re: svn commit: r294727 - head/sys/arm/arm From: Ian Lepore To: Andrew Turner , Konstantin Belousov Cc: Svatopluk Kraus , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Date: Mon, 25 Jan 2016 10:28:46 -0700 In-Reply-To: <20160125170053.7ae20536@zapp.Home> References: <201601251409.u0PE9abE013306@repo.freebsd.org> <20160125154453.GA3942@kib.kiev.ua> <20160125170053.7ae20536@zapp.Home> Content-Type: text/plain; charset="us-ascii" X-Mailer: Evolution 3.16.5 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jan 2016 17:28:55 -0000 On Mon, 2016-01-25 at 17:00 +0000, Andrew Turner wrote: > On Mon, 25 Jan 2016 17:44:53 +0200 > Konstantin Belousov wrote: > > > On Mon, Jan 25, 2016 at 02:09:36PM +0000, Svatopluk Kraus wrote: > > > Author: skra > > > Date: Mon Jan 25 14:09:35 2016 > > > New Revision: 294727 > > > URL: https://svnweb.freebsd.org/changeset/base/294727 > > > > > > Log: > > > Fix an occasional undefined instruction abort during module > > > loading. > > > Even if data cache maintenance was done by IO code, the > > > relocation > > > fixup process creates dirty cache entries that we must write > > > back > > > before doing icache sync. > > Does arm64 need the same fix ? > > > > I don't think so. On arm64 we call cpu_icache_sync_range to sync the > I > and D cache. This will clean the D-Cache to the Point of Coherency, > then invalidate the I-Cache. I think this is slightly wrong as we > only > need to clean to the Point of Unification. > > Looking at the ARMv7 implementation of cpu_icache_sync_all is wrong, > it > only invalidates the I-Cache. cpu_icache_sync_range is almost > correct, > it will clean the D-Cache, with the same issue as arm64, but it is > missing a barrier after this operation, and is missing a branch > predictor invalidation. > > This change seems to be working around the brokenness of the existing > cache sync operations rather than fixing them, however I don't know > the > details on why this approach to fixing the issue was taken. > > Andrew > I disagree that the fact that icache_sync only cleans the icache means it's broken. It means that the callers have to do the right thing with the data cache before calling the icache ops depending on the situation, and that's what arm code has always done. In this case, we weren't failing to do the right thing because of broken code, we were failing due to my flawed analysis of the situation. As the original comment indicated, I believed the data caches were already clean due to the IO code cache operations. I negle cted to take into account the relocation fixup activity after the IO which re-dirtied some of the cache lines. -- Ian