On Wed, May 21, 2025 at 10:00:35AM +0100, Jonathan Cameron wrote:
> On Tue, 20 May 2025 16:50:19 -0500
> Bjorn Helgaas <helg...@kernel.org> wrote:
>
> > From: Bjorn Helgaas <bhelg...@google.com>
> >
> > DPC Error Source ID is only valid when the DPC Trigger Reason indicates
> > that DPC was triggered due to reception of an ERR_NONFATAL or ERR_FATAL
> > Message (PCIe r6.0, sec 7.9.14.5).
> >
> > When DPC was triggered by ERR_NONFATAL (PCI_EXP_DPC_STATUS_TRIGGER_RSN_NFE)
> > or ERR_FATAL (PCI_EXP_DPC_STATUS_TRIGGER_RSN_FE) from a downstream device,
> > log the Error Source ID (decoded into domain/bus/device/function). Don't
> > print the source otherwise, since it's not valid.
> >
> > For DPC trigger due to reception of ERR_NONFATAL or ERR_FATAL, the dmesg
> > logging changes:
> >
> > - pci 0000:00:01.0: DPC: containment event, status:0x000d source:0x0200
> > - pci 0000:00:01.0: DPC: ERR_FATAL detected
> > + pci 0000:00:01.0: DPC: containment event, status:0x000d, ERR_FATAL
> > received from 0000:02:00.0
> >
> > and when DPC triggered for other reasons, where DPC Error Source ID is
> > undefined, e.g., unmasked uncorrectable error:
> >
> > - pci 0000:00:01.0: DPC: containment event, status:0x0009 source:0x0200
> > - pci 0000:00:01.0: DPC: unmasked uncorrectable error detected
> > + pci 0000:00:01.0: DPC: containment event, status:0x0009: unmasked
> > uncorrectable error detected
> >
> > Previously the "containment event" message was at KERN_INFO and the
> > "%s detected" message was at KERN_WARNING. Now the single message is at
> > KERN_WARNING.
> >
> > Signed-off-by: Bjorn Helgaas <bhelg...@google.com>
> > Tested-by: Krzysztof Wilczyński <kwilczyn...@kernel.org>
> > Reviewed-by: Kuppuswamy Sathyanarayanan
> > <sathyanarayanan.kuppusw...@linux.intel.com>
> Matches the spec conditions as far as I can tell.
>
> I guess interesting debate on whether providing extra garbage info is
> a bug or not. Maybe a fixes tag for this one as well?
I added:
Fixes: 26e515713342 ("PCI: Add Downstream Port Containment driver")
since it looks like we've printed the source unconditionally since the
addition of DPC:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pcie/pcie-dpc.c?id=26e515713342#n69
> Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
>
> I briefly wondered if it makes sense to have a prefix string initialized
> outside the switch with "containment event, status:%#06x:"
> made sense but it's probably not worth the effort and maybe makes it
> harder to grep for the error messages. So in the end
> I think your code here is the best option.
>
> Jonathan
