target/riscv: cleanup trigger setup

* Add a warning when eq trigger is setup and it's behavior is different from other triggers. * Make eq trigger's behavior consistent with other triggers in case of length == 1. * Fix a bug in setting chained triggers (LT, GT case). * Improve logging. Change-Id: Id1ed0d11971b8ed875afbb979e6c8a8b51dd3818 Signed-off-by: Evgeniy Naydanov <[email protected]>
riscv-collab · Jul 5, 2023 · 5b30a47 · 5b30a47
1 parent 92c0319
commit 5b30a47
Showing 1 changed file with 21 additions and 12 deletions.
diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c
@@ -655,9 +655,18 @@ struct trigger_request_info {
 	riscv_reg_t tdata1_ignore_mask;
 };
 
+static void log_trigger_request_info(struct trigger_request_info trig_info)
+{
+	LOG_DEBUG("tdata1=%" PRIx64 ", tdata2=%" PRIx64 ", tdata1_ignore_mask=%" PRIx64,
+			trig_info.tdata1, trig_info.tdata2, trig_info.tdata1_ignore_mask);
+};
+
 static int try_setup_single_match_trigger(struct target *target,
 		struct trigger *trigger, struct trigger_request_info trig_info)
 {
+	LOG_TARGET_DEBUG(target, "trying to set up a match trigger");
+	log_trigger_request_info(trig_info);
+
 	int trigger_type =
 		get_field(trig_info.tdata1, CSR_MCONTROL_TYPE(riscv_xlen(target)));
 	int ret = ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
@@ -683,6 +692,9 @@ static int try_setup_chained_match_triggers(struct target *target,
 		struct trigger *trigger, struct trigger_request_info t1,
 		struct trigger_request_info t2)
 {
+	LOG_TARGET_DEBUG(target, "trying to set up a chain of match triggers");
+	log_trigger_request_info(t1);
+	log_trigger_request_info(t2);
 	int trigger_type =
 		get_field(t1.tdata1, CSR_MCONTROL_TYPE(riscv_xlen(target)));
 	int ret = ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
@@ -718,7 +730,6 @@ struct match_triggers_tdata1_fields {
 	riscv_reg_t common;
 	struct {
 		riscv_reg_t any;
-		riscv_reg_t s8bit;
 	} size;
 	struct {
 		riscv_reg_t enable;
@@ -752,10 +763,7 @@ static struct match_triggers_tdata1_fields fill_match_triggers_tdata1_fields_t2(
 		.size = {
 			.any =
 				field_value(CSR_MCONTROL_SIZELO, CSR_MCONTROL_SIZELO_ANY & 3) |
-				field_value(CSR_MCONTROL_SIZEHI, (CSR_MCONTROL_SIZELO_ANY >> 2) & 3),
-			.s8bit =
-				field_value(CSR_MCONTROL_SIZELO, CSR_MCONTROL_SIZELO_8BIT & 3) |
-				field_value(CSR_MCONTROL_SIZEHI, (CSR_MCONTROL_SIZELO_8BIT >> 2) & 3)
+				field_value(CSR_MCONTROL_SIZEHI, (CSR_MCONTROL_SIZELO_ANY >> 2) & 3)
 		},
 		.chain = {
 			.enable = field_value(CSR_MCONTROL_CHAIN, CSR_MCONTROL_CHAIN_ENABLED),
@@ -793,8 +801,7 @@ static struct match_triggers_tdata1_fields fill_match_triggers_tdata1_fields_t6(
 			field_value(CSR_MCONTROL6_LOAD, trigger->is_read) |
 			field_value(CSR_MCONTROL6_STORE, trigger->is_write),
 		.size = {
-			.any = field_value(CSR_MCONTROL6_SIZE, CSR_MCONTROL6_SIZE_ANY),
-			.s8bit = field_value(CSR_MCONTROL6_SIZE, CSR_MCONTROL6_SIZE_8BIT)
+			.any = field_value(CSR_MCONTROL6_SIZE, CSR_MCONTROL6_SIZE_ANY)
 		},
 		.chain = {
 			.enable = field_value(CSR_MCONTROL6_CHAIN, CSR_MCONTROL6_CHAIN_ENABLED),
@@ -852,24 +859,26 @@ static int maybe_add_trigger_t2_t6(struct target *target,
 		struct trigger_request_info lt_1 = {
 			.tdata1 = fields.common | fields.size.any | fields.chain.enable |
 				fields.match.lt,
-			.tdata2 = trigger->address,
+			.tdata2 = trigger->address + trigger->length,
 			.tdata1_ignore_mask = fields.tdata1_ignore_mask
 		};
 		struct trigger_request_info ge_2 = {
 			.tdata1 = fields.common | fields.size.any | fields.chain.disable |
 				fields.match.ge,
-			.tdata2 = trigger->address + trigger->length,
+			.tdata2 = trigger->address,
 			.tdata1_ignore_mask = fields.tdata1_ignore_mask
 		};
 		ret = try_setup_chained_match_triggers(target, trigger, lt_1, ge_2);
 		if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
 			return ret;
 	}
 	LOG_TARGET_DEBUG(target, "trying to setup equality match trigger");
+	if (trigger->length > 1)
+		LOG_TARGET_WARNING(target, "trigger will only match accesses at address 0x%"
+				TARGET_PRIxADDR, trigger->address);
 	struct trigger_request_info eq = {
-		.tdata1 = fields.common |
-			(trigger->length == 1 ? fields.size.s8bit : fields.size.any) |
-			fields.chain.disable | fields.match.eq,
+		.tdata1 = fields.common | fields.size.any | fields.chain.disable |
+			fields.match.eq,
 		.tdata2 = trigger->address,
 		.tdata1_ignore_mask = fields.tdata1_ignore_mask
 	};