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

[Bowman, Terry]

On 9/7/2026 7:51 PM, Jonathan Cameron wrote:
> 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.


It is implementation centric. This can be reduced.

>>
>> 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.
> 

The driver panics on kfifo full error during UCE enqueue. This was recommended by you and
Richard. Changes are at the link below in cxl_forward_error():
https://lore.kernel.org/linux-cxl/20260902133933.2992457-2-terry.bowman@xxxxxxx/

>>
>> 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 ;)
> 

Ok.

-Terry

>> +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
>