Re: [PATCH] pciaer: report config read/write errors

[Loic Prylli <loic@myri.com>]

On 12/02/2008 12:23 PM, Stephen Hemminger wrote:
> This patch does more error checking in the Advanced Error Reporting code.
> Since AER needs to access PCI registers > 255, it won't work without MMCONFIG
> and other quirks may stop it as well. The code must check this by looking
> at return values from pci_read/write_config_XXX calls.  
>
> I don't have any hardware that uses AER routines but discovered this 
> in earlier versions of the sky2 driver that tried to use
> pci AER routines. Ended up just giving up and using other ways to access PCI
> config space on sky2 since there were too many platform glitches.
>   




When experimenting with sky2 driver, was pci_find_ext_capability() 
returning non-zero although further ext-space accesses were failing?



>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
>
> --- a/drivers/pci/pcie/aer/aerdrv_core.c	2008-12-02 07:56:08.000000000 -0800
> +++ b/drivers/pci/pcie/aer/aerdrv_core.c	2008-12-02 09:07:32.000000000 -0800
> @@ -31,80 +31,92 @@ module_param(forceload, bool, 0);
>  int pci_enable_pcie_error_reporting(struct pci_dev *dev)
>  {
>  	u16 reg16 = 0;
> -	int pos;
> +	int pos, err;
> +	u32 status;
>  
>  	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
>  	if (!pos)
>  		return -EIO;
>  
> +	err = pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> +	if (err)
> +		return err;
> +
>
>   



For legacy-conf-space, most kernel code assumes success without 
checking. For ext-conf-space, wouldn't it be convenient to be able to 
make the same assumption when pci_find_ext_capability() returns a valid 
offset?

The patch looks good to me, but I am just asking whether there is a 
known case where pcie_find_ext_capability() returns a valid offset, 
although that offset might turn out unusable (it might be worth 
investigating pci_find_ext_capability() then).


Loic