Re: [RFC] cxl: Device protocol AER injection

Hi Terry,

On Fri, 17 Jul 2026 17:57:00 -0500, Terry Bowman wrote:
> This patch is intended to provide a method of testing the recently
> submitted cxl series "cxl: Enable CXL PCIe Port Protocol Error
> handling and logging"

Ran this on 7.2-rc3 with v18, under QEMU with a switch topology so all
four port classes are present.  v19 wasn't out yet when I ran it; the
findings below are in the RFC's own code and don't move with the delta.
It works -- one CE per class with a different RAS status bit each time,
attribution correct in all four:

  RP  ras=0x1  port=port1 dport=0000:0c:00.0  'Cache Data ECC Error'
  USP ras=0x2  port=port2 dport=              'Memory Data ECC Error'
  DSP ras=0x4  port=port2 dport=0000:0e:00.0  'CRC Threshold Hit'
  EP  ras=0x8  memdev=mem1 port=endpoint4     'Retry Threshold'

A UCE panics via cxl_pci_error_detected() as the series intends, and six
malformed inputs are rejected.  Only VH here, so I can't say anything
about whether the RCH branch is right.

Tested-by: Junjie Cao <[email protected]>

> +	depends on PCIEAER_INJECT

Beyond Jonathan's question about asking at all -- a bool depending on a
tristate means kconfig will give you PCIEAER_INJECT=m with CXL_BUS=y
and this =y.  olddefconfig produces that from a defconfig and it
doesn't link:

  ld: ras.c: undefined reference to `aer_inject'

Deriving it from the combination rather than asking would take the
broken config with the question.

> +	cxl_aer_einj.aer_registers[aer_offset] = aer_status;

Only reference to aer_registers[] in the tree.  aer_inject() builds
its own AER config space and the RCH path skips
cxl_rch_get_aer_info(), so nothing reads it.  Is it still needed, or
can it go with the core/ras_einj.c move?

> +	cxl_aer_einj.dev = NULL;

Clears the pointer but not the reference.  Every non-RCH arm takes a
pci_dev_get() below; to_einj_ras_base() on a match and
cxl_ras_exit() drop it.  Both need .dev to still point there.  So an
arm not consumed before the next write is orphaned.

Thirty back-to-back injections gave 26 trace events, with aer_inject
run 30 times and the kfifo-add-failed, port-not-found, port-unbound,
dport-not-found and RAS-not-mapped counts all zero (kfifo is 128 deep,
so no overflow).  Four arms were never disarmed.  I can't tell whether
the AER core folded them into one pass or the next write replaced .dev
first, and either way the reference goes.  I couldn't make it fire on
demand -- unbinding cxl_port fails the arm-time lookup instead -- so
it's a race, but it's the one a test loop hits.

Behind it the handshake has no single owner: the mutex is writer-only,
while to_einj_ras_base() mutates the same fields and drops a reference
without it.  Both consumers are live: the CE runs consume arms from
the kfifo work, and the UCE backtrace has cxl_pci_error_detected()
doing it from the AER IRQ thread.  A disarm helper
owning the reference would cover this and the asymmetry Jonathan
raised about putting a reference it didn't get.

> +	cxl_aer_einj.is_rch = (nargs == 5 && strcmp(topology, "RCH") == 0);

RCH on a VH device is accepted:

  # echo "0000:0c:00.0 CE 0 0x80 RCH" > .../aer_einj_inject   -> 0

aer_inject() fires and AER logs it, but no CXL trace event appears --
the RCH branch can't match a VH dport.  Reproduces every time.  So a
typo gets you a real AER error with the CXL path silently skipped,
which reads as "the handler didn't fire".  is_cxl_restricted(pdev),
already used further down the file, would make it -EINVAL.

> Also worth discussing is the commandline takes multiple parameters for
> a single sysfs file

One file also suits the state handling above better.  Several files
spread one request over several writes with no commit point.

> How do we incorporate this with the existing CXL EINJ functionality

core/ras_einj.c sounds right, and the Kconfig above is another reason
for both in one place.  Worth a line in the ABI doc that ACPI EINJ is
a real injection but Root Port only while this covers every class with
software registers -- that's what decides which one a test wants.

Last thing, off this patch: with the registers a kernel-side array this
can't exercise the MMIO readback, the offsets against a real capability
block, or the header log.  QEMU has RAS on root and switch ports but
injects only on endpoints, so it can't cover the classes today either.
I'm looking at adding port-level injection there as a second path
through the real reads.

Many thanks,
Junjie