Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Jun 2012 23:14:45 +0200
From:      Marius Strobl <marius@alchemy.franken.de>
To:        John Baldwin <jhb@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r237008 - head/sys/dev/pci
Message-ID:  <20120626211445.GB63893@alchemy.franken.de>
In-Reply-To: <201206251424.24621.jhb@freebsd.org>
References:  <201206131504.q5DF4opt031336@svn.freebsd.org> <201206251000.09052.jhb@freebsd.org> <20120625170811.GI69382@alchemy.franken.de> <201206251424.24621.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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: <OFW PCIe-PCIe bridge> 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: <OFW PCI bus> on pcib3
> > > > <...>
> > > > isab0: <PCI-ISA bridge> at device 30.0 on pci3
> > > > isa0: <ISA bus> on isab0
> > > > <...>
> > > > rtc0: <Real-Time Clock> 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: <Real-Time Clock> 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.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120626211445.GB63893>