Skip to content

Commit 2e5d11b

Browse files
committed
fix(fl): report error when enable fails on single comp, add patch_mode tracking
fl_cmd_enable reported [FLOK] even when fpb_enable_patch failed for a single comparator (e.g. enabling an unset patch). Now it checks the return value and reports [FLERR] with FL_ERR_HW on failure. Also add fl_patch_mode_t to fl_slot_state_t so info display can show the actual patch mode (remap/trampoline/debugmon) from slot state instead of relying on hardware COMP register readback which may be unreliable on FPBv2.
1 parent fe837e1 commit 2e5d11b

3 files changed

Lines changed: 47 additions & 7 deletions

File tree

App/func_loader/fl.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,22 @@ typedef void (*fl_free_cb_t)(void* ptr);
5757
typedef void (*fl_flush_dcache_cb_t)(uintptr_t start, uintptr_t end);
5858
typedef void (*fl_invalidate_icache_cb_t)(uintptr_t start, uintptr_t end);
5959

60+
/**
61+
* @brief Patch mode for slot tracking
62+
*/
63+
typedef enum {
64+
FL_PATCH_MODE_NONE = 0, /* Not patched */
65+
FL_PATCH_MODE_REMAP, /* FPB REMAP (patch command) */
66+
FL_PATCH_MODE_TRAMPOLINE, /* FPB REMAP + trampoline (tpatch command) */
67+
FL_PATCH_MODE_DEBUGMON, /* FPB BKPT + DebugMonitor (dpatch command) */
68+
} fl_patch_mode_t;
69+
6070
/**
6171
* @brief Slot state for tracking injection info
6272
*/
6373
typedef struct {
6474
bool active; /* Slot is in use */
75+
fl_patch_mode_t mode; /* Patch mode used */
6576
uint32_t orig_addr; /* Original function address */
6677
uint32_t target_addr; /* Injected code address */
6778
uint32_t code_size; /* Injected code size */

App/func_loader/fl_cmd_patch.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,18 @@ void fl_cmd_print_fpb_info(fl_context_t* ctx) {
6262
(unsigned long)fpb_info.remap_base, fpb_info.remap_supported ? "supported" : "not supported");
6363

6464
static const char* replace_mode_str[] = {"remap", "bp_lo", "bp_hi", "bp_both"};
65+
static const char* patch_mode_str[] = {"none", "remap", "trampoline", "debugmon"};
6566
for (uint32_t i = 0; i < num_comps && i < FL_MAX_SLOTS && i < FPB_MAX_CODE_COMP; i++) {
6667
fl_slot_state_t* slot = &ctx->slots[i];
6768
fpb_comp_info_t* comp = &fpb_info.comp[i];
68-
const char* mode_str = (comp->replace < 4) ? replace_mode_str[comp->replace] : "?";
69+
const char* mode_str;
70+
71+
/* Use slot's patch mode if active, otherwise fall back to hardware REPLACE field */
72+
if (slot->active && slot->mode < 4) {
73+
mode_str = patch_mode_str[slot->mode];
74+
} else {
75+
mode_str = (comp->replace < 4) ? replace_mode_str[comp->replace] : "?";
76+
}
6977

7078
if (slot->active) {
7179
fl_println("Slot[%u]: 0x%08lX -> 0x%08lX, %u bytes (COMP=0x%08lX, %s, %s)", (unsigned)i,
@@ -120,6 +128,7 @@ fl_error_t fl_cmd_patch(fl_context_t* ctx, const cmd_args_t* args) {
120128

121129
/* Record slot state, transfer last_alloc ownership to slot */
122130
ctx->slots[args->comp].active = true;
131+
ctx->slots[args->comp].mode = FL_PATCH_MODE_REMAP;
123132
ctx->slots[args->comp].orig_addr = args->orig;
124133
ctx->slots[args->comp].target_addr = args->target;
125134
ctx->slots[args->comp].code_size = ctx->last_alloc_size;
@@ -163,6 +172,7 @@ fl_error_t fl_cmd_tpatch(fl_context_t* ctx, const cmd_args_t* args) {
163172

164173
/* Record slot state, transfer last_alloc ownership to slot */
165174
ctx->slots[args->comp].active = true;
175+
ctx->slots[args->comp].mode = FL_PATCH_MODE_TRAMPOLINE;
166176
ctx->slots[args->comp].orig_addr = args->orig;
167177
ctx->slots[args->comp].target_addr = args->target;
168178
ctx->slots[args->comp].code_size = ctx->last_alloc_size;
@@ -211,6 +221,7 @@ fl_error_t fl_cmd_dpatch(fl_context_t* ctx, const cmd_args_t* args) {
211221

212222
/* Record slot state, transfer last_alloc ownership to slot */
213223
ctx->slots[args->comp].active = true;
224+
ctx->slots[args->comp].mode = FL_PATCH_MODE_DEBUGMON;
214225
ctx->slots[args->comp].orig_addr = args->orig;
215226
ctx->slots[args->comp].target_addr = args->target;
216227
ctx->slots[args->comp].code_size = ctx->last_alloc_size;
@@ -266,6 +277,7 @@ fl_error_t fl_cmd_unpatch(fl_context_t* ctx, const cmd_args_t* args) {
266277

267278
/* Clear slot state */
268279
ctx->slots[i].active = false;
280+
ctx->slots[i].mode = FL_PATCH_MODE_NONE;
269281
ctx->slots[i].orig_addr = 0;
270282
ctx->slots[i].target_addr = 0;
271283
ctx->slots[i].code_size = 0;
@@ -300,7 +312,6 @@ fl_error_t fl_cmd_enable(fl_context_t* ctx, const cmd_args_t* args) {
300312
}
301313
}
302314

303-
(void)ctx;
304315
bool en = args->enable != 0;
305316
bool all = args->all;
306317
uint32_t comp = (uint32_t)args->comp;
@@ -315,10 +326,16 @@ fl_error_t fl_cmd_enable(fl_context_t* ctx, const cmd_args_t* args) {
315326

316327
uint32_t changed = 0;
317328
for (uint32_t i = start; i < end && i < FL_MAX_SLOTS; i++) {
329+
if (all && !ctx->slots[i].active) {
330+
continue; /* --all mode: skip inactive slots */
331+
}
332+
318333
fpb_result_t ret = fpb_enable_patch(i, en);
319-
if (ret == FPB_OK) {
320-
changed++;
334+
if (ret != FPB_OK) {
335+
fl_response(false, "Failed to %s patch %lu: %d", en ? "enable" : "disable", (unsigned long)i, ret);
336+
return FL_ERR_HW;
321337
}
338+
changed++;
322339
}
323340

324341
if (all) {

App/tests/test_fl.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,12 +1406,12 @@ void test_loader_cmd_enable_unset_patch(void) {
14061406
setup_loader();
14071407
fl_init(&test_ctx);
14081408

1409-
/* Try to enable a patch that was never set - should succeed but report 0 changed */
1409+
/* Try to enable a patch that was never set - should fail */
14101410
const char* argv[] = {"fl", "--cmd", "enable", "--comp", "0", "--enable", "1"};
14111411
fl_error_t result = fl_exec_cmd(&test_ctx, 7, argv);
14121412

1413-
/* fpb_enable_patch returns FPB_ERR_NOT_SET for unset patches, so changed=0 */
1414-
TEST_ASSERT_EQUAL(FL_OK, result);
1413+
TEST_ASSERT_EQUAL(FL_ERR_HW, result);
1414+
TEST_ASSERT(mock_output_contains("Failed to enable"));
14151415
}
14161416

14171417
/* ============================================================================
@@ -2011,6 +2011,18 @@ void test_loader_cmd_enable_with_crc(void) {
20112011
setup_loader();
20122012
fl_init(&test_ctx);
20132013

2014+
/* Set up a patch first so enable has something to work with */
2015+
const char* alloc_argv[] = {"fl", "--cmd", "alloc", "--size", "64"};
2016+
fl_exec_cmd(&test_ctx, 5, alloc_argv);
2017+
2018+
const char* patch_argv[]
2019+
= {"fl", "--cmd", "patch", "--comp", "0", "--orig", "0x08001000", "--target", "0x20000100"};
2020+
fl_exec_cmd(&test_ctx, 9, patch_argv);
2021+
2022+
/* Disable first so we can test re-enable with CRC */
2023+
const char* disable_argv[] = {"fl", "--cmd", "enable", "--comp", "0", "--enable", "0"};
2024+
fl_exec_cmd(&test_ctx, 7, disable_argv);
2025+
20142026
/* CRC of comp=0 + enable=1 */
20152027
uint32_t comp32 = 0;
20162028
uint32_t enable32 = 1;

0 commit comments

Comments
 (0)