diff mbox series

lib: sbi: Fix DBTR error codes and semantics as per SBI v3.0 spec

Message ID 20260814172329.52258-1-david.garcia@aheadcomputing.com
State New
Headers show
Series lib: sbi: Fix DBTR error codes and semantics as per SBI v3.0 spec | expand

Commit Message

David E. Garcia Porras Aug. 14, 2026, 5:23 p.m. UTC 
Align the Debug Triggers (DBTR) extension implementation with the
error codes and behavior required by the SBI v3.0 specification,
chapter 19:

 - sbi_debug_set_shmem (sec 19.2): accept the flags parameter and
   return SBI_ERR_INVALID_PARAM when it is not zero. Also validate
   the entire shared memory range of trig_max * (XLEN / 2) bytes
   instead of only the base address, returning SBI_ERR_INVALID_ADDRESS
   when the range is not accessible.

 - sbi_debug_install_triggers (sec 19.4, table 101): return
   SBI_ERR_BAD_RANGE when trig_count >= trig_max, and return
   SBI_ERR_NOT_SUPPORTED instead of SBI_ERR_FAILED when the trigger
   type is not supported.

 - sbi_debug_update_triggers (sec 19.5, table 102): return the array
   index of the failing trigger configuration in sbiret.value, and
   return SBI_ERR_INVALID_PARAM when trig_tdata1.type or
   trig_tdata1.chain do not match the originally installed debug
   trigger. Validate the complete request before updating any trigger.

 - sbi_debug_uninstall_triggers, sbi_debug_enable_triggers and
   sbi_debug_disable_triggers (secs 19.6-19.8, tables 103-105): return
   SBI_ERR_INVALID_PARAM when any trigger in the set has
   trig_idx >= trig_max, and validate the complete set before acting
   on any trigger so a partially processed batch is not left behind.

 - Set the trig_state.have_hw_trig bit when a trigger is mapped to a
   HW debug trigger (table 98).

Signed-off-by: David E. Garcia Porras <david.garcia@aheadcomputing.com>
---
 include/sbi/sbi_dbtr.h   |  5 ++-
 lib/sbi/sbi_dbtr.c       | 91 +++++++++++++++++++++++++++++++++-------
 lib/sbi/sbi_ecall_dbtr.c |  4 +-
 3 files changed, 81 insertions(+), 19 deletions(-)

Comments

Anup Patel Sept. 4, 2026, 7:59 a.m. UTC | #1 
On Fri, Aug 14, 2026 at 10:54 PM David E. Garcia Porras
<david.garcia@aheadcomputing.com> wrote:
>
> Align the Debug Triggers (DBTR) extension implementation with the
> error codes and behavior required by the SBI v3.0 specification,
> chapter 19:
>
>  - sbi_debug_set_shmem (sec 19.2): accept the flags parameter and
>    return SBI_ERR_INVALID_PARAM when it is not zero. Also validate
>    the entire shared memory range of trig_max * (XLEN / 2) bytes
>    instead of only the base address, returning SBI_ERR_INVALID_ADDRESS
>    when the range is not accessible.
>
>  - sbi_debug_install_triggers (sec 19.4, table 101): return
>    SBI_ERR_BAD_RANGE when trig_count >= trig_max, and return
>    SBI_ERR_NOT_SUPPORTED instead of SBI_ERR_FAILED when the trigger
>    type is not supported.
>
>  - sbi_debug_update_triggers (sec 19.5, table 102): return the array
>    index of the failing trigger configuration in sbiret.value, and
>    return SBI_ERR_INVALID_PARAM when trig_tdata1.type or
>    trig_tdata1.chain do not match the originally installed debug
>    trigger. Validate the complete request before updating any trigger.
>
>  - sbi_debug_uninstall_triggers, sbi_debug_enable_triggers and
>    sbi_debug_disable_triggers (secs 19.6-19.8, tables 103-105): return
>    SBI_ERR_INVALID_PARAM when any trigger in the set has
>    trig_idx >= trig_max, and validate the complete set before acting
>    on any trigger so a partially processed batch is not left behind.

Too many changes in a single patch. Please try to split each change
into separate patch.

Regards,
Anup

