Re: [PATCH v20 1/9] PCI/AER: Introduce AER-CXL protocol error kfifo

[Jonathan Cameron]

On Wed, 2 Sep 2026 08:39:25 -0500
Terry Bowman <terry.bowman@xxxxxxx> wrote:

> CXL VH RAS handling requires the AER driver to hand off CXL protocol
> errors to cxl_core for logging and recovery before PCIe AER recovery
> tears down the device. Introduce pci/pcie/aer_cxl_vh.c to implement
> this handoff via a kfifo-backed work item.
> 
> The producer, cxl_forward_error(), is gated by is_cxl_error() and
> enqueues the error source PCI device and severity. cxl_core registers a
> consumer via cxl_register_proto_err_work(); the consumer drains the
> kfifo with for_each_cxl_proto_err(). For uncorrectable errors,
> cxl_proto_err_wait_for_empty() lets the AER path block until the CXL
> plane has finished so recovery does not race device teardown.

Do we need most of this last paragraph?
Maybe the bit about letting AER block but the rest smells like implementation
details to me with no info on 'why' or anything unexpected.
> 
> A rwsem serializes registration, deregistration, enqueue, and dequeue
> against concurrent AER IRQ threads; a spinlock serializes concurrent
> kfifo writers. is_aer_internal_error() moves into this file and now
> evaluates info->status & ~info->mask rather than the raw info->status,
> so a masked internal-error bit is treated as not-set. For the RCH RCEC
> path this is equivalent because cxl_rch_enable_rcec() first calls
> pci_aer_unmask_internal_errors(), which clears those mask bits in
> hardware before the AER status is read back.
> 
> A subsequent patch wires cxl_forward_error() into handle_error_source().
> 
> Add MAINTAINERS entries for aer_cxl_vh.c and aer_cxl_rch.c under the CXL
> entry.

I couldn't immediately find any discussion about switching away from panic
on a kfifo overflow.  Was there a reply to an earlier version with a
discussion of that?  I'm not against the change but a 'why' 
here would be good to have.

> 
> Co-developed-by: Dan Williams <djbw@xxxxxxxxxx>
> Signed-off-by: Dan Williams <djbw@xxxxxxxxxx>
> Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx>
One trivial thing inline to add to Ben's nits.
Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>

> +/**
> + * Callback for processing a CXL protocol error from the AER-CXL kfifo.
> + */
> +typedef void (*cxl_proto_err_fn_t)(struct cxl_proto_err_work_data *wd);
> +
> +void cxl_register_proto_err_work(struct work_struct *work,
> +				void (*flush)(void));

Align to after the ( Looks to be 1 space short. If this is a local
style thing ignore me ;)

> +void for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
> +			    cxl_proto_err_fn_t fn);
> +void cxl_unregister_proto_err_work(void);
> +#endif
> +
>  void pci_print_aer(struct pci_dev *dev, int aer_severity,
>  		    struct aer_capability_regs *aer);
>  int cper_severity_to_aer(int cper_severity);
> 
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935