resources: add arch hook for preventing allocation in reserved areas

The HP nx6325 BIOS doesn't report any devices in the [0xf8000000-0xfbffffff] region via ACPI devices or the E820 memory map, but when we assign it to the 00:14.4 bridge as a prefetchable memory window, the machine hangs.

Quoted 4 lines · Bjorn Helgaas

The HP nx6325 BIOS doesn't report any devices in the [0xf8000000-0xfbffffff] region via ACPI devices or the E820 memory map, but when we assign it to the 00:14.4 bridge as a prefetchable memory window, the machine hangs.

Quite frankly, I think this patch sucks.

It sucks because these kinds of hw-specific patches are fundamentally a sign of something else being wrong. Why didn't windows hit this? Why do we need this total hack?

And is there any reason at all to believe that that one particular laptop is really special? I doubt it. And what happens for the next random machine that comes along an hits this?

Maybe we should just say that if we know the bridge is negative decode, and it hasn't been set up by the BIOS, we just don't allocate it at all. And try to look like Windows.

Or figure out what else Windows is doing differently.

The whole "allocate bottom up" old PCI allocation has _years_ of testing and quirk that have been gathered over a long time. We can't just say "we'll do the same thing for the top-down allocator".

The WHOLE AND ONLY POINT of the top-down allocator was to act lik Windows and not need crap like this. If that doesn't work, then I seriously don't think we should change bottom-up to top-down at all, and for 2.6.37 we should just revert the "set to top-down by default".

Seriously. That "whole and only point" thing is important. If we need hacks like this, then we shouldn't do it. We're much better off with the model that has year of testing an not the upheaval. Top-down allocation is in _no_ way inherently better, the only excuse for it was supposed to be "we don't need these kinds of hooks".

Quoted 4 lines · Bjorn Helgaas

The HP nx6325 BIOS doesn't report any devices in the [0xf8000000-0xfbffffff] region via ACPI devices or the E820 memory map, but when we assign it to the 00:14.4 bridge as a prefetchable memory window, the machine hangs.

Quite frankly, I think this patch sucks.

It sucks because these kinds of hw-specific patches are fundamentally a sign of something else being wrong. Why didn't windows hit this? Why do we need this total hack?

And is there any reason at all to believe that that one particular laptop is really special? I doubt it. And what happens for the next random machine that comes along an hits this?

Maybe we should just say that if we know the bridge is negative decode, and it hasn't been set up by the BIOS, we just don't allocate it at all. And try to look like Windows.

Or figure out what else Windows is doing differently.

The whole "allocate bottom up" old PCI allocation has _years_ of testing and quirk that have been gathered over a long time. We can't just say "we'll do the same thing for the top-down allocator".

The WHOLE AND ONLY POINT of the top-down allocator was to act lik Windows and not need crap like this. If that doesn't work, then I seriously don't think we should change bottom-up to top-down at all, and for 2.6.37 we should just revert the "set to top-down by default".

Seriously. That "whole and only point" thing is important. If we need hacks like this, then we shouldn't do it. We're much better off with the model that has year of testing an not the upheaval. Top-down allocation is in _no_ way inherently better, the only excuse for it was supposed to be "we don't need these kinds of hooks".

Not really -- the main point here is to make multi-host bridge machines work reliably, and I really don't see a way to do that without using _CRS.

If we're going to use _CRS, I think in the long run we'll be better off if we do it similarly to Windows, despite these early problems.

That's a maintainable approach. But it's maintainable ONLY if we then don't do other random changes that invalidates all the years of testing we've had.

Quoted 4 lines · Linus Torvalds

That's a maintainable approach. But it's maintainable ONLY if we then don't do other random changes that invalidates all the years of testing we've had.

Btw, looking at all the x86-specific commits that have gone in, I'm *extremely* unhappy that they apparently stopped honoring that "resource_alloc_from_bottom" flag that I explicitly asked for.

So it looks like it's not enough to just set that flag. We have to actually revert all the commits in this area as broken.