>
>  - Set the trig_state.have_hw_trig bit when a trigger is mapped to a
>    HW debug trigger (table 98).
>
> Signed-off-by: David E. Garcia Porras <david.garcia@aheadcomputing.com>
> ---
>  include/sbi/sbi_dbtr.h   |  5 ++-
>  lib/sbi/sbi_dbtr.c       | 91 +++++++++++++++++++++++++++++++++-------
>  lib/sbi/sbi_ecall_dbtr.c |  4 +-
>  3 files changed, 81 insertions(+), 19 deletions(-)
>
> diff --git a/include/sbi/sbi_dbtr.h b/include/sbi/sbi_dbtr.h
> index 5e0bf84e..e6b76086 100644
> --- a/include/sbi/sbi_dbtr.h
> +++ b/include/sbi/sbi_dbtr.h
> @@ -104,7 +104,8 @@ int sbi_dbtr_init(struct sbi_scratch *scratch, bool coldboot);
>  int sbi_dbtr_supported(void);
>  int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
>                          unsigned long shmem_phys_lo,
> -                        unsigned long shmem_phys_hi);
> +                        unsigned long shmem_phys_hi,
> +                        unsigned long flags);
>  int sbi_dbtr_num_trig(unsigned long trig_tdata1, unsigned long *out);
>  int sbi_dbtr_read_trig(unsigned long smode,
>                        unsigned long trig_idx_base, unsigned long trig_count);
> @@ -115,7 +116,7 @@ int sbi_dbtr_uninstall_trig(unsigned long trig_idx_base,
>  int sbi_dbtr_enable_trig(unsigned long trig_idx_base,
>                          unsigned long trig_idx_mask);
>  int sbi_dbtr_update_trig(unsigned long smode,
> -                        unsigned long trig_count);
> +                        unsigned long trig_count, unsigned long *out);
>  int sbi_dbtr_disable_trig(unsigned long trig_idx_base,
>                           unsigned long trig_idx_mask);
>
> diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
> index 01047969..1d4785c7 100644
> --- a/lib/sbi/sbi_dbtr.c
> +++ b/lib/sbi/sbi_dbtr.c
> @@ -261,8 +261,10 @@ int sbi_dbtr_get_total_triggers(void)
>
>  int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
>                          unsigned long shmem_phys_lo,
> -                        unsigned long shmem_phys_hi)
> +                        unsigned long shmem_phys_hi,
> +                        unsigned long flags)
>  {
> +       unsigned long shmem_size;
>         struct sbi_dbtr_hart_triggers_state *hart_state;
>
>         if (dom && !sbi_domain_is_assigned_hart(dom, current_hartindex())) {
> @@ -271,6 +273,9 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
>                 return SBI_ERR_DENIED;
>         }
>
> +       if (flags)
> +               return SBI_ERR_INVALID_PARAM;
> +
>         hart_state = dbtr_thishart_state_ptr();
>         if (!hart_state)
>                 return SBI_ERR_FAILED;
> @@ -304,9 +309,11 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
>         if (shmem_phys_hi)
>                 return SBI_EINVALID_ADDR;
>
> -       if (dom && !sbi_domain_check_addr(dom,
> -                 DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
> -                 SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
> +       /* Must fail if address *range* is not fully accessible - size is assumed to be trig_max * (XLEN /2) bytes */
> +       shmem_size = (unsigned long) hart_state->total_trigs * (__riscv_xlen / 2);
> +       if (dom && !sbi_domain_check_addr_range(dom,
> +                 DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), shmem_size,
> +                 smode, SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
>                 return SBI_ERR_INVALID_ADDRESS;
>
>         hart_state->shmem.phys_lo = shmem_phys_lo;
> @@ -332,6 +339,7 @@ static void dbtr_trigger_setup(struct sbi_dbtr_trigger *trig,
>         trig->state = 0;
>
>         __set_bit(RV_DBTR_BIT(TS, MAPPED), &trig->state);
> +       __set_bit(RV_DBTR_BIT(TS, HAVE_TRIG), &trig->state);
>
>         SET_TRIG_HW_INDEX(trig->state, trig->index);
>
> @@ -616,6 +624,9 @@ int sbi_dbtr_install_trig(unsigned long smode,
>         if (!hs)
>                 return SBI_ERR_FAILED;
>
> +       if (trig_count >= hs->total_trigs)
> +               return SBI_ERR_BAD_RANGE;
> +
>         if (sbi_dbtr_shmem_disabled(hs))
>                 return SBI_ERR_NO_SHMEM;
>
> @@ -641,7 +652,7 @@ int sbi_dbtr_install_trig(unsigned long smode,
>                         *out = _idx;
>                         sbi_hart_protection_unmap_range((unsigned long)shmem_base,
>                                                         trig_count * sizeof(*entry));
> -                       return SBI_ERR_FAILED;
> +                       return SBI_ERR_NOT_SUPPORTED;
>                 }
>
>                 if (!dbtr_trigger_valid(TDATA1_GET_TYPE(ctrl), ctrl)) {
> @@ -702,13 +713,17 @@ int sbi_dbtr_uninstall_trig(unsigned long trig_idx_base,
>         if (!hs)
>                 return SBI_ERR_FAILED;
>
> -       for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
> +       for_each_set_bit_from(idx, &trig_mask, RV_MAX_TRIGGERS) {
>                 trig = INDEX_TO_TRIGGER(idx);
> -               if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)))
> +               if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)) || idx >= hs->total_trigs)
>                         return SBI_ERR_INVALID_PARAM;
> +       }
>
> +       /* Only uninstall after validating the arguments - maintain atomicity */
> +       idx = trig_idx_base;
> +       for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
> +               trig = INDEX_TO_TRIGGER(idx);
>                 dbtr_trigger_clear(trig);
> -
>                 sbi_free_trigger(trig);
>         }
>
> @@ -727,6 +742,14 @@ int sbi_dbtr_enable_trig(unsigned long trig_idx_base,
>         if (!hs)
>                 return SBI_ERR_FAILED;
>
> +       for_each_set_bit_from(idx, &trig_mask, RV_MAX_TRIGGERS) {
> +               trig = INDEX_TO_TRIGGER(idx);
> +               if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)) || idx >= hs->total_trigs)
> +                       return SBI_ERR_INVALID_PARAM;
> +       }
> +
> +       /* Only enable after validating the arguments - maintain atomicity */
> +       idx = trig_idx_base;
>         for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
>                 trig = INDEX_TO_TRIGGER(idx);
>                 sbi_dprintf("%s: enable trigger %lu\n", __func__, idx);
> @@ -737,8 +760,9 @@ int sbi_dbtr_enable_trig(unsigned long trig_idx_base,
>  }
>
>  int sbi_dbtr_update_trig(unsigned long smode,
> -                        unsigned long trig_count)
> +                        unsigned long trig_count, unsigned long *out)
>  {
> +       int stat = SBI_SUCCESS;
>         unsigned long trig_idx;
>         struct sbi_dbtr_trigger *trig;
>         union sbi_dbtr_shmem_entry *entry;
> @@ -772,23 +796,52 @@ int sbi_dbtr_update_trig(unsigned long smode,
>                 trig_idx = entry->id.idx;
>
>                 if (trig_idx >= hs->total_trigs) {
> -                       sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
> -                       return SBI_ERR_INVALID_PARAM;
> +                       stat = SBI_ERR_INVALID_PARAM;
> +                       goto endloop;
>                 }
>
>                 trig = INDEX_TO_TRIGGER(trig_idx);
>
>                 if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) {
> -                       sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
> -                       return SBI_ERR_FAILED;
> +                       stat = SBI_ERR_FAILED;
> +                       goto endloop;
> +               }
> +
> +               if (TDATA1_GET_TYPE(entry->data.tdata1) != TDATA1_GET_TYPE(trig->tdata1)) {
> +                       stat = SBI_ERR_INVALID_PARAM;
> +                       goto endloop;
> +               }
> +
> +               switch(TDATA1_GET_TYPE(entry->data.tdata1)) {
> +                       case RISCV_DBTR_TRIG_MCONTROL:
> +                       case RISCV_DBTR_TRIG_MCONTROL6:
> +                               if ( (entry->data.tdata1 & RV_DBTR_BIT_MASK(MC6, CHAIN)) != (trig->tdata1 & RV_DBTR_BIT_MASK(MC6, CHAIN)) ) {
> +                                       stat = SBI_ERR_INVALID_PARAM;
> +                                       goto endloop;
> +                               }
> +                               break;
> +                       default:
> +                               break;
>                 }
>
>                 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;
> +                       stat = SBI_ERR_NOT_SUPPORTED;
> +                       goto endloop;
>                 }
>
> +               endloop:
> +                       sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
> +                       if (stat != SBI_SUCCESS) {
> +                               *out = _idx;
> +                               return stat;
> +                       }
> +       }
> +
> +       /* Only update after validating request - maintain atomicity */
> +       for_each_trig_entry(shmem_base, trig_count, typeof(*entry), entry) {
> +               sbi_hart_protection_map_range((unsigned long)entry, sizeof(*entry));
> +               trig = INDEX_TO_TRIGGER(entry->id.idx);
>                 dbtr_trigger_setup(trig, &entry->data);
>                 sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
>                 dbtr_trigger_enable(trig);
> @@ -809,6 +862,14 @@ int sbi_dbtr_disable_trig(unsigned long trig_idx_base,
>         if (!hs)
>                 return SBI_ERR_FAILED;
>
> +       for_each_set_bit_from(idx, &trig_mask, RV_MAX_TRIGGERS) {
> +               trig = INDEX_TO_TRIGGER(idx);
> +               if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)) || idx >= hs->total_trigs)
> +                       return SBI_ERR_INVALID_PARAM;
> +       }
> +
> +       /* Only disable after validating the arguments - maintain atomicity */
> +       idx = trig_idx_base;
>         for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
>                 trig = INDEX_TO_TRIGGER(idx);
>                 dbtr_trigger_disable(trig);
> diff --git a/lib/sbi/sbi_ecall_dbtr.c b/lib/sbi/sbi_ecall_dbtr.c
> index 40a437ee..b86ce5b3 100644
> --- a/lib/sbi/sbi_ecall_dbtr.c
> +++ b/lib/sbi/sbi_ecall_dbtr.c
> @@ -28,7 +28,7 @@ static int sbi_ecall_dbtr_handler(unsigned long extid, unsigned long funcid,
>                 break;
>         case SBI_EXT_DBTR_SETUP_SHMEM:
>                 ret = sbi_dbtr_setup_shmem(sbi_domain_thishart_ptr(), smode,
> -                                          regs->a0, regs->a1);
> +                                          regs->a0, regs->a1, regs->a2);
>                 break;
>         case SBI_EXT_DBTR_TRIGGER_READ:
>                 ret = sbi_dbtr_read_trig(smode, regs->a0, regs->a1);
> @@ -43,7 +43,7 @@ static int sbi_ecall_dbtr_handler(unsigned long extid, unsigned long funcid,
>                 ret = sbi_dbtr_enable_trig(regs->a0, regs->a1);
>                 break;
>         case SBI_EXT_DBTR_TRIGGER_UPDATE:
> -               ret = sbi_dbtr_update_trig(smode, regs->a0);
> +               ret = sbi_dbtr_update_trig(smode, regs->a0, &out->value);
>                 break;
>         case SBI_EXT_DBTR_TRIGGER_DISABLE:
>                 ret = sbi_dbtr_disable_trig(regs->a0, regs->a1);
> --
> 2.43.0
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
diff mbox series

Patch

diff --git a/include/sbi/sbi_dbtr.h b/include/sbi/sbi_dbtr.h
index 5e0bf84e..e6b76086 100644
--- a/include/sbi/sbi_dbtr.h
+++ b/include/sbi/sbi_dbtr.h
@@ -104,7 +104,8 @@  int sbi_dbtr_init(struct sbi_scratch *scratch, bool coldboot);
 int sbi_dbtr_supported(void);
 int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
 			 unsigned long shmem_phys_lo,
-			 unsigned long shmem_phys_hi);
+			 unsigned long shmem_phys_hi,
+			 unsigned long flags);
 int sbi_dbtr_num_trig(unsigned long trig_tdata1, unsigned long *out);
 int sbi_dbtr_read_trig(unsigned long smode,
 		       unsigned long trig_idx_base, unsigned long trig_count);
