Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock <[EMAIL PROTECTED]> wrote:
> Yinghai Lu wrote:
> > On 9/22/07, Robert Hancock <[EMAIL PROTECTED]> wrote:
> >> Thomas Gleixner wrote:
> >>> On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
>  Yinghai Lu wrote:
> > No!
> >
> > MMCONFIG will not work with acpi=off any more.
>  I don't think this is unreasonable. The ACPI MCFG table is how we are
>  supposed to learn about the area in the first place. If we can't get the
>  table location via an approved mechanism, and can't validate it doesn't
>  overlap with another memory reservation or something, I really don't
>  think we should be using it.
> >>> We all know how correct ACPI tables are. Specifications are nice,
> >>> reality tells a different story.
> >> MMCONFIG can't be used without ACPI in any case unless we know where the
> >>   table is using chipset-specific knowledge (i.e. reading the registers
> >> directly). Doing that without being told that this area is really
> >> intended to be used, via the ACPI table, is dangerous, i.e. we don't
> >> necessarily know if the MMCONFIG is broken on the platform in some way
> >> we can't detect.
> >
> > the BIOS get these info from the chipset too.
> > for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.
> >
>  I don't think it's much of an issue anyway - the chances that somebody
>  will want to run without ACPI on a system with MCFG are pretty low given
>  that you'll end up losing a bunch of functionality (not least of which
>  is multi-cores).
> >>> acpi=off is an often used debug switch and it _is_ quite useful. Taking
> >>> away debug functionality is not a good idea.
> >> If someone has to turn ACPI off, disabling MMCONFIG is probably the
> >> least of their worries..
> >
> > MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
> > must use ACPI for MMCONFIG?
>
> config PCI_MMCONFIG
>   bool
>   depends on PCI && ACPI && (PCI_GOMMCONFIG || PCI_GOANY)
>   default y
>
> We already depend on ACPI for MMCONFIG support at compile time. This
> patch does not change that.
>
> We could conceivably skip the validation if ACPI was disabled, though
> this would only make a difference in the few cases where we can detect
> the MMCONFIG area without it (looks like currently only Intel E7520 and
> 945, at least in mainline).

i added support for AMD Fam 10h NB, and that patch is in -mm.

I like to see pci_mmcfg_check_hostbridge is called before any acpi check up.

also in your pci_mmcfg_early_init, you may miss calling to
pci_mmcfg_insert_resource...

YH
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock <[EMAIL PROTECTED]> wrote:
> Yinghai Lu wrote:
> > On 9/22/07, Robert Hancock <[EMAIL PROTECTED]> wrote:
> >> Thomas Gleixner wrote:
> >>> On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
>  Yinghai Lu wrote:
> > No!
> >
> > MMCONFIG will not work with acpi=off any more.
>  I don't think this is unreasonable. The ACPI MCFG table is how we are
>  supposed to learn about the area in the first place. If we can't get the
>  table location via an approved mechanism, and can't validate it doesn't
>  overlap with another memory reservation or something, I really don't
>  think we should be using it.
> >>> We all know how correct ACPI tables are. Specifications are nice,
> >>> reality tells a different story.
> >> MMCONFIG can't be used without ACPI in any case unless we know where the
> >>   table is using chipset-specific knowledge (i.e. reading the registers
> >> directly). Doing that without being told that this area is really
> >> intended to be used, via the ACPI table, is dangerous, i.e. we don't
> >> necessarily know if the MMCONFIG is broken on the platform in some way
> >> we can't detect.
> >
> > the BIOS get these info from the chipset too.
> > for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.
> >
>  I don't think it's much of an issue anyway - the chances that somebody
>  will want to run without ACPI on a system with MCFG are pretty low given
>  that you'll end up losing a bunch of functionality (not least of which
>  is multi-cores).
> >>> acpi=off is an often used debug switch and it _is_ quite useful. Taking
> >>> away debug functionality is not a good idea.
> >> If someone has to turn ACPI off, disabling MMCONFIG is probably the
> >> least of their worries..
> >
> > MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
> > must use ACPI for MMCONFIG?
>
> config PCI_MMCONFIG
>   bool
>   depends on PCI && ACPI && (PCI_GOMMCONFIG || PCI_GOANY)
>   default y
>
> We already depend on ACPI for MMCONFIG support at compile time. This
> patch does not change that.
>
> We could conceivably skip the validation if ACPI was disabled, though
> this would only make a difference in the few cases where we can detect
> the MMCONFIG area without it (looks like currently only Intel E7520 and
> 945, at least in mainline).

can you make pci_mmcfg_late_init take one parameter about if acpi is
there or not?

so in acpi_init will be

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 9ba778a..a4a6a6f 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -746,6 +746,7 @@ static int __init acpi_init(void)

if (acpi_disabled) {
printk(KERN_INFO PREFIX "Interpreter disabled.\n");
+   pci_mmcfg_late_init(0);
return -ENODEV;
}

