| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220911162206.5f2uctuotuddqpre@mobilestation>
Date: Sun, 11 Sep 2022 19:22:06 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Zhuo Chen <chenzhuo.1@...edance.com>
Cc: jdmason@...zu.us, dave.jiang@...el.com, allenbh@...il.com,
bhelgaas@...gle.com, ruscur@...sell.cc, oohall@...il.com,
james.smart@...adcom.com, dick.kennedy@...adcom.com,
jejb@...ux.ibm.com, martin.petersen@...cle.com,
ntb@...ts.linux.dev, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
linux-scsi@...r.kernel.org
Subject: Re: [PATCH 1/3] PCI/AER: Use pci_aer_clear_uncorrect_error_status()
to clear uncorrectable error status
Hi Zhuo
On Fri, Sep 02, 2022 at 02:16:32AM +0800, Zhuo Chen wrote:
> Status bits for ERR_NONFATAL errors only are cleared in
> pci_aer_clear_nonfatal_status(), but we want clear uncorrectable
> error status in ntb_hw_idt.c and lpfc_attr.c. So we add
> pci_aer_clear_uncorrect_error_status() and change to use it.
What about the next drivers
drivers/scsi/lpfc/lpfc_attr.c
drivers/crypto/hisilicon/qm.c
drivers/net/ethernet/intel/ice/ice_main.c
which call the pci_aer_clear_nonfatal_status() method too?
>
> Use pci_aer_clear_nonfatal_status() in dpc_process_error(), which has
> no functional changes.
>
> Since pci_aer_clear_nonfatal_status() is used only internally, move
> its declaration to the PCI internal header file. Also, no one cares
> about return value of pci_aer_clear_nonfatal_status(), so make it void.
>
> Signed-off-by: Zhuo Chen <chenzhuo.1@...edance.com>
> ---
> drivers/ntb/hw/idt/ntb_hw_idt.c | 4 ++--
> drivers/pci/pci.h | 2 ++
> drivers/pci/pcie/aer.c | 23 ++++++++++++++++++-----
> drivers/pci/pcie/dpc.c | 3 +--
> drivers/scsi/lpfc/lpfc_attr.c | 4 ++--
> include/linux/aer.h | 4 ++--
> 6 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/ntb/hw/idt/ntb_hw_idt.c b/drivers/ntb/hw/idt/ntb_hw_idt.c
> index 733557231ed0..de1dbbc5b9de 100644
> --- a/drivers/ntb/hw/idt/ntb_hw_idt.c
> +++ b/drivers/ntb/hw/idt/ntb_hw_idt.c
> @@ -2657,8 +2657,8 @@ static int idt_init_pci(struct idt_ntb_dev *ndev)
> ret = pci_enable_pcie_error_reporting(pdev);
> if (ret != 0)
> dev_warn(&pdev->dev, "PCIe AER capability disabled\n");
> - else /* Cleanup nonfatal error status before getting to init */
> - pci_aer_clear_nonfatal_status(pdev);
> + else /* Cleanup uncorrectable error status before getting to init */
> + pci_aer_clear_uncorrect_error_status(pdev);
>From the IDT NTB PCIe initialization procedure point of view both of
these methods are equivalent. So for the IDT NTB part:
Acked-by: Serge Semin <fancer.lancer@...il.com>
-Sergey
>
> /* First enable the PCI device */
> ret = pcim_enable_device(pdev);
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index e10cdec6c56e..574176f43025 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -686,6 +686,7 @@ void pci_aer_init(struct pci_dev *dev);
> void pci_aer_exit(struct pci_dev *dev);
> extern const struct attribute_group aer_stats_attr_group;
> void pci_aer_clear_fatal_status(struct pci_dev *dev);
> +void pci_aer_clear_nonfatal_status(struct pci_dev *dev);
> int pci_aer_clear_status(struct pci_dev *dev);
> int pci_aer_raw_clear_status(struct pci_dev *dev);
> #else
> @@ -693,6 +694,7 @@ static inline void pci_no_aer(void) { }
> static inline void pci_aer_init(struct pci_dev *d) { }
> static inline void pci_aer_exit(struct pci_dev *d) { }
> static inline void pci_aer_clear_fatal_status(struct pci_dev *dev) { }
> +static inline void pci_aer_clear_nonfatal_status(struct pci_dev *dev) { }
> static inline int pci_aer_clear_status(struct pci_dev *dev) { return -EINVAL; }
> static inline int pci_aer_raw_clear_status(struct pci_dev *dev) { return -EINVAL; }
> #endif
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 7952e5efd6cf..d2996afa80f6 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -251,13 +251,13 @@ int pci_disable_pcie_error_reporting(struct pci_dev *dev)
> }
> EXPORT_SYMBOL_GPL(pci_disable_pcie_error_reporting);
>
> -int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
> +void pci_aer_clear_nonfatal_status(struct pci_dev *dev)
> {
> int aer = dev->aer_cap;
> u32 status, sev;
>
> if (!pcie_aer_is_native(dev))
> - return -EIO;
> + return;
>
> /* Clear status bits for ERR_NONFATAL errors only */
> pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
> @@ -265,10 +265,7 @@ int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
> status &= ~sev;
> if (status)
> pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
> -
> - return 0;
> }
> -EXPORT_SYMBOL_GPL(pci_aer_clear_nonfatal_status);
>
> void pci_aer_clear_fatal_status(struct pci_dev *dev)
> {
> @@ -286,6 +283,22 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
> pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
> }
>
> +int pci_aer_clear_uncorrect_error_status(struct pci_dev *dev)
> +{
> + int aer = dev->aer_cap;
> + u32 status;
> +
> + if (!pcie_aer_is_native(dev))
> + return -EIO;
> +
> + pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &status);
> + if (status)
> + pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_aer_clear_uncorrect_error_status);
> +
> /**
> * pci_aer_raw_clear_status - Clear AER error registers.
> * @dev: the PCI device
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 3e9afee02e8d..7942073fbb34 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -288,8 +288,7 @@ void dpc_process_error(struct pci_dev *pdev)
> dpc_get_aer_uncorrect_severity(pdev, &info) &&
> aer_get_device_error_info(pdev, &info)) {
> aer_print_error(pdev, &info);
> - pci_aer_clear_nonfatal_status(pdev);
> - pci_aer_clear_fatal_status(pdev);
> + pci_aer_clear_uncorrect_error_status(pdev);
> }
> }
>
> diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
> index 3caaa7c4af48..1ed8d1640325 100644
> --- a/drivers/scsi/lpfc/lpfc_attr.c
> +++ b/drivers/scsi/lpfc/lpfc_attr.c
> @@ -4712,7 +4712,7 @@ static DEVICE_ATTR_RW(lpfc_aer_support);
> * Description:
> * If the @buf contains 1 and the device currently has the AER support
> * enabled, then invokes the kernel AER helper routine
> - * pci_aer_clear_nonfatal_status() to clean up the uncorrectable
> + * pci_aer_clear_uncorrect_error_status() to clean up the uncorrectable
> * error status register.
> *
> * Notes:
> @@ -4738,7 +4738,7 @@ lpfc_aer_cleanup_state(struct device *dev, struct device_attribute *attr,
> return -EINVAL;
>
> if (phba->hba_flag & HBA_AER_ENABLED)
> - rc = pci_aer_clear_nonfatal_status(phba->pcidev);
> + rc = pci_aer_clear_uncorrect_error_status(phba->pcidev);
>
> if (rc == 0)
> return strlen(buf);
> diff --git a/include/linux/aer.h b/include/linux/aer.h
> index 97f64ba1b34a..f638ad955deb 100644
> --- a/include/linux/aer.h
> +++ b/include/linux/aer.h
> @@ -44,7 +44,7 @@ struct aer_capability_regs {
> /* PCIe port driver needs this function to enable AER */
> int pci_enable_pcie_error_reporting(struct pci_dev *dev);
> int pci_disable_pcie_error_reporting(struct pci_dev *dev);
> -int pci_aer_clear_nonfatal_status(struct pci_dev *dev);
> +int pci_aer_clear_uncorrect_error_status(struct pci_dev *dev);
> void pci_save_aer_state(struct pci_dev *dev);
> void pci_restore_aer_state(struct pci_dev *dev);
> #else
> @@ -56,7 +56,7 @@ static inline int pci_disable_pcie_error_reporting(struct pci_dev *dev)
> {
> return -EINVAL;
> }
> -static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
> +static inline int pci_aer_clear_uncorrect_error_status(struct pci_dev *dev)
> {
> return -EINVAL;
> }
> --
> 2.30.1 (Apple Git-130)
>
Powered by blists - more mailing lists