Which is sad, but since they clearly *are* broken and don't honor the flag that was there explicitly to avoid this problem and make it easy to test reverting it, I'm really pissed off. The WHOLE POINT of that flag was to give people an option to say "use the old resource allocation order because the new one doesn't work for me".

So at this point the only question is whether I should just revert the whole effing lot, or whether there are patches to fix the code to honor the "allocate from bottom" bit and then just set it by default again.

Bjorn? Preferences?

Quoted 4 lines · Linus Torvalds

That's a maintainable approach. But it's maintainable ONLY if we then don't do other random changes that invalidates all the years of testing we've had.

Btw, looking at all the x86-specific commits that have gone in, I'm *extremely* unhappy that they apparently stopped honoring that "resource_alloc_from_bottom" flag that I explicitly asked for.

So it looks like it's not enough to just set that flag. We have to actually revert all the commits in this area as broken.

Which is sad, but since they clearly *are* broken and don't honor the flag that was there explicitly to avoid this problem and make it easy to test reverting it, I'm really pissed off. The WHOLE POINT of that flag was to give people an option to say "use the old resource allocation order because the new one doesn't work for me".

So at this point the only question is whether I should just revert the whole effing lot, or whether there are patches to fix the code to honor the "allocate from bottom" bit and then just set it by default again.

Quoted 7 lines · Bjorn Helgaas

Not really -- the main point here is to make multi-host bridge machines work reliably, and I really don't see a way to do that without using _CRS.

If we're going to use _CRS, I think in the long run we'll be better off if we do it similarly to Windows, despite these early problems.

It's not about any "despite these early problems".

It's about "clearly we're not doing things at all like Windows, and it's just broken".

The thing is, we will never be able to match Windows exactly. It may well have random hardcoded quirks we simply don't know about.

I'm perfectly happy with you aiming to use _CRS. I am _not_ happy with you then using that as an excuse to then do things that don't work.

We will NOT start doing random BIOS-specific quirks just because top-down allocations hit other bugs than bottom-up ones do. Just no. We'll continue doing that we have tried to do, which is to perhaps have quirks that are specific to *hardware* (like the ones in drivers/pci/quirks.c) and just filling in stuff that some BIOSes are known to get wrong.

I don't know whether the other patches in this series make you unhappy.  I'm really not happy with how I implemented the avoidance of ACPI devices when doing PCI allocation, but I do think we need to avoid them *somehow*, and I was looking for a minimal quick fix at this point in the cycle.

Quoted 6 lines · Bjorn Helgaas

I don't know whether the other patches in this series make you unhappy. I'm really not happy with how I implemented the avoidance of ACPI devices when doing PCI allocation, but I do think we need to avoid them *somehow*, and I was looking for a minimal quick fix at this point in the cycle.

So the "avoid ACPI devices" part makes sense, and doesn't involved quirks, so I don't hate it at all the same way I hated the HP quirk.

However, I hate how it makes the allocation logic opaque. You can no longer tell from the regular non-debug dmesg and the /proc/iomem _why_ something got allocated the way it did, because there are hidden rules. That makes things awkward, methinks.

Also, quite frankly, I wonder what happens after release when somebody shows another machine that simply stopped working because the allocation strategy didn't work for it. The hw coverage that -rc6 gets is tiny compared to a real release.

IOW, what's the long-term strategy for this? The only sane long-term strategy I can see is the one we have _always_ done, which is to try to populate the memory resource tree with what simply matches reality. The whole "ok, we know the hardware better than the BIOS does" is a _stable_ strategy. In contrast, the things you propose are NOT stable strategies, they all depend on basically "we match windows exactly and/or trust ACPI". Both of which are *known* to be failing models.

That's why I'm somewhat upset. Your whole strategy seems to depend on a known broken model. We _know_ ACPI tables are crap much of the time. So we know that "avoiding ACPI resources" is inevitably insufficient.

