Re: [PATCH 10/19] coresight: trbe: Always check fault action when updating buffer

On 01/12/2025 11:22, Leo Yan wrote:

The current code checks the fault action only via the IRQ status bit, which is unreliable due to possible hardware latency.

Move the fault action check out of the IRQ status condition. This also causes the buffer size to be calculated for non-WRAP and fault cases, which is fine since the write pointer is trusted for the calculation.

Signed-off-by: Leo Yan leo.yan@arm.com

drivers/hwtracing/coresight/coresight-trbe.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index f56ecdeaa6596afb440e4d53732e08a85f9bf89d..e579ea98523c24d23a0cd265dcdd0a46b52b52da 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -806,7 +806,6 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, enum trbe_fault_action act; unsigned long size, status; unsigned long flags;

• bool wrap = false;

WARN_ON(buf->cpudata != cpudata); WARN_ON(cpudata->cpu != smp_processor_id()); @@ -858,21 +857,11 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev, */ clr_trbe_irq(); isb();

• act = trbe_get_fault_act(handle, status);
  

• /*
  

•  * If this was not due to a WRAP event, we have some
  

•  * errors and as such buffer is empty.
  

•  */
  

• if (act != TRBE_FAULT_ACT_WRAP) {
  

• 	size = 0;
  

• 	goto done;
  

• }
  

• wrap = true;
  

  }

• size = trbe_get_trace_size(handle, buf, wrap);

• act = trbe_get_fault_act(handle, status);

Also act is valid only when there is an IRQ ?

• size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);

We have certain assumptions in trbe_get_trace_size(), which may be broken with this change.

e.g., if we get a fatal error, we don't detect that the buffer is empty and may trigger a WARN_ON() on systems with CPU erratum as below ?

    /*                                                                      
     * If the TRBE is affected by the following erratum, we must fill           
     * the space we skipped with IGNORE packets. And we are always          
     * guaranteed to have at least a PAGE_SIZE space in the buffer.         
     */                                                                     
    if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE) &&
        !WARN_ON(size < overwrite_skip))                                    
            __trbe_pad_buf(buf, start_off, overwrite_skip); 

Suzuki

done: local_irq_restore(flags);