@@ -757,6 +758,7 @@ static int __init acpi_init(void)
result = acpi_bus_init();

if (!result) {
+   pci_mmcfg_late_init(1);
 #ifdef CONFIG_PM_LEGACY
if (!PM_IS_ACTIVE())
pm_active = 1;
@@ -767,8 +769,10 @@ static int __init acpi_init(void)
result = -ENODEV;
}
 #endif
-   } else
+   } else {
+   pci_mmcfg_late_init(0);
disable_acpi();
+   }

return result;
 }

YH
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Robert Hancock 

Yinghai Lu wrote:

On 9/22/07, Robert Hancock <[EMAIL PROTECTED]> wrote:

Thomas Gleixner wrote:

On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:

Yinghai Lu wrote:

No!

MMCONFIG will not work with acpi=off any more.

I don't think this is unreasonable. The ACPI MCFG table is how we are
supposed to learn about the area in the first place. If we can't get the
table location via an approved mechanism, and can't validate it doesn't
overlap with another memory reservation or something, I really don't
think we should be using it.

We all know how correct ACPI tables are. Specifications are nice,
reality tells a different story.

MMCONFIG can't be used without ACPI in any case unless we know where the
  table is using chipset-specific knowledge (i.e. reading the registers
directly). Doing that without being told that this area is really
intended to be used, via the ACPI table, is dangerous, i.e. we don't
necessarily know if the MMCONFIG is broken on the platform in some way
we can't detect.


the BIOS get these info from the chipset too.
for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.


I don't think it's much of an issue anyway - the chances that somebody
will want to run without ACPI on a system with MCFG are pretty low given
that you'll end up losing a bunch of functionality (not least of which
is multi-cores).

acpi=off is an often used debug switch and it _is_ quite useful. Taking
away debug functionality is not a good idea.

If someone has to turn ACPI off, disabling MMCONFIG is probably the
least of their worries..


MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
must use ACPI for MMCONFIG?


config PCI_MMCONFIG
 bool
 depends on PCI && ACPI && (PCI_GOMMCONFIG || PCI_GOANY)
 default y

We already depend on ACPI for MMCONFIG support at compile time. This 
patch does not change that.


We could conceivably skip the validation if ACPI was disabled, though 
this would only make a difference in the few cases where we can detect 
the MMCONFIG area without it (looks like currently only Intel E7520 and 
945, at least in mainline).




For AMD Fam 10h opteron, because using MMCONFIG need via %eax, so BIOS
will stilll stay with MCFG entry for MCP55 SB to not break other
os..., then you can not access ext config space for NB...

with enabling MMCONFIG in NB, and read NNCONFIG BASE from NB,( via
pci_mmcfg_check_hostbridge) that we get full MMCONFIG access for NB...

Anyway this patch alter the feature...

BTW if you trust MCFG in ACPI so much, why do you need to bother to
verify that in DSDT...


One reason is that Windows pre-Vista does not use the MCFG table, it 
does use the ACPI reserved resources. Since the board/system 
manufacturers have been testing against Windows, the resource 
reservations should be more likely correct than the MCFG table which was 
not used until Vista came along.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread H. Peter Anvin 
Yinghai Lu wrote:
>>> We all know how correct ACPI tables are. Specifications are nice,
>>> reality tells a different story.
>> MMCONFIG can't be used without ACPI in any case unless we know where the
>>   table is using chipset-specific knowledge (i.e. reading the registers
>> directly). Doing that without being told that this area is really
>> intended to be used, via the ACPI table, is dangerous, i.e. we don't
>> necessarily know if the MMCONFIG is broken on the platform in some way
>> we can't detect.
> 
> the BIOS get these info from the chipset too.
> for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.

I think he's saying we don't know a safe place to park the MMCONFIG area
if we don't have this information.  However, this applies to *any*
allocation of address space, which we do all the time, so although a
valid argument this has been decided already many times over.

-hpa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock <[EMAIL PROTECTED]> wrote:
> Yinghai Lu wrote:
> > No!
> >
> > MMCONFIG will not work with acpi=off any more.
>
> I don't think this is unreasonable. The ACPI MCFG table is how we are
> supposed to learn about the area in the first place. If we can't get the
> table location via an approved mechanism, and can't validate it doesn't
> overlap with another memory reservation or something, I really don't
> think we should be using it.
>
> I don't think it's much of an issue anyway - the chances that somebody
> will want to run without ACPI on a system with MCFG are pretty low given
> that you'll end up losing a bunch of functionality (not least of which
> is multi-cores).

with acpi=off, that we do lose some features including acpi hotplug
and power management feature...
but we don't lose anything about numa ( multi-cores...) and
bus-numa... (we get these info from NB pci conf for AMD rev C, rev E,
rev F, and Fam 10 opteron)...

Finally we lose bugs introduced by ACPI code ...

