linux-kernel - Re: [PATCH v6 3/5] PCI/AER: Report fatal errors of RCiEP and EP if link recoverd

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: <aPoIDW_Yt90VgHL8@wunner.de>
Date: Thu, 23 Oct 2025 12:48:45 +0200
From: Lukas Wunner <lukas@...ner.de>
To: Shuai Xue <xueshuai@...ux.alibaba.com>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org, bhelgaas@...gle.com,
	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 3/5] PCI/AER: Report fatal errors of RCiEP and EP if
 link recoverd

On Mon, Oct 20, 2025 at 11:20:58PM +0800, Shuai Xue wrote:
> 2025/10/20 22:24, Lukas Wunner:
> > On Mon, Oct 20, 2025 at 10:17:10PM +0800, Shuai Xue wrote:
> > > > >     .slot_reset()
> > > > >       => pci_restore_state()
> > > > >         => pci_aer_clear_status()
> > > > 
> > > > This was added in 2015 by b07461a8e45b.  The commit claims that
> > > > the errors are stale and can be ignored.  It turns out they cannot.
> > > > 
> > > > So maybe pci_restore_state() should print information about the
> > > > errors before clearing them?
> > > 
> > > While that could work, we would lose the error severity information at
> > 
> > Wait, we've got that saved in pci_cap_saved_state, so we could restore
> > the severity register, report leftover errors, then clear those errors?
> 
> You're right that the severity register is also sticky, so we could
> retrieve error severity directly from AER registers.
> 
> However, I have concerns about implementing this approach:
[...]
> 3. Architectural consistency: As you noted earlier, "pci_restore_state()
> is only supposed to restore state, as the name implies, and not clear
> errors." Adding error reporting to this function would further violate
> this principle - we'd be making it do even more than just restore state.
> 
> Would you prefer I implement this broader change, or shall we proceed
> with the targeted helper function approach for now? The helper function
> solves the immediate problem while keeping the changes focused on the
> AER recovery path.

My opinion is that b07461a8e45b was wrong and that reported errors
should not be silently ignored.  What I'd prefer is that if
pci_restore_state() discovers unreported errors, it asks the AER driver
to report them.

We've already got a helper to do that:  aer_recover_queue()
It queues up an entry in AER's kfifo and asks AER to report it.

So far the function is only used by GHES.  GHES allocates the
aer_regs argument from ghes_estatus_pool using gen_pool_alloc().
Consequently aer_recover_work_func() uses ghes_estatus_pool_region_free()
to free the allocation.  That prevents using aer_recover_queue()
for anything else than GHES.  It would first be necessary to
refactor aer_recover_queue() + aer_recover_work_func() such that
it can cope with arbitrary allocations (e.g. kmalloc()).

Thanks,

Lukas