And that's why I hate the "switch everything around" model. Yes, we have a known way to fix things up - namely to actually detect the hardware itself properly when firmware inevitably screws up - but the very act of switching things around will pretty much guarantee that all our years of effort is of dubious value, and we'll end up finding other laptops that used to work and no longer does.

Only switching around when _CRS is used is possible, and shouldn't cause any regressions if we continue to default to not using _CRS. But you want to switch that default around at some point, don't you? At which point we'll be up sh*t creek again. See what I'm saying?

Which all makes me suspect that we'd be much better off just doing the bottom-up allocation even for _CRS. And maybe CRS works fine then when we combine our hardware knowledge with the ACPI region avoidance.

ACPI devices tend to be at high addresses, so allocating top-down is definitely more dangerous unless we explicitly avoid them. I should have realized that and done something like patches 1-3 of this series before the top-down patches.

Doing it bottom-up would very likely work better than the "top-down without avoiding ACPI regions" model we currently have, at least in the short term. We *would* have to do something to avoid E820 reservations to fix this:
https://bugzilla.kernel.org/show_bug.cgi?id=16228,
but that's doable.

So here's my proposal for .37:
- Keep the current state of _CRS enabled by default (for 2008 and newer machines).
- Allocate bottom-up always
- Avoid E820 reservations

That should fix all the regressions I'm aware of. I'll work on the patches this afternoon.

So here's my proposal for .37:
 - Keep the current state of _CRS enabled by default (for 2008 and newer machines).
 - Allocate bottom-up always
 - Avoid E820 reservations

This adds arch_remove_reservations(), which an arch can implement if it needs to protect part of the address space from allocation.

Sometimes that can be done by just requesting a resource. This hook is to cover cases where protected area doesn't fit well in the hierarchical resource tree. For example, x86 BIOS E820 reservations are not related to devices, so they may overlap part of, all of, or more than a device resource.

Signed-off-by Bjorn Helgaas <[email protected]>

---

Quoted 4 lines · On Wed, 08 Dec 2010 14:36:06 -0700 Bjorn Helgaas <[email protected]> wrote:

This adds arch_remove_reservations(), which an arch can implement if it needs to protect part of the address space from allocation.

Quoted 8 lines

Sometimes that can be done by just requesting a resource. This hook is to cover cases where protected area doesn't fit well in the hierarchical resource tree. For example, x86 BIOS E820 reservations are not related to devices, so they may overlap part of, all of, or more than a device resource.

Signed-off-by Bjorn Helgaas <[email protected]>

---

Hm, this is bigger than the simple change of just avoiding the high 2M; Linus have you checked it out yet? It's nicer than simply adjusting PCIBIOS_MAX_MEM since it will affect all resource callers rather than just PCI, but it's definitely bigger.

If you want just the simple change for 2.6.37 I can push that, but we'll need to get a tested-by from Matthew:

  diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
  index ca0437c..aef9f77 100644
  --- a/arch/x86/include/asm/pci.h
  +++ b/arch/x86/include/asm/pci.h
  @@ -141,7 +141,7 @@ void dma32_reserve_bootmem(void);

   /* generic pci stuff */
   #include <asm-generic/pci.h>
  -#define PCIBIOS_MAX_MEM_32 0xffffffff
  +#define PCIBIOS_MAX_MEM_32 0xfff00000

   #ifdef CONFIG_NUMA
   /* Returns the node based on pci bus */

and I'll queue up this set for 2.6.38.

Thanks,

