regression fixed by using pci=rom

regression fixed by using pci=rom

[Dave Airlie]

A user just reported this bug

http://bugs.freedesktop.org/show_bug.cgi?id=15523

He says using pci=rom fixes the regression, do we truly have to start
booting machines with this, sucks to be a distro..

Dave.

Re: regression fixed by using pci=rom

[Jesse Barnes]

On Thursday, May 08, 2008 4:54 pm Dave Airlie wrote:
> A user just reported this bug
>
> http://bugs.freedesktop.org/show_bug.cgi?id=15523
>
> He says using pci=rom fixes the regression, do we truly have to start
> booting machines with this, sucks to be a distro..

Hm, yeah in many cases we definitely *do* want to try to get the expansion ROM 
space allocated.  But maybe it should be a lower priority than other BARs...  
Gary?

Thanks,
Jesse

Re: regression fixed by using pci=rom

[Linus Torvalds]

On Thu, 8 May 2008, Jesse Barnes wrote:
> 
> Hm, yeah in many cases we definitely *do* want to try to get the expansion ROM 
> space allocated.  But maybe it should be a lower priority than other BARs...  
> Gary?

The thing is, a lot of these things have been done this way because not 
doing them that way breaks.

We want to allocate expansion ROM space - even if we don't enable it - 
because not doing so will screw up bus sizing etc, and can make it 
impossible to allocate later.

In general, changing PCI allocation strategy is really _really_ dangerous, 
even when it is "right", because it tends to expose a lot of issues where 
something worked just because it was perhaps indirectly causing a layout 
that worked.

So the reason I immediately reverted this is that it was simply totally 
wrong. If somebody cares about multi-node systems, the onus of making 
those work should be on *that*, not on old systems that already work.

Ingo, Thomas: I would _seriously_ suggest that you don't consider the x86 
PCI setup code to be "x86" code. Because it isn't. Not in that sense. Just 
don't take patches to it.

		Linus

Re: regression fixed by using pci=rom

[Jesse Barnes]

On Thursday, May 08, 2008 6:46 pm Linus Torvalds wrote:
> On Thu, 8 May 2008, Jesse Barnes wrote:
> > Hm, yeah in many cases we definitely *do* want to try to get the
> > expansion ROM space allocated.  But maybe it should be a lower priority
> > than other BARs... Gary?
>
> The thing is, a lot of these things have been done this way because not
> doing them that way breaks.
>
> We want to allocate expansion ROM space - even if we don't enable it -
> because not doing so will screw up bus sizing etc, and can make it
> impossible to allocate later.
>
> In general, changing PCI allocation strategy is really _really_ dangerous,
> even when it is "right", because it tends to expose a lot of issues where
> something worked just because it was perhaps indirectly causing a layout
> that worked.

Yeah, that was my first impression too, but the patch went upstream awhile ago 
and I didn't see any background except for the changelog.

If anything, we should have a pci=norom option instead, so that big systems 
that don't need the ROMs can use their address space more efficiently.

Jesse

Re: regression fixed by using pci=rom

[Gary Hade]

On Thu, May 08, 2008 at 11:29:39PM -0700, Jesse Barnes wrote:
> On Thursday, May 08, 2008 6:46 pm Linus Torvalds wrote:
> > On Thu, 8 May 2008, Jesse Barnes wrote:
> > > Hm, yeah in many cases we definitely *do* want to try to get the
> > > expansion ROM space allocated.  But maybe it should be a lower priority
> > > than other BARs... Gary?
> >
> > The thing is, a lot of these things have been done this way because not
> > doing them that way breaks.
> >
> > We want to allocate expansion ROM space - even if we don't enable it -
> > because not doing so will screw up bus sizing etc, and can make it
> > impossible to allocate later.
> >
> > In general, changing PCI allocation strategy is really _really_ dangerous,
> > even when it is "right", because it tends to expose a lot of issues where
> > something worked just because it was perhaps indirectly causing a layout
> > that worked.
> 
> Yeah, that was my first impression too, but the patch went upstream awhile ago 
> and I didn't see any background except for the changelog.
> 
> If anything, we should have a pci=norom option instead, so that big systems 
> that don't need the ROMs can use their address space more efficiently.

This is actually the solution I was going to suggest.  I will 
work on it.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: regression fixed by using pci=rom

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, 8 May 2008, Jesse Barnes wrote:
> > 
> > Hm, yeah in many cases we definitely *do* want to try to get the 
> > expansion ROM space allocated.  But maybe it should be a lower 
> > priority than other BARs...  Gary?
> 
> The thing is, a lot of these things have been done this way because 
> not doing them that way breaks.
> 
> We want to allocate expansion ROM space - even if we don't enable it - 
> because not doing so will screw up bus sizing etc, and can make it 
> impossible to allocate later.
> 
> In general, changing PCI allocation strategy is really _really_ 
> dangerous, even when it is "right", because it tends to expose a lot 
> of issues where something worked just because it was perhaps 
> indirectly causing a layout that worked.

i tend to believe that the best strategy would be to generally do what 
other OSs do for PCI BAR sizing and PCI resource setup [on desktop 
systems that would be Windows in particular] - and i believe there are 
still a number of gratuitous-looking differences in our code that seem 
unnecessary. Especially on the myriads of desktop systems Windows is 
what gets tested primarily, and by deviating from that legacy layout we 
just set up ourselves for unnecessary failures.

