All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: decode disable fixes
@ 2012-07-09 18:20 Bjorn Helgaas
  2012-07-09 18:20 ` [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too Bjorn Helgaas
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2012-07-09 18:20 UTC (permalink / raw)
  To: linux-pci
  Cc: Jacob Pan, Greg Kroah-Hartman, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Robert Hancock

Since 2.6.36 (253d2e5498), we've disabled MEM & IO decoding while we size
BARs (except for host bridge devices).  These patches tweak this in two
ways:

1) We only kept decoding disabled while sizing the low-order dword
of 64-bit BARs.  I think we should keep it disabled while we do the
high-order dword also.

2) I think we should also disable decoding while updating 64-bit BARs,
e.g., when we move a device into an aperture of an upstream bridge.  I'm
only proposing to disable decoding for 64-bit BAR updates because 32-bit
ones can be done atomically, while 64-bit updates require two writes and
the half-written state is a potential problem.

I'm considering these changes for the 3.6 merge window, which is
approaching fast, so let me know if you see issues with either of these.

---

Bjorn Helgaas (2):
      PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
      PCI: disable MEM decoding while updating 64-bit MEM BARs


 drivers/pci/probe.c     |    6 +++---
 drivers/pci/setup-res.c |   18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

-- 
Bjorn

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
  2012-07-09 18:20 [PATCH 0/2] PCI: decode disable fixes Bjorn Helgaas
@ 2012-07-09 18:20 ` Bjorn Helgaas
  2012-07-09 20:18   ` Jacob Pan
  2012-08-23  7:28     ` Olof Johansson
  2012-07-09 18:20 ` [PATCH 2/2] PCI: disable MEM decoding while updating 64-bit MEM BARs Bjorn Helgaas
  2012-07-10  3:23 ` [PATCH 0/2] PCI: decode disable fixes Bjorn Helgaas
  2 siblings, 2 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2012-07-09 18:20 UTC (permalink / raw)
  To: linux-pci
  Cc: Jacob Pan, Greg Kroah-Hartman, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Robert Hancock

After 253d2e5498, we disable MEM and IO decoding for most devices while we
size 32-bit BARs.  However, we restore the original COMMAND register before
we size the upper 32 bits of 64-bit BARs, so we can still cause a conflict.

This patch waits to restore the original COMMAND register until we're
completely finished sizing the BAR.

Reference: https://lkml.org/lkml/2007/8/25/154
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/probe.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 658ac97..66b3a6f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -152,9 +152,6 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 	pci_read_config_dword(dev, pos, &sz);
 	pci_write_config_dword(dev, pos, l);
 
-	if (!dev->mmio_always_on)
-		pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
-
 	/*
 	 * All bits set in sz means the device isn't working properly.
 	 * If the BAR isn't implemented, all bits must be 0.  If it's a
@@ -239,6 +236,9 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 	}
 
  out:
+	if (!dev->mmio_always_on)
+		pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
+
 	return (res->flags & IORESOURCE_MEM_64) ? 1 : 0;
  fail:
 	res->flags = 0;


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] PCI: disable MEM decoding while updating 64-bit MEM BARs
  2012-07-09 18:20 [PATCH 0/2] PCI: decode disable fixes Bjorn Helgaas
  2012-07-09 18:20 ` [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too Bjorn Helgaas
@ 2012-07-09 18:20 ` Bjorn Helgaas
  2012-07-10  3:23 ` [PATCH 0/2] PCI: decode disable fixes Bjorn Helgaas
  2 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2012-07-09 18:20 UTC (permalink / raw)
  To: linux-pci
  Cc: Jacob Pan, Greg Kroah-Hartman, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Robert Hancock

When we update 64-bit BARs, we have to perform two config writes.  Between
the writes, the half-written BAR value could match a MEM access intended
for another device.  This could result in corruption of this device (for
writes) or an unexpected response machine check (for reads).

To prevent this, disable MEM decoding while updating such BARs.  This uses
the same safety test as 253d2e5498, which disables both MEM and IO while
sizing BARs, namely, we don't disable decoding for host bridge devices.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/setup-res.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index eea85da..1a0e60e 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -30,6 +30,8 @@
 void pci_update_resource(struct pci_dev *dev, int resno)
 {
 	struct pci_bus_region region;
+	bool disable;
+	u16 cmd;
 	u32 new, check, mask;
 	int reg;
 	enum pci_bar_type type;
@@ -67,6 +69,18 @@ void pci_update_resource(struct pci_dev *dev, int resno)
 		new |= PCI_ROM_ADDRESS_ENABLE;
 	}
 
+	/*
+	 * We can't update a 64-bit BAR atomically, so when possible,
+	 * disable decoding so that a half-updated BAR won't conflict
+	 * with another device.
+	 */
+	disable = (res->flags & IORESOURCE_MEM_64) && !dev->mmio_always_on;
+	if (disable) {
+		pci_read_config_word(dev, PCI_COMMAND, &cmd);
+		pci_write_config_word(dev, PCI_COMMAND,
+				      cmd & ~PCI_COMMAND_MEMORY);
+	}
+
 	pci_write_config_dword(dev, reg, new);
 	pci_read_config_dword(dev, reg, &check);
 
