2/2: PCI: disable decode of IO/memory during BAR sizing
PATCH 1/1 2/2: PCI: disable decode of IO/memory during BAR sizing
Change PCI BAR sizing to disable the decode of memory or IO, as appropriate, while we are writing the all-ones value to the BAR to determine the size. If this is not done, the device may spuriously decode accesses to memory areas it should not. On some Intel PCI Express chipsets, this breaks MMCONFIG configuration space access, since the memory the graphics card ends up decoding during this period overlaps the MMCONFIG area, and thus it steals the accesses to the area to do any other configuration space access, including changing the BAR back to its previous value.
However, don't do this disabling on host bridge devices, as it is reported that some of them do silly things like disable CPU to RAM access if this is done.
Based on an original patch by Jesse Barnes.
drivers/pci/probe.c +0 -0
@@ -180,6 +180,58 @@ static inline int is_64bit_memory(u32 ma
return 0; }
+#define BAR_IS_MEMORY(bar) (((bar) & PCI_BASE_ADDRESS_SPACE) == \
+ PCI_BASE_ADDRESS_SPACE_MEMORY)
+
+/**
+ * pci_bar_size - get raw PCI BAR size
+ * @dev: PCI device
+ * @reg: BAR to probe
+ *
+ * Use basic PCI probing:
+ * - save original BAR value + * - disable MEM or IO decode in PCI_COMMAND reg if appropriate + * - write all 1s to the BAR + * - read back value + * - reenble MEM or IO decode as necessary + * - write original value back
+ *
+ * Returns raw BAR size to caller.
+ */
+static u32 pci_bar_size(struct pci_dev *dev, unsigned int reg) +{
+ u32 orig_reg, sz;
+ u16 orig_cmd;
+
+ pci_read_config_dword(dev, reg, &orig_reg);
+ pci_read_config_word(dev, PCI_COMMAND, &orig_cmd);
+
+ /*
+ * Disable memory or IO decode on the device while writing the test
+ * value to the BAR. This prevents possible spurious decoding
+ * of random addresses by the device. Don't do this for host bridges,
+ * however, since some of them do silly things like disable CPU to RAM
+ * access if this is done.
+ */
+ if ((dev->class >> 8) != PCI_CLASS_BRIDGE_HOST) {
+ if (BAR_IS_MEMORY(orig_reg))
+ pci_write_config_word(dev, PCI_COMMAND,
+ orig_cmd & ~PCI_COMMAND_MEMORY);
+ else
+ pci_write_config_word(dev, PCI_COMMAND, + orig_cmd & ~PCI_COMMAND_IO);
+ }
+
+ pci_write_config_dword(dev, reg, 0xffffffff);
+ pci_read_config_dword(dev, reg, &sz);
+ pci_write_config_dword(dev, reg, orig_reg);
+
+ if ((dev->class >> 8) != PCI_CLASS_BRIDGE_HOST)
+ pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
+
+ return sz;
+}
+
static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
{
unsigned int pos, reg, next;
@@ -196,16 +248,13 @@ static void pci_read_bases(struct pci_de
res->name = pci_name(dev);
reg = PCI_BASE_ADDRESS_0 + (pos << 2);
pci_read_config_dword(dev, reg, &l);
- pci_write_config_dword(dev, reg, ~0);
- pci_read_config_dword(dev, reg, &sz);
- pci_write_config_dword(dev, reg, l);
+ sz = pci_bar_size(dev, reg);
if (!sz || sz == 0xffffffff)
continue;
if (l == 0xffffffff)
l = 0;
raw_sz = sz;
- if ((l & PCI_BASE_ADDRESS_SPACE) ==
- PCI_BASE_ADDRESS_SPACE_MEMORY) {
+ if (BAR_IS_MEMORY(l)) {
sz = pci_size(l, sz, (u32)PCI_BASE_ADDRESS_MEM_MASK);
/*
* For 64bit prefetchable memory sz could be 0, if the
@@ -229,9 +278,7 @@ static void pci_read_bases(struct pci_de
u32 szhi, lhi;
pci_read_config_dword(dev, reg+4, &lhi);
- pci_write_config_dword(dev, reg+4, ~0);
- pci_read_config_dword(dev, reg+4, &szhi);
- pci_write_config_dword(dev, reg+4, lhi);
+ szhi = pci_bar_size(dev, reg+4);
sz64 = ((u64)szhi << 32) | raw_sz;
l64 = ((u64)lhi << 32) | l;
sz64 = pci_size64(l64, sz64, PCI_BASE_ADDRESS_MEM_MASK);Quoted 100 lines · On Tuesday, May 29, 2007 9:05:24 Robert Hancock wrote:
Change PCI BAR sizing to disable the decode of memory or IO, as appropriate, while we are writing the all-ones value to the BAR to determine the size. If this is not done, the device may spuriously decode accesses to memory areas it should not. On some Intel PCI Express chipsets, this breaks MMCONFIG configuration space access, since the memory the graphics card ends up decoding during this period overlaps the MMCONFIG area, and thus it steals the accesses to the area to do any other configuration space access, including changing the BAR back to its previous value.
However, don't do this disabling on host bridge devices, as it is reported that some of them do silly things like disable CPU to RAM access if this is done.
Based on an original patch by Jesse Barnes.
Signed-off-by Robert Hancock <[email protected]>---
linux-2.6.22-rc2-mm1/drivers/pci/probe.c2007-05-23 21:21:05.000000000
-0600 +++linux-2.6.22-rc2-mm1edit/drivers/pci/probe.c2007-05-29
21:31:47.000000000 -0600 @@ -180,6 +180,58 @@ static inline int is_64bit_memory(u32 ma(quoted diff) +52
-0return 0; } +#define BAR_IS_MEMORY(bar) (((bar) & PCI_BASE_ADDRESS_SPACE) == \ + PCI_BASE_ADDRESS_SPACE_MEMORY) + +/** + * pci_bar_size - get raw PCI BAR size + * @dev: PCI device + * @reg: BAR to probe + * + * Use basic PCI probing: + * - save original BAR value + * - disable MEM or IO decode in PCI_COMMAND reg if appropriate + * - write all 1s to the BAR + * - read back value + * - reenble MEM or IO decode as necessary + * - write original value back + * + * Returns raw BAR size to caller. + */ +static u32 pci_bar_size(struct pci_dev *dev, unsigned int reg) +{ + u32 orig_reg, sz; + u16 orig_cmd; + + pci_read_config_dword(dev, reg, &orig_reg); + pci_read_config_word(dev, PCI_COMMAND, &orig_cmd); + + /* + * Disable memory or IO decode on the device while writing the test + * value to the BAR. This prevents possible spurious decoding + * of random addresses by the device. Don't do this for host bridges, + * however, since some of them do silly things like disable CPU to RAM + * access if this is done. + */ + if ((dev->class >> 8) != PCI_CLASS_BRIDGE_HOST) { + if (BAR_IS_MEMORY(orig_reg)) + pci_write_config_word(dev, PCI_COMMAND, + orig_cmd & ~PCI_COMMAND_MEMORY); + else + pci_write_config_word(dev, PCI_COMMAND, + orig_cmd & ~PCI_COMMAND_IO); + } + + pci_write_config_dword(dev, reg, 0xffffffff); + pci_read_config_dword(dev, reg, &sz); + pci_write_config_dword(dev, reg, orig_reg); + + if ((dev->class >> 8) != PCI_CLASS_BRIDGE_HOST) + pci_write_config_word(dev, PCI_COMMAND, orig_cmd); + + return sz; +} + static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, introm) {
(quoted diff) +2
-5unsigned int pos, reg, next;@@ -196,16 +248,13 @@ static void pci_read_bases(struct pci_de res->name = pci_name(dev); reg = PCI_BASE_ADDRESS_0 + (pos << 2); pci_read_config_dword(dev, reg, &l); - pci_write_config_dword(dev, reg, ~0); - pci_read_config_dword(dev, reg, &sz); - pci_write_config_dword(dev, reg, l); + sz = pci_bar_size(dev, reg); if (!sz || sz == 0xffffffff) continue; if (l == 0xffffffff) l = 0; raw_sz = sz; - if ((l & PCI_BASE_ADDRESS_SPACE) == - PCI_BASE_ADDRESS_SPACE_MEMORY) { + if (BAR_IS_MEMORY(l)) { sz = pci_size(l, sz, (u32)PCI_BASE_ADDRESS_MEM_MASK); /* * For 64bit prefetchable memory sz could be 0, if the
Review static void pci_read_bases(struct pci_de · line 284
Quoted 10 lines
(quoted diff) +1
-3@@ -229,9 +278,7 @@ static void pci_read_bases(struct pci_de u32 szhi, lhi; pci_read_config_dword(dev, reg+4, &lhi); - pci_write_config_dword(dev, reg+4, ~0); - pci_read_config_dword(dev, reg+4, &szhi); - pci_write_config_dword(dev, reg+4, lhi); + szhi = pci_bar_size(dev, reg+4); sz64 = ((u64)szhi << 32) | raw_sz; l64 = ((u64)lhi << 32) | l; sz64 = pci_size64(l64, sz64, PCI_BASE_ADDRESS_MEM_MASK);
Looks good to me, though like I said, this probing code could be redone to be a little clearer, but that could be done in a separate patch.
Quoted 10 lines · Robert Hancock
Change PCI BAR sizing to disable the decode of memory or IO, as appropriate, while we are writing the all-ones value to the BAR to determine the size. If this is not done, the device may spuriously decode accesses to memory areas it should not. On some Intel PCI Express chipsets, this breaks MMCONFIG configuration space access, since the memory the graphics card ends up decoding during this period overlaps the MMCONFIG area, and thus it steals the accesses to the area to do any other configuration space access, including changing the BAR back to its previous value.
Quoted 7 lines
However, don't do this disabling on host bridge devices, as it is reported that some of them do silly things like disable CPU to RAM access if this is done.
Based on an original patch by Jesse Barnes.
Signed-off-by Robert Hancock <[email protected]>
Robert, the PCI patches for fixing up MCFG detection/usage and probing should go through Greg. Can you resend so we don't miss 2.6.24 this time? (I was really hoping this stuff would get into 2.6.23, but apparently none of the right people were Cc'd at the right time... damn.)
You can add my sign-off to this one too.
Thanks,
Jesse