Re: [PATCH v2] coresight-etm4x: Conditionally access register TRCEXTINSELR

On Thu, 14 Aug 2025 at 10:51, James Clark james.clark@linaro.org wrote:

On 12/08/2025 9:24 am, Yuanfang Zhang wrote:

...

The TRCEXTINSELR is only implemented if TRCIDR5.NUMEXTINSEL > 0. To avoid invalid accesses, introduce a check on numextinsel (derived from TRCIDR5[11:9]) before reading or writing to this register.

Fixes: f5bd523690d2 ("coresight: etm4x: Convert all register accesses")

This tag isn't right. Although this is where the register accesses were last touched, the root issue was present from the introduction of the driver.

Memory mapped access to unimplemented registers are RES0 so won't fail - the issue is the system register access where an undefined exception can be triggered.

Mike

...

Signed-off-by: Yuanfang Zhang yuanfang.zhang@oss.qualcomm.com

Changes in v2:

• Add fixes tag.
• Replace "if (drvdata->nrseqstate)" with "if (drvdata->numextinsel)"
• Link to v1: https://lore.kernel.org/r/20250811-trcextinselr_issue-v1-1-ed78f3215502@oss....

---

drivers/hwtracing/coresight/coresight-etm4x-core.c | 11 ++++++++--- drivers/hwtracing/coresight/coresight-etm4x.h | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 42e5d37403addc6ec81f2e3184522d67d1677c04..4e411427303981104d11720d3c73af91030f8df3 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -528,7 +528,8 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR); etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR); }

• etm4x_relaxed_write32(csa, config->ext_inp, TRCEXTINSELR);
  

• if (drvdata->numextinsel)
  

•         etm4x_relaxed_write32(csa, config->ext_inp, TRCEXTINSELR);
  for (i = 0; i < drvdata->nr_cntr; i++) {
          etm4x_relaxed_write32(csa, config->cntrldvr[i], TRCCNTRLDVRn(i));
          etm4x_relaxed_write32(csa, config->cntr_ctrl[i], TRCCNTCTLRn(i));
  

@@ -1423,6 +1424,7 @@ static void etm4_init_arch_data(void *info) etmidr5 = etm4x_relaxed_read32(csa, TRCIDR5); /* NUMEXTIN, bits[8:0] number of external inputs implemented */ drvdata->nr_ext_inp = FIELD_GET(TRCIDR5_NUMEXTIN_MASK, etmidr5);

• drvdata->numextinsel = FIELD_GET(TRCIDR5_NUMEXTINSEL_MASK, etmidr5);
  /* TRACEIDSIZE, bits[21:16] indicates the trace ID width */
  drvdata->trcid_size = FIELD_GET(TRCIDR5_TRACEIDSIZE_MASK, etmidr5);
  /* ATBTRIG, bit[22] implementation can support ATB triggers? */
  

@@ -1852,7 +1854,9 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR); state->trcseqstr = etm4x_read32(csa, TRCSEQSTR); }

• state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR);
  

• if (drvdata->numextinsel)
  

•         state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR);
  
  for (i = 0; i < drvdata->nr_cntr; i++) {
          state->trccntrldvr[i] = etm4x_read32(csa, TRCCNTRLDVRn(i));
  

@@ -1984,7 +1988,8 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR); etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR); }

• etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR);
  

• if (drvdata->numextinsel)
  

•         etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR);
  
  for (i = 0; i < drvdata->nr_cntr; i++) {
          etm4x_relaxed_write32(csa, state->trccntrldvr[i], TRCCNTRLDVRn(i));
  

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index ac649515054d905fa365203bd35f1d839b03292f..823914fefa90a36a328b652b0dc3828b9bddd990 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -162,6 +162,7 @@ #define TRCIDR4_NUMVMIDC_MASK GENMASK(31, 28)

#define TRCIDR5_NUMEXTIN_MASK GENMASK(8, 0) +#define TRCIDR5_NUMEXTINSEL_MASK GENMASK(11, 9) #define TRCIDR5_TRACEIDSIZE_MASK GENMASK(21, 16) #define TRCIDR5_ATBTRIG BIT(22) #define TRCIDR5_LPOVERRIDE BIT(23) @@ -999,6 +1000,7 @@ struct etmv4_drvdata { u8 nr_cntr; u8 nr_ext_inp; u8 numcidc;

• u8                              numextinsel;
  u8                              numvmidc;
  u8                              nrseqstate;
  u8                              nr_event;
  
  

---

base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 change-id: 20250811-trcextinselr_issue-f267afa0e5ed

Best regards,