@@ -115,7 +116,7 @@  int sbi_dbtr_uninstall_trig(unsigned long trig_idx_base,
 int sbi_dbtr_enable_trig(unsigned long trig_idx_base,
 			 unsigned long trig_idx_mask);
 int sbi_dbtr_update_trig(unsigned long smode,
-			 unsigned long trig_count);
+			 unsigned long trig_count, unsigned long *out);
 int sbi_dbtr_disable_trig(unsigned long trig_idx_base,
 			  unsigned long trig_idx_mask);
 
diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
index 01047969..1d4785c7 100644
--- a/lib/sbi/sbi_dbtr.c
+++ b/lib/sbi/sbi_dbtr.c
@@ -261,8 +261,10 @@  int sbi_dbtr_get_total_triggers(void)
 
 int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
 			 unsigned long shmem_phys_lo,
-			 unsigned long shmem_phys_hi)
+			 unsigned long shmem_phys_hi,
+			 unsigned long flags)
 {
+	unsigned long shmem_size;
 	struct sbi_dbtr_hart_triggers_state *hart_state;
 
 	if (dom && !sbi_domain_is_assigned_hart(dom, current_hartindex())) {
@@ -271,6 +273,9 @@  int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
 		return SBI_ERR_DENIED;
 	}
 
+	if (flags)
+		return SBI_ERR_INVALID_PARAM;
+
 	hart_state = dbtr_thishart_state_ptr();
 	if (!hart_state)
 		return SBI_ERR_FAILED;
@@ -304,9 +309,11 @@  int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
 	if (shmem_phys_hi)
 		return SBI_EINVALID_ADDR;
 
-	if (dom && !sbi_domain_check_addr(dom,
-		  DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
-		  SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
+	/* Must fail if address *range* is not fully accessible - size is assumed to be trig_max * (XLEN /2) bytes */
+	shmem_size = (unsigned long) hart_state->total_trigs * (__riscv_xlen / 2);
+	if (dom && !sbi_domain_check_addr_range(dom,
+		  DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), shmem_size,
+		  smode, SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
 		return SBI_ERR_INVALID_ADDRESS;
 
 	hart_state->shmem.phys_lo = shmem_phys_lo;
@@ -332,6 +339,7 @@  static void dbtr_trigger_setup(struct sbi_dbtr_trigger *trig,
 	trig->state = 0;
 
 	__set_bit(RV_DBTR_BIT(TS, MAPPED), &trig->state);
+	__set_bit(RV_DBTR_BIT(TS, HAVE_TRIG), &trig->state);
 
 	SET_TRIG_HW_INDEX(trig->state, trig->index);
 
@@ -616,6 +624,9 @@  int sbi_dbtr_install_trig(unsigned long smode,
 	if (!hs)
 		return SBI_ERR_FAILED;
 
+	if (trig_count >= hs->total_trigs)
+		return SBI_ERR_BAD_RANGE;
+
 	if (sbi_dbtr_shmem_disabled(hs))
 		return SBI_ERR_NO_SHMEM;
 
@@ -641,7 +652,7 @@  int sbi_dbtr_install_trig(unsigned long smode,
 			*out = _idx;
 			sbi_hart_protection_unmap_range((unsigned long)shmem_base,
 							trig_count * sizeof(*entry));
-			return SBI_ERR_FAILED;
+			return SBI_ERR_NOT_SUPPORTED;
 		}
 
 		if (!dbtr_trigger_valid(TDATA1_GET_TYPE(ctrl), ctrl)) {
@@ -702,13 +713,17 @@  int sbi_dbtr_uninstall_trig(unsigned long trig_idx_base,
 	if (!hs)
 		return SBI_ERR_FAILED;
 
-	for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
+	for_each_set_bit_from(idx, &trig_mask, RV_MAX_TRIGGERS) {
 		trig = INDEX_TO_TRIGGER(idx);
-		if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)))
+		if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)) || idx >= hs->total_trigs)
 			return SBI_ERR_INVALID_PARAM;
+	}
 
+	/* Only uninstall after validating the arguments - maintain atomicity */
+	idx = trig_idx_base;
+	for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
+		trig = INDEX_TO_TRIGGER(idx);
 		dbtr_trigger_clear(trig);
-
 		sbi_free_trigger(trig);
 	}
 
@@ -727,6 +742,14 @@  int sbi_dbtr_enable_trig(unsigned long trig_idx_base,
 	if (!hs)
 		return SBI_ERR_FAILED;
 
+	for_each_set_bit_from(idx, &trig_mask, RV_MAX_TRIGGERS) {
+		trig = INDEX_TO_TRIGGER(idx);
+		if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)) || idx >= hs->total_trigs)
+			return SBI_ERR_INVALID_PARAM;
+	}
+
+	/* Only enable after validating the arguments - maintain atomicity */
+	idx = trig_idx_base;
 	for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
 		trig = INDEX_TO_TRIGGER(idx);
 		sbi_dprintf("%s: enable trigger %lu\n", __func__, idx);