YH
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock <[EMAIL PROTECTED]> wrote:
> Thomas Gleixner wrote:
> > On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
> >> Yinghai Lu wrote:
> >>> No!
> >>>
> >>> MMCONFIG will not work with acpi=off any more.
> >> I don't think this is unreasonable. The ACPI MCFG table is how we are
> >> supposed to learn about the area in the first place. If we can't get the
> >> table location via an approved mechanism, and can't validate it doesn't
> >> overlap with another memory reservation or something, I really don't
> >> think we should be using it.
> >
> > We all know how correct ACPI tables are. Specifications are nice,
> > reality tells a different story.
>
> MMCONFIG can't be used without ACPI in any case unless we know where the
>   table is using chipset-specific knowledge (i.e. reading the registers
> directly). Doing that without being told that this area is really
> intended to be used, via the ACPI table, is dangerous, i.e. we don't
> necessarily know if the MMCONFIG is broken on the platform in some way
> we can't detect.

the BIOS get these info from the chipset too.
for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.

>
> >
> >> I don't think it's much of an issue anyway - the chances that somebody
> >> will want to run without ACPI on a system with MCFG are pretty low given
> >> that you'll end up losing a bunch of functionality (not least of which
> >> is multi-cores).
> >
> > acpi=off is an often used debug switch and it _is_ quite useful. Taking
> > away debug functionality is not a good idea.
>
> If someone has to turn ACPI off, disabling MMCONFIG is probably the
> least of their worries..

MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
must use ACPI for MMCONFIG?

For AMD Fam 10h opteron, because using MMCONFIG need via %eax, so BIOS
will stilll stay with MCFG entry for MCP55 SB to not break other
os..., then you can not access ext config space for NB...

with enabling MMCONFIG in NB, and read NNCONFIG BASE from NB,( via
pci_mmcfg_check_hostbridge) that we get full MMCONFIG access for NB...

Anyway this patch alter the feature...

BTW if you trust MCFG in ACPI so much, why do you need to bother to
verify that in DSDT...

YH
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Robert Hancock 

Thomas Gleixner wrote:

On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:

Yinghai Lu wrote:

No!

MMCONFIG will not work with acpi=off any more.
I don't think this is unreasonable. The ACPI MCFG table is how we are 
supposed to learn about the area in the first place. If we can't get the 
table location via an approved mechanism, and can't validate it doesn't 
overlap with another memory reservation or something, I really don't 
think we should be using it.


We all know how correct ACPI tables are. Specifications are nice,
reality tells a different story.


MMCONFIG can't be used without ACPI in any case unless we know where the 
 table is using chipset-specific knowledge (i.e. reading the registers 
directly). Doing that without being told that this area is really 
intended to be used, via the ACPI table, is dangerous, i.e. we don't 
necessarily know if the MMCONFIG is broken on the platform in some way 
we can't detect.




I don't think it's much of an issue anyway - the chances that somebody 
will want to run without ACPI on a system with MCFG are pretty low given 
that you'll end up losing a bunch of functionality (not least of which 
is multi-cores).


acpi=off is an often used debug switch and it _is_ quite useful. Taking
away debug functionality is not a good idea.


If someone has to turn ACPI off, disabling MMCONFIG is probably the 
least of their worries..


--
Robert Hancock  Saskatoon, SK, Canada
To email, remove "nospam" from [EMAIL PROTECTED]
Home Page: http://www.roberthancock.com/


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Thomas Gleixner 
On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
> Yinghai Lu wrote:
> > No!
> > 
> > MMCONFIG will not work with acpi=off any more.
> 
> I don't think this is unreasonable. The ACPI MCFG table is how we are 
> supposed to learn about the area in the first place. If we can't get the 
> table location via an approved mechanism, and can't validate it doesn't 
> overlap with another memory reservation or something, I really don't 
> think we should be using it.

We all know how correct ACPI tables are. Specifications are nice,
reality tells a different story.

> I don't think it's much of an issue anyway - the chances that somebody 
> will want to run without ACPI on a system with MCFG are pretty low given 
> that you'll end up losing a bunch of functionality (not least of which 
> is multi-cores).

acpi=off is an often used debug switch and it _is_ quite useful. Taking
away debug functionality is not a good idea.

tglx


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Robert Hancock 

Yinghai Lu wrote:

No!

MMCONFIG will not work with acpi=off any more.


I don't think this is unreasonable. The ACPI MCFG table is how we are 
supposed to learn about the area in the first place. If we can't get the 
table location via an approved mechanism, and can't validate it doesn't 
overlap with another memory reservation or something, I really don't 
think we should be using it.


I don't think it's much of an issue anyway - the chances that somebody 
will want to run without ACPI on a system with MCFG are pretty low given 
that you'll end up losing a bunch of functionality (not least of which 
is multi-cores).




because acpi_init==>pci_mmcfg_late_init==>pci_mmcfg_check_hostbridge...

can you move pci_mmcfg_later_init out of acpi_init and just call
somewhere after acpi_init...
or put pci_mmcfg_check_hostbridge back to pci_mmcfg_early_init?

