lib: sbi: dbtr: do not unconditionally access tdata2/tdata3 CSRs

The current SBI DBTR extension implementation accesses tdata2 and tdata3
without first checking whether either register is implemented on the
underlying hart. This produces an illegal instruction exception on
otherwise spec-compliant cores that legitimately omit one or both
registers.

Per the RISC-V Debug Specification, Chapter 5 (Sdtrig ISA Extension)
and Section 5.7 (Trigger Module Registers):

  Section 5 (Sdtrig introduction):
    "If Sdtrig is implemented, the Trigger Module must support at least
     one trigger. Accessing trigger CSRs that are not used by any of the
     implemented triggers must result in an illegal instruction
     exception. M-Mode and Debug Mode accesses to trigger CSRs that are
     used by any of the implemented triggers must succeed, regardless of
     the current type of the currently selected trigger."

  Section 5.7 (Trigger Module Registers):
    "Attempts to access an unimplemented Trigger Module Register raise
     an illegal instruction exception."

Per-register optionality is also explicit:

  Section 5.7.3 (Trigger Data 2, at 0x7a2):
    "Trigger-specific data. It is optional if no implemented triggers
     use it."

  Section 5.7.4 (Trigger Data 3, at 0x7a3):
    "Trigger-specific data. It is optional if no implemented triggers
     use it."

  Section 5.7.17 (Trigger Extra (RV32), at 0x7a3), which also applies
  via textra64 on RV64:
    "All functionality in this register is optional. Any number of
     upper bits of mhvalue and svalue may be tied to 0. mhselect and
     sselect may only support 0 (ignore)."

Unconditionally accessing tdata2/tdata3 in the install/update/read/
uninstall paths causes SBI calls to fail with an illegal instruction
exception on hardware that does not implement one or both CSRs, even
if the supervisor-supplied trigger configuration does not require the
missing CSR(s).

This patch:

  1. Introduces tdata_read_safe() / tdata_write_safe() helpers that
     wrap csr_read_allowed / csr_write_allowed so that an illegal-
     instruction trap raised by an unimplemented CSR is caught locally
     rather than propagated. On the read path, a trapped read yields
     zero; on the write path, the trap is silently absorbed (writes to
     an unimplemented CSR are no-ops by definition). Every tdata2/tdata3
     read and write in the install/update/read/uninstall paths is
     converted to these helpers.

  2. On the install and update paths, rejects requests that program
     a non-zero trig_tdata2 or trig_tdata3 into an unimplemented CSR
     with SBI_ERR_NOT_SUPPORTED, matching the SBI spec
     wording in sections 19.4 / 19.5:

       "One of the trigger configuration can't be programmed due to
        unimplemented optional bits in tdata1, tdata2, or tdata3
        CSRs."

     Implementation status is probed once per call via the
     tdata_implemented() helper. This only catches the "whole CSR
     unimplemented" case; tied-off WARL bits inside an otherwise-
     implemented CSR are not caught here and would require programming
     the trigger and reading the value back for comparison, which can
     be addressed separately.

  3. Enable tdata3 configuration in the debug trigger install path.

References:
  - RISC-V Debug Specification, Chapter 5 (Sdtrig), sections 5, 5.7,
    5.7.3, 5.7.4, 5.7.17.
  - RISC-V SBI Specification v3.0, Chapter 19 (Debug Triggers
    Extension), sections 19.4, 19.5.

Fixes: 97f234f15c ("lib: sbi: Introduce the SBI debug triggers extension support")
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Suggested-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Signed-off-by: David E. Garcia Porras <david.garcia@aheadcomputing.com>
Reviewed-By: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260616170118.3515676-1-david.garcia@aheadcomputing.com
Signed-off-by: Anup Patel <anup@brainfault.org>

lib/sbi/sbi_dbtr.c

@@ -34,6 +34,25 @@ static unsigned long hart_state_ptr_offset;
 	sbi_scratch_write_type((__scratch), void *, hart_state_ptr_offset, \
 			       (__hart_state))
 
+#define tdata_read_safe(__csr)						\
+	({								\
+		struct sbi_trap_info __trap = {0};			\
+		csr_read_allowed((__csr), &__trap);			\
+	})
+
+#define tdata_write_safe(__csr, __value)				\
+	({								\
+		struct sbi_trap_info __trap = {0};			\
+		csr_write_allowed((__csr), &__trap, (__value));		\
+	})
+
+#define tdata_implemented(__csr)					\
+	({								\
+		struct sbi_trap_info __trap = {0};			\
+		csr_read_allowed((__csr), &__trap);			\
+		!__trap.cause;						\
+	})
+
 #define INDEX_TO_TRIGGER(_index)					\
 	({								\
 		struct sbi_dbtr_trigger *__trg = NULL;			\
@@ -418,7 +437,8 @@ static void dbtr_trigger_enable(struct sbi_dbtr_trigger *trig)
 	 */
 	csr_write(CSR_TSELECT, trig->index);
 	csr_write(CSR_TDATA1, 0x0);
-	csr_write(CSR_TDATA2, trig->tdata2);
+	tdata_write_safe(CSR_TDATA2, trig->tdata2);
+	tdata_write_safe(CSR_TDATA3, trig->tdata3);
 	csr_write(CSR_TDATA1, trig->tdata1);
 }
 
@@ -463,7 +483,8 @@ static void dbtr_trigger_clear(struct sbi_dbtr_trigger *trig)
 
 	csr_write(CSR_TSELECT, trig->index);
 	csr_write(CSR_TDATA1, 0x0);
-	csr_write(CSR_TDATA2, 0x0);
+	tdata_write_safe(CSR_TDATA2, 0x0);
+	tdata_write_safe(CSR_TDATA3, 0x0);
 }
 
 static int dbtr_trigger_supported(unsigned long type)
