This path adds validation of the MMCONFIG table against the ACPI reserved
motherboard resources. If the MMCONFIG table is found to be reserved in
ACPI, we don't bother checking the E820 table. The PCI Express firmware spec
apparently tells BIOS developers that reservation in ACPI is required and
E820 reservation is optional, so checking against ACPI first makes sense.
Many BIOSes don't reserve the MMCONFIG region in E820 even though it is
perfectly functional, the existing check needlessly disables MMCONFIG in
these cases.
In order to do this, MMCONFIG setup has been split into two phases. If PCI
configuration type 1 is not available then MMCONFIG is enabled early as before.
Otherwise, it is enabled later after the ACPI interpreter is enabled, since we
need to be able to execute control methods in order to check the ACPI reserved
resources. Presently this is just triggered off the end of ACPI interpreter
initialization.
There are a few other behavioral changes here:
-Validate all MMCONFIG configurations provided, not just the first one.
-Validate the entire required length of each configuration according to the
provided ending bus number is reserved, not just the minimum required allocation.
-Validate that the area is reserved even if we read it from the chipset directly
and not from the MCFG table. This catches the case where the BIOS didn't set the
location properly in the chipset and has mapped it over other things it shouldn't
have.
Signed-off-by: Robert Hancock <[email protected]>
diff -up linux-2.6.22-rc2-mm1/arch/i386/pci/init.c linux-2.6.22-rc2-mm1edit/arch/i386/pci/init.c
--- linux-2.6.22-rc2-mm1/arch/i386/pci/init.c 2007-05-23 21:20:43.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/arch/i386/pci/init.c 2007-05-23 21:31:50.000000000 -0600
@@ -12,7 +12,7 @@ static __init int pci_access_init(void)
type = pci_direct_probe();
#endif
#ifdef CONFIG_PCI_MMCONFIG
- pci_mmcfg_init(type);
+ pci_mmcfg_early_init(type);
#endif
if (raw_pci_ops)
return 0;
diff -up linux-2.6.22-rc2-mm1/arch/i386/pci/mmconfig-shared.c linux-2.6.22-rc2-mm1edit/arch/i386/pci/mmconfig-shared.c
--- linux-2.6.22-rc2-mm1/arch/i386/pci/mmconfig-shared.c 2007-05-23 21:21:04.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/arch/i386/pci/mmconfig-shared.c 2007-05-23 21:38:19.000000000 -0600
@@ -206,9 +206,77 @@ static void __init pci_mmcfg_insert_reso
pci_mmcfg_resources_inserted = 1;
}
-static void __init pci_mmcfg_reject_broken(int type)
+static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
+ void *data)
+{
+ struct resource *mcfg_res = data;
+ struct acpi_resource_address64 address;
+ acpi_status status;
+
+ if (res->type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32) {
+ struct acpi_resource_fixed_memory32 *fixmem32 =
+ &res->data.fixed_memory32;
+ if (!fixmem32)
+ return AE_OK;
+ if ((mcfg_res->start >= fixmem32->address) &&
+ (mcfg_res->end <= (fixmem32->address +
+ fixmem32->address_length))) {
+ mcfg_res->flags = 1;
+ return AE_CTRL_TERMINATE;
+ }
+ }
+ if ((res->type != ACPI_RESOURCE_TYPE_ADDRESS32) &&
+ (res->type != ACPI_RESOURCE_TYPE_ADDRESS64))
+ return AE_OK;
+
+ status = acpi_resource_to_address64(res, &address);
+ if (ACPI_FAILURE(status) || (address.address_length <= 0) ||
+ (address.resource_type != ACPI_MEMORY_RANGE))
+ return AE_OK;
+
+ if ((mcfg_res->start >= address.minimum) &&
+ (mcfg_res->end <=
+ (address.minimum +address.address_length))) {
+ mcfg_res->flags = 1;
+ return AE_CTRL_TERMINATE;
+ }
+ return AE_OK;
+}
+
+static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
+ void *context, void **rv)
+{
+ struct resource *mcfg_res = context;
+
+ acpi_walk_resources(handle, METHOD_NAME__CRS,
+ check_mcfg_resource, context);
+
+ if (mcfg_res->flags)
+ return AE_CTRL_TERMINATE;
+
+ return AE_OK;
+}
+
+static int __init is_acpi_reserved(unsigned long start, unsigned long end)
+{
+ struct resource mcfg_res;
+
+ mcfg_res.start = start;
+ mcfg_res.end = end;
+ mcfg_res.flags = 0;
+
+ acpi_get_devices("PNP0C01", find_mboard_resource, &mcfg_res, NULL);
+
+ if( !mcfg_res.flags )
+ acpi_get_devices("PNP0C02", find_mboard_resource, &mcfg_res, NULL);
+
+ return mcfg_res.flags;
+}
+
+static void __init pci_mmcfg_reject_broken(void)
{
typeof(pci_mmcfg_config[0]) *cfg;
+ int i;
if ((pci_mmcfg_config_num == 0) ||
(pci_mmcfg_config == NULL) ||
@@ -228,18 +296,36 @@ static void __init pci_mmcfg_reject_brok
"Rejected as broken MCFG.\n");
goto reject;
}
-
- /*
- * Only do this check when type 1 works. If it doesn't work
- * assume we run on a Mac and always use MCFG
- */
- if (type == 1 && !e820_all_mapped(cfg->address,
- cfg->address + MMCONFIG_APER_MIN,
- E820_RESERVED)) {
- printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %Lx is not"
- " E820-reserved\n", cfg->address);
- goto reject;
+
+ for(i=0; i < pci_mmcfg_config_num; i++) {
+ u32 size = (cfg->end_bus_number + 1) << 20;
+ cfg = &pci_mmcfg_config[i];
+ printk(KERN_NOTICE "PCI: MCFG configuration %d: base %p segment %hu buses %u - %u\n",
+ i, (void*)cfg->address, cfg->pci_segment,
+ (unsigned int)cfg->start_bus_number,
+ (unsigned int)cfg->end_bus_number);
+ if(is_acpi_reserved(cfg->address,
+ cfg->address + size - 1))
+ printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved "
+ "in ACPI motherboard resources\n",
+ cfg->address);
+ else {
+ printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %Lx is not "
+ "reserved in ACPI motherboard resources\n",
+ cfg->address);
+ /* Don't try to do this check unless configuration type 1 is
+ available. */
+ if((pci_probe & PCI_PROBE_CONF1) &&
+ e820_all_mapped(cfg->address,
+ cfg->address + size - 1,
+ E820_RESERVED))
+ printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved in E820\n",
+ cfg->address);
+ else
+ goto reject;
+ }
}
+
return;
reject:
@@ -249,20 +335,46 @@ reject:
pci_mmcfg_config_num = 0;
}
-void __init pci_mmcfg_init(int type)
+void __init pci_mmcfg_early_init(int type)
+{
+ if ((pci_probe & PCI_PROBE_MMCONF) == 0)
+ return;
+
+ /* If type 1 access is available, no need to enable MMCONFIG yet, we can
+ defer until later when the ACPI interpreter is available to better
+ validate things. */
+ if( type == 1 )
+ return;
+
+ acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg);
+
+ if ((pci_mmcfg_config_num == 0) ||
+ (pci_mmcfg_config == NULL) ||
+ (pci_mmcfg_config[0].address == 0))
+ return;
+
+ if (pci_mmcfg_arch_init())
+ pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
+}
+
+void __init pci_mmcfg_late_init(void)
{
int known_bridge = 0;
+ /* MMCONFIG disabled */
if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return;
+
+ /* MMCONFIG already enabled */
+ if (!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
+ return;
- if (type == 1 && pci_mmcfg_check_hostbridge())
+ if ((pci_probe & PCI_PROBE_CONF1) && pci_mmcfg_check_hostbridge())
known_bridge = 1;
-
- if (!known_bridge) {
+ else
acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg);
- pci_mmcfg_reject_broken(type);
- }
+
+ pci_mmcfg_reject_broken();
if ((pci_mmcfg_config_num == 0) ||
(pci_mmcfg_config == NULL) ||
@@ -270,7 +382,7 @@ void __init pci_mmcfg_init(int type)
return;
if (pci_mmcfg_arch_init()) {
- if (type == 1)
+ if (pci_probe & PCI_PROBE_CONF1)
unreachable_devices();
if (known_bridge)
pci_mmcfg_insert_resources(IORESOURCE_BUSY);
diff -up linux-2.6.22-rc2-mm1/arch/i386/pci/pci.h linux-2.6.22-rc2-mm1edit/arch/i386/pci/pci.h
--- linux-2.6.22-rc2-mm1/arch/i386/pci/pci.h 2007-04-25 21:08:32.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/arch/i386/pci/pci.h 2007-05-23 21:31:50.000000000 -0600
@@ -91,7 +91,8 @@ extern int pci_conf1_read(unsigned int s
extern int pci_direct_probe(void);
extern void pci_direct_init(int type);
extern void pci_pcbios_init(void);
-extern void pci_mmcfg_init(int type);
+extern void pci_mmcfg_early_init(int type);
+extern void pci_mmcfg_late_init(void);
extern void pcibios_sort(void);
/* pci-mmconfig.c */
--- linux-2.6.22-rc2-mm1/drivers/acpi/bus.c 2007-05-23 21:20:44.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/drivers/acpi/bus.c 2007-05-23 21:31:50.000000000 -0600
@@ -42,6 +42,7 @@
ACPI_MODULE_NAME("bus");
#ifdef CONFIG_X86
extern void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger);
+extern void __init pci_mmcfg_late_init(void);
#endif
struct acpi_device *acpi_root;
@@ -755,6 +756,9 @@ static int __init acpi_init(void)
result = acpi_bus_init();
if (!result) {
+#ifdef CONFIG_X86
+ pci_mmcfg_late_init();
+#endif
#ifdef CONFIG_PM_LEGACY
if (!PM_IS_ACTIVE())
pm_active = 1;
On Tue, 29 May 2007, Robert Hancock wrote:
>
> This path adds validation of the MMCONFIG table against the ACPI reserved
> motherboard resources.
Please fix the formatting of your code.
"for" and "if" are not functions, and they have a space before the
parenthesis.
And pretty much every single conditional in this patch is spread out over
two or more lines and has at least three different indentations. There's
something wrong here. Code can't look this bad and still be fine. Some of
this looks like random whitespace noise:
+ if(is_acpi_reserved(cfg->address,
+ cfg->address + size - 1))
+ printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved "
+ "in ACPI motherboard resources\n",
+ cfg->address);
+ else {
That's just horrid. Please try to make the code _look_ nicer.
For example, just making "is_acpi_reserved()" take a start/len thing
instead, would allow you to at least do
if (is_acpi_reserved(cfg->address, size)) {
printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved "
"in ACPI motherboard resources\n",
cfg->address);
} else {
...
(and has the braces rigth too - don't pair an unbraced "if ()" with a
braced "else" statement), and that together with making the body of the
for-loop a separate function would possibly make that code read a lot
better.
Same goes for this thing:
+ if((pci_probe & PCI_PROBE_CONF1) &&
+ e820_all_mapped(cfg->address,
+ cfg->address + size - 1,
+ E820_RESERVED))
+ printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved in E820\n",
+ cfg->address);
+ else
+ goto reject;
there really is *not* a highly coveted prize for having the most different
indentation in the fewest possible lines of code!
Yeah, I realize that maybe this is nit-picking, but trying to read this
patch really does make you go blind. It violates so many coding standards
that it's almost impossible to read the code itself. It's made worse by
the fact that you then also used Thunderbird to send the patch, and had it
set for
Content-type: text/plain; charset=ISO-8859-1; format=flowed
where that "format=flowed" means that basically no mail-client will be
able to read it sanely (because a lot of them will re-flow the text), and
you have to save it to a file before you can even comment on it.
Gaah. See
http://mbligh.org/linuxdocs/Email/Clients/Thunderbird
where the most important sentence is "Thunderbird is written by aged whore
monkeys stoned on crack". But it also talks about how to disable that
idiotic format=flowed for patches, and how to make sure it's not wrapping.
But as mentioned, your patch itself had some whitespace issues even aside
from the regular Thunderbird breakage.
Linus
Linus Torvalds wrote:
> On Tue, 29 May 2007, Robert Hancock wrote:
>
>> This path adds validation of the MMCONFIG table against the ACPI reserved
>> motherboard resources.
>>
>
> Please fix the formatting of your code.
>
>
[snip snip]
> Same goes for this thing:
>
> + if((pci_probe & PCI_PROBE_CONF1) &&
> + e820_all_mapped(cfg->address,
> + cfg->address + size - 1,
> + E820_RESERVED))
> + printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved in E820\n",
> + cfg->address);
> + else
> + goto reject;
>
> there really is *not* a highly coveted prize for having the most different
> indentation in the fewest possible lines of code!
>
>
That looks like standard lisp style indenting (yes I realise that C !=
lisp). At a guess I would say that code was written in emacs (which can
be fixed by the necessary changes in your ~/.emacs). Grand parent post
should check out these resources, they may help...
http://www.bloomington.in.us/~brutt/emacs-c-dev.html
http://lug.umbc.edu/tutorials/adv-emacs.html
or if you are really gung ho the CC-mode documentation
http://www.chemie.fu-berlin.de/chemnet/use/info/cc-mode/cc-mode_6.html
Matt
Linus Torvalds wrote:
> On Tue, 29 May 2007, Robert Hancock wrote:
>> This path adds validation of the MMCONFIG table against the ACPI reserved
>> motherboard resources.
>
> Please fix the formatting of your code.
>
> "for" and "if" are not functions, and they have a space before the
> parenthesis.
>
> And pretty much every single conditional in this patch is spread out over
> two or more lines and has at least three different indentations. There's
> something wrong here. Code can't look this bad and still be fine. Some of
> this looks like random whitespace noise:
>
> + if(is_acpi_reserved(cfg->address,
> + cfg->address + size - 1))
> + printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved "
> + "in ACPI motherboard resources\n",
> + cfg->address);
> + else {
>
> That's just horrid. Please try to make the code _look_ nicer.
I'll try and fix up the formatting and repost this patch. I suspect some
of the issues are from the added code clashing with the way the existing
code was formatted.
--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/
On Wed, 30 May 2007, Robert Hancock wrote:
>
> I'll try and fix up the formatting and repost this patch.
Thanks.
> I suspect some
> of the issues are from the added code clashing with the way the existing
> code was formatted.
Well, I have to admit that I might not have reacted so much if it hadn't
been for the Thunderbird thing, which made it look _really_ strange at
first, so then I had to go outside my mail client to look closer. And once
I looked closer, I just went "aiieee, it wasn't all the email client" ;)
Linus
Linus Torvalds wrote:
> And once I looked closer, I just went "aiieee, it wasn't all the email client" ;)
Not long ago, Tejun pointed out the "External Editor" extension for Thunderbird,
which turns out to be the only really sane way to submit patches with that client.
Download and install it, then add a button for it using View->Toolbars->Customize...
and finally just click on it when in the Compose dialog.
A very useful tip. Thanks again to Tejun for pointing it out.
On Wed, May 30, 2007 at 08:05:28AM -0700, Linus Torvalds wrote:
>
>
> On Wed, 30 May 2007, Robert Hancock wrote:
> >
> > I'll try and fix up the formatting and repost this patch.
>
> Thanks.
>
> > I suspect some
> > of the issues are from the added code clashing with the way the existing
> > code was formatted.
>
> Well, I have to admit that I might not have reacted so much if it hadn't
> been for the Thunderbird thing, which made it look _really_ strange at
> first, so then I had to go outside my mail client to look closer. And once
> I looked closer, I just went "aiieee, it wasn't all the email client" ;)
I got fed up of telling people to reconfigure their MUAs a long time
ago and ended up with this in my .procmailrc
:0fw
| /usr/bin/perl -pe 's/^(Content-Type: .*)format=flowed/\1format=flawed/'
It doesn't solve all the worlds problems, but it least makes that crap
readable in _my_ MUA. Sadly, if it's a patch that I have to apply
then chances are I'll have to get them to resend it with something
else anyway, as it's inevitably buggered in some other way because
thunderbird really is that dire.
I'm convinced there's some contest to see who can make the worst
graphical mail client for Linux. I'm not sure what the prize is,
or who's winning, but the entries so far are horrific.
Dave
--
http://www.codemonkey.org.uk
On Wed, 30 May 2007, Dave Jones wrote:
>
> I'm convinced there's some contest to see who can make the worst
> graphical mail client for Linux. I'm not sure what the prize is,
> or who's winning, but the entries so far are horrific.
LOL.
Although in this case it was actually not a Linux MUA:
User-Agent: Thunderbird 2.0.0.0 (Windows/20070326)
but I didn't want to embarrass Robert publicly before you brought up the
issue of operating systems ;)
Your procmail filter looks somewhat broken, btw. It will trigger on things
that aren't headers, and it will potentially _not_ trigger on headers (ie
if they are line continuations). But I guess it works well enough in
practice.
Linus
On Wed, 30 May 2007 13:23:31 -0400
Dave Jones <[email protected]> wrote:
> On Wed, May 30, 2007 at 08:05:28AM -0700, Linus Torvalds wrote:
> >
> >
> > On Wed, 30 May 2007, Robert Hancock wrote:
> > >
> > > I'll try and fix up the formatting and repost this patch.
> >
> > Thanks.
> >
> > > I suspect some
> > > of the issues are from the added code clashing with the way the existing
> > > code was formatted.
> >
> > Well, I have to admit that I might not have reacted so much if it hadn't
> > been for the Thunderbird thing, which made it look _really_ strange at
> > first, so then I had to go outside my mail client to look closer. And once
> > I looked closer, I just went "aiieee, it wasn't all the email client" ;)
>
> I got fed up of telling people to reconfigure their MUAs a long time
> ago and ended up with this in my .procmailrc
>
> :0fw
> | /usr/bin/perl -pe 's/^(Content-Type: .*)format=flowed/\1format=flawed/'
>
> It doesn't solve all the worlds problems, but it least makes that crap
> readable in _my_ MUA. Sadly, if it's a patch that I have to apply
> then chances are I'll have to get them to resend it with something
> else anyway, as it's inevitably buggered in some other way because
> thunderbird really is that dire.
>
> I'm convinced there's some contest to see who can make the worst
> graphical mail client for Linux. I'm not sure what the prize is,
> or who's winning, but the entries so far are horrific.
>
Lotus Notes has no serious competition.
Andy's patch-checking script will (should) detect wordwrapping,
tab-expansion and hopefully space-stuffing. When we get that sorted out,
people who submit broken patches to one of the lists should get a robot
reply within minutes telling them what they did wrong, so things will
become largely self-correcting.
I am sooooo looking forward to that thing. <Sends note to Nobel prize
committee>
On Wed, May 30, 2007 at 10:37:49AM -0700, Linus Torvalds wrote:
> Although in this case it was actually not a Linux MUA:
>
> User-Agent: Thunderbird 2.0.0.0 (Windows/20070326)
>
> but I didn't want to embarrass Robert publicly before you brought up the
> issue of operating systems ;)
Oh well, there's no accounting for taste ;-)
> Your procmail filter looks somewhat broken, btw. It will trigger on things
> that aren't headers, and it will potentially _not_ trigger on headers (ie
> if they are line continuations). But I guess it works well enough in
> practice.
Yeah, now that I look again, it actually missed on Robert's message because
it isn't case-insensitive. It's far from perfect.
Dave
--
http://www.codemonkey.org.uk
Mark Lord wrote:
> Linus Torvalds wrote:
>> And once I looked closer, I just went "aiieee, it wasn't all the email
>> client" ;)
>
> Not long ago, Tejun pointed out the "External Editor" extension for
> Thunderbird,
> which turns out to be the only really sane way to submit patches with
> that client.
>
> Download and install it, then add a button for it using
> View->Toolbars->Customize...
> and finally just click on it when in the Compose dialog.
>
> A very useful tip. Thanks again to Tejun for pointing it out.
Yes, I've been using that one, as well as changing the word wrap length
to 0 characters to switch that off. Apparently disabling format=flowed
is needed as well, however :-)
--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/
This path adds validation of the MMCONFIG table against the ACPI reserved
motherboard resources. If the MMCONFIG table is found to be reserved in
ACPI, we don't bother checking the E820 table. The PCI Express firmware spec
apparently tells BIOS developers that reservation in ACPI is required and
E820 reservation is optional, so checking against ACPI first makes sense.
Many BIOSes don't reserve the MMCONFIG region in E820 even though it is
perfectly functional, the existing check needlessly disables MMCONFIG in
these cases.
In order to do this, MMCONFIG setup has been split into two phases. If PCI
configuration type 1 is not available then MMCONFIG is enabled early as before.
Otherwise, it is enabled later after the ACPI interpreter is enabled, since we
need to be able to execute control methods in order to check the ACPI reserved
resources. Presently this is just triggered off the end of ACPI interpreter
initialization.
There are a few other behavioral changes here:
-Validate all MMCONFIG configurations provided, not just the first one.
-Validate the entire required length of each configuration according to the
provided ending bus number is reserved, not just the minimum required allocation.
-Validate that the area is reserved even if we read it from the chipset directly
and not from the MCFG table. This catches the case where the BIOS didn't set the
location properly in the chipset and has mapped it over other things it shouldn't
have.
Based on an original patch by Rajesh Shah from Intel.
Signed-off-by: Robert Hancock <[email protected]>
---
This should fix up some of the whitespace/formatting problems in the previous
version. There were actually some bugs in the check_mcfg_resource function,
there were some <= that should have been <. Also forgot the attribution
for Rajesh Shah who wrote the original version of some of this code.
diff -rup --exclude-from=linux-2.6.22-rc2-mm1/Documentation/dontdiff linux-2.6.22-rc2-mm1/arch/i386/pci/init.c linux-2.6.22-rc2-mm1edit/arch/i386/pci/init.c
--- linux-2.6.22-rc2-mm1/arch/i386/pci/init.c 2007-05-23 21:20:43.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/arch/i386/pci/init.c 2007-05-23 21:31:50.000000000 -0600
@@ -12,7 +12,7 @@ static __init int pci_access_init(void)
type = pci_direct_probe();
#endif
#ifdef CONFIG_PCI_MMCONFIG
- pci_mmcfg_init(type);
+ pci_mmcfg_early_init(type);
#endif
if (raw_pci_ops)
return 0;
diff -rup --exclude-from=linux-2.6.22-rc2-mm1/Documentation/dontdiff linux-2.6.22-rc2-mm1/arch/i386/pci/mmconfig-shared.c linux-2.6.22-rc2-mm1edit/arch/i386/pci/mmconfig-shared.c
--- linux-2.6.22-rc2-mm1/arch/i386/pci/mmconfig-shared.c 2007-05-23 21:21:04.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/arch/i386/pci/mmconfig-shared.c 2007-05-30 18:40:31.000000000 -0600
@@ -206,9 +206,78 @@ static void __init pci_mmcfg_insert_reso
pci_mmcfg_resources_inserted = 1;
}
-static void __init pci_mmcfg_reject_broken(int type)
+static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
+ void *data)
+{
+ struct resource *mcfg_res = data;
+ struct acpi_resource_address64 address;
+ acpi_status status;
+
+ if (res->type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32) {
+ struct acpi_resource_fixed_memory32 *fixmem32 =
+ &res->data.fixed_memory32;
+ if (!fixmem32)
+ return AE_OK;
+ if ((mcfg_res->start >= fixmem32->address) &&
+ (mcfg_res->end < (fixmem32->address +
+ fixmem32->address_length))) {
+ mcfg_res->flags = 1;
+ return AE_CTRL_TERMINATE;
+ }
+ }
+ if ((res->type != ACPI_RESOURCE_TYPE_ADDRESS32) &&
+ (res->type != ACPI_RESOURCE_TYPE_ADDRESS64))
+ return AE_OK;
+
+ status = acpi_resource_to_address64(res, &address);
+ if (ACPI_FAILURE(status) ||
+ (address.address_length <= 0) ||
+ (address.resource_type != ACPI_MEMORY_RANGE))
+ return AE_OK;
+
+ if ((mcfg_res->start >= address.minimum) &&
+ (mcfg_res->end < (address.minimum + address.address_length))) {
+ mcfg_res->flags = 1;
+ return AE_CTRL_TERMINATE;
+ }
+ return AE_OK;
+}
+
+static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
+ void *context, void **rv)
+{
+ struct resource *mcfg_res = context;
+
+ acpi_walk_resources(handle, METHOD_NAME__CRS,
+ check_mcfg_resource, context);
+
+ if (mcfg_res->flags)
+ return AE_CTRL_TERMINATE;
+
+ return AE_OK;
+}
+
+static int __init is_acpi_reserved(unsigned long start, unsigned long end)
+{
+ struct resource mcfg_res;
+
+ mcfg_res.start = start;
+ mcfg_res.end = end;
+ mcfg_res.flags = 0;
+
+ acpi_get_devices("PNP0C01", find_mboard_resource, &mcfg_res, NULL);
+
+ if (!mcfg_res.flags)
+ acpi_get_devices("PNP0C02", find_mboard_resource, &mcfg_res,
+ NULL);
+
+ return mcfg_res.flags;
+}
+
+static void __init pci_mmcfg_reject_broken(void)
{
typeof(pci_mmcfg_config[0]) *cfg;
+ int i;
if ((pci_mmcfg_config_num == 0) ||
(pci_mmcfg_config == NULL) ||
@@ -228,18 +297,37 @@ static void __init pci_mmcfg_reject_brok
"Rejected as broken MCFG.\n");
goto reject;
}
-
- /*
- * Only do this check when type 1 works. If it doesn't work
- * assume we run on a Mac and always use MCFG
- */
- if (type == 1 && !e820_all_mapped(cfg->address,
- cfg->address + MMCONFIG_APER_MIN,
- E820_RESERVED)) {
- printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %Lx is not"
- " E820-reserved\n", cfg->address);
- goto reject;
+
+ for (i = 0; i < pci_mmcfg_config_num; i++) {
+ u32 size = (cfg->end_bus_number + 1) << 20;
+ cfg = &pci_mmcfg_config[i];
+ printk(KERN_NOTICE "PCI: MCFG configuration %d: base %p "
+ "segment %hu buses %u - %u\n",
+ i, (void*)cfg->address, cfg->pci_segment,
+ (unsigned int)cfg->start_bus_number,
+ (unsigned int)cfg->end_bus_number);
+ if (is_acpi_reserved(cfg->address, cfg->address + size - 1)) {
+ printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved "
+ "in ACPI motherboard resources\n",
+ cfg->address);
+ } else {
+ printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %Lx is not "
+ "reserved in ACPI motherboard resources\n",
+ cfg->address);
+ /* Don't try to do this check unless configuration
+ type 1 is available. */
+ if ((pci_probe & PCI_PROBE_CONF1) &&
+ e820_all_mapped(cfg->address,
+ cfg->address + size - 1,
+ E820_RESERVED))
+ printk(KERN_NOTICE
+ "PCI: MCFG area at %Lx reserved in E820\n",
+ cfg->address);
+ else
+ goto reject;
+ }
}
+
return;
reject:
@@ -249,20 +337,46 @@ reject:
pci_mmcfg_config_num = 0;
}
-void __init pci_mmcfg_init(int type)
+void __init pci_mmcfg_early_init(int type)
+{
+ if ((pci_probe & PCI_PROBE_MMCONF) == 0)
+ return;
+
+ /* If type 1 access is available, no need to enable MMCONFIG yet, we can
+ defer until later when the ACPI interpreter is available to better
+ validate things. */
+ if (type == 1)
+ return;
+
+ acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg);
+
+ if ((pci_mmcfg_config_num == 0) ||
+ (pci_mmcfg_config == NULL) ||
+ (pci_mmcfg_config[0].address == 0))
+ return;
+
+ if (pci_mmcfg_arch_init())
+ pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
+}
+
+void __init pci_mmcfg_late_init(void)
{
int known_bridge = 0;
+ /* MMCONFIG disabled */
if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return;
- if (type == 1 && pci_mmcfg_check_hostbridge())
- known_bridge = 1;
+ /* MMCONFIG already enabled */
+ if (!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
+ return;
- if (!known_bridge) {
+ if ((pci_probe & PCI_PROBE_CONF1) && pci_mmcfg_check_hostbridge())
+ known_bridge = 1;
+ else
acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg);
- pci_mmcfg_reject_broken(type);
- }
+
+ pci_mmcfg_reject_broken();
if ((pci_mmcfg_config_num == 0) ||
(pci_mmcfg_config == NULL) ||
@@ -270,7 +384,7 @@ void __init pci_mmcfg_init(int type)
return;
if (pci_mmcfg_arch_init()) {
- if (type == 1)
+ if (pci_probe & PCI_PROBE_CONF1)
unreachable_devices();
if (known_bridge)
pci_mmcfg_insert_resources(IORESOURCE_BUSY);
diff -rup --exclude-from=linux-2.6.22-rc2-mm1/Documentation/dontdiff linux-2.6.22-rc2-mm1/arch/i386/pci/pci.h linux-2.6.22-rc2-mm1edit/arch/i386/pci/pci.h
--- linux-2.6.22-rc2-mm1/arch/i386/pci/pci.h 2007-04-25 21:08:32.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/arch/i386/pci/pci.h 2007-05-23 21:31:50.000000000 -0600
@@ -91,7 +91,8 @@ extern int pci_conf1_read(unsigned int s
extern int pci_direct_probe(void);
extern void pci_direct_init(int type);
extern void pci_pcbios_init(void);
-extern void pci_mmcfg_init(int type);
+extern void pci_mmcfg_early_init(int type);
+extern void pci_mmcfg_late_init(void);
extern void pcibios_sort(void);
/* pci-mmconfig.c */
diff -rup --exclude-from=linux-2.6.22-rc2-mm1/Documentation/dontdiff linux-2.6.22-rc2-mm1/drivers/acpi/bus.c linux-2.6.22-rc2-mm1edit/drivers/acpi/bus.c
--- linux-2.6.22-rc2-mm1/drivers/acpi/bus.c 2007-05-23 21:20:44.000000000 -0600
+++ linux-2.6.22-rc2-mm1edit/drivers/acpi/bus.c 2007-05-23 21:31:50.000000000 -0600
@@ -42,6 +42,7 @@
ACPI_MODULE_NAME("bus");
#ifdef CONFIG_X86
extern void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger);
+extern void __init pci_mmcfg_late_init(void);
#endif
struct acpi_device *acpi_root;
@@ -755,6 +756,9 @@ static int __init acpi_init(void)
result = acpi_bus_init();
if (!result) {
+#ifdef CONFIG_X86
+ pci_mmcfg_late_init();
+#endif
#ifdef CONFIG_PM_LEGACY
if (!PM_IS_ACTIVE())
pm_active = 1;
On Tue, May 29, 2007 at 10:03:32PM -0600, Robert Hancock wrote:
> -Validate that the area is reserved even if we read it from the
> chipset directly and not from the MCFG table. This catches the case
> where the BIOS didn't set the location properly in the chipset and
> has mapped it over other things it shouldn't have.
Just for the record, I still fundamentally disagree with that part.
You're not catching what you think you're catching, since the chipset
tells you what it is going to decode as mmconfig, no matter what is
connected to it.
OG.
Olivier Galibert wrote:
> On Tue, May 29, 2007 at 10:03:32PM -0600, Robert Hancock wrote:
>> -Validate that the area is reserved even if we read it from the
>> chipset directly and not from the MCFG table. This catches the case
>> where the BIOS didn't set the location properly in the chipset and
>> has mapped it over other things it shouldn't have.
>
> Just for the record, I still fundamentally disagree with that part.
> You're not catching what you think you're catching, since the chipset
> tells you what it is going to decode as mmconfig, no matter what is
> connected to it.
But we don't - not in the case where it's overlapped some other area
with the MMCONFIG region. We saw a case of this on some of the Intel
boards, where the MMCONFIG is a 128MB area at 0xf0000000, and when
sizing the BARs on a PCI Express video device with a 256MB region, it
ended up being located momentarily at f0000000-ffffffff, which
overlapped the MMCONFIG area. That caused MMCONFIG to stop working, so
apparently on that chipset, PCI Express devices connected to the
northbridge have a higher decode priority than the MMCONFIG area.
If the BIOS is so screwed up that this becomes an issue, I don't think
we can sanely try to use the table, since we can't anticipate all the
potential problems that might result.
--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/
On Monday, June 4, 2007 5:12:50 Olivier Galibert wrote:
> On Tue, May 29, 2007 at 10:03:32PM -0600, Robert Hancock wrote:
> > -Validate that the area is reserved even if we read it from the
> > chipset directly and not from the MCFG table. This catches the case
> > where the BIOS didn't set the location properly in the chipset and
> > has mapped it over other things it shouldn't have.
>
> Just for the record, I still fundamentally disagree with that part.
> You're not catching what you think you're catching, since the chipset
> tells you what it is going to decode as mmconfig, no matter what is
> connected to it.
I'm not sure what you're objecting to? Are you suggesting that we use
mmconfig space as defined by the bridge register even if it's not mentioned
(or doesn't match) the MCFG table?
Jesse