Re: [PATCH 2/2] PCI/AER: Check validity of Error Source ID register

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

On Tue, 3 Nov 2026, Bjorn Helgaas wrote:

> On Tue, Nov 03, 2026 at 02:57:58PM +0200, Ilpo Järvinen wrote:
> > On Tue, 3 Nov 2026, Lukas Wunner wrote:
> > 
> > > Certain Intel and AMD Root Ports are known to populate the Error Source ID
> > > register with an invalid value.  The AER driver seeks to recognize such
> > > buggy silicon by assuming invalidity if the bus number is zero.  It then
> > > searches for the error-signaling device in the hierarchy below the Root
> > > Port.  This was introduced by commit 28eb27cf0839 ("PCI AER: support
> > > invalid error source IDs").
> > > 
> > > Alex has provided dmesg output which shows an invalid Error Source ID with
> > > a non-zero bus number.  The AER driver's heuristic fails to recognize its
> > > invalidity:
> > > 
> > >   pcieport 0000:00:07.1: AER: Correctable Error message received from 0000:01:00.0 (no details found)
> > > 
> > > The Root Port at 0000:00:07.1 has subordinate bus 04, so an error source
> 
> I always think "subordinate" is just slightly ambiguous because it's
> used for both "anything below this bridge" and the more specific
> "Subordinate Bus Number" (the highest numbered bus below the bridge).
> 
> It's perfectly accurate as-is especially because in this case it looks
> like Secondary Bus Number and Subordinate Bus Number are *both* 04 for
> 00:07.1.  But "secondary bus 04" seems less ambiguous to me.
> 
> > > on bus 01 is obviously invalid.  The range of subordinate buses is cached
> > > and checking against it is cheap.
> > ...
> 
> > > +	return root->subordinate->busn_res.start <= PCI_BUS_NUM(id) &&
> > > +	       root->subordinate->busn_res.end   >= PCI_BUS_NUM(id);
> > 
> > I kind of hate the order of this logic. I understand you're probably 
> > doing it for the alignment which has some value but...
> > 
> > ...For me, the more logical order would be to keep these in increasing 
> > order even if doing it results in losing the alignment:
> > 
> >    start <= val &&
> >    val <= end
> > 
> > Maybe that's just me and it's not an end of the world either way.
> 
> I agree, I think "start <= val && val <= end" is more readable, though
> not quite as pretty.

I suppose a local var could be used to shorten the derefs and it would 
even fit to a single line then.

-- 
 i.