* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END
@ 2010-04-26 23:02 H. Peter Anvin
2010-04-26 23:25 ` Jesse Barnes
0 siblings, 1 reply; 32+ messages in thread
From: H. Peter Anvin @ 2010-04-26 23:02 UTC (permalink / raw)
To: Jesse Barnes
Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai,
Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds,
linux-pci, x86, linux-kernel, Thomas Renninger, yaneti
[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]
I don't think it's sufficient, actually. We regularly see machines where devices point into e820_reserved memory above 1 MB - because it's a platform device or because firmware (e.g. smm) is touching the device. I think Bjorn's fix is great for .34, but longer term I think we need to structure the code to actually handle reserved regions differently from occupied/forbidden regions.
"Jesse Barnes" <jbarnes@virtuousgeek.org> wrote:
>On Mon, 26 Apr 2010 14:44:50 -0700
>"H. Peter Anvin" <hpa@zytor.com> wrote:
>
>> >
>> > Agreed. The trickier part is handling any platform devices that
>> > request_resource against that space. But maybe we don't need to do
>> > anything special; just making sure we avoid it in the PCI "BIOS" code
>> > as Bjorn did may be sufficient.
>> >
>>
>> Why is that hard? If a platform device does a request_resource against
>> that space, it's a request for specific address space and it should be
>> granted.
>
>I was thinking if we made it a special resource type we'd have to
>change any platform drivers to use it; i.e. it wouldn't be
>IORESOURCE_MEM or IORESOURCE_IO but IORESOURCE_DRAGONS. That way it
>wouldn't be part of the normal resource space.
>
>But that's definitely overkill. I think Bjorn's fix is sufficient.
>
>--
>Jesse Barnes, Intel Open Source Technology Center
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 23:02 [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END H. Peter Anvin @ 2010-04-26 23:25 ` Jesse Barnes 2010-04-26 23:49 ` H. Peter Anvin 2010-04-26 23:49 ` H. Peter Anvin 0 siblings, 2 replies; 32+ messages in thread From: Jesse Barnes @ 2010-04-26 23:25 UTC (permalink / raw) To: H. Peter Anvin Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti Sorry, sounds like we're talking about a few different things here: 1) devices which sit in e820 reserved space (whether at < 1M or > 1M) 2) devices which sit in e820 ram or other space below 1M 3) how to hand out space from the 0-1M region Bjorn's patch fixes (3) so that regular PCI devices don't get space there, which makes sense. Some devices may be in the special region, but were pointed there by the BIOS. Generally this should be ok, so drivers requesting this space should be allowed to get at it, but we should avoid putting anything else there. And below it sounds like you're talking about (1). If so, then yes I guess we need a solution there, which will allow drivers to bind to these "reserved" devices, even though the BIOS has marked them as off limits, at least as far as e820 goes. So perhaps both (1) and (2) could be put into a new, special IO resource space, or could use a new flag, since "busy" doesn't really reflect what's going on there very well, as Bjorn pointed out. Jesse On Mon, 26 Apr 2010 16:02:57 -0700 "H. Peter Anvin" <hpa@zytor.com> wrote: > I don't think it's sufficient, actually. We regularly see machines where devices point into e820_reserved memory above 1 MB - because it's a platform device or because firmware (e.g. smm) is touching the device. I think Bjorn's fix is great for .34, but longer term I think we need to structure the code to actually handle reserved regions differently from occupied/forbidden regions. > > "Jesse Barnes" <jbarnes@virtuousgeek.org> wrote: > > >On Mon, 26 Apr 2010 14:44:50 -0700 > >"H. Peter Anvin" <hpa@zytor.com> wrote: > > > >> > > >> > Agreed. The trickier part is handling any platform devices that > >> > request_resource against that space. But maybe we don't need to do > >> > anything special; just making sure we avoid it in the PCI "BIOS" code > >> > as Bjorn did may be sufficient. > >> > > >> > >> Why is that hard? If a platform device does a request_resource against > >> that space, it's a request for specific address space and it should be > >> granted. > > > >I was thinking if we made it a special resource type we'd have to > >change any platform drivers to use it; i.e. it wouldn't be > >IORESOURCE_MEM or IORESOURCE_IO but IORESOURCE_DRAGONS. That way it > >wouldn't be part of the normal resource space. > > > >But that's definitely overkill. I think Bjorn's fix is sufficient. > > > >-- > >Jesse Barnes, Intel Open Source Technology Center > -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 23:25 ` Jesse Barnes @ 2010-04-26 23:49 ` H. Peter Anvin 2010-04-26 23:49 ` H. Peter Anvin 1 sibling, 0 replies; 32+ messages in thread From: H. Peter Anvin @ 2010-04-26 23:49 UTC (permalink / raw) To: Jesse Barnes Cc: H. Peter Anvin, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti > Sorry, sounds like we're talking about a few different things here: > 1) devices which sit in e820 reserved space (whether at < 1M or > 1M) > 2) devices which sit in e820 ram or other space below 1M > 3) how to hand out space from the 0-1M region > > Bjorn's patch fixes (3) so that regular PCI devices don't get space > there, which makes sense. > > Some devices may be in the special region, but were pointed there by > the BIOS. Generally this should be ok, so drivers requesting this > space should be allowed to get at it, but we should avoid putting > anything else there. > > And below it sounds like you're talking about (1). If so, then yes I > guess we need a solution there, which will allow drivers to bind to > these "reserved" devices, even though the BIOS has marked them as off > limits, at least as far as e820 goes. > > So perhaps both (1) and (2) could be put into a new, special IO > resource space, or could use a new flag, since "busy" doesn't really > reflect what's going on there very well, as Bjorn pointed out. > > Jesse Properly done, these aren't separate cases at all. The E820 interface as specified doesn't reserve the address space below 1 MB, because it is legacy space -- which is another way of saying "everyone already knows to reserve it." The Right Thing[TM] to do is simply to modify the data output by E820 to reserve all non-memory space below 1 MB; this can (and should) be done in platform-specific initialization code to allow overrides. Once that is done, both your (2) and (3) cases are nothing but special subcases of (1). That's what I would like to see as the right solution, but it is clearly too late to do that in .34. Bjorn's solution is very attractive for .34 since it is so simple, but it's not a complete solution. -hpa ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 23:25 ` Jesse Barnes 2010-04-26 23:49 ` H. Peter Anvin @ 2010-04-26 23:49 ` H. Peter Anvin 2010-04-27 0:05 ` Jesse Barnes 1 sibling, 1 reply; 32+ messages in thread From: H. Peter Anvin @ 2010-04-26 23:49 UTC (permalink / raw) To: Jesse Barnes Cc: H. Peter Anvin, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti > Sorry, sounds like we're talking about a few different things here: > 1) devices which sit in e820 reserved space (whether at < 1M or > 1M) > 2) devices which sit in e820 ram or other space below 1M > 3) how to hand out space from the 0-1M region > > Bjorn's patch fixes (3) so that regular PCI devices don't get space > there, which makes sense. > > Some devices may be in the special region, but were pointed there by > the BIOS. Generally this should be ok, so drivers requesting this > space should be allowed to get at it, but we should avoid putting > anything else there. > > And below it sounds like you're talking about (1). If so, then yes I > guess we need a solution there, which will allow drivers to bind to > these "reserved" devices, even though the BIOS has marked them as off > limits, at least as far as e820 goes. > > So perhaps both (1) and (2) could be put into a new, special IO > resource space, or could use a new flag, since "busy" doesn't really > reflect what's going on there very well, as Bjorn pointed out. > > Jesse Properly done, these aren't separate cases at all. The E820 interface as specified doesn't reserve the address space below 1 MB, because it is legacy space -- which is another way of saying "everyone already knows to reserve it." The Right Thing[TM] to do is simply to modify the data output by E820 to reserve all non-memory space below 1 MB; this can (and should) be done in platform-specific initialization code to allow overrides. Once that is done, both your (2) and (3) cases are nothing but special subcases of (1). That's what I would like to see as the right solution, but it is clearly too late to do that in .34. Bjorn's solution is very attractive for .34 since it is so simple, but it's not a complete solution. -hpa ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 23:49 ` H. Peter Anvin @ 2010-04-27 0:05 ` Jesse Barnes 2010-04-27 1:27 ` Linus Torvalds 0 siblings, 1 reply; 32+ messages in thread From: Jesse Barnes @ 2010-04-27 0:05 UTC (permalink / raw) To: H. Peter Anvin Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Mon, 26 Apr 2010 16:49:55 -0700 "H. Peter Anvin" <hpa@zytor.com> wrote: > > Sorry, sounds like we're talking about a few different things here: > > 1) devices which sit in e820 reserved space (whether at < 1M or > 1M) > > 2) devices which sit in e820 ram or other space below 1M > > 3) how to hand out space from the 0-1M region > > > > Bjorn's patch fixes (3) so that regular PCI devices don't get space > > there, which makes sense. > > > > Some devices may be in the special region, but were pointed there by > > the BIOS. Generally this should be ok, so drivers requesting this > > space should be allowed to get at it, but we should avoid putting > > anything else there. > > > > And below it sounds like you're talking about (1). If so, then yes I > > guess we need a solution there, which will allow drivers to bind to > > these "reserved" devices, even though the BIOS has marked them as off > > limits, at least as far as e820 goes. > > > > So perhaps both (1) and (2) could be put into a new, special IO > > resource space, or could use a new flag, since "busy" doesn't really > > reflect what's going on there very well, as Bjorn pointed out. > > > > Jesse > > Properly done, these aren't separate cases at all. > > The E820 interface as specified doesn't reserve the address space below 1 > MB, because it is legacy space -- which is another way of saying "everyone > already knows to reserve it." The Right Thing[TM] to do is simply to > modify the data output by E820 to reserve all non-memory space below 1 MB; > this can (and should) be done in platform-specific initialization code to > allow overrides. > > Once that is done, both your (2) and (3) cases are nothing but special > subcases of (1). That's what I would like to see as the right solution, > but it is clearly too late to do that in .34. > > Bjorn's solution is very attractive for .34 since it is so simple, but > it's not a complete solution. Glad we agree. As I said (and echoing Bjorn), I think it would be best to reserve this space in a way that doesn't just use IORESOURCE_BUSY. We want and need to do allocations from the special region, so we should mark it as such. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-27 0:05 ` Jesse Barnes @ 2010-04-27 1:27 ` Linus Torvalds 2010-04-27 1:40 ` Jesse Barnes 2010-04-27 1:41 ` Yinghai 0 siblings, 2 replies; 32+ messages in thread From: Linus Torvalds @ 2010-04-27 1:27 UTC (permalink / raw) To: Jesse Barnes Cc: H. Peter Anvin, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Mon, 26 Apr 2010, Jesse Barnes wrote: > > Glad we agree. As I said (and echoing Bjorn), I think it would be best > to reserve this space in a way that doesn't just use IORESOURCE_BUSY. > We want and need to do allocations from the special region, so we > should mark it as such. I think Bjorn's patch to pcibios_align_resource() is really good and clever, and I think it should take care of the need for IORESOURCE_BUSY, no? We do want to let devices that are _already_ allocated there insert their resources, it's just that we never want to allocate new ones in the low 1M region. Do we actually have a regression left with Bjorn's patch? Linus ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-27 1:27 ` Linus Torvalds @ 2010-04-27 1:40 ` Jesse Barnes 2010-04-27 1:41 ` Yinghai 1 sibling, 0 replies; 32+ messages in thread From: Jesse Barnes @ 2010-04-27 1:40 UTC (permalink / raw) To: Linus Torvalds Cc: H. Peter Anvin, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Mon, 26 Apr 2010 18:27:27 -0700 (PDT) Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > On Mon, 26 Apr 2010, Jesse Barnes wrote: > > > > Glad we agree. As I said (and echoing Bjorn), I think it would be best > > to reserve this space in a way that doesn't just use IORESOURCE_BUSY. > > We want and need to do allocations from the special region, so we > > should mark it as such. > > I think Bjorn's patch to pcibios_align_resource() is really good and > clever, and I think it should take care of the need for IORESOURCE_BUSY, > no? We do want to let devices that are _already_ allocated there insert > their resources, it's just that we never want to allocate new ones in the > low 1M region. > > Do we actually have a regression left with Bjorn's patch? No, I think we're covered. But it sounded like Peter was also concerned about making new allocations from the 1M space, which would mean we'd need something other than the IORESOURCE_BUSY bit. But maybe Bjorn's patch plus simply removing the IORESOURCE_BUSY line is sufficient for that. The downside there is that it doesn't clearly communicate the special nature of the 1M region. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-27 1:27 ` Linus Torvalds 2010-04-27 1:40 ` Jesse Barnes @ 2010-04-27 1:41 ` Yinghai 2010-04-27 15:11 ` Bjorn Helgaas 1 sibling, 1 reply; 32+ messages in thread From: Yinghai @ 2010-04-27 1:41 UTC (permalink / raw) To: Linus Torvalds Cc: Jesse Barnes, H. Peter Anvin, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On 04/26/2010 06:27 PM, Linus Torvalds wrote: > > > On Mon, 26 Apr 2010, Jesse Barnes wrote: >> >> Glad we agree. As I said (and echoing Bjorn), I think it would be best >> to reserve this space in a way that doesn't just use IORESOURCE_BUSY. >> We want and need to do allocations from the special region, so we >> should mark it as such. > > I think Bjorn's patch to pcibios_align_resource() is really good and > clever, and I think it should take care of the need for IORESOURCE_BUSY, > no? We do want to let devices that are _already_ allocated there insert > their resources, it's just that we never want to allocate new ones in the > low 1M region. case A: bus 0: --- bus X --- device Y if the BIOS only assign range to to BUS X bridge with 0xB0000, and device Y is not assigned. then with Bojorn's patch, device Y can not get right resource allocated on first try. > > Do we actually have a regression left with Bjorn's patch? also find one AMD system: [ 6.960006] PCI: Using host bridge windows from ACPI; if necessary, use "pci=nocrs" and report a bug [ 6.984225] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-06]) [ 7.023528] pci_root PNP0A03:00: host bridge window [io 0x0000-0x03af] [ 7.024014] pci_root PNP0A03:00: host bridge window [io 0x03b0-0x03bb] [ 7.028005] pci_root PNP0A03:00: host bridge window [io 0x03bc-0x03bf] [ 7.032005] pci_root PNP0A03:00: host bridge window [io 0x03c0-0x03df] [ 7.036005] pci_root PNP0A03:00: host bridge window [io 0x03e0-0xefff] [ 7.040011] pci_root PNP0A03:00: host bridge window [mem 0xd8000000-0xe7ffffff] [ 7.044005] pci_root PNP0A03:00: host bridge window [mem 0xf0000000-0xfe9fffff] [ 7.048005] pci_root PNP0A03:00: host bridge window [mem 0xfec00000-0xfed0ffff] [ 7.052005] pci_root PNP0A03:00: host bridge window [mem 0x000a0000-0x000bffff] [ 7.056011] pci_root PNP0A03:00: host bridge window [mem 0xfed20000-0xffffffff] [ 0.000000] BIOS-e820: 0000000000000100 - 0000000000098c00 (usable) [ 0.000000] BIOS-e820: 0000000000098c00 - 00000000000a0000 (reserved) [ 0.000000] BIOS-e820: 00000000000e6000 - 0000000000100000 (reserved) [ 0.000000] BIOS-e820: 0000000000100000 - 00000000d7fa0000 (usable) [ 0.000000] BIOS-e820: 00000000d7fae000 - 00000000d7fb0000 (reserved) [ 0.000000] BIOS-e820: 00000000d7fb0000 - 00000000d7fbe000 (ACPI data) [ 0.000000] BIOS-e820: 00000000d7fbe000 - 00000000d7ff0000 (ACPI NVS) [ 0.000000] BIOS-e820: 00000000d7ff0000 - 00000000f0000000 (reserved) [ 0.000000] BIOS-e820: 00000000fec00000 - 00000000fec01000 (reserved) [ 0.000000] BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved) [ 0.000000] BIOS-e820: 00000000ff700000 - 0000000100000000 (reserved) [ 0.000000] BIOS-e820: 0000000100000000 - 0000008028000000 (usable) pci assign unassign code could use range like [mem 0xfed20000-0xffffffff] wrongly. YH ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-27 1:41 ` Yinghai @ 2010-04-27 15:11 ` Bjorn Helgaas 2010-04-28 16:07 ` Bjorn Helgaas 0 siblings, 1 reply; 32+ messages in thread From: Bjorn Helgaas @ 2010-04-27 15:11 UTC (permalink / raw) To: Yinghai Cc: Linus Torvalds, Jesse Barnes, H. Peter Anvin, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Monday 26 April 2010 07:41:55 pm Yinghai wrote: > On 04/26/2010 06:27 PM, Linus Torvalds wrote: > > Do we actually have a regression left with Bjorn's patch? After the pcibios_align_resource() patch, I'm not aware of any regressions. But let's double-check this: > also find one AMD system: > [ 7.056011] pci_root PNP0A03:00: host bridge window [mem 0xfed20000-0xffffffff] > ... > pci assign unassign code could use range like [mem 0xfed20000-0xffffffff] wrongly. I agree, it's very unlikely that it's safe to put PCI devices all the way up to 0xffffffff. I suspect this might be fixed by d558b483d5a, which computes the end of the bridge window using _MAX rather than _LEN. See https://bugzilla.kernel.org/show_bug.cgi?id=15480#c15 for an example similar to the one above: we originally thought the window was [mem 0xcff00000-0xffffffff], but d558b483d5a changes that to [mem 0xcff00000-0xfebfffff], which matches what Windows found. Yinghai, can you take a look at your AMD system again with a kernel that includes d558b483d5a, and see whether we still have a problem? If we *do* still have a problem, please open a bugzilla and attach a dmesg log with ACPI resource info collected with the debug patch here: https://bugzilla.kernel.org/show_bug.cgi?id=15533#c5 Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-27 15:11 ` Bjorn Helgaas @ 2010-04-28 16:07 ` Bjorn Helgaas 2010-04-28 17:14 ` Yinghai Lu 0 siblings, 1 reply; 32+ messages in thread From: Bjorn Helgaas @ 2010-04-28 16:07 UTC (permalink / raw) To: Yinghai Cc: Linus Torvalds, Jesse Barnes, H. Peter Anvin, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti Yinghai, ping, do you have any more information about this? On Tuesday 27 April 2010 09:11:10 am Bjorn Helgaas wrote: > On Monday 26 April 2010 07:41:55 pm Yinghai wrote: > But let's double-check this: > > > also find one AMD system: > > [ 7.056011] pci_root PNP0A03:00: host bridge window [mem 0xfed20000-0xffffffff] > > ... > > pci assign unassign code could use range like [mem 0xfed20000-0xffffffff] wrongly. > > I agree, it's very unlikely that it's safe to put PCI devices all the > way up to 0xffffffff. I suspect this might be fixed by d558b483d5a, > which computes the end of the bridge window using _MAX rather than _LEN. > > See https://bugzilla.kernel.org/show_bug.cgi?id=15480#c15 for an example > similar to the one above: we originally thought the window was > [mem 0xcff00000-0xffffffff], but d558b483d5a changes that to > [mem 0xcff00000-0xfebfffff], which matches what Windows found. > > Yinghai, can you take a look at your AMD system again with a kernel that > includes d558b483d5a, and see whether we still have a problem? If we > *do* still have a problem, please open a bugzilla and attach a dmesg log > with ACPI resource info collected with the debug patch here: > https://bugzilla.kernel.org/show_bug.cgi?id=15533#c5 > > Bjorn > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-28 16:07 ` Bjorn Helgaas @ 2010-04-28 17:14 ` Yinghai Lu 2010-04-28 19:06 ` Bjorn Helgaas 0 siblings, 1 reply; 32+ messages in thread From: Yinghai Lu @ 2010-04-28 17:14 UTC (permalink / raw) To: Bjorn Helgaas Cc: Linus Torvalds, Jesse Barnes, H. Peter Anvin, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti Never mind, for 2.6.34 your patch should be good enough. On 04/28/2010 09:07 AM, Bjorn Helgaas wrote: > Yinghai, ping, do you have any more information about this? > > On Tuesday 27 April 2010 09:11:10 am Bjorn Helgaas wrote: > >> On Monday 26 April 2010 07:41:55 pm Yinghai wrote: >> But let's double-check this: >> >> >>> also find one AMD system: >>> [ 7.056011] pci_root PNP0A03:00: host bridge window [mem 0xfed20000-0xffffffff] >>> ... >>> pci assign unassign code could use range like [mem 0xfed20000-0xffffffff] wrongly. >>> >> I agree, it's very unlikely that it's safe to put PCI devices all the >> way up to 0xffffffff. I suspect this might be fixed by d558b483d5a, >> which computes the end of the bridge window using _MAX rather than _LEN. >> >> See https://bugzilla.kernel.org/show_bug.cgi?id=15480#c15 for an example >> similar to the one above: we originally thought the window was >> [mem 0xcff00000-0xffffffff], but d558b483d5a changes that to >> [mem 0xcff00000-0xfebfffff], which matches what Windows found. >> >> Yinghai, can you take a look at your AMD system again with a kernel that >> includes d558b483d5a, and see whether we still have a problem? If we >> *do* still have a problem, please open a bugzilla and attach a dmesg log >> with ACPI resource info collected with the debug patch here: >> https://bugzilla.kernel.org/show_bug.cgi?id=15533#c5 >> >> Bjorn >> >> > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-28 17:14 ` Yinghai Lu @ 2010-04-28 19:06 ` Bjorn Helgaas 2010-04-28 19:10 ` Yinghai 0 siblings, 1 reply; 32+ messages in thread From: Bjorn Helgaas @ 2010-04-28 19:06 UTC (permalink / raw) To: Yinghai Lu Cc: Linus Torvalds, Jesse Barnes, H. Peter Anvin, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Wednesday 28 April 2010 11:14:45 am Yinghai Lu wrote: > Never mind, for 2.6.34 your patch should be good enough. Uh, OK. I'm not trying to be a nuisance, but if there's a machine where we think there's a bridge window like [mem 0xfed20000-0xffffffff], I want to fix it, even if you think it's "good enough for 2.6.34." I was hoping you could specifically confirm that "yes, that AMD machine was fixed by d558b483d5a," or else give me some more information that would help me figure out what's going on. Bjorn > On 04/28/2010 09:07 AM, Bjorn Helgaas wrote: > > Yinghai, ping, do you have any more information about this? > > > > On Tuesday 27 April 2010 09:11:10 am Bjorn Helgaas wrote: > > > >> On Monday 26 April 2010 07:41:55 pm Yinghai wrote: > >> But let's double-check this: > >> > >> > >>> also find one AMD system: > >>> [ 7.056011] pci_root PNP0A03:00: host bridge window [mem 0xfed20000-0xffffffff] > >>> ... > >>> pci assign unassign code could use range like [mem 0xfed20000-0xffffffff] wrongly. > >>> > >> I agree, it's very unlikely that it's safe to put PCI devices all the > >> way up to 0xffffffff. I suspect this might be fixed by d558b483d5a, > >> which computes the end of the bridge window using _MAX rather than _LEN. > >> > >> See https://bugzilla.kernel.org/show_bug.cgi?id=15480#c15 for an example > >> similar to the one above: we originally thought the window was > >> [mem 0xcff00000-0xffffffff], but d558b483d5a changes that to > >> [mem 0xcff00000-0xfebfffff], which matches what Windows found. > >> > >> Yinghai, can you take a look at your AMD system again with a kernel that > >> includes d558b483d5a, and see whether we still have a problem? If we > >> *do* still have a problem, please open a bugzilla and attach a dmesg log > >> with ACPI resource info collected with the debug patch here: > >> https://bugzilla.kernel.org/show_bug.cgi?id=15533#c5 > >> > >> Bjorn > >> > >> > > > > > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-28 19:06 ` Bjorn Helgaas @ 2010-04-28 19:10 ` Yinghai 2010-04-28 19:23 ` Bjorn Helgaas 0 siblings, 1 reply; 32+ messages in thread From: Yinghai @ 2010-04-28 19:10 UTC (permalink / raw) To: Bjorn Helgaas Cc: Linus Torvalds, Jesse Barnes, H. Peter Anvin, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On 04/28/2010 12:06 PM, Bjorn Helgaas wrote: > On Wednesday 28 April 2010 11:14:45 am Yinghai Lu wrote: >> Never mind, for 2.6.34 your patch should be good enough. > > Uh, OK. > > I'm not trying to be a nuisance, but if there's a machine where > we think there's a bridge window like [mem 0xfed20000-0xffffffff], > I want to fix it, even if you think it's "good enough for 2.6.34." > > I was hoping you could specifically confirm that "yes, that AMD > machine was fixed by d558b483d5a," or else give me some more > information that would help me figure out what's going on. [ 0.000000] BIOS-e820: 0000000000000100 - 0000000000098c00 (usable) [ 0.000000] BIOS-e820: 0000000000098c00 - 00000000000a0000 (reserved) [ 0.000000] BIOS-e820: 00000000000e6000 - 0000000000100000 (reserved) [ 0.000000] BIOS-e820: 0000000000100000 - 00000000d7fa0000 (usable) [ 0.000000] BIOS-e820: 00000000d7fae000 - 00000000d7fb0000 (reserved) [ 0.000000] BIOS-e820: 00000000d7fb0000 - 00000000d7fbe000 (ACPI data) [ 0.000000] BIOS-e820: 00000000d7fbe000 - 00000000d7ff0000 (ACPI NVS) [ 0.000000] BIOS-e820: 00000000d7ff0000 - 00000000f0000000 (reserved) [ 0.000000] BIOS-e820: 00000000fec00000 - 00000000fec01000 (reserved) [ 0.000000] BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved) [ 0.000000] BIOS-e820: 00000000ff700000 - 0000000100000000 (reserved) [ 0.000000] BIOS-e820: 0000000100000000 - 0000008028000000 (usable) [ 6.960006] PCI: Using host bridge windows from ACPI; if necessary, use "pci=nocrs" and report a bug [ 6.984225] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-06]) [ 7.023528] pci_root PNP0A03:00: host bridge window [io 0x0000-0x03af] [ 7.024014] pci_root PNP0A03:00: host bridge window [io 0x03b0-0x03bb] [ 7.028005] pci_root PNP0A03:00: host bridge window [io 0x03bc-0x03bf] [ 7.032005] pci_root PNP0A03:00: host bridge window [io 0x03c0-0x03df] [ 7.036005] pci_root PNP0A03:00: host bridge window [io 0x03e0-0xefff] [ 7.040011] pci_root PNP0A03:00: host bridge window [mem 0xd8000000-0xe7ffffff] [ 7.044005] pci_root PNP0A03:00: host bridge window [mem 0xf0000000-0xfe9fffff] [ 7.048005] pci_root PNP0A03:00: host bridge window [mem 0xfec00000-0xfed0ffff] [ 7.052005] pci_root PNP0A03:00: host bridge window [mem 0x000a0000-0x000bffff] [ 7.056011] pci_root PNP0A03:00: host bridge window [mem 0xfed20000-0xffffffff] d7ff0000-efffffff : reserved d8000000-e7ffffff : PCI Bus 0000:00 d8000000-dfffffff : GART e8000000-efffffff : PCI Bus 0000:80 fec00000-fed0ffff : PCI Bus 0000:00 fec00000-fec00fff : reserved fec00000-fec003ff : IOAPIC 0 fed00000-fed003ff : HPET 0 fed10000-fed1ffff : PCI Bus 0000:80 fed20000-ffffffff : PCI Bus 0000:00 fee00000-fee00fff : Local APIC fee00000-fee00fff : reserved fefff000-feffffff : pnp 00:0a ff700000-ffffffff : reserved ffb80000-fffffffe : pnp 00:06 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-28 19:10 ` Yinghai @ 2010-04-28 19:23 ` Bjorn Helgaas 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Helgaas @ 2010-04-28 19:23 UTC (permalink / raw) To: Yinghai Cc: Linus Torvalds, Jesse Barnes, H. Peter Anvin, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Wednesday 28 April 2010 01:10:34 pm Yinghai wrote: > On 04/28/2010 12:06 PM, Bjorn Helgaas wrote: > > On Wednesday 28 April 2010 11:14:45 am Yinghai Lu wrote: > >> Never mind, for 2.6.34 your patch should be good enough. > > > > Uh, OK. > > > > I'm not trying to be a nuisance, but if there's a machine where > > we think there's a bridge window like [mem 0xfed20000-0xffffffff], > > I want to fix it, even if you think it's "good enough for 2.6.34." > > > > I was hoping you could specifically confirm that "yes, that AMD > > machine was fixed by d558b483d5a," or else give me some more > > information that would help me figure out what's going on. I can see this is going nowhere. The information below is essentially just what you already posted, and I'm sorry, but it's not enough for me to fix anything. Can you tell me the model and BIOS version? Maybe I can find another one somewhere and do my own poking around. What I'd like to know is (a) what Windows reports as PCI bus resources, and (b) what the Linux dmesg looks like with the patch here: https://bugzilla.kernel.org/show_bug.cgi?id=15533#c5 CONFIG_ACPI_DEBUG=y and the arguments "acpi.debug_level=0x00010000 acpi.debug_layer=0x00000100". Bjorn > [ 0.000000] BIOS-e820: 0000000000000100 - 0000000000098c00 (usable) > [ 0.000000] BIOS-e820: 0000000000098c00 - 00000000000a0000 (reserved) > [ 0.000000] BIOS-e820: 00000000000e6000 - 0000000000100000 (reserved) > [ 0.000000] BIOS-e820: 0000000000100000 - 00000000d7fa0000 (usable) > [ 0.000000] BIOS-e820: 00000000d7fae000 - 00000000d7fb0000 (reserved) > [ 0.000000] BIOS-e820: 00000000d7fb0000 - 00000000d7fbe000 (ACPI data) > [ 0.000000] BIOS-e820: 00000000d7fbe000 - 00000000d7ff0000 (ACPI NVS) > [ 0.000000] BIOS-e820: 00000000d7ff0000 - 00000000f0000000 (reserved) > [ 0.000000] BIOS-e820: 00000000fec00000 - 00000000fec01000 (reserved) > [ 0.000000] BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved) > [ 0.000000] BIOS-e820: 00000000ff700000 - 0000000100000000 (reserved) > [ 0.000000] BIOS-e820: 0000000100000000 - 0000008028000000 (usable) > > [ 6.960006] PCI: Using host bridge windows from ACPI; if necessary, use "pci=nocrs" and report a bug > [ 6.984225] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-06]) > [ 7.023528] pci_root PNP0A03:00: host bridge window [io 0x0000-0x03af] > [ 7.024014] pci_root PNP0A03:00: host bridge window [io 0x03b0-0x03bb] > [ 7.028005] pci_root PNP0A03:00: host bridge window [io 0x03bc-0x03bf] > [ 7.032005] pci_root PNP0A03:00: host bridge window [io 0x03c0-0x03df] > [ 7.036005] pci_root PNP0A03:00: host bridge window [io 0x03e0-0xefff] > [ 7.040011] pci_root PNP0A03:00: host bridge window [mem 0xd8000000-0xe7ffffff] > [ 7.044005] pci_root PNP0A03:00: host bridge window [mem 0xf0000000-0xfe9fffff] > [ 7.048005] pci_root PNP0A03:00: host bridge window [mem 0xfec00000-0xfed0ffff] > [ 7.052005] pci_root PNP0A03:00: host bridge window [mem 0x000a0000-0x000bffff] > [ 7.056011] pci_root PNP0A03:00: host bridge window [mem 0xfed20000-0xffffffff] > > > d7ff0000-efffffff : reserved > d8000000-e7ffffff : PCI Bus 0000:00 > d8000000-dfffffff : GART > e8000000-efffffff : PCI Bus 0000:80 > fec00000-fed0ffff : PCI Bus 0000:00 > fec00000-fec00fff : reserved > fec00000-fec003ff : IOAPIC 0 > fed00000-fed003ff : HPET 0 > fed10000-fed1ffff : PCI Bus 0000:80 > fed20000-ffffffff : PCI Bus 0000:00 > fee00000-fee00fff : Local APIC > fee00000-fee00fff : reserved > fefff000-feffffff : pnp 00:0a > ff700000-ffffffff : reserved > ffb80000-fffffffe : pnp 00:06 > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END
@ 2010-04-27 2:02 H. Peter Anvin
0 siblings, 0 replies; 32+ messages in thread
From: H. Peter Anvin @ 2010-04-27 2:02 UTC (permalink / raw)
To: Jesse Barnes, Linus Torvalds
Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai,
Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86,
linux-kernel, Thomas Renninger, yaneti
[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]
The 1 MB range is only one case of a prereserved space. It's special only in the sense that it is *always* reserved, even if the map doesn't mark it as such. Fixed (or SMM-used) resources in reserved space above 1 MB are exactly the same issue.
"Jesse Barnes" <jbarnes@virtuousgeek.org> wrote:
>On Mon, 26 Apr 2010 18:27:27 -0700 (PDT)
>Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>>
>>
>> On Mon, 26 Apr 2010, Jesse Barnes wrote:
>> >
>> > Glad we agree. As I said (and echoing Bjorn), I think it would be best
>> > to reserve this space in a way that doesn't just use IORESOURCE_BUSY.
>> > We want and need to do allocations from the special region, so we
>> > should mark it as such.
>>
>> I think Bjorn's patch to pcibios_align_resource() is really good and
>> clever, and I think it should take care of the need for IORESOURCE_BUSY,
>> no? We do want to let devices that are _already_ allocated there insert
>> their resources, it's just that we never want to allocate new ones in the
>> low 1M region.
>>
>> Do we actually have a regression left with Bjorn's patch?
>
>No, I think we're covered. But it sounded like Peter was also
>concerned about making new allocations from the 1M space, which would
>mean we'd need something other than the IORESOURCE_BUSY bit. But maybe
>Bjorn's patch plus simply removing the IORESOURCE_BUSY line is
>sufficient for that. The downside there is that it doesn't clearly
>communicate the special nature of the 1M region.
>
>--
>Jesse Barnes, Intel Open Source Technology Center
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH -v2 1/2] x86: Reserve [0xa0000, 0x100000] in e820 map
@ 2010-04-13 21:42 Yinghai
2010-04-21 5:33 ` [PATCH -v4 1/3] " Yinghai
0 siblings, 1 reply; 32+ messages in thread
From: Yinghai @ 2010-04-13 21:42 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Bjorn Helgaas, Thomas Gleixner, Ingo Molnar, Andy Isaacson,
guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel,
Thomas Renninger
On 04/13/2010 02:18 PM, H. Peter Anvin wrote:
> On 04/13/2010 02:11 PM, Yinghai wrote:
>>>
>>> I guess the real question (which I haven't looked at myself) is if the
>>> E820_RESERVED -> BUSY will cause an explicitly assigned BAR from being
>>> moved. That's bad, not so much for this particular range, but from BARs
>>> which may be assigned by SMM. Hacking that up in a simulator
>>> (Qemu/Bochs) and testing it is probably on the to do list...
>>
>> no, if some device BAR fall in that range, it should still use that range, and will not be relocated.
>>
>> will update the change log.
>>
>
> Good, that's what we want.
the driver for that device later can not use pci_request_region(). because that region is BUSY already.
YH
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH -v4 1/3] x86: Reserve [0xa0000, 0x100000] in e820 map @ 2010-04-21 5:33 ` Yinghai 2010-04-21 19:31 ` Andy Isaacson 0 siblings, 1 reply; 32+ messages in thread From: Yinghai @ 2010-04-21 5:33 UTC (permalink / raw) To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar Cc: Bjorn Helgaas, Andy Isaacson, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger Update e820 at first, and later put them resource tree. Reserved that early, will not be allocated to unassigned PCI BAR v3: remove probe_roms() that is not needed, because whole range is reserved already Signed-off-by: Yinghai Lu <yinghai@kernel.org> Cc: Guenter Roeck <guenter.roeck@ericsson.com> Cc: Andy Isaacson <adi@hexapodia.org> Tested-by: Andy Isaacson <adi@hexapodia.org> --- arch/x86/include/asm/setup.h | 3 arch/x86/include/asm/x86_init.h | 4 arch/x86/kernel/Makefile | 1 arch/x86/kernel/head32.c | 2 arch/x86/kernel/mrst.c | 2 arch/x86/kernel/probe_roms_32.c | 166 ---------------------------------------- arch/x86/kernel/setup.c | 25 ------ arch/x86/kernel/x86_init.c | 2 8 files changed, 8 insertions(+), 197 deletions(-) Index: linux-2.6/arch/x86/include/asm/setup.h =================================================================== --- linux-2.6.orig/arch/x86/include/asm/setup.h +++ linux-2.6/arch/x86/include/asm/setup.h @@ -43,8 +43,8 @@ static inline void visws_early_detect(vo extern unsigned long saved_video_mode; +void trim_bios_range(void); extern void reserve_standard_io_resources(void); -extern void i386_reserve_resources(void); extern void setup_default_timer_irq(void); #ifdef CONFIG_X86_MRST @@ -96,7 +96,6 @@ void *extend_brk(size_t size, size_t ali #ifdef __i386__ void __init i386_start_kernel(void); -extern void probe_roms(void); #else void __init x86_64_start_kernel(char *real_mode); Index: linux-2.6/arch/x86/include/asm/x86_init.h =================================================================== --- linux-2.6.orig/arch/x86/include/asm/x86_init.h +++ linux-2.6/arch/x86/include/asm/x86_init.h @@ -32,14 +32,14 @@ struct x86_init_mpparse { /** * struct x86_init_resources - platform specific resource related ops - * @probe_roms: probe BIOS roms + * @trim_bios_range: trim BIOS related range in E820 * @reserve_resources: reserve the standard resources for the * platform * @memory_setup: platform specific memory setup * */ struct x86_init_resources { - void (*probe_roms)(void); + void (*trim_bios_range)(void); void (*reserve_resources)(void); char *(*memory_setup)(void); }; Index: linux-2.6/arch/x86/kernel/Makefile =================================================================== --- linux-2.6.orig/arch/x86/kernel/Makefile +++ linux-2.6/arch/x86/kernel/Makefile @@ -34,7 +34,6 @@ obj-y += traps.o irq.o irq_$(BITS).o d obj-y += time.o ioport.o ldt.o dumpstack.o obj-y += setup.o x86_init.o i8259.o irqinit.o obj-$(CONFIG_X86_VISWS) += visws_quirks.o -obj-$(CONFIG_X86_32) += probe_roms_32.o obj-$(CONFIG_X86_32) += sys_i386_32.o i386_ksyms_32.o obj-$(CONFIG_X86_64) += sys_x86_64.o x8664_ksyms_64.o obj-$(CONFIG_X86_64) += syscall_64.o vsyscall_64.o Index: linux-2.6/arch/x86/kernel/head32.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/head32.c +++ linux-2.6/arch/x86/kernel/head32.c @@ -21,8 +21,6 @@ static void __init i386_default_early_setup(void) { /* Initilize 32bit specific setup functions */ - x86_init.resources.probe_roms = probe_roms; - x86_init.resources.reserve_resources = i386_reserve_resources; x86_init.mpparse.setup_ioapic_ids = setup_ioapic_ids_from_mpc; reserve_ebda_region(); Index: linux-2.6/arch/x86/kernel/mrst.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/mrst.c +++ linux-2.6/arch/x86/kernel/mrst.c @@ -222,7 +222,7 @@ static void __init mrst_setup_boot_clock */ void __init x86_mrst_early_setup(void) { - x86_init.resources.probe_roms = x86_init_noop; + x86_init.resources.trim_bios_range = x86_init_noop; x86_init.resources.reserve_resources = x86_init_noop; x86_init.timers.timer_init = mrst_time_init; Index: linux-2.6/arch/x86/kernel/probe_roms_32.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/probe_roms_32.c +++ /dev/null @@ -1,166 +0,0 @@ -#include <linux/sched.h> -#include <linux/mm.h> -#include <linux/uaccess.h> -#include <linux/mmzone.h> -#include <linux/ioport.h> -#include <linux/seq_file.h> -#include <linux/console.h> -#include <linux/init.h> -#include <linux/edd.h> -#include <linux/dmi.h> -#include <linux/pfn.h> -#include <linux/pci.h> -#include <asm/pci-direct.h> - - -#include <asm/e820.h> -#include <asm/mmzone.h> -#include <asm/setup.h> -#include <asm/sections.h> -#include <asm/io.h> -#include <asm/setup_arch.h> - -static struct resource system_rom_resource = { - .name = "System ROM", - .start = 0xf0000, - .end = 0xfffff, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}; - -static struct resource extension_rom_resource = { - .name = "Extension ROM", - .start = 0xe0000, - .end = 0xeffff, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}; - -static struct resource adapter_rom_resources[] = { { - .name = "Adapter ROM", - .start = 0xc8000, - .end = 0, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}, { - .name = "Adapter ROM", - .start = 0, - .end = 0, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}, { - .name = "Adapter ROM", - .start = 0, - .end = 0, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}, { - .name = "Adapter ROM", - .start = 0, - .end = 0, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}, { - .name = "Adapter ROM", - .start = 0, - .end = 0, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}, { - .name = "Adapter ROM", - .start = 0, - .end = 0, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -} }; - -static struct resource video_rom_resource = { - .name = "Video ROM", - .start = 0xc0000, - .end = 0xc7fff, - .flags = IORESOURCE_BUSY | IORESOURCE_READONLY | IORESOURCE_MEM -}; - -#define ROMSIGNATURE 0xaa55 - -static int __init romsignature(const unsigned char *rom) -{ - const unsigned short * const ptr = (const unsigned short *)rom; - unsigned short sig; - - return probe_kernel_address(ptr, sig) == 0 && sig == ROMSIGNATURE; -} - -static int __init romchecksum(const unsigned char *rom, unsigned long length) -{ - unsigned char sum, c; - - for (sum = 0; length && probe_kernel_address(rom++, c) == 0; length--) - sum += c; - return !length && !sum; -} - -void __init probe_roms(void) -{ - const unsigned char *rom; - unsigned long start, length, upper; - unsigned char c; - int i; - - /* video rom */ - upper = adapter_rom_resources[0].start; - for (start = video_rom_resource.start; start < upper; start += 2048) { - rom = isa_bus_to_virt(start); - if (!romsignature(rom)) - continue; - - video_rom_resource.start = start; - - if (probe_kernel_address(rom + 2, c) != 0) - continue; - - /* 0 < length <= 0x7f * 512, historically */ - length = c * 512; - - /* if checksum okay, trust length byte */ - if (length && romchecksum(rom, length)) - video_rom_resource.end = start + length - 1; - - request_resource(&iomem_resource, &video_rom_resource); - break; - } - - start = (video_rom_resource.end + 1 + 2047) & ~2047UL; - if (start < upper) - start = upper; - - /* system rom */ - request_resource(&iomem_resource, &system_rom_resource); - upper = system_rom_resource.start; - - /* check for extension rom (ignore length byte!) */ - rom = isa_bus_to_virt(extension_rom_resource.start); - if (romsignature(rom)) { - length = extension_rom_resource.end - extension_rom_resource.start + 1; - if (romchecksum(rom, length)) { - request_resource(&iomem_resource, &extension_rom_resource); - upper = extension_rom_resource.start; - } - } - - /* check for adapter roms on 2k boundaries */ - for (i = 0; i < ARRAY_SIZE(adapter_rom_resources) && start < upper; start += 2048) { - rom = isa_bus_to_virt(start); - if (!romsignature(rom)) - continue; - - if (probe_kernel_address(rom + 2, c) != 0) - continue; - - /* 0 < length <= 0x7f * 512, historically */ - length = c * 512; - - /* but accept any length that fits if checksum okay */ - if (!length || start + length > upper || !romchecksum(rom, length)) - continue; - - adapter_rom_resources[i].start = start; - adapter_rom_resources[i].end = start + length - 1; - request_resource(&iomem_resource, &adapter_rom_resources[i]); - - start = adapter_rom_resources[i++].end & ~2047UL; - } -} - Index: linux-2.6/arch/x86/kernel/setup.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/setup.c +++ linux-2.6/arch/x86/kernel/setup.c @@ -680,7 +680,7 @@ static struct dmi_system_id __initdata b {} }; -static void __init trim_bios_range(void) +void __init trim_bios_range(void) { /* * A special case is the first 4Kb of memory; @@ -693,7 +693,7 @@ static void __init trim_bios_range(void) * area (640->1Mb) as ram even though it is not. * take them out. */ - e820_remove_range(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RAM, 1); + e820_add_region(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RESERVED); sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map); } @@ -853,14 +853,12 @@ void __init setup_arch(char **cmdline_p) */ init_hypervisor_platform(); - x86_init.resources.probe_roms(); - /* after parse_early_param, so could debug it */ insert_resource(&iomem_resource, &code_resource); insert_resource(&iomem_resource, &data_resource); insert_resource(&iomem_resource, &bss_resource); - trim_bios_range(); + x86_init.resources.trim_bios_range(); #ifdef CONFIG_X86_32 if (ppro_with_ram_bug()) { e820_update_range(0x70000000ULL, 0x40000ULL, E820_RAM, @@ -1052,20 +1050,3 @@ void __init setup_arch(char **cmdline_p) mcheck_init(); } - -#ifdef CONFIG_X86_32 - -static struct resource video_ram_resource = { - .name = "Video RAM area", - .start = 0xa0000, - .end = 0xbffff, - .flags = IORESOURCE_BUSY | IORESOURCE_MEM -}; - -void __init i386_reserve_resources(void) -{ - request_resource(&iomem_resource, &video_ram_resource); - reserve_standard_io_resources(); -} - -#endif /* CONFIG_X86_32 */ Index: linux-2.6/arch/x86/kernel/x86_init.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/x86_init.c +++ linux-2.6/arch/x86/kernel/x86_init.c @@ -32,7 +32,7 @@ void iommu_shutdown_noop(void) { } struct x86_init_ops x86_init __initdata = { .resources = { - .probe_roms = x86_init_noop, + .trim_bios_range = trim_bios_range, .reserve_resources = reserve_standard_io_resources, .memory_setup = default_machine_specific_memory_setup, }, ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH -v4 1/3] x86: Reserve [0xa0000, 0x100000] in e820 map 2010-04-21 5:33 ` [PATCH -v4 1/3] " Yinghai @ 2010-04-21 19:31 ` Andy Isaacson 2010-04-23 23:05 ` [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END Bjorn Helgaas 0 siblings, 1 reply; 32+ messages in thread From: Andy Isaacson @ 2010-04-21 19:31 UTC (permalink / raw) To: Yinghai Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Bjorn Helgaas, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger On Tue, Apr 20, 2010 at 10:33:50PM -0700, Yinghai wrote: > Update e820 at first, and later put them resource tree. > Reserved that early, will not be allocated to unassigned PCI BAR > > v3: remove probe_roms() that is not needed, because whole range is reserved > already Test booted this patch series on the problematic t3400, seems to work fine. dmesg attached to bug 15744. -andy ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-21 19:31 ` Andy Isaacson @ 2010-04-23 23:05 ` Bjorn Helgaas 2010-04-23 23:44 ` H. Peter Anvin ` (2 more replies) 0 siblings, 3 replies; 32+ messages in thread From: Bjorn Helgaas @ 2010-04-23 23:05 UTC (permalink / raw) To: Andy Isaacson, R. Andrew Bailey Cc: Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Wednesday 21 April 2010 01:31:20 pm Andy Isaacson wrote: > On Tue, Apr 20, 2010 at 10:33:50PM -0700, Yinghai wrote: > > Update e820 at first, and later put them resource tree. > > Reserved that early, will not be allocated to unassigned PCI BAR > > > > v3: remove probe_roms() that is not needed, because whole range is reserved > > already > > Test booted this patch series on the problematic t3400, seems to work > fine. dmesg attached to bug 15744. Thanks for testing (again). I'm not confident that this series is going to be successful, so I started looking for other approaches. I can't reproduce the exact problem you're seeing, but in my kludged-up attempt, the patch below is enough to keep us from assigning the space below 1MB to a device. Would you guys (Andy & Andy, what a coincidence :-)) mind giving it a try? This is intended to work on top of current upstream, with no other patches required. Bjorn commit 7fb707eb97fdf6dc4fa4b127f127f8d00223afc7 Author: Bjorn Helgaas <bjorn.helgaas@hp.com> Date: Fri Apr 23 15:22:10 2010 -0600 x86/PCI: never allocate PCI MMIO resources below BIOS_END When we move a PCI device or assign resources to a device not configured by the BIOS, we want to avoid the BIOS region below 1MB. Note that if the BIOS places devices below 1MB, we leave them there. See https://bugzilla.kernel.org/show_bug.cgi?id=15744 and https://bugzilla.kernel.org/show_bug.cgi?id=15841 Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c index 46fd43f..97da2ba 100644 --- a/arch/x86/pci/i386.c +++ b/arch/x86/pci/i386.c @@ -72,6 +72,9 @@ pcibios_align_resource(void *data, const struct resource *res, return start; if (start & 0x300) start = (start + 0x3ff) & ~0x3ff; + } else if (res->flags & IORESOURCE_MEM) { + if (start < BIOS_END) + start = BIOS_END; } return start; } ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-23 23:05 ` [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END Bjorn Helgaas @ 2010-04-23 23:44 ` H. Peter Anvin 2010-04-24 0:36 ` Yinghai Lu 2010-04-26 12:50 ` R. Andrew Bailey 2010-04-26 18:34 ` Andy Isaacson 2 siblings, 1 reply; 32+ messages in thread From: H. Peter Anvin @ 2010-04-23 23:44 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On 04/23/2010 04:05 PM, Bjorn Helgaas wrote: > On Wednesday 21 April 2010 01:31:20 pm Andy Isaacson wrote: >> On Tue, Apr 20, 2010 at 10:33:50PM -0700, Yinghai wrote: >>> Update e820 at first, and later put them resource tree. >>> Reserved that early, will not be allocated to unassigned PCI BAR >>> >>> v3: remove probe_roms() that is not needed, because whole range is reserved >>> already >> >> Test booted this patch series on the problematic t3400, seems to work >> fine. dmesg attached to bug 15744. > > Thanks for testing (again). I'm not confident that this series is > going to be successful, so I started looking for other approaches. > > I can't reproduce the exact problem you're seeing, but in my > kludged-up attempt, the patch below is enough to keep us from > assigning the space below 1MB to a device. > > Would you guys (Andy & Andy, what a coincidence :-)) mind giving > it a try? This is intended to work on top of current upstream, > with no other patches required. > This certainly wins from a simplicity standpoint! -hpa ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-23 23:44 ` H. Peter Anvin @ 2010-04-24 0:36 ` Yinghai Lu 0 siblings, 0 replies; 32+ messages in thread From: Yinghai Lu @ 2010-04-24 0:36 UTC (permalink / raw) To: H. Peter Anvin Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On 04/23/2010 04:44 PM, H. Peter Anvin wrote: > On 04/23/2010 04:05 PM, Bjorn Helgaas wrote: > >> On Wednesday 21 April 2010 01:31:20 pm Andy Isaacson wrote: >> >>> On Tue, Apr 20, 2010 at 10:33:50PM -0700, Yinghai wrote: >>> >>>> Update e820 at first, and later put them resource tree. >>>> Reserved that early, will not be allocated to unassigned PCI BAR >>>> >>>> v3: remove probe_roms() that is not needed, because whole range is reserved >>>> already >>>> >>> Test booted this patch series on the problematic t3400, seems to work >>> fine. dmesg attached to bug 15744. >>> >> Thanks for testing (again). I'm not confident that this series is >> going to be successful, so I started looking for other approaches. >> >> I can't reproduce the exact problem you're seeing, but in my >> kludged-up attempt, the patch below is enough to keep us from >> assigning the space below 1MB to a device. >> >> Would you guys (Andy & Andy, what a coincidence :-)) mind giving >> it a try? This is intended to work on top of current upstream, >> with no other patches required. >> >> > This certainly wins from a simplicity standpoint! > > indeed. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-23 23:05 ` [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END Bjorn Helgaas 2010-04-23 23:44 ` H. Peter Anvin @ 2010-04-26 12:50 ` R. Andrew Bailey 2010-04-26 15:40 ` Bjorn Helgaas 2010-04-26 18:34 ` Andy Isaacson 2 siblings, 1 reply; 32+ messages in thread From: R. Andrew Bailey @ 2010-04-26 12:50 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andy Isaacson, Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On 23/04/10 17:05 -0600, Bjorn Helgaas wrote: >On Wednesday 21 April 2010 01:31:20 pm Andy Isaacson wrote: >> On Tue, Apr 20, 2010 at 10:33:50PM -0700, Yinghai wrote: >> > Update e820 at first, and later put them resource tree. >> > Reserved that early, will not be allocated to unassigned PCI BAR >> > >> > v3: remove probe_roms() that is not needed, because whole range is reserved >> > already >> >> Test booted this patch series on the problematic t3400, seems to work >> fine. dmesg attached to bug 15744. > >Thanks for testing (again). I'm not confident that this series is >going to be successful, so I started looking for other approaches. > >I can't reproduce the exact problem you're seeing, but in my >kludged-up attempt, the patch below is enough to keep us from >assigning the space below 1MB to a device. > >Would you guys (Andy & Andy, what a coincidence :-)) mind giving >it a try? This is intended to work on top of current upstream, >with no other patches required. > >Bjorn > Good news- that solved it. I tried Yinghai's patches saturday to no avail (sorry it took me so long to get back to you, I was about 5 bios revisions behind on this machine and wanted to update it before I tried any more tests). .andy ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 12:50 ` R. Andrew Bailey @ 2010-04-26 15:40 ` Bjorn Helgaas 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Helgaas @ 2010-04-26 15:40 UTC (permalink / raw) To: R. Andrew Bailey Cc: Andy Isaacson, Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Monday 26 April 2010 06:50:32 am R. Andrew Bailey wrote: > On 23/04/10 17:05 -0600, Bjorn Helgaas wrote: > >On Wednesday 21 April 2010 01:31:20 pm Andy Isaacson wrote: > >Would you guys (Andy & Andy, what a coincidence :-)) mind giving > >it a try? This is intended to work on top of current upstream, > >with no other patches required. > > > Good news- that solved it. I tried Yinghai's patches saturday to no > avail (sorry it took me so long to get back to you, I was about 5 bios > revisions behind on this machine and wanted to update it before I > tried any more tests). Great, thanks for testing this! The only problem here is that we changed two things at once -- the BIOS and the patch, and we need to figure out which change fixed the problem. I want Linux to work correctly even on the old BIOS, on the theory that "if Windows works, Linux should work, too." Changing a BIOS is relatively risky, and it's not something I want users to have to diagnose and deal with. If we're lucky, the kernel without the patch will still fail on the updated BIOS. If the pre-patch kernel fails and the post-patch kernel works, and you can attach the entire dmesg log of the post-patch kernel to the bugzilla, we should be able to see Linux making more sensible BAR assignments when working around the BIOS bug. Then we can be pretty confident that my patch fixed the problem. If the pre-patch kernel works on the updated BIOS, that means one of the BIOS updates fixed the BIOS bug, and we didn't actually exercise my patch at all. If that's the case, we'll have to wait for a report from the other Andy. (Or you could temporarily down-grade your BIOS to the original version you had. But I don't know whether that's even possible... it probably depends on the BIOS update tools.) Thanks again for all your help. All progress in Linux depends on early adopters like you who are willing to test kernels and help work through issues :-) Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-23 23:05 ` [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END Bjorn Helgaas 2010-04-23 23:44 ` H. Peter Anvin 2010-04-26 12:50 ` R. Andrew Bailey @ 2010-04-26 18:34 ` Andy Isaacson 2010-04-26 19:31 ` Jesse Barnes 2 siblings, 1 reply; 32+ messages in thread From: Andy Isaacson @ 2010-04-26 18:34 UTC (permalink / raw) To: Bjorn Helgaas Cc: R. Andrew Bailey, Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Fri, Apr 23, 2010 at 05:05:24PM -0600, Bjorn Helgaas wrote: > On Wednesday 21 April 2010 01:31:20 pm Andy Isaacson wrote: > > On Tue, Apr 20, 2010 at 10:33:50PM -0700, Yinghai wrote: > > > Update e820 at first, and later put them resource tree. > > > Reserved that early, will not be allocated to unassigned PCI BAR > > > > > > v3: remove probe_roms() that is not needed, because whole range is reserved > > > already > > > > Test booted this patch series on the problematic t3400, seems to work > > fine. dmesg attached to bug 15744. > > Thanks for testing (again). I'm not confident that this series is > going to be successful, so I started looking for other approaches. > > I can't reproduce the exact problem you're seeing, but in my > kludged-up attempt, the patch below is enough to keep us from > assigning the space below 1MB to a device. > > Would you guys (Andy & Andy, what a coincidence :-)) mind giving > it a try? This is intended to work on top of current upstream, > with no other patches required. > > Bjorn > > > commit 7fb707eb97fdf6dc4fa4b127f127f8d00223afc7 > Author: Bjorn Helgaas <bjorn.helgaas@hp.com> > Date: Fri Apr 23 15:22:10 2010 -0600 > > x86/PCI: never allocate PCI MMIO resources below BIOS_END > > When we move a PCI device or assign resources to a device not configured > by the BIOS, we want to avoid the BIOS region below 1MB. Note that if the > BIOS places devices below 1MB, we leave them there. Works for me. dmesg at https://bugzilla.kernel.org/attachment.cgi?id=26150 -andy ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 18:34 ` Andy Isaacson @ 2010-04-26 19:31 ` Jesse Barnes 2010-04-26 20:27 ` Bjorn Helgaas 0 siblings, 1 reply; 32+ messages in thread From: Jesse Barnes @ 2010-04-26 19:31 UTC (permalink / raw) To: Andy Isaacson Cc: Bjorn Helgaas, R. Andrew Bailey, Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Mon, 26 Apr 2010 11:34:36 -0700 Andy Isaacson <adi@hexapodia.org> wrote: > On Fri, Apr 23, 2010 at 05:05:24PM -0600, Bjorn Helgaas wrote: > > On Wednesday 21 April 2010 01:31:20 pm Andy Isaacson wrote: > > > On Tue, Apr 20, 2010 at 10:33:50PM -0700, Yinghai wrote: > > > > Update e820 at first, and later put them resource tree. > > > > Reserved that early, will not be allocated to unassigned PCI BAR > > > > > > > > v3: remove probe_roms() that is not needed, because whole range is reserved > > > > already > > > > > > Test booted this patch series on the problematic t3400, seems to work > > > fine. dmesg attached to bug 15744. > > > > Thanks for testing (again). I'm not confident that this series is > > going to be successful, so I started looking for other approaches. > > > > I can't reproduce the exact problem you're seeing, but in my > > kludged-up attempt, the patch below is enough to keep us from > > assigning the space below 1MB to a device. > > > > Would you guys (Andy & Andy, what a coincidence :-)) mind giving > > it a try? This is intended to work on top of current upstream, > > with no other patches required. > > > > Bjorn > > > > > > commit 7fb707eb97fdf6dc4fa4b127f127f8d00223afc7 > > Author: Bjorn Helgaas <bjorn.helgaas@hp.com> > > Date: Fri Apr 23 15:22:10 2010 -0600 > > > > x86/PCI: never allocate PCI MMIO resources below BIOS_END > > > > When we move a PCI device or assign resources to a device not configured > > by the BIOS, we want to avoid the BIOS region below 1MB. Note that if the > > BIOS places devices below 1MB, we leave them there. > > Works for me. dmesg at > https://bugzilla.kernel.org/attachment.cgi?id=26150 Great, thanks for testing. Applied this one to my for-linus tree. I still thing Yinghai's patches should go in as well (marking regions as busy seems like good housekeeping), but with this fixed they're a better fit for -next. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 19:31 ` Jesse Barnes @ 2010-04-26 20:27 ` Bjorn Helgaas 2010-04-26 20:37 ` Jesse Barnes 0 siblings, 1 reply; 32+ messages in thread From: Bjorn Helgaas @ 2010-04-26 20:27 UTC (permalink / raw) To: Jesse Barnes Cc: Andy Isaacson, R. Andrew Bailey, Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Monday 26 April 2010 01:31:35 pm Jesse Barnes wrote: > On Mon, 26 Apr 2010 11:34:36 -0700 > Andy Isaacson <adi@hexapodia.org> wrote: > > On Fri, Apr 23, 2010 at 05:05:24PM -0600, Bjorn Helgaas wrote: > > > commit 7fb707eb97fdf6dc4fa4b127f127f8d00223afc7 > > > Author: Bjorn Helgaas <bjorn.helgaas@hp.com> > > > Date: Fri Apr 23 15:22:10 2010 -0600 > > > > > > x86/PCI: never allocate PCI MMIO resources below BIOS_END > > > > > > When we move a PCI device or assign resources to a device not configured > > > by the BIOS, we want to avoid the BIOS region below 1MB. Note that if the > > > BIOS places devices below 1MB, we leave them there. > > > > Works for me. dmesg at > > https://bugzilla.kernel.org/attachment.cgi?id=26150 > > Great, thanks for testing. Applied this one to my for-linus tree. Thanks! > I still think Yinghai's patches should go in as well (marking regions as > busy seems like good housekeeping), but with this fixed they're a better > fit for -next. I'm a little concerned that those patches are a sledgehammer approach. Previously, IORESOURCE_BUSY has basically been used for mutual exclusion between drivers that would otherwise claim the same resource. It hasn't been used to guide resource assignment in the PCI/PNP/etc core. Maybe it's a good idea to also use IORESOURCE_BUSY there, but I'm not sure. Right now it feels like undesirable overloading to me. I think it also leads to at least one problem: Guenter's machine has no VGA but has a PCI device that lives at 0xa0000. The driver for that device won't be able to request that region if the arch code has marked it busy. Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 20:27 ` Bjorn Helgaas @ 2010-04-26 20:37 ` Jesse Barnes 2010-04-26 21:07 ` Yinghai 2010-04-26 21:12 ` H. Peter Anvin 0 siblings, 2 replies; 32+ messages in thread From: Jesse Barnes @ 2010-04-26 20:37 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andy Isaacson, R. Andrew Bailey, Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Mon, 26 Apr 2010 14:27:56 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > I'm a little concerned that those patches are a sledgehammer approach. > Previously, IORESOURCE_BUSY has basically been used for mutual exclusion > between drivers that would otherwise claim the same resource. It hasn't > been used to guide resource assignment in the PCI/PNP/etc core. Maybe > it's a good idea to also use IORESOURCE_BUSY there, but I'm not sure. > Right now it feels like undesirable overloading to me. I guess that's true, removing those regions from the pool entirely might be better? Or some other, clear way of expressing that the regions aren't available to drivers. Maybe we need a new IO resource type for platform ranges. > I think it also leads to at least one problem: Guenter's machine has no > VGA but has a PCI device that lives at 0xa0000. The driver for that > device won't be able to request that region if the arch code has marked > it busy. Ah good point, so we'll want another approach at any rate. Yinghai? -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 20:37 ` Jesse Barnes @ 2010-04-26 21:07 ` Yinghai 2010-04-26 21:19 ` H. Peter Anvin 2010-04-26 21:12 ` H. Peter Anvin 1 sibling, 1 reply; 32+ messages in thread From: Yinghai @ 2010-04-26 21:07 UTC (permalink / raw) To: Jesse Barnes, Linus Torvalds Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On 04/26/2010 01:37 PM, Jesse Barnes wrote: > On Mon, 26 Apr 2010 14:27:56 -0600 >> I think it also leads to at least one problem: Guenter's machine has no >> VGA but has a PCI device that lives at 0xa0000. The driver for that >> device won't be able to request that region if the arch code has marked >> it busy. > > Ah good point, so we'll want another approach at any rate. Yinghai? With current linus's tree, there is some difference between mmio > 1M and MMIO < 1M. Actually it does not care about E820_RESERVED for > 1M range, because it doesn't have _BUSY. For < 1M range, a. MMIO is not with E820_RESERVED, the device driver will work. b. MMIO is with E820_RESERVED, the device driver can not reserve it's mmio later. because that MMIO could have parent resource with "reserved" name and _BUSY flag My patch -v3 will break a. but my patch -v4 patch will make it work. but looks -v4 is too tricky. maybe we just need remove change like following. diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c index 7bca3c6..9849824 100644 --- a/arch/x86/kernel/e820.c +++ b/arch/x86/kernel/e820.c @@ -1094,7 +1094,7 @@ void __init e820_reserve_resources(void) * pci device BAR resource and insert them later in * pcibios_resource_survey() */ - if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20)) { + if (e820.map[i].type != E820_RESERVED) { res->flags |= IORESOURCE_BUSY; insert_resource(&iomem_resource, res); } May need Linus to ack it. Those strange devices could be some kind of virtual debug devices. and they like to use those low mmio range. YH ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 21:07 ` Yinghai @ 2010-04-26 21:19 ` H. Peter Anvin 0 siblings, 0 replies; 32+ messages in thread From: H. Peter Anvin @ 2010-04-26 21:19 UTC (permalink / raw) To: Yinghai Cc: Jesse Barnes, Linus Torvalds, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti > > but looks -v4 is too tricky. > Let's distinguish between what we need for .34 -- we need something minimal and as soon as possible at this stage -- and concentrate on getting the right thing to happen going forward. -hpa ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 20:37 ` Jesse Barnes 2010-04-26 21:07 ` Yinghai @ 2010-04-26 21:12 ` H. Peter Anvin 2010-04-26 21:25 ` Jesse Barnes 2010-04-26 21:44 ` jacob pan 1 sibling, 2 replies; 32+ messages in thread From: H. Peter Anvin @ 2010-04-26 21:12 UTC (permalink / raw) To: Jesse Barnes Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti > On Mon, 26 Apr 2010 14:27:56 -0600 > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: >> I'm a little concerned that those patches are a sledgehammer approach. >> Previously, IORESOURCE_BUSY has basically been used for mutual exclusion >> between drivers that would otherwise claim the same resource. It hasn't >> been used to guide resource assignment in the PCI/PNP/etc core. Maybe >> it's a good idea to also use IORESOURCE_BUSY there, but I'm not sure. >> Right now it feels like undesirable overloading to me. > > I guess that's true, removing those regions from the pool entirely > might be better? Or some other, clear way of expressing that the > regions aren't available to drivers. Maybe we need a new IO resource > type for platform ranges. > >> I think it also leads to at least one problem: Guenter's machine has no >> VGA but has a PCI device that lives at 0xa0000. The driver for that >> device won't be able to request that region if the arch code has marked >> it busy. > > Ah good point, so we'll want another approach at any rate. Yinghai? What we need is to keep track of the areas available for address space allocation by dynamically addressed devices, as distinct from address space that is in use by a kernel-known device. There is an in-between, which one can call "here there be dragons" space, which should never be used for dynamic device allocation, but if a platform device or pre-assigned device uses that space then it should be allowed to be allocated. In the case of x86, anything that is E820_RESERVED, *or* in the legacy region (below 1 MB) and is not RAM, is "here there be dragons" space. -hpa ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 21:12 ` H. Peter Anvin @ 2010-04-26 21:25 ` Jesse Barnes 2010-04-26 21:44 ` H. Peter Anvin 2010-04-26 21:59 ` Yinghai Lu 2010-04-26 21:44 ` jacob pan 1 sibling, 2 replies; 32+ messages in thread From: Jesse Barnes @ 2010-04-26 21:25 UTC (permalink / raw) To: H. Peter Anvin Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Mon, 26 Apr 2010 14:12:35 -0700 "H. Peter Anvin" <hpa@zytor.com> wrote: > > On Mon, 26 Apr 2010 14:27:56 -0600 > > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > >> I'm a little concerned that those patches are a sledgehammer approach. > >> Previously, IORESOURCE_BUSY has basically been used for mutual exclusion > >> between drivers that would otherwise claim the same resource. It hasn't > >> been used to guide resource assignment in the PCI/PNP/etc core. Maybe > >> it's a good idea to also use IORESOURCE_BUSY there, but I'm not sure. > >> Right now it feels like undesirable overloading to me. > > > > I guess that's true, removing those regions from the pool entirely > > might be better? Or some other, clear way of expressing that the > > regions aren't available to drivers. Maybe we need a new IO resource > > type for platform ranges. > > > >> I think it also leads to at least one problem: Guenter's machine has no > >> VGA but has a PCI device that lives at 0xa0000. The driver for that > >> device won't be able to request that region if the arch code has marked > >> it busy. > > > > Ah good point, so we'll want another approach at any rate. Yinghai? > > What we need is to keep track of the areas available for address space > allocation by dynamically addressed devices, as distinct from address > space that is in use by a kernel-known device. There is an in-between, > which one can call "here there be dragons" space, which should never be > used for dynamic device allocation, but if a platform device or > pre-assigned device uses that space then it should be allowed to be > allocated. > > In the case of x86, anything that is E820_RESERVED, *or* in the legacy > region (below 1 MB) and is not RAM, is "here there be dragons" space. Agreed. The trickier part is handling any platform devices that request_resource against that space. But maybe we don't need to do anything special; just making sure we avoid it in the PCI "BIOS" code as Bjorn did may be sufficient. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 21:25 ` Jesse Barnes @ 2010-04-26 21:44 ` H. Peter Anvin 2010-04-26 21:53 ` Jesse Barnes 2010-04-26 21:59 ` Yinghai Lu 1 sibling, 1 reply; 32+ messages in thread From: H. Peter Anvin @ 2010-04-26 21:44 UTC (permalink / raw) To: Jesse Barnes Cc: H. Peter Anvin, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti > > Agreed. The trickier part is handling any platform devices that > request_resource against that space. But maybe we don't need to do > anything special; just making sure we avoid it in the PCI "BIOS" code > as Bjorn did may be sufficient. > Why is that hard? If a platform device does a request_resource against that space, it's a request for specific address space and it should be granted. -hpa ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 21:44 ` H. Peter Anvin @ 2010-04-26 21:53 ` Jesse Barnes 0 siblings, 0 replies; 32+ messages in thread From: Jesse Barnes @ 2010-04-26 21:53 UTC (permalink / raw) To: H. Peter Anvin Cc: Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On Mon, 26 Apr 2010 14:44:50 -0700 "H. Peter Anvin" <hpa@zytor.com> wrote: > > > > Agreed. The trickier part is handling any platform devices that > > request_resource against that space. But maybe we don't need to do > > anything special; just making sure we avoid it in the PCI "BIOS" code > > as Bjorn did may be sufficient. > > > > Why is that hard? If a platform device does a request_resource against > that space, it's a request for specific address space and it should be > granted. I was thinking if we made it a special resource type we'd have to change any platform drivers to use it; i.e. it wouldn't be IORESOURCE_MEM or IORESOURCE_IO but IORESOURCE_DRAGONS. That way it wouldn't be part of the normal resource space. But that's definitely overkill. I think Bjorn's fix is sufficient. -- Jesse Barnes, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 21:25 ` Jesse Barnes 2010-04-26 21:44 ` H. Peter Anvin @ 2010-04-26 21:59 ` Yinghai Lu 1 sibling, 0 replies; 32+ messages in thread From: Yinghai Lu @ 2010-04-26 21:59 UTC (permalink / raw) To: Jesse Barnes Cc: H. Peter Anvin, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Thomas Gleixner, Ingo Molnar, guenter.roeck, Linus Torvalds, linux-pci, x86, linux-kernel, Thomas Renninger, yaneti On 04/26/2010 02:25 PM, Jesse Barnes wrote: > On Mon, 26 Apr 2010 14:12:35 -0700 > "H. Peter Anvin" <hpa@zytor.com> wrote: > > >>> On Mon, 26 Apr 2010 14:27:56 -0600 >>> Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: >>> >>>> I'm a little concerned that those patches are a sledgehammer approach. >>>> Previously, IORESOURCE_BUSY has basically been used for mutual exclusion >>>> between drivers that would otherwise claim the same resource. It hasn't >>>> been used to guide resource assignment in the PCI/PNP/etc core. Maybe >>>> it's a good idea to also use IORESOURCE_BUSY there, but I'm not sure. >>>> Right now it feels like undesirable overloading to me. >>>> >>> I guess that's true, removing those regions from the pool entirely >>> might be better? Or some other, clear way of expressing that the >>> regions aren't available to drivers. Maybe we need a new IO resource >>> type for platform ranges. >>> >>> >>>> I think it also leads to at least one problem: Guenter's machine has no >>>> VGA but has a PCI device that lives at 0xa0000. The driver for that >>>> device won't be able to request that region if the arch code has marked >>>> it busy. >>>> >>> Ah good point, so we'll want another approach at any rate. Yinghai? >>> >> What we need is to keep track of the areas available for address space >> allocation by dynamically addressed devices, as distinct from address >> space that is in use by a kernel-known device. There is an in-between, >> which one can call "here there be dragons" space, which should never be >> used for dynamic device allocation, but if a platform device or >> pre-assigned device uses that space then it should be allowed to be >> allocated. >> >> In the case of x86, anything that is E820_RESERVED, *or* in the legacy >> region (below 1 MB) and is not RAM, is "here there be dragons" space. >> > Agreed. The trickier part is handling any platform devices that > request_resource against that space. But maybe we don't need to do > anything special; just making sure we avoid it in the PCI "BIOS" code > as Bjorn did may be sufficient. > > the two regressions from the reporters: BIOS put 0xa0000-0xb0000, 0xc0000- 0xd0000 with E820_RESERVED. BIOS ACPI _CRS keep 0xa0000-0xb0000, 0xc0000-0xd0000 as part resources for peer root bus: BUS 0. kernel insert 0xa0000-0xb0000 into resource tree with _BUSY in e820_reserve_resources() at first. last pci bus scan code, will insert 0xa0000-0xb0000, and it is under previous reserved entry. later pci_assign_unassign code, will use bus 0 resources directly, and don't care if the parent's have _BUSY bit. solutions: 1. mark _BUSY under bus 0 resource: ==> -v3 2. split e820 reserve entries to small pieces to fit into bus 0 resources, so will have holder under bus0 resources. it will prevent those range to be used. -v4 3. reject any dynamically allocation under 1M. ==> Bjorn's new patch. till now, driver can reserve resource under 1M, only when those range is not in e820. case A: bus 0: --- bus X --- device Y if the BIOS only assign range to to BUS X bridge with 0xB0000, and device Y is not assigned. then with Bojorn's patch, device Y can not get right resource allocated on first try. my -v4 can handle that case. YH ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END 2010-04-26 21:12 ` H. Peter Anvin 2010-04-26 21:25 ` Jesse Barnes @ 2010-04-26 21:44 ` jacob pan 1 sibling, 0 replies; 32+ messages in thread From: jacob pan @ 2010-04-26 21:44 UTC (permalink / raw) To: H. Peter Anvin Cc: Jesse Barnes, Bjorn Helgaas, Andy Isaacson, R. Andrew Bailey, Yinghai, Thomas Gleixner, Ingo Molnar, linux-kernel H. Peter Anvin Mon, 26 Apr 2010 14:12:35 -0700 >> On Mon, 26 Apr 2010 14:27:56 -0600 >> Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: >>> I'm a little concerned that those patches are a sledgehammer approach. >>> Previously, IORESOURCE_BUSY has basically been used for mutual exclusion >>> between drivers that would otherwise claim the same resource. It hasn't >>> been used to guide resource assignment in the PCI/PNP/etc core. Maybe >>> it's a good idea to also use IORESOURCE_BUSY there, but I'm not sure. >>> Right now it feels like undesirable overloading to me. >> >> I guess that's true, removing those regions from the pool entirely >> might be better? Or some other, clear way of expressing that the >> regions aren't available to drivers. Maybe we need a new IO resource >> type for platform ranges. >> >>> I think it also leads to at least one problem: Guenter's machine has no >>> VGA but has a PCI device that lives at 0xa0000. The driver for that >>> device won't be able to request that region if the arch code has marked >>> it busy. >> >> Ah good point, so we'll want another approach at any rate. Yinghai? > >What we need is to keep track of the areas available for address space >allocation by dynamically addressed devices, as distinct from address >space that is in use by a kernel-known device. There is an in-between, >which one can call "here there be dragons" space, which should never be >used for dynamic device allocation, but if a platform device or >pre-assigned device uses that space then it should be allowed to be >allocated. > >In the case of x86, anything that is E820_RESERVED, *or* in the legacy >region (below 1 MB) and is not RAM, is "here there be dragons" space. > > -hpa > Moorestown has a similar situation where one of the PCI devices have a BAR below 1MB. Moorestown also has the option not to have VGA. ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2010-04-28 19:23 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-04-26 23:02 [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END H. Peter Anvin 2010-04-26 23:25 ` Jesse Barnes 2010-04-26 23:49 ` H. Peter Anvin 2010-04-26 23:49 ` H. Peter Anvin 2010-04-27 0:05 ` Jesse Barnes 2010-04-27 1:27 ` Linus Torvalds 2010-04-27 1:40 ` Jesse Barnes 2010-04-27 1:41 ` Yinghai 2010-04-27 15:11 ` Bjorn Helgaas 2010-04-28 16:07 ` Bjorn Helgaas 2010-04-28 17:14 ` Yinghai Lu 2010-04-28 19:06 ` Bjorn Helgaas 2010-04-28 19:10 ` Yinghai 2010-04-28 19:23 ` Bjorn Helgaas -- strict thread matches above, loose matches on Subject: below -- 2010-04-27 2:02 H. Peter Anvin 2010-04-13 21:42 [PATCH -v2 1/2] x86: Reserve [0xa0000, 0x100000] in e820 map Yinghai 2010-04-21 5:33 ` [PATCH -v4 1/3] " Yinghai 2010-04-21 19:31 ` Andy Isaacson 2010-04-23 23:05 ` [PATCH] x86/PCI: never allocate PCI MMIO resources below BIOS_END Bjorn Helgaas 2010-04-23 23:44 ` H. Peter Anvin 2010-04-24 0:36 ` Yinghai Lu 2010-04-26 12:50 ` R. Andrew Bailey 2010-04-26 15:40 ` Bjorn Helgaas 2010-04-26 18:34 ` Andy Isaacson 2010-04-26 19:31 ` Jesse Barnes 2010-04-26 20:27 ` Bjorn Helgaas 2010-04-26 20:37 ` Jesse Barnes 2010-04-26 21:07 ` Yinghai 2010-04-26 21:19 ` H. Peter Anvin 2010-04-26 21:12 ` H. Peter Anvin 2010-04-26 21:25 ` Jesse Barnes 2010-04-26 21:44 ` H. Peter Anvin 2010-04-26 21:53 ` Jesse Barnes 2010-04-26 21:59 ` Yinghai Lu 2010-04-26 21:44 ` jacob pan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.