YH


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/21/07, Yinghai Lu <[EMAIL PROTECTED]> wrote:
> On 9/21/07, Andi Kleen <[EMAIL PROTECTED]> wrote:
> >
> > From: Robert Hancock <[EMAIL PROTECTED]>
> >
> > 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.
> >
> > This also cleans up the MMCONFIG initialization functions so that they
> > simply do nothing if MMCONFIG is not compiled in.
> >
> > Based on an original patch by Rajesh Shah from Intel.
> >
> > [EMAIL PROTECTED]: many fixes and cleanups]
> > Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>
> > Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
> > Cc: Rajesh Shah <[EMAIL PROTECTED]>
> > Cc: Jesse Barnes <[EMAIL PROTECTED]>
> > Acked-by: Linus Torvalds <[EMAIL PROTECTED]>
> > Cc: Andi Kleen <[EMAIL PROTECTED]>
> > Cc: Greg KH <[EMAIL PROTECTED]>
> > Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>

Also the titile is misleading: it is x86 instead of i386.. because it
will affect x86_64 too.

YH
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/21/07, Andi Kleen <[EMAIL PROTECTED]> wrote:
>
> From: Robert Hancock <[EMAIL PROTECTED]>
>
> 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.
>
> This also cleans up the MMCONFIG initialization functions so that they
> simply do nothing if MMCONFIG is not compiled in.
>
> Based on an original patch by Rajesh Shah from Intel.
>
> [EMAIL PROTECTED]: many fixes and cleanups]
> Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>
> Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
> Cc: Rajesh Shah <[EMAIL PROTECTED]>
> Cc: Jesse Barnes <[EMAIL PROTECTED]>
> Acked-by: Linus Torvalds <[EMAIL PROTECTED]>
> Cc: Andi Kleen <[EMAIL PROTECTED]>
> Cc: Greg KH <[EMAIL PROTECTED]>
> Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
> ---
>
>  arch/i386/pci/init.c|4 -
>  arch/i386/pci/mmconfig-shared.c |  151 
> +++-
>  arch/i386/pci/pci.h |1
>  drivers/acpi/bus.c  |2
>  include/linux/pci.h |8 ++
>  5 files changed, 144 insertions(+), 22 deletions(-)
>
> Index: linux/arch/i386/pci/init.c
> ===
> --- linux.orig/arch/i386/pci/init.c
> +++ linux/arch/i386/pci/init.c
> @@ -11,9 +11,7 @@ static __init int pci_access_init(void)
>  #ifdef CONFIG_PCI_DIRECT
> type = pci_direct_probe();
>  #endif
> -#ifdef CONFIG_PCI_MMCONFIG
> -   pci_mmcfg_init(type);
> -#endif
> +   pci_mmcfg_early_init(type);
> if (raw_pci_ops)
> return 0;
>  #ifdef CONFIG_PCI_BIOS
> Index: linux/arch/i386/pci/mmconfig-shared.c
> ===
> --- linux.orig/arch/i386/pci/mmconfig-shared.c
> +++ linux/arch/i386/pci/mmconfig-shared.c
> @@ -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 =
> +   >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, );
> +   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,
> +

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/21/07, Andi Kleen [EMAIL PROTECTED] wrote:

 From: Robert Hancock [EMAIL PROTECTED]

 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.

 This also cleans up the MMCONFIG initialization functions so that they
 simply do nothing if MMCONFIG is not compiled in.

 Based on an original patch by Rajesh Shah from Intel.

 [EMAIL PROTECTED]: many fixes and cleanups]
 Signed-off-by: Robert Hancock [EMAIL PROTECTED]
 Signed-off-by: Andi Kleen [EMAIL PROTECTED]
 Cc: Rajesh Shah [EMAIL PROTECTED]
 Cc: Jesse Barnes [EMAIL PROTECTED]
 Acked-by: Linus Torvalds [EMAIL PROTECTED]
 Cc: Andi Kleen [EMAIL PROTECTED]
 Cc: Greg KH [EMAIL PROTECTED]
 Signed-off-by: Andrew Morton [EMAIL PROTECTED]
 ---

  arch/i386/pci/init.c|4 -
  arch/i386/pci/mmconfig-shared.c |  151 
 +++-
  arch/i386/pci/pci.h |1
  drivers/acpi/bus.c  |2
  include/linux/pci.h |8 ++
  5 files changed, 144 insertions(+), 22 deletions(-)

 Index: linux/arch/i386/pci/init.c
 ===
 --- linux.orig/arch/i386/pci/init.c
 +++ linux/arch/i386/pci/init.c
 @@ -11,9 +11,7 @@ static __init int pci_access_init(void)
  #ifdef CONFIG_PCI_DIRECT
 type = pci_direct_probe();
  #endif
 -#ifdef CONFIG_PCI_MMCONFIG
 -   pci_mmcfg_init(type);
 -#endif
 +   pci_mmcfg_early_init(type);
 if (raw_pci_ops)
 return 0;
  #ifdef CONFIG_PCI_BIOS
 Index: linux/arch/i386/pci/mmconfig-shared.c
 ===
 --- linux.orig/arch/i386/pci/mmconfig-shared.c
 +++ linux/arch/i386/pci/mmconfig-shared.c
 @@ -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,
 +

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/21/07, Yinghai Lu [EMAIL PROTECTED] wrote:
 On 9/21/07, Andi Kleen [EMAIL PROTECTED] wrote:
 
  From: Robert Hancock [EMAIL PROTECTED]
 
  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.
 
  This also cleans up the MMCONFIG initialization functions so that they
  simply do nothing if MMCONFIG is not compiled in.
 
  Based on an original patch by Rajesh Shah from Intel.
 
  [EMAIL PROTECTED]: many fixes and cleanups]
  Signed-off-by: Robert Hancock [EMAIL PROTECTED]
  Signed-off-by: Andi Kleen [EMAIL PROTECTED]
  Cc: Rajesh Shah [EMAIL PROTECTED]
  Cc: Jesse Barnes [EMAIL PROTECTED]
  Acked-by: Linus Torvalds [EMAIL PROTECTED]
  Cc: Andi Kleen [EMAIL PROTECTED]
  Cc: Greg KH [EMAIL PROTECTED]
  Signed-off-by: Andrew Morton [EMAIL PROTECTED]