To Linux the primary interest is in devices not failing due to 
unexpected layout and them not overlapping any magic areas - good 
resource compression is something that larger systems can opt-in into 
anyway if it's really needed. In that sense this commit that came 
through us failed that strict benchmark :-/

> So the reason I immediately reverted this is that it was simply 
> totally wrong. If somebody cares about multi-node systems, the onus of 
> making those work should be on *that*, not on old systems that already 
> work.
> 
> Ingo, Thomas: I would _seriously_ suggest that you don't consider the 
> x86 PCI setup code to be "x86" code. Because it isn't. Not in that 
> sense. Just don't take patches to it.

yeah, we stopped doing that in this window (the commits you got form us 
in this window were all leftovers and acked by Jesse) - and we've 
already bounced over all PCI-looking patches to Jesse and asked Jesse 
about all PCI affecting pulls as well in this window. Even this window's 
leftovers we made separate topics and sent separate pull requests for 
them (all tested and acked by Jesse) because it's not really arch/x86 
and PCI commits just need to be considered separately from all other 
changes.

	Ingo

Re: regression fixed by using pci=rom

[Gary Hade]

On Fri, May 09, 2008 at 08:44:03AM +0200, Ingo Molnar wrote:
> 
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Thu, 8 May 2008, Jesse Barnes wrote:
> > > 
> > > Hm, yeah in many cases we definitely *do* want to try to get the 
> > > expansion ROM space allocated.  But maybe it should be a lower 
> > > priority than other BARs...  Gary?
> > 
> > The thing is, a lot of these things have been done this way because 
> > not doing them that way breaks.
> > 
> > We want to allocate expansion ROM space - even if we don't enable it - 
> > because not doing so will screw up bus sizing etc, and can make it 
> > impossible to allocate later.
> > 
> > In general, changing PCI allocation strategy is really _really_ 
> > dangerous, even when it is "right", because it tends to expose a lot 
> > of issues where something worked just because it was perhaps 
> > indirectly causing a layout that worked.
> 
> i tend to believe that the best strategy would be to generally do what 
> other OSs do for PCI BAR sizing and PCI resource setup [on desktop 
> systems that would be Windows in particular] - and i believe there are 
> still a number of gratuitous-looking differences in our code that seem 
> unnecessary. Especially on the myriads of desktop systems Windows is 
> what gets tested primarily, and by deviating from that legacy layout we 
> just set up ourselves for unnecessary failures.

Ingo, Would you (or others listening to this discussion) know
exactly what Windows does with respect to BIOS unassigned
expansion ROMs?  Does Windows attempt to obtain space for the
expansion ROMs at boot-time or does it perhaps have some sort
of strategy where space is allocated on an "as needed" basis
at run-time?

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: regression fixed by using pci=rom

[Linus Torvalds]

On Fri, 9 May 2008, Gary Hade wrote:
> 
> Ingo, Would you (or others listening to this discussion) know
> exactly what Windows does with respect to BIOS unassigned
> expansion ROMs?

Nobody really seems to know what windows does for *any* resources. And it 
probably depends on windows version too, and on detailed chipset issues 
(ie does Windows do the same thing as Linux wrt transparent bridges that 
still have bus translation resources set up?).

So the "try to do what Windows does" is something we strive for in 
particular cases, but it's not an absolute thing, and probably can never 
even be that in theory. It's more a guideline, especially when we don't 
know which particular choice is better and both seem otherwise equally 
good.

But "doesn't work" always trumps "that's what windows does". Because even 
if something works under Windows, sometimes Linux drivers simply then act 
differently (ie they may use PIO vs MMIO, or in the case of graphics they 
may need ROMs for re-POST'ing while a windows driver is written by the 
chipset vendor and DTRT without even looking at the BIOS tables)

			Linus

Re: regression fixed by using pci=rom

[Gary Hade]

On Fri, May 09, 2008 at 10:26:47AM -0700, Linus Torvalds wrote:
> 
> 
> On Fri, 9 May 2008, Gary Hade wrote:
> > 
> > Ingo, Would you (or others listening to this discussion) know
> > exactly what Windows does with respect to BIOS unassigned
> > expansion ROMs?
> 
> Nobody really seems to know what windows does for *any* resources. And it 
> probably depends on windows version too, and on detailed chipset issues 
> (ie does Windows do the same thing as Linux wrt transparent bridges that 
> still have bus translation resources set up?).
> 
> So the "try to do what Windows does" is something we strive for in 
> particular cases, but it's not an absolute thing, and probably can never 
> even be that in theory. It's more a guideline, especially when we don't 
> know which particular choice is better and both seem otherwise equally 
> good.
> 
> But "doesn't work" always trumps "that's what windows does". Because even 
> if something works under Windows, sometimes Linux drivers simply then act 
> differently (ie they may use PIO vs MMIO, or in the case of graphics they 
> may need ROMs for re-POST'ing while a windows driver is written by the 
> chipset vendor and DTRT without even looking at the BIOS tables)

Yea, this is probably a good explanation of why Windows does run,
presumably without PCI resource allocation issues, on those same
systems where we have struggling with the expansion ROM resource
allocation failures from Linux.

Thanks.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: regression fixed by using pci=rom

[Linus Torvalds]

On Fri, 9 May 2008, Dave Airlie wrote:
>
> A user just reported this bug
> 
> http://bugs.freedesktop.org/show_bug.cgi?id=15523
> 
> He says using pci=rom fixes the regression, do we truly have to start
> booting machines with this, sucks to be a distro..

Yeah, that commit he bisected to is bogus crap.

I'll revert it.

		Linus