Re: [PATCH]PCI:disable resource decode in PCI BAR detection

[Robert Hancock <hancockr@shaw.ca>]

Ivan Kokshaysky wrote:
> On Thu, Sep 13, 2007 at 09:32:42PM -0600, Robert Hancock wrote:
>> Disabling the BAR decoding does not mean disabling the device (aside 
>> from the one  group of host bridge that apparently disables CPU to RAM 
>> access, whose designers were apparently on crack when they read the PCI 
>> spec and which is why we don't disable the decode on host bridges). 
>> There really is no other sane way to do it.
> 
> Disabling IO and MEM decode effectively disables device as a PCI target.
> Worse, it's often not just BARs that get disabled, but some non-standard
> or hidden address ranges as well. Host bridges are just one common
> example, there is a lot of other insane hardware, more than one could
> expect.

Do you have an example of specific hardware that exhibits this problem? 
So far after a similar patch has been in -mm for months there have been 
no reports of any such problems.

> BTW, the way how the internal address decode works on these latest Intel
> whippets doesn't look sane either. ;-)
> 
>> If we do encounter other devices that choke on having the BAR disabled 
>> during probing then we can add additional quirk logic, but we haven't 
>> run into anything like that yet.
> 
> As Greg said, the main point is "no regression".
> 
> Folks, can you check if the patch below helps?

This isn't guaranteed to help. I don't think it is only integrated 
graphics that could cause this problem, I think that an add-on PCI 
Express card can do this as well. Depending on the chipset memory decode 
rules, any PCI or PCI Express device with a BAR large enough could cause 
issues.

> 
> Ivan.
> 
> --- 2.6.23-rc6-git4/include/linux/pci.h	2007-09-12 17:22:57.000000000 +0400
> +++ linux/include/linux/pci.h	2007-09-14 10:26:29.000000000 +0400
> @@ -183,6 +183,7 @@ struct pci_dev {
>  	unsigned int 	msi_enabled:1;
>  	unsigned int	msix_enabled:1;
>  	unsigned int	is_managed:1;
> +	unsigned int	disable_while_probe:1; /* Probe BARs with IO and MMIO turned off */
>  	atomic_t	enable_cnt;	/* pci_enable_device has been called */
>  
>  	u32		saved_config_space[16]; /* config space saved at suspend time */
> --- 2.6.23-rc6-git4/arch/i386/pci/fixup.c	2007-09-12 17:22:55.000000000 +0400
> +++ linux/arch/i386/pci/fixup.c	2007-09-14 14:43:13.000000000 +0400
> @@ -444,3 +444,21 @@ static void __devinit pci_siemens_interr
>  }
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SIEMENS, 0x0015,
>  			  pci_siemens_interrupt_controller);
> +
> +/*
> + * Work around apparent quirk in internal address decoding on
> + * some Intel chipsets (G31, G33, Q965 etc.): 
> + * when some MMIO address range of integrated peripheral overlaps
> + * mmconfig area, mmconfig accesses are blocked. This can happen
> + * when we size the BAR writing all ones to it.
> + * In practice, only integrated graphics controller has MMIO range
> + * large enough (BAR2, 256Mb) to cause a trouble, so we disable
> + * address decoders just on that function during PCI BAR probe.
> + */
> +static void __devinit pci_intel_mmconfig(struct pci_dev *dev)
> +{
> +	if ((dev->class >> 8) == PCI_CLASS_DISPLAY_VGA &&
> +	    pci_domain_nr(dev->bus) == 0 && dev->bus->number == 0)
> +		dev->disable_while_probe = 1;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_intel_mmconfig);
> --- 2.6.23-rc6-git4/drivers/pci/probe.c	2007-09-14 00:35:47.000000000 +0400
> +++ linux/drivers/pci/probe.c	2007-09-14 10:27:45.000000000 +0400
> @@ -185,6 +185,13 @@ static void pci_read_bases(struct pci_de
>  	unsigned int pos, reg, next;
>  	u32 l, sz;
>  	struct resource *res;
> +	u16 cmd, orig_cmd;
> +
> +	if (dev->disable_while_probe) {
> +		pci_read_config_word(dev, PCI_COMMAND, &orig_cmd);
> +		cmd |= orig_cmd & ~(PCI_COMMAND_MEMORY | PCI_COMMAND_IO);
> +		pci_write_config_word(dev, PCI_COMMAND, cmd);
> +	}
>  
>  	for(pos=0; pos<howmany; pos = next) {
>  		u64 l64;
> @@ -283,6 +290,8 @@ static void pci_read_bases(struct pci_de
>  			}
>  		}
>  	}
> +	if (dev->disable_while_probe)
> +		pci_write_config_word(dev, PCI_COMMAND, orig_cmd);
>  }
>  
>  void pci_read_bridge_bases(struct pci_bus *child)
>