[Actually cc'ing Matthew this time]

Quoted 20 lines · On Fri, 10 Dec 2010 12:30:08 -0800 Jesse Barnes <[email protected]>…

Quoted 4 lines · On Wed, 08 Dec 2010 14:36:06 -0700 Bjorn Helgaas <[email protected]> wrote:

This adds arch_remove_reservations(), which an arch can implement if it needs to protect part of the address space from allocation.

Quoted 8 lines

Sometimes that can be done by just requesting a resource. This hook is to cover cases where protected area doesn't fit well in the hierarchical resource tree. For example, x86 BIOS E820 reservations are not related to devices, so they may overlap part of, all of, or more than a device resource.

Signed-off-by Bjorn Helgaas <[email protected]>

---

Hm, this is bigger than the simple change of just avoiding the high 2M; Linus have you checked it out yet? It's nicer than simply adjusting PCIBIOS_MAX_MEM since it will affect all resource callers rather than just PCI, but it's definitely bigger.

Quoted 20 lines

If you want just the simple change for 2.6.37 I can push that, but we'll need to get a tested-by from Matthew:

  diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
  index ca0437c..aef9f77 100644
  --- a/arch/x86/include/asm/pci.h
  +++ b/arch/x86/include/asm/pci.h
  @@ -141,7 +141,7 @@ void dma32_reserve_bootmem(void);

   /* generic pci stuff */
   #include <asm-generic/pci.h>
  -#define PCIBIOS_MAX_MEM_32 0xffffffff
  +#define PCIBIOS_MAX_MEM_32 0xfff00000

   #ifdef CONFIG_NUMA
   /* Returns the node based on pci bus */

and I'll queue up this set for 2.6.38.

Thanks,

Quoted 25 lines · Jesse Barnes

[Actually cc'ing Matthew this time]

Quoted 20 lines · On Fri, 10 Dec 2010 12:30:08 -0800 Jesse Barnes <[email protected]>…

Quoted 4 lines · On Wed, 08 Dec 2010 14:36:06 -0700 Bjorn Helgaas <[email protected]> wrote:

This adds arch_remove_reservations(), which an arch can implement if it needs to protect part of the address space from allocation.

Quoted 8 lines

Sometimes that can be done by just requesting a resource. This hook is to cover cases where protected area doesn't fit well in the hierarchical resource tree. For example, x86 BIOS E820 reservations are not related to devices, so they may overlap part of, all of, or more than a device resource.

Signed-off-by Bjorn Helgaas <[email protected]>

---

Hm, this is bigger than the simple change of just avoiding the high 2M; Linus have you checked it out yet? It's nicer than simply adjusting PCIBIOS_MAX_MEM since it will affect all resource callers rather than just PCI, but it's definitely bigger.

Dan Williams also reproduced the problem on a 2530p and tested this series (though I think he actually tested a backport in Fedora):
https://bugzilla.kernel.org/show_bug.cgi?id=23542#c7
https://bugzilla.kernel.org/show_bug.cgi?id=23542#c23

The simple PCIBIOS_MAX_MEM_32 change below will fix the 2530p problem, but it won't help fix the nx6325 problem Rafael reported; details at:
https://bugzilla.kernel.org/show_bug.cgi?id=23332#c20

Rafael did confirm on #acpi Wednesday that the full series fixed his nx6325.

Thanks, I'll add Dan and Rafael's tested-bys to the patches (they're already in my for-linus tree).  Unless Linus has a problem with them I'll send them over to him this weekend or Monday.

Quoted 4 lines · Jesse Barnes

Thanks, I'll add Dan and Rafael's tested-bys to the patches (they're already in my for-linus tree).  Unless Linus has a problem with them I'll send them over to him this weekend or Monday.

See my other email I just sent out.

I really am not going to take some totally new experimental and hacky major PCI resource management thing this late in the -rc game. No way, no how.

If the top-down allocator is causing regressions that cannot be fixed by _simple_ patches, we're simply going to have to undo it. What's the advantage of top-down? None. Not if we then need all this crap, which we could as easily do on top of the bottom-up one WITHOUT any regressions.

Why isn't anybody else questioning the whole basic premise here?

Quoted 12 lines · On Sat, 11 Dec 2010 19:34:05 -0800 Linus Torvalds…

Quoted 4 lines · Jesse Barnes

Thanks, I'll add Dan and Rafael's tested-bys to the patches (they're already in my for-linus tree). Unless Linus has a problem with them I'll send them over to him this weekend or Monday.

See my other email I just sent out.

I really am not going to take some totally new experimental and hacky major PCI resource management thing this late in the -rc game. No way, no how.

Quoted 7 lines

If the top-down allocator is causing regressions that cannot be fixed by _simple_ patches, we're simply going to have to undo it. What's the advantage of top-down? None. Not if we then need all this crap, which we could as easily do on top of the bottom-up one WITHOUT any regressions.

Why isn't anybody else questioning the whole basic premise here?

Questioning the whole premise is fine, but so far we've gone in (or at least think we're going in) a consistent direction: behave like Windows on platforms designed for Windows to avoid bugs that Windows doesn't hit and enable all the same devices Windows allows.

But yes, I really don't like the nx6325 patch either; there's obviously something we're still missing that's preventing us from doing the right thing on that platform. Quirking it isn't a good long term answer.

Quoted 28 lines · Jesse Barnes

Quoted 12 lines · On Sat, 11 Dec 2010 19:34:05 -0800 Linus Torvalds…

Quoted 4 lines · Jesse Barnes

Thanks, I'll add Dan and Rafael's tested-bys to the patches (they're already in my for-linus tree). Unless Linus has a problem with them I'll send them over to him this weekend or Monday.

See my other email I just sent out.

I really am not going to take some totally new experimental and hacky major PCI resource management thing this late in the -rc game. No way, no how.

Quoted 7 lines

If the top-down allocator is causing regressions that cannot be fixed by _simple_ patches, we're simply going to have to undo it. What's the advantage of top-down? None. Not if we then need all this crap, which we could as easily do on top of the bottom-up one WITHOUT any regressions.

Why isn't anybody else questioning the whole basic premise here?

Questioning the whole premise is fine, but so far we've gone in (or at least think we're going in) a consistent direction: behave like Windows on platforms designed for Windows to avoid bugs that Windows doesn't hit and enable all the same devices Windows allows.

Quoted 3 lines

But yes, I really don't like the nx6325 patch either; there's obviously something we're still missing that's preventing us from doing the right thing on that platform. Quirking it isn't a good long term answer.

OK, so I guess the best thing we can do for 2.6.37 is to revert 1af3c2e (x86: allocate space within a region top-down), right?

Quoted 2 lines

OK, so I guess the best thing we can do for 2.6.37 is to revert 1af3c2e (x86:
allocate space within a region top-down), right?

That's a possibility, but I'm not sure it's the best. It would avoid the nx6325 landmine, but we could easily step on a different one on other machines.

If Windows uses the end of available space first, that's the best-tested territory, and I think it's better to try to stay in it rather revert 1af3c2e and start mapping different territory that really isn't tested by Windows at all.

On the nx6325, we stray out of that tested territory because Linux opens windows on subtractive-decode bridges and allocates more space to CardBus bridges.

I think it'd be fairly simple to leave subtractive-decode bridges alone, and that might make the nx6325 work well enough for .37, even without the quirk. This feels like a change that might be the right thing to do in general. Using subtractive rather than positive decode would give up a little performance, but I doubt it's important

Quoted 13 lines · Bjorn Helgaas

Quoted 2 lines

OK, so I guess the best thing we can do for 2.6.37 is to revert 1af3c2e (x86:
allocate space within a region top-down), right?

That's a possibility, but I'm not sure it's the best. It would avoid the nx6325 landmine, but we could easily step on a different one on other machines.

If Windows uses the end of available space first, that's the best-tested territory, and I think it's better to try to stay in it rather revert 1af3c2e and start mapping different territory that really isn't tested by Windows at all.

On the nx6325, we stray out of that tested territory because Linux opens windows on subtractive-decode bridges and allocates more space to CardBus bridges.

Quoted 5 lines

I think it'd be fairly simple to leave subtractive-decode bridges alone, and that might make the nx6325 work well enough for .37, even without the quirk. This feels like a change that might be the right thing to do in general. Using subtractive rather than positive decode would give up a little performance, but I doubt it's important

Is there a patch for that that people could try?