@@ -84,6 +98,10 @@ void pci_update_resource(struct pci_dev *dev, int resno)
 			       "(high %#08x != %#08x)\n", resno, new, check);
 		}
 	}
+
+	if (disable)
+		pci_write_config_word(dev, PCI_COMMAND, cmd);
+
 	res->flags &= ~IORESOURCE_UNSET;
 	dev_dbg(&dev->dev, "BAR %d: set to %pR (PCI address [%#llx-%#llx])\n",
 		resno, res, (unsigned long long)region.start,


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
  2012-07-09 18:20 ` [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too Bjorn Helgaas
@ 2012-07-09 20:18   ` Jacob Pan
  2012-08-23  7:28     ` Olof Johansson
  1 sibling, 0 replies; 11+ messages in thread
From: Jacob Pan @ 2012-07-09 20:18 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Greg Kroah-Hartman, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Robert Hancock

On Mon, 09 Jul 2012 12:20:18 -0600
Bjorn Helgaas <bhelgaas@google.com> wrote:

> After 253d2e5498, we disable MEM and IO decoding for most devices
> while we size 32-bit BARs.  However, we restore the original COMMAND
> register before we size the upper 32 bits of 64-bit BARs, so we can
> still cause a conflict.
> 
> This patch waits to restore the original COMMAND register until we're
> completely finished sizing the BAR.
agreed.

Thanks,

Jacob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] PCI: decode disable fixes
  2012-07-09 18:20 [PATCH 0/2] PCI: decode disable fixes Bjorn Helgaas
  2012-07-09 18:20 ` [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too Bjorn Helgaas
  2012-07-09 18:20 ` [PATCH 2/2] PCI: disable MEM decoding while updating 64-bit MEM BARs Bjorn Helgaas
@ 2012-07-10  3:23 ` Bjorn Helgaas
  2 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2012-07-10  3:23 UTC (permalink / raw)
  To: linux-pci
  Cc: Jacob Pan, Greg Kroah-Hartman, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Robert Hancock

On Mon, Jul 9, 2012 at 12:20 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Since 2.6.36 (253d2e5498), we've disabled MEM & IO decoding while we size
> BARs (except for host bridge devices).  These patches tweak this in two
> ways:
>
> 1) We only kept decoding disabled while sizing the low-order dword
> of 64-bit BARs.  I think we should keep it disabled while we do the
> high-order dword also.
>
> 2) I think we should also disable decoding while updating 64-bit BARs,
> e.g., when we move a device into an aperture of an upstream bridge.  I'm
> only proposing to disable decoding for 64-bit BAR updates because 32-bit
> ones can be done atomically, while 64-bit updates require two writes and
> the half-written state is a potential problem.
>
> I'm considering these changes for the 3.6 merge window, which is
> approaching fast, so let me know if you see issues with either of these.
>
> ---
>
> Bjorn Helgaas (2):
>       PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
>       PCI: disable MEM decoding while updating 64-bit MEM BARs
>
>
>  drivers/pci/probe.c     |    6 +++---
>  drivers/pci/setup-res.c |   18 ++++++++++++++++++
>  2 files changed, 21 insertions(+), 3 deletions(-)