Also the titile is misleading: it is x86 instead of i386.. because it
will affect x86_64 too.

YH
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Robert Hancock 

Yinghai Lu wrote:

No!

MMCONFIG will not work with acpi=off any more.


I don't think this is unreasonable. The ACPI MCFG table is how we are 
supposed to learn about the area in the first place. If we can't get the 
table location via an approved mechanism, and can't validate it doesn't 
overlap with another memory reservation or something, I really don't 
think we should be using it.


I don't think it's much of an issue anyway - the chances that somebody 
will want to run without ACPI on a system with MCFG are pretty low given 
that you'll end up losing a bunch of functionality (not least of which 
is multi-cores).




because acpi_init==pci_mmcfg_late_init==pci_mmcfg_check_hostbridge...

can you move pci_mmcfg_later_init out of acpi_init and just call
somewhere after acpi_init...
or put pci_mmcfg_check_hostbridge back to pci_mmcfg_early_init?

YH


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Thomas Gleixner 
On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
 Yinghai Lu wrote:
  No!
  
  MMCONFIG will not work with acpi=off any more.
 
 I don't think this is unreasonable. The ACPI MCFG table is how we are 
 supposed to learn about the area in the first place. If we can't get the 
 table location via an approved mechanism, and can't validate it doesn't 
 overlap with another memory reservation or something, I really don't 
 think we should be using it.

We all know how correct ACPI tables are. Specifications are nice,
reality tells a different story.

 I don't think it's much of an issue anyway - the chances that somebody 
 will want to run without ACPI on a system with MCFG are pretty low given 
 that you'll end up losing a bunch of functionality (not least of which 
 is multi-cores).

acpi=off is an often used debug switch and it _is_ quite useful. Taking
away debug functionality is not a good idea.

tglx


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Robert Hancock 

Thomas Gleixner wrote:

On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:

Yinghai Lu wrote:

No!

MMCONFIG will not work with acpi=off any more.
I don't think this is unreasonable. The ACPI MCFG table is how we are 
supposed to learn about the area in the first place. If we can't get the 
table location via an approved mechanism, and can't validate it doesn't 
overlap with another memory reservation or something, I really don't 
think we should be using it.


We all know how correct ACPI tables are. Specifications are nice,
reality tells a different story.


MMCONFIG can't be used without ACPI in any case unless we know where the 
 table is using chipset-specific knowledge (i.e. reading the registers 
directly). Doing that without being told that this area is really 
intended to be used, via the ACPI table, is dangerous, i.e. we don't 
necessarily know if the MMCONFIG is broken on the platform in some way 
we can't detect.




I don't think it's much of an issue anyway - the chances that somebody 
will want to run without ACPI on a system with MCFG are pretty low given 
that you'll end up losing a bunch of functionality (not least of which 
is multi-cores).


acpi=off is an often used debug switch and it _is_ quite useful. Taking
away debug functionality is not a good idea.


If someone has to turn ACPI off, disabling MMCONFIG is probably the 
least of their worries..


--
Robert Hancock  Saskatoon, SK, Canada
To email, remove nospam from [EMAIL PROTECTED]
Home Page: http://www.roberthancock.com/


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock [EMAIL PROTECTED] wrote:
 Thomas Gleixner wrote:
  On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
  Yinghai Lu wrote:
  No!
 
  MMCONFIG will not work with acpi=off any more.
  I don't think this is unreasonable. The ACPI MCFG table is how we are
  supposed to learn about the area in the first place. If we can't get the
  table location via an approved mechanism, and can't validate it doesn't
  overlap with another memory reservation or something, I really don't
  think we should be using it.
 
  We all know how correct ACPI tables are. Specifications are nice,
  reality tells a different story.

 MMCONFIG can't be used without ACPI in any case unless we know where the
   table is using chipset-specific knowledge (i.e. reading the registers
 directly). Doing that without being told that this area is really
 intended to be used, via the ACPI table, is dangerous, i.e. we don't
 necessarily know if the MMCONFIG is broken on the platform in some way
 we can't detect.

