Re: [RFC PATCH 05/14] PCI: add access functions for PCIe capabilitiesto hide PCIe spec differences

[Jiang Liu]

On 07/13/2012 04:49 AM, Bjorn Helgaas wrote:
>> Hi Bjorn,
>>         It's a little risk to change these PCIe capabilities access
>> functions as void. On some platform with hardware error detecting/correcting
>> capabilities, such as EEH on Power, it would be better to return
>> error code if hardware error happens during accessing configuration registers.
>>         As I know, coming Intel Xeon processor may provide PCIe hardware
>> error detecting capability similar to EEH on power.
> 
> I guess I'm playing devil's advocate here.  As a general rule, people
> don't check the return value of pci_read_config_*() or
> pci_write_config_*().  Unless you change them all, most callers of
> pci_pcie_capability_read_*() and _write_*() won't check the returns
> either.  So I'm not sure return values are an effective way to detect
> those hardware errors.
> 
> How do these EEH errors get detected or reported today?  Do the
> drivers check every config access for success?  Adding those checks
> and figuring out how to handle errors at every possible point doesn't
> seem like a recipe for success.

Hi Bjorn,
	Sorry for later reply, on travel these days.
	Yeah, it's true that most driver doesn't check return values of configuration
access functions, but there are still some drivers which do check return value of
pci_read_config_xxx(). For example, pciehp driver checks return value of CFG access
functions.

	It's not realistic to enhance all drivers, but we may focus on a small set of
drivers for hardwares on specific high-end servers. For RAS features, we can never provide
perfect solutions, so we prefer some improvements. After all a small improvement is still
an improvement:)

	I'm only familiar with PCI on IA64 and x86. For PowerPC, I just know that the OS
may query firmware whether there's some hardware faults if pci_cfg_read_xxx() returns
all 1s. For PCI on IA64, SAL may handle PCI hardware errors and return error code to
pci_cfg_read_xxx(). For x86, I think it will have some mechanisms to report hardware faults
like SAL on IA64.

	So how about keeping consistence with pci_cfg_read_xxx() and pci_user_cfg_read_xxx()?
	Thanks!
	Gerry

--
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/