@@ -737,8 +760,9 @@  int sbi_dbtr_enable_trig(unsigned long trig_idx_base,
 }
 
 int sbi_dbtr_update_trig(unsigned long smode,
-			 unsigned long trig_count)
+			 unsigned long trig_count, unsigned long *out)
 {
+	int stat = SBI_SUCCESS;
 	unsigned long trig_idx;
 	struct sbi_dbtr_trigger *trig;
 	union sbi_dbtr_shmem_entry *entry;
@@ -772,23 +796,52 @@  int sbi_dbtr_update_trig(unsigned long smode,
 		trig_idx = entry->id.idx;
 
 		if (trig_idx >= hs->total_trigs) {
-			sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
-			return SBI_ERR_INVALID_PARAM;
+			stat = SBI_ERR_INVALID_PARAM;
+			goto endloop;
 		}
 
 		trig = INDEX_TO_TRIGGER(trig_idx);
 
 		if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) {
-			sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
-			return SBI_ERR_FAILED;
+			stat = SBI_ERR_FAILED;
+			goto endloop;
+		}
+
+		if (TDATA1_GET_TYPE(entry->data.tdata1) != TDATA1_GET_TYPE(trig->tdata1)) {
+			stat = SBI_ERR_INVALID_PARAM;
+			goto endloop;
+		}
+
+		switch(TDATA1_GET_TYPE(entry->data.tdata1)) {
+			case RISCV_DBTR_TRIG_MCONTROL:
+			case RISCV_DBTR_TRIG_MCONTROL6:
+				if ( (entry->data.tdata1 & RV_DBTR_BIT_MASK(MC6, CHAIN)) != (trig->tdata1 & RV_DBTR_BIT_MASK(MC6, CHAIN)) ) {
+					stat = SBI_ERR_INVALID_PARAM;
+					goto endloop;
+				}
+				break;
+			default:
+				break;
 		}
 
 		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;
+			stat = SBI_ERR_NOT_SUPPORTED;
+			goto endloop;
 		}
 