the BIOS get these info from the chipset too.
for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.


 
  I don't think it's much of an issue anyway - the chances that somebody
  will want to run without ACPI on a system with MCFG are pretty low given
  that you'll end up losing a bunch of functionality (not least of which
  is multi-cores).
 
  acpi=off is an often used debug switch and it _is_ quite useful. Taking
  away debug functionality is not a good idea.

 If someone has to turn ACPI off, disabling MMCONFIG is probably the
 least of their worries..

MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
must use ACPI for MMCONFIG?

For AMD Fam 10h opteron, because using MMCONFIG need via %eax, so BIOS
will stilll stay with MCFG entry for MCP55 SB to not break other
os..., then you can not access ext config space for NB...

with enabling MMCONFIG in NB, and read NNCONFIG BASE from NB,( via
pci_mmcfg_check_hostbridge) that we get full MMCONFIG access for NB...

Anyway this patch alter the feature...

BTW if you trust MCFG in ACPI so much, why do you need to bother to
verify that in DSDT...

YH
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock [EMAIL PROTECTED] wrote:
 Yinghai Lu wrote:
  No!
 
  MMCONFIG will not work with acpi=off any more.

 I don't think this is unreasonable. The ACPI MCFG table is how we are
 supposed to learn about the area in the first place. If we can't get the
 table location via an approved mechanism, and can't validate it doesn't
 overlap with another memory reservation or something, I really don't
 think we should be using it.

 I don't think it's much of an issue anyway - the chances that somebody
 will want to run without ACPI on a system with MCFG are pretty low given
 that you'll end up losing a bunch of functionality (not least of which
 is multi-cores).

with acpi=off, that we do lose some features including acpi hotplug
and power management feature...
but we don't lose anything about numa ( multi-cores...) and
bus-numa... (we get these info from NB pci conf for AMD rev C, rev E,
rev F, and Fam 10 opteron)...

Finally we lose bugs introduced by ACPI code ...

YH
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread H. Peter Anvin 
Yinghai Lu wrote:
 We all know how correct ACPI tables are. Specifications are nice,
 reality tells a different story.
 MMCONFIG can't be used without ACPI in any case unless we know where the
   table is using chipset-specific knowledge (i.e. reading the registers
 directly). Doing that without being told that this area is really
 intended to be used, via the ACPI table, is dangerous, i.e. we don't
 necessarily know if the MMCONFIG is broken on the platform in some way
 we can't detect.
 
 the BIOS get these info from the chipset too.
 for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.

I think he's saying we don't know a safe place to park the MMCONFIG area
if we don't have this information.  However, this applies to *any*
allocation of address space, which we do all the time, so although a
valid argument this has been decided already many times over.

-hpa
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Robert Hancock 

Yinghai Lu wrote:

On 9/22/07, Robert Hancock [EMAIL PROTECTED] wrote:

Thomas Gleixner wrote:

On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:

Yinghai Lu wrote:

No!

MMCONFIG will not work with acpi=off any more.

I don't think this is unreasonable. The ACPI MCFG table is how we are
supposed to learn about the area in the first place. If we can't get the
table location via an approved mechanism, and can't validate it doesn't
overlap with another memory reservation or something, I really don't
think we should be using it.

We all know how correct ACPI tables are. Specifications are nice,
reality tells a different story.

MMCONFIG can't be used without ACPI in any case unless we know where the
  table is using chipset-specific knowledge (i.e. reading the registers
directly). Doing that without being told that this area is really
intended to be used, via the ACPI table, is dangerous, i.e. we don't
necessarily know if the MMCONFIG is broken on the platform in some way
we can't detect.


the BIOS get these info from the chipset too.
for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.


I don't think it's much of an issue anyway - the chances that somebody
will want to run without ACPI on a system with MCFG are pretty low given
that you'll end up losing a bunch of functionality (not least of which
is multi-cores).

acpi=off is an often used debug switch and it _is_ quite useful. Taking
away debug functionality is not a good idea.

If someone has to turn ACPI off, disabling MMCONFIG is probably the
least of their worries..


MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
must use ACPI for MMCONFIG?


config PCI_MMCONFIG
 bool
 depends on PCI  ACPI  (PCI_GOMMCONFIG || PCI_GOANY)
 default y

We already depend on ACPI for MMCONFIG support at compile time. This 
patch does not change that.


We could conceivably skip the validation if ACPI was disabled, though 
this would only make a difference in the few cases where we can detect 
the MMCONFIG area without it (looks like currently only Intel E7520 and 
945, at least in mainline).




