RE: [PATCH 2.6.34-rcX] Do not expect PCI devices to return zeroesin PCIe space

[Pan, Jacob jun]

Hi Petr,

There are other code in the kernel makes similar assumption of accessing pci cfg above 0x100. (but they do not hang in a loop)
e.g. in drivers/pci/probe.c
* accesses, or the device is behind a reverse Express bridge.  So we try
 * reading the dword at 0x100 which must either be 0 or a valid extended
 * capability header.
 */
int pci_cfg_space_size_ext(struct pci_dev *dev)
{
	u32 status;
	int pos = PCI_CFG_SPACE_SIZE;

	if (pci_read_config_dword(dev, pos, &status) != PCIBIOS_SUCCESSFUL)
		goto fail;
	if (status == 0xffffffff)
		goto fail;


Back to the problem itself, hpa has suggested a better fix might be using cfg_size for checking in fixed_bar_cap. But we can not use it right now since we have cfg_size set to 0x100 on MRST (due to lack of PCI_CAP_ID_EXP in the PCI shim). I will negotiate with FW guys so that we have the correct return from pci_cfg_space_size() for Moorestown.

Until then, your current fix should be good.

Thanks,

Jacob
> -----Original Message-----
> From: Petr Vandrovec [mailto:petr@xxxxxxxxxxxxxx]
> Sent: Friday, April 30, 2010 7:54 PM
> To: linux-kernel@xxxxxxxxxxxxxxx
> Cc: akpm@xxxxxxxxxxxxxxxxxxxx; Thomas Gleixner; Ingo Molnar; H. Peter
> Anvin; x86@xxxxxxxxxx; Pan, Jacob jun; Jesse Barnes
> Subject: [PATCH 2.6.34-rcX] Do not expect PCI devices to return zeroes
> in PCIe space
> 
> Hello,
>   openSUSE11.3 32bit kernels hang when installed to the VMware's VMs
> because Moorestown
> fixed capabilities detection code enters endless loop on Intel's AGP
> bridges (with
> device ID=7191).  See https://bugzilla.kernel.org/show_bug.cgi?id=15888
> for additional
> details.  arch/x86/pci/mrst.c was introduced after 2.6.33, so only
> 2.6.34-rcX are
> affected.
> 				Thanks,
> 					Petr Vandrovec
> 
> 
> commit 11a35e56ad8275cbf62882d9c0dc2f17c2b5628b
> Author: Petr Vandrovec <petr@xxxxxxxxxxxxxx>
> Date:   Fri Apr 30 19:17:43 2010 -0700
> 
>     Do not expect PCI devices to return zeroes in PCIe space
> 
>     There is no reason why old pre-PCIe/PCI-X devices should return
> zeroes when
>     configuration space above 0x100 is accessed.  If these devices
> decode just
>     low 8 bits of register number, conventional space repeats 15 times
> in
>     PCIe config space.  And Moorestown parser for fixed bars then can
> enter
>     endless loop when finding Intel AGP bridge device 0x7191 with
> secondary
>     latency timer programmed to 0x40 - when such device is encountered,
> code
>     will enter endless loop of reading registers 0x718 (reading
> 0x40010100)
>     and 0x400 (reading 0x71918086).
> 
>     This change adds additional condition to the test: if device
> id/vendor
>     match first PCIe capability, then device is not really PCIe.  It
> should
>     not cause any problems: fixed_bar_cap is invoked only on Intel's
> devices,
>     so only time there is possibilty to have false match would be if
> first
>     PCIe capability would have ID 0x8086, and even then that address of
>     next capability pointer and capability version will match device ID
> seems
>     highly unlikely.
> 
>     This fix unbreaks 32bit 2.6.33+ kernels configured with Moorestown
>     support to boot on AMD rev 10h+ processors under VMware in VMs
> which
>     lack PCIe support.
> 
>     Signed-off-by: Petr Vandrovec <petr@xxxxxxxxxxxxxx>
> 
> diff --git a/arch/x86/pci/mrst.c b/arch/x86/pci/mrst.c
> index 8bf2fcb..cd6c277 100644
> --- a/arch/x86/pci/mrst.c
> +++ b/arch/x86/pci/mrst.c
> @@ -55,12 +55,16 @@ static int fixed_bar_cap(struct pci_bus *bus,
> unsigned int devfn)
>  {
>  	int pos;
>  	u32 pcie_cap = 0, cap_data;
> +	u32 devid;
> 
>  	pos = PCIE_CAP_OFFSET;
> 
>  	if (!raw_pci_ext_ops)
>  		return 0;
> 
> +	if (raw_pci_ops->read(pci_domain_nr(bus), bus->number, devfn, 0,
> 4, &devid))
> +		return 0;
> +
>  	while (pos) {
>  		if (raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,
>  					  devfn, pos, 4, &pcie_cap))
> @@ -69,6 +73,9 @@ static int fixed_bar_cap(struct pci_bus *bus,
> unsigned int devfn)
>  		if (pcie_cap == 0xffffffff)
>  			return 0;
> 
> +		if (pcie_cap == devid && pos == PCIE_CAP_OFFSET)
> +			return 0;
> +
>  		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
>  			raw_pci_ext_ops->read(pci_domain_nr(bus), bus-
> >number,
>  					      devfn, pos + 4, 4, &cap_data);

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