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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1eaf1f94-e26b-4313-b6b7-51ad966fe28e@linux.alibaba.com>
Date: Fri, 24 Oct 2025 11:09:25 +0800
From: Shuai Xue <xueshuai@...ux.alibaba.com>
To: Lukas Wunner <lukas@...ner.de>, Bjorn Helgaas <bhelgaas@...gle.com>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
 linuxppc-dev@...ts.ozlabs.org, kbusch@...nel.org,
 sathyanarayanan.kuppuswamy@...ux.intel.com, mahesh@...ux.ibm.com,
 oohall@...il.com, Jonathan.Cameron@...wei.com, terry.bowman@....com,
 tianruidong@...ux.alibaba.com
Subject: Re: [PATCH v6 4/5] PCI/ERR: Use pcie_aer_is_native() to check for
 native AER control



在 2025/10/23 18:29, Lukas Wunner 写道:
> On Mon, Oct 20, 2025 at 10:45:31PM +0800, Shuai Xue wrote:
>> 	if (host->native_aer || pcie_ports_native) {
>> 		pcie_clear_device_status(bridge);
>> 		pci_aer_clear_nonfatal_status(bridge);
>> 	}
>>
>> This code clears both the PCIe Device Status register and AER status
>> registers when in native AER mode.
>>
>> pcie_clear_device_status() is renamed from
>> pci_aer_clear_device_status(). Does it intends to clear only AER error
>> status?
>>
>> - BIT 0: Correctable Error Detected
>> - BIT 1: Non-Fatal Error Detected
>> - BIT 2: Fatal Error Detected
>> - BIT 3: Unsupported Request Detected
>>
>>  From PCIe spec, BIT 0-2 are logged for functions supporting Advanced
>> Error Handling.
>>
>> I am not sure if we should clear BIT 3, and also BIT 6 (Emergency Power
>> Reduction Detected) and in case a AER error.
> 
> AFAIUI, bits 0 to 3 are what the PCIe r7.0 sec 6.2.1 calls
> "baseline capability" error reporting.  They're supported
> even if AER is not supported.
> 
> Bit 6 has nothing to do with this AFAICS.

Hi, Lukas,

Per PCIe r7.0 section 7.5.3.5:

   **For Functions supporting Advanced Error Handling**, errors are logged
   in this register regardless of the settings of the Uncorrectable Error
   Mask register. Default value of this bit is 0b.

 From this, it's clear that bits 0 to 2 are not logged unless AER is supported.

So, if dev->aer_cap is not true, there’s no need to clear bits 0 to 2.
This validates the dev->aer_cap sanity check in pcie_aer_is_native():

   int pcie_aer_is_native(struct pci_dev *dev)
   {
       struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);

       if (!dev->aer_cap)
           return 0;

       return pcie_ports_native || host->native_aer;
   }
   EXPORT_SYMBOL_NS_GPL(pcie_aer_is_native, "CXL");

Based on this, the introduction of pcie_aer_is_native() in the patch
seems reasonable and consistent with the PCIe specification.

Further, should we rename pcie_clear_device_status() back to
pci_aer_clear_device_status():

-void pcie_clear_device_status(struct pci_dev *dev)
+void pci_aer_clear_device_status(struct pci_dev *dev)
  {
         u16 sta;

         pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &sta);
-       pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta);
+       /* Bits 0-2 are logged if AER is supported */
+       pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta & 0x7);
  }

I am still uncertain whether bit 3 ("Unsupported Request Detected")
should be cleared in this function. It’s not directly tied to AER
capability.


I’d love to hear your thoughts, as well as @Bjorn’s, on both the renaming
suggestion and whether bit 3 should be cleared alongside bits 0 to 2.

Thanks.
Shuai

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