For AMD Fam 10h opteron, because using MMCONFIG need via %eax, so BIOS
will stilll stay with MCFG entry for MCP55 SB to not break other
os..., then you can not access ext config space for NB...

with enabling MMCONFIG in NB, and read NNCONFIG BASE from NB,( via
pci_mmcfg_check_hostbridge) that we get full MMCONFIG access for NB...

Anyway this patch alter the feature...

BTW if you trust MCFG in ACPI so much, why do you need to bother to
verify that in DSDT...


One reason is that Windows pre-Vista does not use the MCFG table, it 
does use the ACPI reserved resources. Since the board/system 
manufacturers have been testing against Windows, the resource 
reservations should be more likely correct than the MCFG table which was 
not used until Vista came along.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock [EMAIL PROTECTED] wrote:
 Yinghai Lu wrote:
  On 9/22/07, Robert Hancock [EMAIL PROTECTED] wrote:
  Thomas Gleixner wrote:
  On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
  Yinghai Lu wrote:
  No!
 
  MMCONFIG will not work with acpi=off any more.
  I don't think this is unreasonable. The ACPI MCFG table is how we are
  supposed to learn about the area in the first place. If we can't get the
  table location via an approved mechanism, and can't validate it doesn't
  overlap with another memory reservation or something, I really don't
  think we should be using it.
  We all know how correct ACPI tables are. Specifications are nice,
  reality tells a different story.
  MMCONFIG can't be used without ACPI in any case unless we know where the
table is using chipset-specific knowledge (i.e. reading the registers
  directly). Doing that without being told that this area is really
  intended to be used, via the ACPI table, is dangerous, i.e. we don't
  necessarily know if the MMCONFIG is broken on the platform in some way
  we can't detect.
 
  the BIOS get these info from the chipset too.
  for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.
 
  I don't think it's much of an issue anyway - the chances that somebody
  will want to run without ACPI on a system with MCFG are pretty low given
  that you'll end up losing a bunch of functionality (not least of which
  is multi-cores).
  acpi=off is an often used debug switch and it _is_ quite useful. Taking
  away debug functionality is not a good idea.
  If someone has to turn ACPI off, disabling MMCONFIG is probably the
  least of their worries..
 
  MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
  must use ACPI for MMCONFIG?

 config PCI_MMCONFIG
   bool
   depends on PCI  ACPI  (PCI_GOMMCONFIG || PCI_GOANY)
   default y

 We already depend on ACPI for MMCONFIG support at compile time. This
 patch does not change that.

 We could conceivably skip the validation if ACPI was disabled, though
 this would only make a difference in the few cases where we can detect
 the MMCONFIG area without it (looks like currently only Intel E7520 and
 945, at least in mainline).

can you make pci_mmcfg_late_init take one parameter about if acpi is
there or not?

so in acpi_init will be

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 9ba778a..a4a6a6f 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -746,6 +746,7 @@ static int __init acpi_init(void)

if (acpi_disabled) {
printk(KERN_INFO PREFIX Interpreter disabled.\n);
+   pci_mmcfg_late_init(0);
return -ENODEV;
}

@@ -757,6 +758,7 @@ static int __init acpi_init(void)
result = acpi_bus_init();

if (!result) {
+   pci_mmcfg_late_init(1);
 #ifdef CONFIG_PM_LEGACY
if (!PM_IS_ACTIVE())
pm_active = 1;
@@ -767,8 +769,10 @@ static int __init acpi_init(void)
result = -ENODEV;
}
 #endif
-   } else
+   } else {
+   pci_mmcfg_late_init(0);
disable_acpi();
+   }

return result;
 }

YH
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-22 Thread Yinghai Lu 
On 9/22/07, Robert Hancock [EMAIL PROTECTED] wrote:
 Yinghai Lu wrote:
  On 9/22/07, Robert Hancock [EMAIL PROTECTED] wrote:
  Thomas Gleixner wrote:
  On Sat, 2007-09-22 at 10:28 -0600, Robert Hancock wrote:
  Yinghai Lu wrote:
  No!
 
  MMCONFIG will not work with acpi=off any more.
  I don't think this is unreasonable. The ACPI MCFG table is how we are
  supposed to learn about the area in the first place. If we can't get the
  table location via an approved mechanism, and can't validate it doesn't
  overlap with another memory reservation or something, I really don't
  think we should be using it.
  We all know how correct ACPI tables are. Specifications are nice,
  reality tells a different story.
  MMCONFIG can't be used without ACPI in any case unless we know where the
