| Message ID | 20180723200118.23657-1-mr.nuke.me@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v2] PCI/AER: Do not clear AER bits if we don't own AER | expand |
Hi Alexandru,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on v4.18-rc6 next-20180724]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Alexandru-Gagniuc/PCI-AER-Do-not-clear-AER-bits-if-we-don-t-own-AER/20180725-003708
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-s0-07250049 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/linux/string.h:6:0,
from include/linux/uuid.h:20,
from include/linux/cper.h:24,
from drivers/pci/pcie/aer.c:15:
drivers/pci/pcie/aer.c: In function 'pci_enable_pcie_error_reporting':
drivers/pci/pcie/aer.c:371:7: error: implicit declaration of function 'pcie_aer_is_kernel_first' [-Werror=implicit-function-declaration]
if (!pcie_aer_is_kernel_first(dev))
^
include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/pci/pcie/aer.c:371:2: note: in expansion of macro 'if'
if (!pcie_aer_is_kernel_first(dev))
^~
cc1: some warnings being treated as errors
vim +/if +371 drivers/pci/pcie/aer.c
365
366 #define PCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \
367 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
368
369 int pci_enable_pcie_error_reporting(struct pci_dev *dev)
370 {
> 371 if (!pcie_aer_is_kernel_first(dev))
372 return -EIO;
373
374 return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);
375 }
376 EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting);
377
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index a2e88386af28..85c3e173c025 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -307,6 +307,12 @@ int pcie_aer_get_firmware_first(struct pci_dev *dev) aer_set_firmware_first(dev); return dev->__aer_firmware_first; } + +static bool pcie_aer_is_kernel_first(struct pci_dev *dev) +{ + return !!dev->aer_cap && !pcie_aer_get_firmware_first(dev); +} + #define PCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) @@ -337,10 +343,7 @@ bool aer_acpi_firmware_first(void) int pci_enable_pcie_error_reporting(struct pci_dev *dev) { - if (pcie_aer_get_firmware_first(dev)) - return -EIO; - - if (!dev->aer_cap) + if (!pcie_aer_is_kernel_first(dev)) return -EIO; return pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS); @@ -349,7 +352,7 @@ EXPORT_SYMBOL_GPL(pci_enable_pcie_error_reporting); int pci_disable_pcie_error_reporting(struct pci_dev *dev) { - if (pcie_aer_get_firmware_first(dev)) + if (!pcie_aer_is_kernel_first(dev)) return -EIO; return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL, @@ -383,10 +386,10 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev) if (!pci_is_pcie(dev)) return -ENODEV; - pos = dev->aer_cap; - if (!pos) + if (pcie_aer_is_kernel_first(dev)) return -EIO; + pos = dev->aer_cap; port_type = pci_pcie_type(dev); if (port_type == PCI_EXP_TYPE_ROOT_PORT) { pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, &status);
When we don't own AER, we shouldn't touch the AER error bits. Clearing error bits willy-nilly might cause firmware to miss some errors. In theory, these bits get cleared by FFS, or via ACPI _HPX method. These mechanisms are not subject to the problem. This race is mostly of theoretical significance, since I can't reasonably demonstrate this race in the lab. On a side-note, pcie_aer_is_kernel_first() is created to alleviate the need for two checks: aer_cap and get_firmware_first(). Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> --- drivers/pci/pcie/aer.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)