I applied these to my "next" branch.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
  2012-07-09 18:20 ` [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too Bjorn Helgaas
@ 2012-08-23  7:28     ` Olof Johansson
  2012-08-23  7:28     ` Olof Johansson
  1 sibling, 0 replies; 11+ messages in thread
From: Olof Johansson @ 2012-08-23  7:28 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Jacob Pan, Greg Kroah-Hartman, linux-kernel,
	Jesse Barnes, Ivan Kokshaysky, Matthew Wilcox, Robert Hancock,
	linuxppc-dev

Hi,

On Mon, Jul 9, 2012 at 11:20 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> After 253d2e5498, we disable MEM and IO decoding for most devices while we
> size 32-bit BARs.  However, we restore the original COMMAND register before
> we size the upper 32 bits of 64-bit BARs, so we can still cause a conflict.
>
> This patch waits to restore the original COMMAND register until we're
> completely finished sizing the BAR.
>
> Reference: https://lkml.org/lkml/2007/8/25/154
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

This patch causes boot lockup on PA Semi hardware, since it disables
the bar on the UART that is used for console, and it has printks
between the old and the new re-enable location. If I boot with 'debug'
level for printk, I hit this. If I boot with just regular console
args, I don't.

I'm guessing any other platform that uses MMIO-based UART on PCI for
console will have similar issues. I can verify on Chrome OS x86
hardware tomorrow if legacy powerpc isn't important enough to care
about. :-)

I have no proposal for a fix for this. Can you please consider
reverting for 3.6 unless someone has a better idea?


-Olof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
@ 2012-08-23  7:28     ` Olof Johansson
  0 siblings, 0 replies; 11+ messages in thread
From: Olof Johansson @ 2012-08-23  7:28 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Jacob Pan, linux-pci, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Greg Kroah-Hartman,
	linuxppc-dev, Robert Hancock

Hi,

On Mon, Jul 9, 2012 at 11:20 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> After 253d2e5498, we disable MEM and IO decoding for most devices while we
> size 32-bit BARs.  However, we restore the original COMMAND register before
> we size the upper 32 bits of 64-bit BARs, so we can still cause a conflict.
>
> This patch waits to restore the original COMMAND register until we're
> completely finished sizing the BAR.
>
> Reference: https://lkml.org/lkml/2007/8/25/154
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

This patch causes boot lockup on PA Semi hardware, since it disables
the bar on the UART that is used for console, and it has printks
between the old and the new re-enable location. If I boot with 'debug'
level for printk, I hit this. If I boot with just regular console
args, I don't.

I'm guessing any other platform that uses MMIO-based UART on PCI for
console will have similar issues. I can verify on Chrome OS x86
hardware tomorrow if legacy powerpc isn't important enough to care
about. :-)

I have no proposal for a fix for this. Can you please consider
reverting for 3.6 unless someone has a better idea?


-Olof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
  2012-08-23  7:28     ` Olof Johansson
@ 2012-08-23 18:02       ` Bjorn Helgaas
  -1 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2012-08-23 18:02 UTC (permalink / raw)
  To: Olof Johansson
  Cc: linux-pci, Jacob Pan, Greg Kroah-Hartman, linux-kernel,
	Jesse Barnes, Ivan Kokshaysky, Matthew Wilcox, Robert Hancock,
	linuxppc-dev

On Thu, Aug 23, 2012 at 12:28:23AM -0700, Olof Johansson wrote:
> Hi,
> 
> On Mon, Jul 9, 2012 at 11:20 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > After 253d2e5498, we disable MEM and IO decoding for most devices while we
> > size 32-bit BARs.  However, we restore the original COMMAND register before
> > we size the upper 32 bits of 64-bit BARs, so we can still cause a conflict.
> >
> > This patch waits to restore the original COMMAND register until we're
> > completely finished sizing the BAR.
> >
> > Reference: https://lkml.org/lkml/2007/8/25/154
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> This patch causes boot lockup on PA Semi hardware, since it disables
> the bar on the UART that is used for console, and it has printks
> between the old and the new re-enable location. If I boot with 'debug'
> level for printk, I hit this. If I boot with just regular console
> args, I don't.
> 
> I'm guessing any other platform that uses MMIO-based UART on PCI for
> console will have similar issues. I can verify on Chrome OS x86
> hardware tomorrow if legacy powerpc isn't important enough to care
> about. :-)
> 
> I have no proposal for a fix for this. Can you please consider
> reverting for 3.6 unless someone has a better idea?

Thanks a lot for finding and debugging this!

Can you try the patch below?