table is using chipset-specific knowledge (i.e. reading the registers
  directly). Doing that without being told that this area is really
  intended to be used, via the ACPI table, is dangerous, i.e. we don't
  necessarily know if the MMCONFIG is broken on the platform in some way
  we can't detect.
 
  the BIOS get these info from the chipset too.
  for AMD Fam 10h opteron, we can read that MSR for MMCONFIG base.
 
  I don't think it's much of an issue anyway - the chances that somebody
  will want to run without ACPI on a system with MCFG are pretty low given
  that you'll end up losing a bunch of functionality (not least of which
  is multi-cores).
  acpi=off is an often used debug switch and it _is_ quite useful. Taking
  away debug functionality is not a good idea.
  If someone has to turn ACPI off, disabling MMCONFIG is probably the
  least of their worries..
 
  MMCONFIG has nothing to do ACPI..., just becase MCFG in the ACPI, we
  must use ACPI for MMCONFIG?

 config PCI_MMCONFIG
   bool
   depends on PCI  ACPI  (PCI_GOMMCONFIG || PCI_GOANY)
   default y

 We already depend on ACPI for MMCONFIG support at compile time. This
 patch does not change that.

 We could conceivably skip the validation if ACPI was disabled, though
 this would only make a difference in the few cases where we can detect
 the MMCONFIG area without it (looks like currently only Intel E7520 and
 945, at least in mainline).

i added support for AMD Fam 10h NB, and that patch is in -mm.

I like to see pci_mmcfg_check_hostbridge is called before any acpi check up.

also in your pci_mmcfg_early_init, you may miss calling to
pci_mmcfg_insert_resource...

YH
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-21 Thread Andi Kleen 

From: Robert Hancock <[EMAIL PROTECTED]>

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.

This also cleans up the MMCONFIG initialization functions so that they
simply do nothing if MMCONFIG is not compiled in.

Based on an original patch by Rajesh Shah from Intel.

[EMAIL PROTECTED]: many fixes and cleanups]
Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>
Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
Cc: Rajesh Shah <[EMAIL PROTECTED]>
Cc: Jesse Barnes <[EMAIL PROTECTED]>
Acked-by: Linus Torvalds <[EMAIL PROTECTED]>
Cc: Andi Kleen <[EMAIL PROTECTED]>
Cc: Greg KH <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---

 arch/i386/pci/init.c|4 -
 arch/i386/pci/mmconfig-shared.c |  151 +++-
 arch/i386/pci/pci.h |1 
 drivers/acpi/bus.c  |2 
 include/linux/pci.h |8 ++
 5 files changed, 144 insertions(+), 22 deletions(-)

Index: linux/arch/i386/pci/init.c
===
--- linux.orig/arch/i386/pci/init.c
+++ linux/arch/i386/pci/init.c
@@ -11,9 +11,7 @@ static __init int pci_access_init(void)
 #ifdef CONFIG_PCI_DIRECT
type = pci_direct_probe();
 #endif
-#ifdef CONFIG_PCI_MMCONFIG
-   pci_mmcfg_init(type);
-#endif
+   pci_mmcfg_early_init(type);
if (raw_pci_ops)
return 0;
 #ifdef CONFIG_PCI_BIOS
Index: linux/arch/i386/pci/mmconfig-shared.c
===
--- linux.orig/arch/i386/pci/mmconfig-shared.c
+++ linux/arch/i386/pci/mmconfig-shared.c
@@ -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 =
+   >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, );
+   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

[PATCH] [9/50] i386: validate against ACPI motherboard resources

2007-09-21 Thread Andi Kleen 

From: Robert Hancock [EMAIL PROTECTED]

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.

This also cleans up the MMCONFIG initialization functions so that they
simply do nothing if MMCONFIG is not compiled in.

Based on an original patch by Rajesh Shah from Intel.

[EMAIL PROTECTED]: many fixes and cleanups]
Signed-off-by: Robert Hancock [EMAIL PROTECTED]
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
Cc: Rajesh Shah [EMAIL PROTECTED]
Cc: Jesse Barnes [EMAIL PROTECTED]
Acked-by: Linus Torvalds [EMAIL PROTECTED]
Cc: Andi Kleen [EMAIL PROTECTED]
Cc: Greg KH [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---

 arch/i386/pci/init.c|4 -
 arch/i386/pci/mmconfig-shared.c |  151 +++-
 arch/i386/pci/pci.h |1 
 drivers/acpi/bus.c  |2 
 include/linux/pci.h |8 ++
 5 files changed, 144 insertions(+), 22 deletions(-)

Index: linux/arch/i386/pci/init.c
===
--- linux.orig/arch/i386/pci/init.c
+++ linux/arch/i386/pci/init.c
@@ -11,9 +11,7 @@ static __init int pci_access_init(void)
 #ifdef CONFIG_PCI_DIRECT
type = pci_direct_probe();
 #endif
-#ifdef CONFIG_PCI_MMCONFIG
-   pci_mmcfg_init(type);
-#endif
+   pci_mmcfg_early_init(type);
if (raw_pci_ops)
return 0;
 #ifdef CONFIG_PCI_BIOS
Index: linux/arch/i386/pci/mmconfig-shared.c
===
--- linux.orig/arch/i386/pci/mmconfig-shared.c
+++ linux/arch/i386/pci/mmconfig-shared.c
@@ -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;
+}
+