Hello Bjorn,
On 5/5/20 11:04 PM, Bjorn Helgaas wrote:
On Tue, May 05, 2020 at 07:15:13PM +0200, refactormyself@xxxxxxxxx wrote:
From: Bolarinwa Olayemi Saheed <refactormyself@xxxxxxxxx>
PCIe capability accessors return 0 on success, otherwise it could return
either a negative or positive error value. Positive error values are from
PCIBIOS_* error values. This behaviour contradicts that of PCI config.
accessors which only returns PCIBIOS_* error values.
There is no caller of these accessors that directly utilize the returned
positive error values. They either use pcibios_err_to_errno() to convert
it to a generic error value or simply pass it on or in some cases discard
any positive error value.
We *do* still need to check all the callers to see that they do the
right thing. The intent of a patch like this one is that it causes no
functional change -- everything should work exactly the same before
and after the patch.
For example, the case we talked about earlier in this chain:
local_pci_probe
pci_drv->probe(..)
init_one # hfi1_pci_driver.probe method
hfi1_init_dd
pcie_speeds
pcie_capability_read_dword
Before this patch, pcie_capability_read_dword() could return:
- 0 (success)
- a negative value (e.g., -EINVAL)
- a positive value (e.g., PCIBIOS_BAD_REGISTER_NUMBER (0x87))
The positive return value would cause local_pci_probe() to warn that
"Driver probe function unexpectedly returned %d".
After this patch, pcie_capability_read_dword() will never return a
positive value because PCIBIOS_BAD_REGISTER_NUMBER is now -EFAULT, and
local_pci_probe() will no longer warn.
In this case, that's a bug fix, but we don't want bugs to be silently,
magically fixed by patches that don't seem to be connected to the
problem.
So we need to account for that somehow. We could:
- Add a preparatory patch that calls pcibios_err_to_errno() from
pcie_speeds() as I mentioned before. Of course, we plan to remove
that soon anyway.
- Maybe call pcibios_err_to_errno() inside
pcie_capability_read_dword()?
We really need to identify all places that have this problem so we can
see which way makes more sense.
Can you start posting these to linux-pci so they're visible on the
mailing list? Sometimes other folks have great ideas, like Yicong's.
I will start working on it.
Thank you.
Saheed