commit cfc29ece86d648e63fb46de81b2bf8e3e107672c
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Aug 23 10:53:08 2012 -0600

    PCI: Don't print anything while decoding is disabled
    
    If we try to print to the console device while its decoding is disabled,
    the system will hang.
    
    Reported-by: Olof Johansson <olof@lixom.net>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6c143b4..9f8a6b7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -144,15 +144,13 @@ static inline unsigned long decode_bar(struct pci_dev *dev, u32 bar)
 	case PCI_BASE_ADDRESS_MEM_TYPE_32:
 		break;
 	case PCI_BASE_ADDRESS_MEM_TYPE_1M:
-		dev_info(&dev->dev, "1M mem BAR treated as 32-bit BAR\n");
+		/* 1M mem BAR treated as 32-bit BAR */
 		break;
 	case PCI_BASE_ADDRESS_MEM_TYPE_64:
 		flags |= IORESOURCE_MEM_64;
 		break;
 	default:
-		dev_warn(&dev->dev,
-			 "mem unknown type %x treated as 32-bit BAR\n",
-			 mem_type);
+		/* mem unknown type treated as 32-bit BAR */
 		break;
 	}
 	return flags;
@@ -173,9 +171,11 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 	u32 l, sz, mask;
 	u16 orig_cmd;
 	struct pci_bus_region region;
+	bool bar_too_big = false, bar_disabled = false;
 
 	mask = type ? PCI_ROM_ADDRESS_MASK : ~0;
 
+	/* No printks while decoding is disabled! */
 	if (!dev->mmio_always_on) {
 		pci_read_config_word(dev, PCI_COMMAND, &orig_cmd);
 		pci_write_config_word(dev, PCI_COMMAND,
@@ -240,8 +240,7 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 			goto fail;
 
 		if ((sizeof(resource_size_t) < 8) && (sz64 > 0x100000000ULL)) {
-			dev_err(&dev->dev, "reg %x: can't handle 64-bit BAR\n",
-				pos);
+			bar_too_big = true;
 			goto fail;
 		}
 
@@ -252,12 +251,11 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 			region.start = 0;
 			region.end = sz64;
 			pcibios_bus_to_resource(dev, res, &region);
+			bar_disabled = true;
 		} else {
 			region.start = l64;
 			region.end = l64 + sz64;
 			pcibios_bus_to_resource(dev, res, &region);
-			dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pR\n",
-				   pos, res);
 		}
 	} else {
 		sz = pci_size(l, sz, mask);
@@ -268,18 +266,23 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 		region.start = l;
 		region.end = l + sz;
 		pcibios_bus_to_resource(dev, res, &region);
-
-		dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pR\n", pos, res);
 	}
 
- out:
+	goto out;
+
+
+fail:
+	res->flags = 0;
+out:
 	if (!dev->mmio_always_on)
 		pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
 
+	if (bar_too_big)
+		dev_err(&dev->dev, "reg %x: can't handle 64-bit BAR\n", pos);
+	if (res->flags && !bar_disabled)
+		dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pR\n", pos, res);
+
 	return (res->flags & IORESOURCE_MEM_64) ? 1 : 0;
- fail:
-	res->flags = 0;
-	goto out;
 }
 
 static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
