| 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: <53a3795b-4ccd-458e-88cd-d510031bc6b6@linux.intel.com>
Date: Thu, 13 Jun 2024 14:26:42 -0700
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
To: Zhenzhong Duan <zhenzhong.duan@...el.com>, linux-pci@...r.kernel.org
Cc: linuxppc-dev@...ts.ozlabs.org, linux-acpi@...r.kernel.org,
rafael@...nel.org, lenb@...nel.org, james.morse@....com,
tony.luck@...el.com, bp@...en8.de, dave@...olabs.net,
jonathan.cameron@...wei.com, dave.jiang@...el.com,
alison.schofield@...el.com, vishal.l.verma@...el.com, ira.weiny@...el.com,
bhelgaas@...gle.com, helgaas@...nel.org, mahesh@...ux.ibm.com,
oohall@...il.com, linmiaohe@...wei.com, shiju.jose@...wei.com,
adam.c.preble@...el.com, lukas@...ner.de,
Smita.KoralahalliChannabasappa@....com, rrichter@....com,
linux-cxl@...r.kernel.org, linux-edac@...r.kernel.org,
linux-kernel@...r.kernel.org, erwin.tsaur@...el.com,
sathyanarayanan.kuppuswamy@...el.com, dan.j.williams@...el.com,
feiting.wanyan@...el.com, yudong.wang@...el.com, chao.p.peng@...el.com,
qingshun.wang@...ux.intel.com
Subject: Re: [PATCH v4 1/3] PCI/AER: Store UNCOR_STATUS bits that might be
ANFE in aer_err_info
Hi,
On 5/9/24 1:48 AM, Zhenzhong Duan wrote:
> In some cases the detector of a Non-Fatal Error(NFE) is not the most
> appropriate agent to determine the type of the error. For example,
> when software performs a configuration read from a non-existent
> device or Function, completer will send an ERR_NONFATAL Message.
> On some platforms, ERR_NONFATAL results in a System Error, which
> breaks normal software probing.
>
> Advisory Non-Fatal Error(ANFE) is a special case that can be used
> in above scenario. It is predominantly determined by the role of the
> detecting agent (Requester, Completer, or Receiver) and the specific
> error. In such cases, an agent with AER signals the NFE (if enabled)
> by sending an ERR_COR Message as an advisory to software, instead of
> sending ERR_NONFATAL.
>
> When processing an ANFE, ideally both correctable error(CE) status and
> uncorrectable error(UE) status should be cleared. However, there is no
> way to fully identify the UE associated with ANFE. Even worse, Non-Fatal
> Error(NFE) may set the same UE status bit as ANFE. Treating an ANFE as
> NFE will reproduce above mentioned issue, i.e., breaking softwore probing;
> treating NFE as ANFE will make us ignoring some UEs which need active
> recover operation. To avoid clearing UEs that are not ANFE by accident,
> the most conservative route is taken here: If any of the NFE Detected
> bits is set in Device Status, do not touch UE status, they should be
> cleared later by the UE handler. Otherwise, a specific set of UEs that
> may be raised as ANFE according to the PCIe specification will be cleared
> if their corresponding severity is Non-Fatal.
>
> To achieve above purpose, store UNCOR_STATUS bits that might be ANFE
> in aer_err_info.anfe_status. So that those bits could be printed and
> processed later.
>
> Tested-by: Yudong Wang <yudong.wang@...el.com>
> Co-developed-by: "Wang, Qingshun" <qingshun.wang@...ux.intel.com>
> Signed-off-by: "Wang, Qingshun" <qingshun.wang@...ux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@...el.com>
> ---
> drivers/pci/pci.h | 1 +
> drivers/pci/pcie/aer.c | 53 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 54 insertions(+)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 17fed1846847..3f9eb807f9fd 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -412,6 +412,7 @@ struct aer_err_info {
>
> unsigned int status; /* COR/UNCOR Error Status */
> unsigned int mask; /* COR/UNCOR Error Mask */
> + unsigned int anfe_status; /* UNCOR Error Status for ANFE */
> struct pcie_tlp_log tlp; /* TLP Header */
> };
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index ac6293c24976..f2839b51321a 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -107,6 +107,12 @@ struct aer_stats {
> PCI_ERR_ROOT_MULTI_COR_RCV | \
> PCI_ERR_ROOT_MULTI_UNCOR_RCV)
>
> +#define AER_ERR_ANFE_UNC_MASK (PCI_ERR_UNC_POISON_TLP | \
> + PCI_ERR_UNC_COMP_TIME | \
> + PCI_ERR_UNC_COMP_ABORT | \
> + PCI_ERR_UNC_UNX_COMP | \
> + PCI_ERR_UNC_UNSUP)
> +
> static int pcie_aer_disable;
> static pci_ers_result_t aer_root_reset(struct pci_dev *dev);
>
> @@ -1196,6 +1202,49 @@ void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
> EXPORT_SYMBOL_GPL(aer_recover_queue);
> #endif
>
> +static void anfe_get_uc_status(struct pci_dev *dev, struct aer_err_info *info)
> +{
> + u32 uncor_mask, uncor_status, anfe_status;
> + u16 device_status;
> + int aer = dev->aer_cap;
> +
> + pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, &uncor_status);
> + pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &uncor_mask);
> + /*
> + * According to PCIe Base Specification Revision 6.1,
> + * Section 6.2.3.2.4, if an UNCOR error is raised as
> + * Advisory Non-Fatal error, it will match the following
> + * conditions:
> + * a. The severity of the error is Non-Fatal.
> + * b. The error is one of the following:
> + * 1. Poisoned TLP (Section 6.2.3.2.4.3)
> + * 2. Completion Timeout (Section 6.2.3.2.4.4)
> + * 3. Completer Abort (Section 6.2.3.2.4.1)
> + * 4. Unexpected Completion (Section 6.2.3.2.4.5)
> + * 5. Unsupported Request (Section 6.2.3.2.4.1)
> + */
> + anfe_status = uncor_status & ~uncor_mask & ~info->severity &
> + AER_ERR_ANFE_UNC_MASK;
> +
> + if (pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &device_status))
> + return;
> + /*
> + * Take the most conservative route here. If there are Non-Fatal errors
> + * detected, do not assume any bit in uncor_status is set by ANFE.
> + */
> + if (device_status & PCI_EXP_DEVSTA_NFED)
> + return;
You can move this check to the top of the function. You don't need to check
the rest if NFE error is detected in device status.
> +
> + /*
> + * If there is another ANFE between reading uncor_status and clearing
> + * PCI_ERR_COR_ADV_NFAT bit in cor_status register, that ANFE isn't
> + * recorded in info->anfe_status. It will be read out as NFE in
> + * following uncor_status register reading and processed by NFE
> + * handler.
> + */
> + info->anfe_status = anfe_status;
> +}
> +
> /**
> * aer_get_device_error_info - read error status from dev and store it to info
> * @dev: pointer to the device expected to have a error record
> @@ -1213,6 +1262,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>
> /* Must reset in this function */
> info->status = 0;
> + info->anfe_status = 0;
> info->tlp_header_valid = 0;
>
> /* The device might not support AER */
> @@ -1226,6 +1276,9 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
> &info->mask);
> if (!(info->status & ~info->mask))
> return 0;
> +
> + if (info->status & PCI_ERR_COR_ADV_NFAT)
> + anfe_get_uc_status(dev, info);
> } else if (type == PCI_EXP_TYPE_ROOT_PORT ||
> type == PCI_EXP_TYPE_RC_EC ||
> type == PCI_EXP_TYPE_DOWNSTREAM ||
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Powered by blists - more mailing lists