Patchwork SNB PCI root information

login
register
mail settings
Submitter Ulrich Drepper
Date June 20, 2012, 8:04 p.m.
Message ID <CAOPLpQdi-8NZL1rwt3APU_is4GcyGaAN0Vq0T23BMSHob--YYg@mail.gmail.com>
Download mbox | patch
Permalink /patch/166138/
State Superseded
Headers show

Comments

Ulrich Drepper - June 20, 2012, 8:04 p.m.
On Wed, Jun 20, 2012 at 3:34 PM, Ingo Molnar <mingo@kernel.org> wrote:
> I mean, if we create a parameter space that tweaks data then why
> not make it complete and allow *all* firmware data to be
> (optionally) modified, from the kernel boot line?

If there is proof that BIOSes get it wrong then just use this small
additional patch:


Signed-off-by: Ulrich Drepper <drepper@gmail.com>

 		node = -1;
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas - June 20, 2012, 8:16 p.m.
On Wed, Jun 20, 2012 at 2:04 PM, Ulrich Drepper <drepper@gmail.com> wrote:
> On Wed, Jun 20, 2012 at 3:34 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> I mean, if we create a parameter space that tweaks data then why
>> not make it complete and allow *all* firmware data to be
>> (optionally) modified, from the kernel boot line?
>
> If there is proof that BIOSes get it wrong then just use this small
> additional patch:

It's not a question of "do BIOSes get this wrong?"  The question is
"what does a user expect to happen when she supplies
'pci=busnum_node=00:00,80:01'?"  I contend that the user expects us to
use that info whether the BIOS gave us correct info, wrong info, or
nothing at all.

Normally I'd be glad to munge this all together myself, but I'm
feeling a bit swamped, so if anybody wants this, it would help me out
to get a single complete patch.

> Signed-off-by: Ulrich Drepper <drepper@gmail.com>
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index fc09c27..7aceb84 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -387,16 +387,16 @@ struct pci_bus * __devinit
> pci_acpi_scan_root(struct acpi_pci_root *root)
>                return NULL;
>        }
>
> -       node = -1;
> +       node = get_mp_bus_to_node(busnum);
>  #ifdef CONFIG_ACPI_NUMA
> -       pxm = acpi_get_pxm(device->handle);
> -       if (pxm >= 0)
> -               node = pxm_to_node(pxm);
> -       if (node != -1)
> -               set_mp_bus_to_node(busnum, node);
> -       else
> +       if (node == -1) {
> +               pxm = acpi_get_pxm(device->handle);
> +               if (pxm >= 0)
> +                       node = pxm_to_node(pxm);
> +               if (node != -1)
> +                       set_mp_bus_to_node(busnum, node);
> +       }
>  #endif
> -               node = get_mp_bus_to_node(busnum);
>
>        if (node != -1 && !node_online(node))
>                node = -1;
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulrich Drepper - June 20, 2012, 9:21 p.m.
On Wed, Jun 20, 2012 at 4:16 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> It's not a question of "do BIOSes get this wrong?"  The question is
> "what does a user expect to happen when she supplies
> 'pci=busnum_node=00:00,80:01'?"  I contend that the user expects us to
> use that info whether the BIOS gave us correct info, wrong info, or
> nothing at all.

Think about busnum_node as busnum_default_node.  Then the behavior
makes perfect sense.

I contend that the really the only reason for overwriting is if there
are wrong BIOS.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yinghai Lu - June 20, 2012, 11:58 p.m.
On Wed, Jun 20, 2012 at 1:04 PM, Ulrich Drepper <drepper@gmail.com> wrote:
> On Wed, Jun 20, 2012 at 3:34 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> I mean, if we create a parameter space that tweaks data then why
>> not make it complete and allow *all* firmware data to be
>> (optionally) modified, from the kernel boot line?
>
> If there is proof that BIOSes get it wrong then just use this small
> additional patch:
>
>
> Signed-off-by: Ulrich Drepper <drepper@gmail.com>
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index fc09c27..7aceb84 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -387,16 +387,16 @@ struct pci_bus * __devinit
> pci_acpi_scan_root(struct acpi_pci_root *root)
>                return NULL;
>        }
>
> -       node = -1;
> +       node = get_mp_bus_to_node(busnum);
>  #ifdef CONFIG_ACPI_NUMA
> -       pxm = acpi_get_pxm(device->handle);
> -       if (pxm >= 0)
> -               node = pxm_to_node(pxm);
> -       if (node != -1)
> -               set_mp_bus_to_node(busnum, node);
> -       else
> +       if (node == -1) {
> +               pxm = acpi_get_pxm(device->handle);
> +               if (pxm >= 0)
> +                       node = pxm_to_node(pxm);
> +               if (node != -1)
> +                       set_mp_bus_to_node(busnum, node);
> +       }
>  #endif
> -               node = get_mp_bus_to_node(busnum);
>
>        if (node != -1 && !node_online(node))
>                node = -1;

it will have problem for amd platform that have _PXM and also have
hostbridge detection.

before the patch, _PXM will always be used at first.

after this patch, the one from host bridge will always get used at first.

need to check AMD cpu with two silicon chip in one package.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index fc09c27..7aceb84 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -387,16 +387,16 @@  struct pci_bus * __devinit
pci_acpi_scan_root(struct acpi_pci_root *root)
 		return NULL;
 	}

-	node = -1;
+	node = get_mp_bus_to_node(busnum);
 #ifdef CONFIG_ACPI_NUMA
-	pxm = acpi_get_pxm(device->handle);
-	if (pxm >= 0)
-		node = pxm_to_node(pxm);
-	if (node != -1)
-		set_mp_bus_to_node(busnum, node);
-	else
+	if (node == -1) {
+		pxm = acpi_get_pxm(device->handle);
+		if (pxm >= 0)
+			node = pxm_to_node(pxm);
+		if (node != -1)
+			set_mp_bus_to_node(busnum, node);
+	}
 #endif
-		node = get_mp_bus_to_node(busnum);

 	if (node != -1 && !node_online(node))