From owner-svn-src-head@FreeBSD.ORG Tue Jun 26 21:14:53 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C98C1106564A; Tue, 26 Jun 2012 21:14:53 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 3097A8FC1A; Tue, 26 Jun 2012 21:14:53 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.4/8.14.4/ALCHEMY.FRANKEN.DE) with ESMTP id q5QLEjpp066251; Tue, 26 Jun 2012 23:14:45 +0200 (CEST) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.4/8.14.4/Submit) id q5QLEjT7066250; Tue, 26 Jun 2012 23:14:45 +0200 (CEST) (envelope-from marius) Date: Tue, 26 Jun 2012 23:14:45 +0200 From: Marius Strobl To: John Baldwin Message-ID: <20120626211445.GB63893@alchemy.franken.de> References: <201206131504.q5DF4opt031336@svn.freebsd.org> <201206251000.09052.jhb@freebsd.org> <20120625170811.GI69382@alchemy.franken.de> <201206251424.24621.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201206251424.24621.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r237008 - head/sys/dev/pci X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 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: Tue, 26 Jun 2012 21:14:53 -0000 On Mon, Jun 25, 2012 at 02:24:24PM -0400, John Baldwin wrote: > On Monday, June 25, 2012 1:08:11 pm Marius Strobl wrote: > > On Mon, Jun 25, 2012 at 10:00:08AM -0400, John Baldwin wrote: > > > On Saturday, June 23, 2012 6:16:26 pm Marius Strobl wrote: > > > > On Wed, Jun 13, 2012 at 03:04:50PM +0000, John Baldwin wrote: > > > > > Author: jhb > > > > > Date: Wed Jun 13 15:04:50 2012 > > > > > New Revision: 237008 > > > > > URL: http://svn.freebsd.org/changeset/base/237008 > > > > > > > > > > Log: > > > > > Fix a couple of bugs that prevented windows in PCI-PCI bridges from > > > > > growing "downward" (moving the start address down). First, an off by > > > > > one error caused the end address to be moved down an extra alignment > > > > > chunk unnecessarily. Second, when aligning the new candidate starting > > > > > address, the wrong bits were masked off. > > > > > > > > > > > > > Unfortunately, this now panics a sparc64 machine on the first attempt > > > > to use a grown resource via bus_space(9) for me: > > > > pcib3: at device 0.0 on pci2 > > > > pcib2: allocated I/O port range (0x1000-0x1fff) for rid 1c of pcib3 > > > > pcib2: allocated memory range (0x200000-0x3ffffff) for rid 20 of pcib3 > > > > pcib3: domain 0 > > > > pcib3: secondary bus 5 > > > > pcib3: subordinate bus 5 > > > > pcib3: I/O decode 0x1000-0x1fff > > > > pcib3: memory decode 0x200000-0x3ffffff > > > > pcib3: no prefetched decode > > > > pcib3: Subtractively decoded bridge. > > > > <...> > > > > pci3: on pcib3 > > > > <...> > > > > isab0: at device 30.0 on pci3 > > > > isa0: on isab0 > > > > <...> > > > > rtc0: at port 0x70-0x73 on isa0 > > > > pcib3: attempting to grow I/O port window for (0x70-0x73,0x4) > > > > front candidate range: 0x70-0x73 > > > > pcib3: grew I/O port window to 0x70-0x1fff > > > > panic: start address is not aligned > > > > Alternatively, this may also be a data access trap, which also indicates > > > > that some invalid address being used for the access. > > > > > > I think this was fixed in the next commit to this file (I had gotten the > > > mask bits on 'front' wrong). Yes, it should be fixed by r237271: > > > > > > Old version: > > > > > > (gdb) p/x (0x70 & (~(1ul << 12) - 1)) > > > $1 = 0x70 > > > > > > Fixed version: > > > > > > (gdb) p/x (0x70 & (~((1ul << 12) - 1))) > > > $2 = 0x0 > > > > Well, a stock r237433 still panics with a data access trap when > > trying to use the resource via bus_space(9). So while the math for > > growing the window is probably right now, there still is a problem. > > Hmm. It would be interesting to know if it used to grow before (it might > not have due to the bugs I fixed in the growing code). Prior to r237008 growing was attempted but failed due to the bug. See the "before:" snippet in my original email. > > > > > > > > before: > > > > rtc0: at port 0x70-0x73 on isa0 > > > > pcib3: attempting to grow I/O port window for (0x70-0x73,0x4) > > > > pcib2: allocated I/O port range (0x70-0x73) for rid 0 of rtc0 > > > > > > > > Shouldn't a subtractively decoded resource actually be outside of > > > > the window of the parent PCI-PCI bridge, i.e. it seems we shouldn't > > > > try to grow the window in that case? The below patch fixes this for > > > > me, I'm not sure whether that actually is the right approach though. > > > > > > Well, I've seen subtractive bridges with programmed windows, and the resource > > > will decode properly either way. What the current code does is allow the > > > request to pass up the tree if growing fails. > > > > By growing the window to 0x0-0x1fff in this case we are effectively > > turning the formerly subtractively decoded resource in a positively > > decodeded one. Maybe there's some additional bit, probably in the > > PCI-ISA bridge, that needs to be switch for that, too? In any case, > > growing the window in this case and by that changing the type of > > decoding seems like a strange approach to me. Why do subtractive > > decoders exist in the first place when the windows alternatively > > could be grown/set up to only just do positive decoding instead? > > The PCI-ISA bridge should already be decoding that range. Note that > subtractive decoding is slower (it has to wait for an extra cycle to > give other devices a chance to snag a request). I would not mind a > tunable to control growing or not growing a window on a subtractively > decoded bridge. Does the firmware assign a window to this bridge > btw? We probably should not allocate a new window for a subtractively > decoded bridge, but if the firmware has already assigned a window, > growing an existing window seems less problematic. > Yes, it does but switching to positive decoding nevertheless is problematic in this case. According to the PCI 2.3 specification, a child wanting its addresses to be decoded subtractively needs to provide special DEVSEL# behavior. It seems that it should work for a bridge to actually positively decoding such transactions, but I'm not familiar with the physical layer of PCI and thus not sure. On the other hand, both the PCI-ISA and PCI-PCI bridges in question are ALi/ULi crap and based on fun I had with their chips in the past, I also wouldn't be surprised their again being a magic bit in the configuration space needing to be set to make this work. Unfortunately, I don't have a datasheet for this M5249 PCI-PCI bridge. If we'd implement a tunable for changing the decoding of subtractive bridges, the way to go would seem to be to have it off by default in order to preserve pre-NEW_PCIB/ r237008 as at this time it's unknown how many bridges are affected. If you'd want to keep the new/current behavior by default, introducing something like a PCIB_SUBTRACTIVE_FORCED flag along a PCI_QUIRK_SUB_FORCED quirk in addition to a tuneable defaulting to on probably would be a good idea.