+		endloop:
+			sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
+			if (stat != SBI_SUCCESS) {
+				*out = _idx;
+				return stat;
+			}
+	}
+
+	/* Only update after validating request - maintain atomicity */
+	for_each_trig_entry(shmem_base, trig_count, typeof(*entry), entry) {
+		sbi_hart_protection_map_range((unsigned long)entry, sizeof(*entry));
+		trig = INDEX_TO_TRIGGER(entry->id.idx);
 		dbtr_trigger_setup(trig, &entry->data);
 		sbi_hart_protection_unmap_range((unsigned long)entry, sizeof(*entry));
 		dbtr_trigger_enable(trig);
@@ -809,6 +862,14 @@  int sbi_dbtr_disable_trig(unsigned long trig_idx_base,
 	if (!hs)
 		return SBI_ERR_FAILED;
 
+	for_each_set_bit_from(idx, &trig_mask, RV_MAX_TRIGGERS) {
+		trig = INDEX_TO_TRIGGER(idx);
+		if (!(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED)) || idx >= hs->total_trigs)
+			return SBI_ERR_INVALID_PARAM;
+	}
+
+	/* Only disable after validating the arguments - maintain atomicity */
+	idx = trig_idx_base;
 	for_each_set_bit_from(idx, &trig_mask, hs->total_trigs) {
 		trig = INDEX_TO_TRIGGER(idx);
 		dbtr_trigger_disable(trig);
diff --git a/lib/sbi/sbi_ecall_dbtr.c b/lib/sbi/sbi_ecall_dbtr.c
index 40a437ee..b86ce5b3 100644
--- a/lib/sbi/sbi_ecall_dbtr.c
+++ b/lib/sbi/sbi_ecall_dbtr.c
@@ -28,7 +28,7 @@  static int sbi_ecall_dbtr_handler(unsigned long extid, unsigned long funcid,
 		break;
 	case SBI_EXT_DBTR_SETUP_SHMEM:
 		ret = sbi_dbtr_setup_shmem(sbi_domain_thishart_ptr(), smode,
-					   regs->a0, regs->a1);
+					   regs->a0, regs->a1, regs->a2);
 		break;
 	case SBI_EXT_DBTR_TRIGGER_READ:
 		ret = sbi_dbtr_read_trig(smode, regs->a0, regs->a1);
@@ -43,7 +43,7 @@  static int sbi_ecall_dbtr_handler(unsigned long extid, unsigned long funcid,
 		ret = sbi_dbtr_enable_trig(regs->a0, regs->a1);
 		break;
 	case SBI_EXT_DBTR_TRIGGER_UPDATE:
-		ret = sbi_dbtr_update_trig(smode, regs->a0);
+		ret = sbi_dbtr_update_trig(smode, regs->a0, &out->value);
 		break;
 	case SBI_EXT_DBTR_TRIGGER_DISABLE:
 		ret = sbi_dbtr_disable_trig(regs->a0, regs->a1);