Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1276646 > unrolled thread

Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Bjorn Helgaas <helgaas@kernel.org> - 2015-11-24 17:50 +0100
    Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 "Rafael J. Wysocki" <rafael@kernel.org> - 2015-11-24 23:20 +0100
      Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Keith Busch <keith.busch@intel.com> - 2015-11-25 00:10 +0100
        Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Arthur Marsh <arthur.marsh@internode.on.net> - 2015-11-25 01:40 +0100
          Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Jiang Liu <jiang.liu@linux.intel.com> - 2015-11-25 09:30 +0100
            Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Arthur Marsh <arthur.marsh@internode.on.net> - 2015-11-25 11:20 +0100
      Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Jiang Liu <jiang.liu@linux.intel.com> - 2015-11-25 09:20 +0100
        Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245 "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> - 2015-11-25 14:20 +0100
          [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Jiang Liu <jiang.liu@linux.intel.com> - 2015-11-27 04:10 +0100
            Re: [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245 "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-11-27 16:40 +0100
              Re: [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245 Jiang Liu <jiang.liu@linux.intel.com> - 2015-11-30 02:30 +0100
                Re: [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245 "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-11-30 02:50 +0100

#1276646 — Re: [Bugfix] x86/PCI: Fix regression caused by commit 4d6b4e69a245

On Mon, Nov 16, 2015 at 12:27:37PM +0800, Jiang Liu wrote:
> From: Liu Jiang <jiang.liu@linux.intel.com>
> 
> Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
> PCI host bridge") converted x86 to use the common interface
> acpi_pci_root_create, but the conversion missed on code piece in
> arch/x86/pci/bus_numa.c, which causes regression on some legacy
> AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
> The root causes is that acpi_pci_root_create() fails to insert
> host bridge resources into iomem_resource/ioport_resource because
> x86_pci_root_bus_resources() has already inserted those resources.
> So change x86_pci_root_bus_resources() to not insert resources into
> iomem_resource/ioport_resource.

Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")

> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>

What's the status of this?  It looks like a regression we need to fix
for v4.4.

AFAICT, Arthur did *not* test this patch (rather, his response says he
did test it and the test failed).

4d6b4e69a245 was merged by Rafael, and I assume he'll merge the fix
unless I hear otherwise.

Bjorn

> Cc: Keith Busch <keith.busch@intel.com>
> Cc: Arthur Marsh <arthur.marsh@internode.on.net>
> ---
>  arch/x86/pci/bus_numa.c |   16 +++-------------
>  drivers/acpi/pci_root.c |    7 +++++++
>  2 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
> index 7bcf06a7cd12..ce53b5b64f51 100644
> --- a/arch/x86/pci/bus_numa.c
> +++ b/arch/x86/pci/bus_numa.c
> @@ -41,27 +41,17 @@ void x86_pci_root_bus_resources(int bus, struct list_head *resources)
>  	       bus);
>  
>  	/* already added by acpi ? */
> -	resource_list_for_each_entry(window, resources)
> +	resource_list_for_each_entry(window, &info->resources)
>  		if (window->res->flags & IORESOURCE_BUS) {
>  			found = true;
>  			break;
>  		}
> -
>  	if (!found)
>  		pci_add_resource(resources, &info->busn);
>  
> -	list_for_each_entry(root_res, &info->resources, list) {
> -		struct resource *res;
> -		struct resource *root;
> +	list_for_each_entry(root_res, &info->resources, list)
> +		pci_add_resource(resources, &root_res->res);
>  
> -		res = &root_res->res;
> -		pci_add_resource(resources, res);
> -		if (res->flags & IORESOURCE_IO)
> -			root = &ioport_resource;
> -		else
> -			root = &iomem_resource;
> -		insert_resource(root, res);
> -	}
>  	return;
>  
>  default_resources:
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 850d7bf0c873..ae3fe4e64203 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -768,6 +768,13 @@ static void pci_acpi_root_add_resources(struct acpi_pci_root_info *info)
>  		else
>  			continue;
>  
> +		/*
> +		 * Some legacy x86 host bridge drivers use iomem_resource and
> +		 * ioport_resource as default resource pool, skip it.
> +		 */
> +		if (res == root)
> +			continue;
> +
>  		conflict = insert_resource_conflict(root, res);
>  		if (conflict) {
>  			dev_info(&info->bridge->dev,
> -- 
> 1.7.10.4
> 
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1276816

On Tue, Nov 24, 2015 at 5:49 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Nov 16, 2015 at 12:27:37PM +0800, Jiang Liu wrote:
>> From: Liu Jiang <jiang.liu@linux.intel.com>
>>
>> Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
>> PCI host bridge") converted x86 to use the common interface
>> acpi_pci_root_create, but the conversion missed on code piece in
>> arch/x86/pci/bus_numa.c, which causes regression on some legacy
>> AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
>> The root causes is that acpi_pci_root_create() fails to insert
>> host bridge resources into iomem_resource/ioport_resource because
>> x86_pci_root_bus_resources() has already inserted those resources.
>> So change x86_pci_root_bus_resources() to not insert resources into
>> iomem_resource/ioport_resource.
>
> Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")
>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
>
> What's the status of this?  It looks like a regression we need to fix
> for v4.4.
>
> AFAICT, Arthur did *not* test this patch (rather, his response says he
> did test it and the test failed).
>
> 4d6b4e69a245 was merged by Rafael, and I assume he'll merge the fix
> unless I hear otherwise.

Quite frankly, I'm more likely to revert the offending commit at this
point as that's not the only regression reported against it and the
fix only helps in one case (out of three known to me).

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

[toc] | [prev] | [next] | [standalone]

#1276892

On Tue, Nov 24, 2015 at 11:19:34PM +0100, Rafael J. Wysocki wrote:
> Quite frankly, I'm more likely to revert the offending commit at this
> point as that's not the only regression reported against it and the
> fix only helps in one case (out of three known to me).

Using 4.4-rc1 and can confirm the patch fixes my regression report. The
revert also fixes it, so either way is good for me!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276930

Keith Busch wrote on 25/11/15 09:34:
> On Tue, Nov 24, 2015 at 11:19:34PM +0100, Rafael J. Wysocki wrote:
>> Quite frankly, I'm more likely to revert the offending commit at this
>> point as that's not the only regression reported against it and the
>> fix only helps in one case (out of three known to me).
>
> Using 4.4-rc1 and can confirm the patch fixes my regression report. The
> revert also fixes it, so either way is good for me!
>

To re-cap, all was fine for me until:

4d6b4e69a245e9df4b84dba387596086cb66887d is the first bad commit
commit 4d6b4e69a245e9df4b84dba387596086cb66887d
Author: Jiang Liu <jiang.liu@linux.intel.com>
Date:   Wed Oct 14 14:29:41 2015 +0800

      x86/PCI/ACPI: Use common interface to support PCI host bridge

      Use common interface to simplify ACPI PCI host bridge implementation.

      Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
      Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
      Acked-by: Bjorn Helgaas <bhelgaas@google.com>
      Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

:040000 040000 a3447eea376b5a3e6f57deb35cf064c5481b45e3 
f64d8e49fd87b776933dfa3dfefcb33509004d3f M      arch

 From the boot-up I get the message as shown in the images at: 
http://www.users.on.net/~arthur.marsh/20151107601.jpg and
http://www.users.on.net/~arthur.marsh/20151107602.jpg

The boot-up suggests trying rebooting with pci=alloc but that didn't help.

The errors shown include
"BAR 0: trying firmware assignment [io  size 0x0020]"
"BAR 0: [io  size 0x0020] conflicts with PCI Bus #00 [io  0x0000-0xffff]
"BAR 0: failed to assign [io  size 0x0020]

Applying the following patch on top of the patch above from 14 October 
2015 worked for me:


 From 02818ba34bfa76d93f2a29c85660da0323b0b457 Mon Sep 17 00:00:00 2001
From: Liu Jiang <jiang.liu@linux.intel.com>
Date: Mon, 9 Nov 2015 13:36:48 +0800
Subject: [PATCH]


Signed-off-by: Liu Jiang <jiang.liu@linux.intel.com>
---
  arch/x86/pci/bus_numa.c |    3 +++
  1 file changed, 3 insertions(+)

diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
index 7bcf06a7cd12..022d83158cdb 100644
--- a/arch/x86/pci/bus_numa.c
+++ b/arch/x86/pci/bus_numa.c
@@ -51,6 +51,8 @@ void x86_pci_root_bus_resources(int bus, struct 
list_head *resources)
          pci_add_resource(resources, &info->busn);

      list_for_each_entry(root_res, &info->resources, list) {
+        pci_add_resource(resources, &root_res->res);
+#if 0
          struct resource *res;
          struct resource *root;

@@ -61,6 +63,7 @@ void x86_pci_root_bus_resources(int bus, struct 
list_head *resources)
          else
              root = &iomem_resource;
          insert_resource(root, res);
+#endif
      }
      return;

###

The patch postd by Jian Liu on 16 November 2015 "[Bugfix] x86/PCI: Fix 
regression caused by commit 4d6b4e69a245" had *not* been seen or tested 
by me before being posted to the linux-acpi list and when I did test it 
(after removing the patch above from 9 November 2015), things broke:
http://www.users.on.net/~arthur.marsh/20151116611.jpg

So if "commit 4d6b4e69a245e9df4b84dba387596086cb66887d
x86/PCI/ACPI: Use common interface to support PCI host bridge" stays, 
then the patch "16 November 2015 [Bugfix] x86/PCI: Fix regression caused 
by commit 4d6b4e69a245" would need to go and the patch above from 9 
November 2015 would need to be accepted into the mainline for my machine 
to boot from the mainline code.

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

[toc] | [prev] | [next] | [standalone]

#1277094

[Multipart message — attachments visible in raw view] — view raw

On 2015/11/25 8:32, Arthur Marsh wrote:
> Keith Busch wrote on 25/11/15 09:34:
>> On Tue, Nov 24, 2015 at 11:19:34PM +0100, Rafael J. Wysocki wrote:
>>> Quite frankly, I'm more likely to revert the offending commit at this
>>> point as that's not the only regression reported against it and the
>>> fix only helps in one case (out of three known to me).
>>
>> Using 4.4-rc1 and can confirm the patch fixes my regression report. The
>> revert also fixes it, so either way is good for me!
>>
> 
> To re-cap, all was fine for me until:
> 
> 4d6b4e69a245e9df4b84dba387596086cb66887d is the first bad commit
> commit 4d6b4e69a245e9df4b84dba387596086cb66887d
> Author: Jiang Liu <jiang.liu@linux.intel.com>
> Date:   Wed Oct 14 14:29:41 2015 +0800
> 
>      x86/PCI/ACPI: Use common interface to support PCI host bridge
> 
>      Use common interface to simplify ACPI PCI host bridge implementation.
> 
>      Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>      Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
>      Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>      Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> :040000 040000 a3447eea376b5a3e6f57deb35cf064c5481b45e3
> f64d8e49fd87b776933dfa3dfefcb33509004d3f M      arch
> 
> From the boot-up I get the message as shown in the images at:
> http://www.users.on.net/~arthur.marsh/20151107601.jpg and
> http://www.users.on.net/~arthur.marsh/20151107602.jpg
> 
> The boot-up suggests trying rebooting with pci=alloc but that didn't help.
> 
> The errors shown include
> "BAR 0: trying firmware assignment [io  size 0x0020]"
> "BAR 0: [io  size 0x0020] conflicts with PCI Bus #00 [io  0x0000-0xffff]
> "BAR 0: failed to assign [io  size 0x0020]
> 
> Applying the following patch on top of the patch above from 14 October
> 2015 worked for me:
> 
> 
> From 02818ba34bfa76d93f2a29c85660da0323b0b457 Mon Sep 17 00:00:00 2001
> From: Liu Jiang <jiang.liu@linux.intel.com>
> Date: Mon, 9 Nov 2015 13:36:48 +0800
> Subject: [PATCH]
> 
> 
> Signed-off-by: Liu Jiang <jiang.liu@linux.intel.com>
> ---
>  arch/x86/pci/bus_numa.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
> index 7bcf06a7cd12..022d83158cdb 100644
> --- a/arch/x86/pci/bus_numa.c
> +++ b/arch/x86/pci/bus_numa.c
> @@ -51,6 +51,8 @@ void x86_pci_root_bus_resources(int bus, struct
> list_head *resources)
>          pci_add_resource(resources, &info->busn);
> 
>      list_for_each_entry(root_res, &info->resources, list) {
> +        pci_add_resource(resources, &root_res->res);
> +#if 0
>          struct resource *res;
>          struct resource *root;
> 
> @@ -61,6 +63,7 @@ void x86_pci_root_bus_resources(int bus, struct
> list_head *resources)
>          else
>              root = &iomem_resource;
>          insert_resource(root, res);
> +#endif
>      }
>      return;
> 
> ###
> 
> The patch postd by Jian Liu on 16 November 2015 "[Bugfix] x86/PCI: Fix
> regression caused by commit 4d6b4e69a245" had *not* been seen or tested
> by me before being posted to the linux-acpi list and when I did test it
> (after removing the patch above from 9 November 2015), things broke:
> http://www.users.on.net/~arthur.marsh/20151116611.jpg
> 
> So if "commit 4d6b4e69a245e9df4b84dba387596086cb66887d
> x86/PCI/ACPI: Use common interface to support PCI host bridge" stays,
> then the patch "16 November 2015 [Bugfix] x86/PCI: Fix regression caused
> by commit 4d6b4e69a245" would need to go and the patch above from 9
> November 2015 would need to be accepted into the mainline for my machine
> to boot from the mainline code.
Hi Arthur,
	Thanks for reminder again!
It's a little strange, the formal patch "[Bugfix] x86/PCI: Fix
regression caused by commit 4d6b4e69a245" is based on the debug patch
I sent to you at 9 November 2015.
	Could you please help to try the attached patch again?
Thanks,
Gerry

> 
> Arthur.

[toc] | [prev] | [next] | [standalone]

#1277155

Jiang Liu wrote on 25/11/15 18:57:

> Hi Arthur,
> 	Thanks for reminder again!
> It's a little strange, the formal patch "[Bugfix] x86/PCI: Fix
> regression caused by commit 4d6b4e69a245" is based on the debug patch
> I sent to you at 9 November 2015.
> 	Could you please help to try the attached patch again?
> Thanks,
> Gerry

OK, I'm booting alright now with git diff reporting the following (just 
the patch that Jiang Liu posted in the parent):

diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
index 7bcf06a..6eb3c8a 100644
--- a/arch/x86/pci/bus_numa.c
+++ b/arch/x86/pci/bus_numa.c
@@ -50,18 +50,9 @@ void x86_pci_root_bus_resources(int bus, struct 
list_head *resources)
  	if (!found)
  		pci_add_resource(resources, &info->busn);

-	list_for_each_entry(root_res, &info->resources, list) {
-		struct resource *res;
-		struct resource *root;
+	list_for_each_entry(root_res, &info->resources, list)
+		pci_add_resource(resources, &root_res->res);

-		res = &root_res->res;
-		pci_add_resource(resources, res);
-		if (res->flags & IORESOURCE_IO)
-			root = &ioport_resource;
-		else
-			root = &iomem_resource;
-		insert_resource(root, res);
-	}
  	return;

  default_resources:
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 850d7bf..ae3fe4e 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -768,6 +768,13 @@ static void pci_acpi_root_add_resources(struct 
acpi_pci_root_info *info)
  		else
  			continue;

+		/*
+		 * Some legacy x86 host bridge drivers use iomem_resource and
+		 * ioport_resource as default resource pool, skip it.
+		 */
+		if (res == root)
+			continue;
+
  		conflict = insert_resource_conflict(root, res);
  		if (conflict) {
  			dev_info(&info->bridge->dev,
####


There have been at least two gcc-5 updates on this machine since 
originally reporting the problem so that may have affected results.

Thanks to all for help with this.

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

[toc] | [prev] | [next] | [standalone]

#1277089

On 2015/11/25 6:19, Rafael J. Wysocki wrote:
> On Tue, Nov 24, 2015 at 5:49 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> On Mon, Nov 16, 2015 at 12:27:37PM +0800, Jiang Liu wrote:
>>> From: Liu Jiang <jiang.liu@linux.intel.com>
>>>
>>> Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
>>> PCI host bridge") converted x86 to use the common interface
>>> acpi_pci_root_create, but the conversion missed on code piece in
>>> arch/x86/pci/bus_numa.c, which causes regression on some legacy
>>> AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
>>> The root causes is that acpi_pci_root_create() fails to insert
>>> host bridge resources into iomem_resource/ioport_resource because
>>> x86_pci_root_bus_resources() has already inserted those resources.
>>> So change x86_pci_root_bus_resources() to not insert resources into
>>> iomem_resource/ioport_resource.
>>
>> Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")
>>
>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>>> Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
>>
>> What's the status of this?  It looks like a regression we need to fix
>> for v4.4.
>>
>> AFAICT, Arthur did *not* test this patch (rather, his response says he
>> did test it and the test failed).
>>
>> 4d6b4e69a245 was merged by Rafael, and I assume he'll merge the fix
>> unless I hear otherwise.
> 
> Quite frankly, I'm more likely to revert the offending commit at this
> point as that's not the only regression reported against it and the
> fix only helps in one case (out of three known to me).
Hi Rafael,
	I got regression report from Hans de Bruin<jmdebruin@xmsnet.nl>,
Keith Busch <keith.busch@intel.com>, and Arthur Marsh
<arthur.marsh@internode.on.net>. Hans and Keith also reports
the patch fixes the regression. For Arthur's case, the debug
patch works for him, but the formal patch based on the debug
patch fails, so I need to do more investigation about this.
Is there any other report related to commit 4d6b4e69a245 so
I could help to investigate?
Thanks,
Gerry

> 
> Thanks,
> Rafael
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1277385

On 11/25/2015 9:12 AM, Jiang Liu wrote:
> On 2015/11/25 6:19, Rafael J. Wysocki wrote:
>> On Tue, Nov 24, 2015 at 5:49 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>>> On Mon, Nov 16, 2015 at 12:27:37PM +0800, Jiang Liu wrote:
>>>> From: Liu Jiang <jiang.liu@linux.intel.com>
>>>>
>>>> Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
>>>> PCI host bridge") converted x86 to use the common interface
>>>> acpi_pci_root_create, but the conversion missed on code piece in
>>>> arch/x86/pci/bus_numa.c, which causes regression on some legacy
>>>> AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
>>>> The root causes is that acpi_pci_root_create() fails to insert
>>>> host bridge resources into iomem_resource/ioport_resource because
>>>> x86_pci_root_bus_resources() has already inserted those resources.
>>>> So change x86_pci_root_bus_resources() to not insert resources into
>>>> iomem_resource/ioport_resource.
>>> Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")
>>>
>>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>>>> Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
>>> What's the status of this?  It looks like a regression we need to fix
>>> for v4.4.
>>>
>>> AFAICT, Arthur did *not* test this patch (rather, his response says he
>>> did test it and the test failed).
>>>
>>> 4d6b4e69a245 was merged by Rafael, and I assume he'll merge the fix
>>> unless I hear otherwise.
>> Quite frankly, I'm more likely to revert the offending commit at this
>> point as that's not the only regression reported against it and the
>> fix only helps in one case (out of three known to me).
> Hi Rafael,
> 	I got regression report from Hans de Bruin<jmdebruin@xmsnet.nl>,
> Keith Busch <keith.busch@intel.com>, and Arthur Marsh
> <arthur.marsh@internode.on.net>. Hans and Keith also reports
> the patch fixes the regression. For Arthur's case, the debug
> patch works for him, but the formal patch based on the debug
> patch fails, so I need to do more investigation about this.
> Is there any other report related to commit 4d6b4e69a245 so
> I could help to investigate?

OK, so the proposed bug fix works for everybody, right?

In that case there's no reason not to apply it.

If there's anyone who can reproduce the problem and for whom that patch 
doesn't work, please let me know.

Thanks,
Rafael

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

[toc] | [prev] | [next] | [standalone]

#1278511 — [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245

From: Liu Jiang <jiang.liu@linux.intel.com>

Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
PCI host bridge") converted x86 to use the common interface
acpi_pci_root_create, but the conversion missed on code piece in
arch/x86/pci/bus_numa.c, which causes regression on some legacy
AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
The root causes is that acpi_pci_root_create() fails to insert
host bridge resources into iomem_resource/ioport_resource because
x86_pci_root_bus_resources() has already inserted those resources.
So change x86_pci_root_bus_resources() to not insert resources into
iomem_resource/ioport_resource.

Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Arthur Marsh <arthur.marsh@internode.on.net>
Cc: Hans de Bruin <jmdebruin@xmsnet.nl>
---
 arch/x86/pci/bus_numa.c |   13 ++-----------
 drivers/acpi/pci_root.c |    7 +++++++
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
index 7bcf06a7cd12..6eb3c8af96e2 100644
--- a/arch/x86/pci/bus_numa.c
+++ b/arch/x86/pci/bus_numa.c
@@ -50,18 +50,9 @@ void x86_pci_root_bus_resources(int bus, struct list_head *resources)
 	if (!found)
 		pci_add_resource(resources, &info->busn);
 
-	list_for_each_entry(root_res, &info->resources, list) {
-		struct resource *res;
-		struct resource *root;
+	list_for_each_entry(root_res, &info->resources, list)
+		pci_add_resource(resources, &root_res->res);
 
-		res = &root_res->res;
-		pci_add_resource(resources, res);
-		if (res->flags & IORESOURCE_IO)
-			root = &ioport_resource;
-		else
-			root = &iomem_resource;
-		insert_resource(root, res);
-	}
 	return;
 
 default_resources:
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 850d7bf0c873..ae3fe4e64203 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -768,6 +768,13 @@ static void pci_acpi_root_add_resources(struct acpi_pci_root_info *info)
 		else
 			continue;
 
+		/*
+		 * Some legacy x86 host bridge drivers use iomem_resource and
+		 * ioport_resource as default resource pool, skip it.
+		 */
+		if (res == root)
+			continue;
+
 		conflict = insert_resource_conflict(root, res);
 		if (conflict) {
 			dev_info(&info->bridge->dev,
-- 
1.7.10.4

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

[toc] | [prev] | [next] | [standalone]

#1278850 — Re: [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245

On Friday, November 27, 2015 11:12:33 AM Jiang Liu wrote:
> From: Liu Jiang <jiang.liu@linux.intel.com>
> 
> Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
> PCI host bridge") converted x86 to use the common interface
> acpi_pci_root_create, but the conversion missed on code piece in
> arch/x86/pci/bus_numa.c, which causes regression on some legacy
> AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
> The root causes is that acpi_pci_root_create() fails to insert
> host bridge resources into iomem_resource/ioport_resource because
> x86_pci_root_bus_resources() has already inserted those resources.
> So change x86_pci_root_bus_resources() to not insert resources into
> iomem_resource/ioport_resource.
> 
> Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
> Cc: Keith Busch <keith.busch@intel.com>
> Cc: Arthur Marsh <arthur.marsh@internode.on.net>
> Cc: Hans de Bruin <jmdebruin@xmsnet.nl>

What exactly has changed between this version and the previous one?


> ---
>  arch/x86/pci/bus_numa.c |   13 ++-----------
>  drivers/acpi/pci_root.c |    7 +++++++
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
> index 7bcf06a7cd12..6eb3c8af96e2 100644
> --- a/arch/x86/pci/bus_numa.c
> +++ b/arch/x86/pci/bus_numa.c
> @@ -50,18 +50,9 @@ void x86_pci_root_bus_resources(int bus, struct list_head *resources)
>  	if (!found)
>  		pci_add_resource(resources, &info->busn);
>  
> -	list_for_each_entry(root_res, &info->resources, list) {
> -		struct resource *res;
> -		struct resource *root;
> +	list_for_each_entry(root_res, &info->resources, list)
> +		pci_add_resource(resources, &root_res->res);
>  
> -		res = &root_res->res;
> -		pci_add_resource(resources, res);
> -		if (res->flags & IORESOURCE_IO)
> -			root = &ioport_resource;
> -		else
> -			root = &iomem_resource;
> -		insert_resource(root, res);
> -	}
>  	return;
>  
>  default_resources:
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 850d7bf0c873..ae3fe4e64203 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -768,6 +768,13 @@ static void pci_acpi_root_add_resources(struct acpi_pci_root_info *info)
>  		else
>  			continue;
>  
> +		/*
> +		 * Some legacy x86 host bridge drivers use iomem_resource and
> +		 * ioport_resource as default resource pool, skip it.
> +		 */
> +		if (res == root)
> +			continue;
> +
>  		conflict = insert_resource_conflict(root, res);
>  		if (conflict) {
>  			dev_info(&info->bridge->dev,
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279486 — Re: [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245

On 2015/11/28 0:06, Rafael J. Wysocki wrote:
> On Friday, November 27, 2015 11:12:33 AM Jiang Liu wrote:
>> From: Liu Jiang <jiang.liu@linux.intel.com>
>>
>> Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
>> PCI host bridge") converted x86 to use the common interface
>> acpi_pci_root_create, but the conversion missed on code piece in
>> arch/x86/pci/bus_numa.c, which causes regression on some legacy
>> AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
>> The root causes is that acpi_pci_root_create() fails to insert
>> host bridge resources into iomem_resource/ioport_resource because
>> x86_pci_root_bus_resources() has already inserted those resources.
>> So change x86_pci_root_bus_resources() to not insert resources into
>> iomem_resource/ioport_resource.
>>
>> Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
>> Cc: Keith Busch <keith.busch@intel.com>
>> Cc: Arthur Marsh <arthur.marsh@internode.on.net>
>> Cc: Hans de Bruin <jmdebruin@xmsnet.nl>
> 
> What exactly has changed between this version and the previous one?
Hi Rafael,
	I have removed following changes against the original patch
posted at Nov 16.
 	       bus);

 	/* already added by acpi ? */
-	resource_list_for_each_entry(window, resources)
+	resource_list_for_each_entry(window, &info->resources)
 		if (window->res->flags & IORESOURCE_BUS) {
 			found = true;
 			break;
 		}
-
 	if (!found)
 		pci_add_resource(resources, &info->busn);

And I only refined the commit message based on the test patch
I sent to Authur as an attachment at Nov 25.
Thanks,
Gerry
> 
> 
>> ---
>>  arch/x86/pci/bus_numa.c |   13 ++-----------
>>  drivers/acpi/pci_root.c |    7 +++++++
>>  2 files changed, 9 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
>> index 7bcf06a7cd12..6eb3c8af96e2 100644
>> --- a/arch/x86/pci/bus_numa.c
>> +++ b/arch/x86/pci/bus_numa.c
>> @@ -50,18 +50,9 @@ void x86_pci_root_bus_resources(int bus, struct list_head *resources)
>>  	if (!found)
>>  		pci_add_resource(resources, &info->busn);
>>  
>> -	list_for_each_entry(root_res, &info->resources, list) {
>> -		struct resource *res;
>> -		struct resource *root;
>> +	list_for_each_entry(root_res, &info->resources, list)
>> +		pci_add_resource(resources, &root_res->res);
>>  
>> -		res = &root_res->res;
>> -		pci_add_resource(resources, res);
>> -		if (res->flags & IORESOURCE_IO)
>> -			root = &ioport_resource;
>> -		else
>> -			root = &iomem_resource;
>> -		insert_resource(root, res);
>> -	}
>>  	return;
>>  
>>  default_resources:
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 850d7bf0c873..ae3fe4e64203 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -768,6 +768,13 @@ static void pci_acpi_root_add_resources(struct acpi_pci_root_info *info)
>>  		else
>>  			continue;
>>  
>> +		/*
>> +		 * Some legacy x86 host bridge drivers use iomem_resource and
>> +		 * ioport_resource as default resource pool, skip it.
>> +		 */
>> +		if (res == root)
>> +			continue;
>> +
>>  		conflict = insert_resource_conflict(root, res);
>>  		if (conflict) {
>>  			dev_info(&info->bridge->dev,
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279493 — Re: [PATCH] x86/PCI: Fix regression caused by commit 4d6b4e69a245

On Monday, November 30, 2015 09:20:06 AM Jiang Liu wrote:
> On 2015/11/28 0:06, Rafael J. Wysocki wrote:
> > On Friday, November 27, 2015 11:12:33 AM Jiang Liu wrote:
> >> From: Liu Jiang <jiang.liu@linux.intel.com>
> >>
> >> Commit 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support
> >> PCI host bridge") converted x86 to use the common interface
> >> acpi_pci_root_create, but the conversion missed on code piece in
> >> arch/x86/pci/bus_numa.c, which causes regression on some legacy
> >> AMD platforms as reported by Arthur Marsh <arthur.marsh@internode.on.net>.
> >> The root causes is that acpi_pci_root_create() fails to insert
> >> host bridge resources into iomem_resource/ioport_resource because
> >> x86_pci_root_bus_resources() has already inserted those resources.
> >> So change x86_pci_root_bus_resources() to not insert resources into
> >> iomem_resource/ioport_resource.
> >>
> >> Fixes: 4d6b4e69a245 ("x86/PCI/ACPI: Use common interface to support PCI host bridge")
> >> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> >> Reported-and-tested-by: Arthur Marsh <arthur.marsh@internode.on.net>
> >> Cc: Keith Busch <keith.busch@intel.com>
> >> Cc: Arthur Marsh <arthur.marsh@internode.on.net>
> >> Cc: Hans de Bruin <jmdebruin@xmsnet.nl>
> > 
> > What exactly has changed between this version and the previous one?
> Hi Rafael,
> 	I have removed following changes against the original patch
> posted at Nov 16.
>  	       bus);
> 
>  	/* already added by acpi ? */
> -	resource_list_for_each_entry(window, resources)
> +	resource_list_for_each_entry(window, &info->resources)
>  		if (window->res->flags & IORESOURCE_BUS) {
>  			found = true;
>  			break;
>  		}
> -
>  	if (!found)
>  		pci_add_resource(resources, &info->busn);
> 
> And I only refined the commit message based on the test patch
> I sent to Authur as an attachment at Nov 25.

OK, thanks.

Keith, Hans, can you test this version too please?

Bjorn, any more comments from you on this one?

> >> ---
> >>  arch/x86/pci/bus_numa.c |   13 ++-----------
> >>  drivers/acpi/pci_root.c |    7 +++++++
> >>  2 files changed, 9 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
> >> index 7bcf06a7cd12..6eb3c8af96e2 100644
> >> --- a/arch/x86/pci/bus_numa.c
> >> +++ b/arch/x86/pci/bus_numa.c
> >> @@ -50,18 +50,9 @@ void x86_pci_root_bus_resources(int bus, struct list_head *resources)
> >>  	if (!found)
> >>  		pci_add_resource(resources, &info->busn);
> >>  
> >> -	list_for_each_entry(root_res, &info->resources, list) {
> >> -		struct resource *res;
> >> -		struct resource *root;
> >> +	list_for_each_entry(root_res, &info->resources, list)
> >> +		pci_add_resource(resources, &root_res->res);
> >>  
> >> -		res = &root_res->res;
> >> -		pci_add_resource(resources, res);
> >> -		if (res->flags & IORESOURCE_IO)
> >> -			root = &ioport_resource;
> >> -		else
> >> -			root = &iomem_resource;
> >> -		insert_resource(root, res);
> >> -	}
> >>  	return;
> >>  
> >>  default_resources:
> >> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> >> index 850d7bf0c873..ae3fe4e64203 100644
> >> --- a/drivers/acpi/pci_root.c
> >> +++ b/drivers/acpi/pci_root.c
> >> @@ -768,6 +768,13 @@ static void pci_acpi_root_add_resources(struct acpi_pci_root_info *info)
> >>  		else
> >>  			continue;
> >>  
> >> +		/*
> >> +		 * Some legacy x86 host bridge drivers use iomem_resource and
> >> +		 * ioport_resource as default resource pool, skip it.
> >> +		 */
> >> +		if (res == root)
> >> +			continue;
> >> +
> >>  		conflict = insert_resource_conflict(root, res);
> >>  		if (conflict) {
> >>  			dev_info(&info->bridge->dev,
> >>
> > 

Thanks,
Rafael

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web