@ 2012-08-23 18:02       ` Bjorn Helgaas
  0 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2012-08-23 18:02 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Jacob Pan, linux-pci, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Greg Kroah-Hartman,
	linuxppc-dev, Robert Hancock

On Thu, Aug 23, 2012 at 12:28:23AM -0700, Olof Johansson wrote:
> Hi,
> 
> On Mon, Jul 9, 2012 at 11:20 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > After 253d2e5498, we disable MEM and IO decoding for most devices while we
> > size 32-bit BARs.  However, we restore the original COMMAND register before
> > we size the upper 32 bits of 64-bit BARs, so we can still cause a conflict.
> >
> > This patch waits to restore the original COMMAND register until we're
> > completely finished sizing the BAR.
> >
> > Reference: https://lkml.org/lkml/2007/8/25/154
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> This patch causes boot lockup on PA Semi hardware, since it disables
> the bar on the UART that is used for console, and it has printks
> between the old and the new re-enable location. If I boot with 'debug'
> level for printk, I hit this. If I boot with just regular console
> args, I don't.
> 
> I'm guessing any other platform that uses MMIO-based UART on PCI for
> console will have similar issues. I can verify on Chrome OS x86
> hardware tomorrow if legacy powerpc isn't important enough to care
> about. :-)
> 
> I have no proposal for a fix for this. Can you please consider
> reverting for 3.6 unless someone has a better idea?

Thanks a lot for finding and debugging this!

Can you try the patch below?


commit cfc29ece86d648e63fb46de81b2bf8e3e107672c
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Aug 23 10:53:08 2012 -0600

    PCI: Don't print anything while decoding is disabled
    
    If we try to print to the console device while its decoding is disabled,
    the system will hang.
    
    Reported-by: Olof Johansson <olof@lixom.net>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6c143b4..9f8a6b7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -144,15 +144,13 @@ static inline unsigned long decode_bar(struct pci_dev *dev, u32 bar)
 	case PCI_BASE_ADDRESS_MEM_TYPE_32:
 		break;
 	case PCI_BASE_ADDRESS_MEM_TYPE_1M:
-		dev_info(&dev->dev, "1M mem BAR treated as 32-bit BAR\n");
+		/* 1M mem BAR treated as 32-bit BAR */
 		break;
 	case PCI_BASE_ADDRESS_MEM_TYPE_64:
 		flags |= IORESOURCE_MEM_64;
 		break;
 	default:
-		dev_warn(&dev->dev,
-			 "mem unknown type %x treated as 32-bit BAR\n",
-			 mem_type);
+		/* mem unknown type treated as 32-bit BAR */
 		break;
 	}
 	return flags;
@@ -173,9 +171,11 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 	u32 l, sz, mask;
 	u16 orig_cmd;
 	struct pci_bus_region region;
+	bool bar_too_big = false, bar_disabled = false;
 
 	mask = type ? PCI_ROM_ADDRESS_MASK : ~0;
 
+	/* No printks while decoding is disabled! */
 	if (!dev->mmio_always_on) {
 		pci_read_config_word(dev, PCI_COMMAND, &orig_cmd);
 		pci_write_config_word(dev, PCI_COMMAND,
@@ -240,8 +240,7 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 			goto fail;
 
 		if ((sizeof(resource_size_t) < 8) && (sz64 > 0x100000000ULL)) {
-			dev_err(&dev->dev, "reg %x: can't handle 64-bit BAR\n",
-				pos);
+			bar_too_big = true;
 			goto fail;
 		}
 
@@ -252,12 +251,11 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 			region.start = 0;
 			region.end = sz64;
 			pcibios_bus_to_resource(dev, res, &region);
+			bar_disabled = true;
 		} else {
 			region.start = l64;
 			region.end = l64 + sz64;
 			pcibios_bus_to_resource(dev, res, &region);
-			dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pR\n",
-				   pos, res);
 		}
 	} else {
 		sz = pci_size(l, sz, mask);
@@ -268,18 +266,23 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 		region.start = l;
 		region.end = l + sz;
 		pcibios_bus_to_resource(dev, res, &region);
-
-		dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pR\n", pos, res);
 	}
 
- out:
+	goto out;
+
+
+fail:
+	res->flags = 0;
+out:
 	if (!dev->mmio_always_on)
 		pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
 
+	if (bar_too_big)
+		dev_err(&dev->dev, "reg %x: can't handle 64-bit BAR\n", pos);
+	if (res->flags && !bar_disabled)
+		dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pR\n", pos, res);
+
 	return (res->flags & IORESOURCE_MEM_64) ? 1 : 0;
- fail:
-	res->flags = 0;
-	goto out;
 }
 
 static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
  2012-08-23 18:02       ` Bjorn Helgaas
