* [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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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 2012-08-23 18:02 ` Bjorn Helgaas 1 sibling, 1 reply; 8+ 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] 8+ 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 2012-08-23 18:20 ` Olof Johansson 0 siblings, 1 reply; 8+ 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, ®ion); + bar_disabled = true; } else { region.start = l64; region.end = l64 + sz64; pcibios_bus_to_resource(dev, res, ®ion); - 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, ®ion); - - 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] 8+ 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 0 siblings, 0 replies; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2012-08-23 18:20 UTC | newest] Thread overview: 8+ 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 18:02 ` Bjorn Helgaas 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).