@@ -566,8 +587,8 @@ int sbi_dbtr_read_trig(unsigned long smode,
 		trig = INDEX_TO_TRIGGER((_idx + trig_idx_base));
 		csr_write(CSR_TSELECT, trig->index);
 		trig->tdata1 = csr_read(CSR_TDATA1);
-		trig->tdata2 = csr_read(CSR_TDATA2);
+		trig->tdata2 = tdata_read_safe(CSR_TDATA2);
-		trig->tdata3 = csr_read(CSR_TDATA3);
+		trig->tdata3 = tdata_read_safe(CSR_TDATA3);
 		xmit->tstate = cpu_to_lle(trig->state);
 		xmit->tdata1 = cpu_to_lle(trig->tdata1);
 		xmit->tdata2 = cpu_to_lle(trig->tdata2);
@@ -589,6 +610,7 @@ int sbi_dbtr_install_trig(unsigned long smode,
 	unsigned long ctrl;
 	struct sbi_dbtr_trigger *trig;
 	struct sbi_dbtr_hart_triggers_state *hs = NULL;
+	bool tdata2_impl, tdata3_impl;
 
 	hs = dbtr_thishart_state_ptr();
 	if (!hs)
@@ -601,6 +623,15 @@ int sbi_dbtr_install_trig(unsigned long smode,
 	sbi_hart_protection_map_range((unsigned long)shmem_base,
 				      trig_count * sizeof(*entry));
 
+	/*
+	 * SBI v3.0 sec 19.4 requires SBI_ERR_NOT_SUPPORTED when a trigger
+	 * programs a non-zero value into an unimplemented optional CSR. Only
+	 * the "whole CSR unimplemented" case is caught; WARL bits tied off
+	 * inside an otherwise-implemented CSR are not.
+	 */
+	tdata2_impl = tdata_implemented(CSR_TDATA2);
+	tdata3_impl = tdata_implemented(CSR_TDATA3);
+
 	/* Check requested triggers configuration */
 	for_each_trig_entry(shmem_base, trig_count, typeof(*entry), entry) {
 		recv = (struct sbi_dbtr_data_msg *)(&entry->data);
@@ -619,6 +650,14 @@ int sbi_dbtr_install_trig(unsigned long smode,
 							trig_count * sizeof(*entry));
 			return SBI_ERR_FAILED;
 		}
+
+		if ((recv->tdata2 && !tdata2_impl) ||
+		    (recv->tdata3 && !tdata3_impl)) {
+			*out = _idx;
+			sbi_hart_protection_unmap_range((unsigned long)shmem_base,
+							trig_count * sizeof(*entry));
+			return SBI_ERR_NOT_SUPPORTED;
+		}
 	}
 
 	if (hs->available_trigs < trig_count) {
@@ -705,6 +744,7 @@ int sbi_dbtr_update_trig(unsigned long smode,
 	union sbi_dbtr_shmem_entry *entry;
 	void *shmem_base = NULL;
 	struct sbi_dbtr_hart_triggers_state *hs = NULL;
+	bool tdata2_impl, tdata3_impl;
 
 	hs = dbtr_thishart_state_ptr();
 	if (!hs)
@@ -718,6 +758,15 @@ int sbi_dbtr_update_trig(unsigned long smode,
 	if (trig_count >= hs->total_trigs)
 		return SBI_ERR_BAD_RANGE;
 
+	/*
+	 * SBI v3.0 sec 19.5 requires SBI_ERR_NOT_SUPPORTED when a trigger
+	 * programs a non-zero value into an unimplemented optional CSR. Only
+	 * the "whole CSR unimplemented" case is caught; WARL bits tied off
+	 * inside an otherwise-implemented CSR are not.
+	 */
+	tdata2_impl = tdata_implemented(CSR_TDATA2);
+	tdata3_impl = tdata_implemented(CSR_TDATA3);
+
 	for_each_trig_entry(shmem_base, trig_count, typeof(*entry), entry) {
 		sbi_hart_protection_map_range((unsigned long)entry, sizeof(*entry));
 		trig_idx = entry->id.idx;
@@ -734,6 +783,12 @@ int sbi_dbtr_update_trig(unsigned long smode,
 			return SBI_ERR_FAILED;
 		}
 
+		if ((entry->data.tdata2 && !tdata2_impl) ||
+		    (entry->data.tdata3 && !tdata3_impl)) {
+			sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
+			return SBI_ERR_NOT_SUPPORTED;
+		}
+
 		dbtr_trigger_setup(trig, &entry->data);
 		sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
 		dbtr_trigger_enable(trig);