@ 2012-08-23 18:20         ` Olof Johansson
  -1 siblings, 0 replies; 11+ messages in thread
From: Olof Johansson @ 2012-08-23 18:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Jacob Pan, Greg Kroah-Hartman, linux-kernel,
	Jesse Barnes, Ivan Kokshaysky, Matthew Wilcox, Robert Hancock,
	linuxppc-dev

Hi,

On Thu, Aug 23, 2012 at 11:02 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Aug 23, 2012 at 12:28:23AM -0700, Olof Johansson wrote:
>> Hi,
>>
>> On Mon, Jul 9, 2012 at 11:20 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> > After 253d2e5498, we disable MEM and IO decoding for most devices while we
>> > size 32-bit BARs.  However, we restore the original COMMAND register before
>> > we size the upper 32 bits of 64-bit BARs, so we can still cause a conflict.
>> >
>> > This patch waits to restore the original COMMAND register until we're
>> > completely finished sizing the BAR.
>> >
>> > Reference: https://lkml.org/lkml/2007/8/25/154
>> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>
>> This patch causes boot lockup on PA Semi hardware, since it disables
>> the bar on the UART that is used for console, and it has printks
>> between the old and the new re-enable location. If I boot with 'debug'
>> level for printk, I hit this. If I boot with just regular console
>> args, I don't.
>>
>> I'm guessing any other platform that uses MMIO-based UART on PCI for
>> console will have similar issues. I can verify on Chrome OS x86
>> hardware tomorrow if legacy powerpc isn't important enough to care
>> about. :-)
>>
>> I have no proposal for a fix for this. Can you please consider
>> reverting for 3.6 unless someone has a better idea?
>
> Thanks a lot for finding and debugging this!
>
> Can you try the patch below?
>
>
> commit cfc29ece86d648e63fb46de81b2bf8e3e107672c
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Thu Aug 23 10:53:08 2012 -0600
>
>     PCI: Don't print anything while decoding is disabled
>
>     If we try to print to the console device while its decoding is disabled,
>     the system will hang.
>
>     Reported-by: Olof Johansson <olof@lixom.net>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Ah yes, of course. Teaches me to try to do anything productive late at
night. Tested fine here.

Acked-by: Olof Johansson <olof@lixom.net>


Thanks!

-Olof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too
@ 2012-08-23 18:20         ` Olof Johansson
  0 siblings, 0 replies; 11+ messages in thread
From: Olof Johansson @ 2012-08-23 18:20 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Jacob Pan, linux-pci, linux-kernel, Jesse Barnes,
	Ivan Kokshaysky, Matthew Wilcox, Greg Kroah-Hartman,
	linuxppc-dev, Robert Hancock

Hi,

On Thu, Aug 23, 2012 at 11:02 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Aug 23, 2012 at 12:28:23AM -0700, Olof Johansson wrote:
>> Hi,
>>
>> On Mon, Jul 9, 2012 at 11:20 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> > After 253d2e5498, we disable MEM and IO decoding for most devices while we
>> > size 32-bit BARs.  However, we restore the original COMMAND register before
>> > we size the upper 32 bits of 64-bit BARs, so we can still cause a conflict.
>> >
>> > This patch waits to restore the original COMMAND register until we're
>> > completely finished sizing the BAR.
>> >
>> > Reference: https://lkml.org/lkml/2007/8/25/154
>> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>
>> This patch causes boot lockup on PA Semi hardware, since it disables
>> the bar on the UART that is used for console, and it has printks
>> between the old and the new re-enable location. If I boot with 'debug'
>> level for printk, I hit this. If I boot with just regular console
>> args, I don't.
>>
>> I'm guessing any other platform that uses MMIO-based UART on PCI for
>> console will have similar issues. I can verify on Chrome OS x86
>> hardware tomorrow if legacy powerpc isn't important enough to care
>> about. :-)
>>
>> I have no proposal for a fix for this. Can you please consider
>> reverting for 3.6 unless someone has a better idea?
>
> Thanks a lot for finding and debugging this!
>
> Can you try the patch below?
>
>
> commit cfc29ece86d648e63fb46de81b2bf8e3e107672c
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Thu Aug 23 10:53:08 2012 -0600
>
>     PCI: Don't print anything while decoding is disabled
>
>     If we try to print to the console device while its decoding is disabled,
>     the system will hang.
>
>     Reported-by: Olof Johansson <olof@lixom.net>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Ah yes, of course. Teaches me to try to do anything productive late at
night. Tested fine here.

Acked-by: Olof Johansson <olof@lixom.net>


Thanks!

-Olof

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-08-23 18:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-09 18:20 [PATCH 0/2] PCI: decode disable fixes Bjorn Helgaas
2012-07-09 18:20 ` [PATCH 1/2] PCI: leave MEM and IO decoding disabled during 64-bit BAR sizing, too Bjorn Helgaas
2012-07-09 20:18   ` Jacob Pan
2012-08-23  7:28   ` Olof Johansson
2012-08-23  7:28     ` Olof Johansson
2012-08-23 18:02     ` Bjorn Helgaas
2012-08-23 18:02       ` Bjorn Helgaas
2012-08-23 18:20       ` Olof Johansson
2012-08-23 18:20         ` Olof Johansson
2012-07-09 18:20 ` [PATCH 2/2] PCI: disable MEM decoding while updating 64-bit MEM BARs Bjorn Helgaas
2012-07-10  3:23 ` [PATCH 0/2] PCI: decode disable fixes Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.