Skip to content

minerva-ag: Block vr access during update to prevent update fail #2639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Victor-Jhong wants to merge 2 commits into
base: main
Choose a base branch
from
Open

minerva-ag: Block vr access during update to prevent update fail #2639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f439c6
minerva-ag/vr_update_version_and_remain_check
Victor-Jhong Dec 5, 2025
38fd1ff
minerva-ag/block_vr_access_during_update_to_prevent_update_fail
Victor-Jhong Jan 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions common/service/mctp/mctp_ctrl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@
CHECK_NULL_ARG_WITH_RETURN(resp, MCTP_ERROR);
CHECK_NULL_ARG_WITH_RETURN(resp_len, MCTP_ERROR);

#ifdef ENABLE_MCTP_GET_MSG_TYPE_SUPPORT_DEBUG_LOG
LOG_INF("mctp get_message_type_support received");
#endif

struct _get_message_type_resp *p = (struct _get_message_type_resp *)resp;

uint8_t type_len = 0;
Expand All @@ -142,7 +146,7 @@

ret = load_mctp_support_types(&type_len, types);

if (ret < 0) {

Check failure on line 149 in common/service/mctp/mctp_ctrl.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
Condition 'ret<0' is always true

Raw Output:
common/service/mctp/mctp_ctrl.c:149:10:Condition 'ret<0' is always true
LOG_ERR("Command not supported, %d", ret);
p->completion_code = MCTP_ERROR;
} else {
Expand Down
35 changes: 33 additions & 2 deletions common/service/pldm/pldm_firmware_update.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,15 @@
return comp_id;
}

#ifdef ENABLE_PLDM_PASS_COMPONENT_CHECK
__weak uint8_t plat_pldm_pass_component_table_check(uint16_t num_of_comp,
const uint8_t *comp_image_version_str,
uint8_t comp_image_version_str_len)
{
return PLDM_SUCCESS;
Copy link
Contributor

@jamesatha jamesatha Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we put an real implementation in here? If not, can you address the cppcheck error on line 1082?

Copy link
Contributor Author

@Victor-Jhong Victor-Jhong Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning can be safely ignored; it's a limitation of static analysis, not an understanding of the weak function override mechanism.

}
#endif

int get_descriptor_type_length(uint16_t type)
{
switch (type) {
Expand Down Expand Up @@ -649,7 +658,7 @@
}
}

static void pldm_status_reset()
void pldm_status_reset()
{
state_update(STATE_IDLE);
cur_aux_state = STATE_AUX_NOT_IN_UPDATE;
Expand All @@ -662,7 +671,7 @@

static void exit_update_mode()
{
printk("PLDM update mode timeout, exiting update mode...\n");
LOG_WRN("PLDM update mode timeout, exiting update mode...");
pldm_status_reset();
}

Expand All @@ -676,7 +685,7 @@
CHECK_NULL_ARG_WITH_RETURN(ext_params, 0);

pldm_msg msg = { 0 };
mctp_ext_params *extra_data = (mctp_ext_params *)ext_params;

Check failure on line 688 in common/service/pldm/pldm_firmware_update.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
Variable 'extra_data' can be declared as pointer to const

Raw Output:
common/service/pldm/pldm_firmware_update.c:688:19:Variable 'extra_data' can be declared as pointer to const

msg.ext_params = *extra_data;

Expand Down Expand Up @@ -1086,6 +1095,16 @@
LOG_HEXDUMP_INF(buf + sizeof(struct pldm_pass_component_table_req), req_p->comp_ver_str_len,
"");

#ifdef ENABLE_PLDM_PASS_COMPONENT_CHECK
uint8_t check_result = plat_pldm_pass_component_table_check(
req_p->comp_identifier, buf + sizeof(struct pldm_pass_component_table_req),
req_p->comp_ver_str_len);
if (check_result != PLDM_SUCCESS) {

Check failure on line 1102 in common/service/pldm/pldm_firmware_update.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
Condition 'check_result!=PLDM_SUCCESS' is always false

Raw Output:
common/service/pldm/pldm_firmware_update.c:1102:19:Condition 'check_result!=PLDM_SUCCESS' is always false
resp_p->completion_code = check_result;
goto exit;
}
#endif

if (current_state != STATE_LEARN_COMP) {
LOG_ERR("Firmware update failed because current state %d is not %d", current_state,
STATE_LEARN_COMP);
Expand Down Expand Up @@ -1257,7 +1276,7 @@
CHECK_NULL_ARG_WITH_RETURN(resp_len, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(ext_params, PLDM_ERROR);

struct pldm_activate_firmware_req *req_p = (struct pldm_activate_firmware_req *)buf;

Check failure on line 1279 in common/service/pldm/pldm_firmware_update.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
Variable 'req_p' can be declared as pointer to const

Raw Output:
common/service/pldm/pldm_firmware_update.c:1279:37:Variable 'req_p' can be declared as pointer to const
struct pldm_activate_firmware_resp *resp_p = (struct pldm_activate_firmware_resp *)resp;

*resp_len = 1;
Expand Down Expand Up @@ -1413,6 +1432,10 @@
CHECK_NULL_ARG_WITH_RETURN(resp_len, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(ext_params, PLDM_ERROR);

#ifdef ENABLE_PLDM_GET_FW_PARAM_DEBUG_LOG
LOG_INF("pldm get_firmware_parameter received");
#endif

struct pldm_get_firmware_parameters_resp *resp_p =
(struct pldm_get_firmware_parameters_resp *)resp;

Expand Down Expand Up @@ -1734,7 +1757,7 @@
descriptor_count -= type_length;

for (index = 0; index < type_length; ++index) {
strncpy(data, &descriptor->descriptor_data[index * 2], 2);

Check failure on line 1760 in common/service/pldm/pldm_firmware_update.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
The buffer 'data' may not be null-terminated after the call to strncpy().

Raw Output:
common/service/pldm/pldm_firmware_update.c:1760:4:The buffer 'data' may not be null-terminated after the call to strncpy().
val = strtol(data, NULL, 16);
tlv_ptr->descriptor_data[index] = val;
}
Expand Down Expand Up @@ -1785,3 +1808,11 @@
}
return false;
}

bool is_update_state_idle_or_learn_comp()
{
if ((current_state == STATE_IDLE) || (current_state == STATE_LEARN_COMP)) {
return true;
}
return false;
}
5 changes: 5 additions & 0 deletions common/service/pldm/pldm_firmware_update.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,11 @@ uint8_t fill_descriptor_into_buf(struct pldm_descriptor_string *descriptor, uint
bool is_update_state_download_phase();
bool is_update_state_idle();
uint8_t pldm_fw_update(void *fw_update_param, const int flash_position);
bool is_update_state_idle_or_learn_comp();
void pldm_status_reset();
uint8_t plat_pldm_pass_component_table_check(uint16_t num_of_comp,
const uint8_t *comp_image_version_str,
uint8_t comp_image_version_str_len);

#ifdef __cplusplus
}
Expand Down
38 changes: 38 additions & 0 deletions common/service/pldm/pldm_oem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,42 @@
void *ext_params);
#endif

__weak uint8_t force_update_flag_set_cmd(void *mctp_inst, uint8_t *buf, uint16_t len,
uint8_t instance_id, uint8_t *resp, uint16_t *resp_len,
void *ext_params)
{
CHECK_NULL_ARG_WITH_RETURN(mctp_inst, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(buf, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(resp, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(resp_len, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(ext_params, PLDM_ERROR);

uint8_t *completion_code_p = resp;

*completion_code_p = PLDM_ERROR_UNSUPPORTED_PLDM_CMD;
*resp_len = 1;

return PLDM_ERROR_UNSUPPORTED_PLDM_CMD;
}

__weak uint8_t force_update_flag_get_cmd(void *mctp_inst, uint8_t *buf, uint16_t len,
uint8_t instance_id, uint8_t *resp, uint16_t *resp_len,
void *ext_params)
{
CHECK_NULL_ARG_WITH_RETURN(mctp_inst, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(buf, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(resp, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(resp_len, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(ext_params, PLDM_ERROR);

uint8_t *completion_code_p = resp;

*completion_code_p = PLDM_ERROR_UNSUPPORTED_PLDM_CMD;
*resp_len = 1;

return PLDM_ERROR_UNSUPPORTED_PLDM_CMD;
}

__weak uint8_t sensor_polling_cmd(void *mctp_inst, uint8_t *buf, uint16_t len, uint8_t instance_id,
uint8_t *resp, uint16_t *resp_len, void *ext_params)
{
Expand Down Expand Up @@ -91,7 +127,7 @@
CHECK_NULL_ARG_WITH_RETURN(resp_len, PLDM_ERROR);
CHECK_NULL_ARG_WITH_RETURN(ext_params, PLDM_ERROR);

struct _cmd_echo_req *req_p = (struct _cmd_echo_req *)buf;

Check failure on line 130 in common/service/pldm/pldm_oem.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
Variable 'req_p' can be declared as pointer to const

Raw Output:
common/service/pldm/pldm_oem.c:130:24:Variable 'req_p' can be declared as pointer to const
struct _cmd_echo_resp *resp_p = (struct _cmd_echo_resp *)resp;

if (check_iana(req_p->iana) == PLDM_ERROR) {
Expand Down Expand Up @@ -184,7 +220,7 @@

#ifdef ENABLE_PLDM
#ifdef ENABLE_EVENT_TO_BMC
uint8_t send_event_log_to_bmc(struct pldm_addsel_data sel_msg)

Check failure on line 223 in common/service/pldm/pldm_oem.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
Function 'send_event_log_to_bmc' argument 1 names different: declaration 'msg' definition 'sel_msg'.

Raw Output:
common/service/pldm/pldm_oem.c:223:55:Function 'send_event_log_to_bmc' argument 1 names different: declaration 'msg' definition 'sel_msg'.
{
pldm_msg msg = { 0 };
uint8_t bmc_bus = I2C_BUS_BMC;
Expand Down Expand Up @@ -251,6 +287,8 @@
#ifdef ENABLE_VISTARA
{ PLDM_OEM_WF_READ_SPD_CHUNK, oem_wf_read_spd_chunk },
#endif
{ PLDM_OEM_FORCE_UPDATE_SETTING_CMD, force_update_flag_set_cmd },
{ PLDM_OEM_FORCE_UPDATE_GETTING_CMD, force_update_flag_get_cmd },
};

uint8_t pldm_oem_handler_query(uint8_t code, void **ret_fn)
Expand Down Expand Up @@ -295,7 +333,7 @@
return PLDM_SUCCESS;
}

uint8_t *iana = &buf[0];

Check failure on line 336 in common/service/pldm/pldm_oem.c

View workflow job for this annotation

GitHub Actions / Aggregate-Lint-Output 

[] reported by reviewdog 🐶
Variable 'iana' can be declared as pointer to const

Raw Output:
common/service/pldm/pldm_oem.c:336:11:Variable 'iana' can be declared as pointer to const
uint8_t cxl_id = buf[3];
uint8_t dimm_idx = buf[4];
uint16_t offset = (uint16_t)buf[5] | ((uint16_t)buf[6] << 8);
Expand Down
23 changes: 23 additions & 0 deletions common/service/pldm/pldm_oem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ extern "C" {
#define PLDM_OEM_READ_FILE_IO 0x03
#define PLDM_OEM_SENSOR_POLLING_CMD 0x04
#define PLDM_OEM_WF_READ_SPD_CHUNK 0x05
#define PLDM_OEM_FORCE_UPDATE_SETTING_CMD 0x06
#define PLDM_OEM_FORCE_UPDATE_GETTING_CMD 0x07

#define POWER_CONTROL_LEN 0x01

Expand Down Expand Up @@ -331,6 +333,27 @@ struct _sensor_polling_cmd_resp {
uint8_t set_value;
} __attribute__((packed));

struct _force_update_flag_set_cmd_req {
uint8_t iana[IANA_LEN];
uint8_t set_value;
} __attribute__((packed));

struct _force_update_flag_set_cmd_resp {
uint8_t completion_code;
uint8_t iana[IANA_LEN];
uint8_t set_value;
} __attribute__((packed));

struct _force_update_flag_get_cmd_req {
uint8_t iana[IANA_LEN];
} __attribute__((packed));

struct _force_update_flag_get_cmd_resp {
uint8_t completion_code;
uint8_t iana[IANA_LEN];
uint8_t get_value;
} __attribute__((packed));

uint8_t check_iana(const uint8_t *iana);
uint8_t set_iana(uint8_t *buf, uint8_t buf_len);
uint8_t send_event_log_to_bmc(struct pldm_addsel_data msg);
Expand Down
3 changes: 2 additions & 1 deletion meta-facebook/minerva-ag/boards/npcm400f_evb.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ CONFIG_I2C_NPCM4XX=y
CONFIG_I3C_NPCM4XX=y
CONFIG_ADC_NPCM4XX=y
CONFIG_USB_DC_NPCM4XX=y
CONFIG_JTAG_NPCM4XX=y
CONFIG_JTAG_NPCM4XX=y
CONFIG_LOG_STRDUP_BUF_COUNT=16
3 changes: 3 additions & 0 deletions meta-facebook/minerva-ag/src/platform/plat_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
#define PLDM_FW_UPDATE_TIMEOUT_MS 5000

#define DISABLE_SENSOR_RESP_DURING_FW_UPDATE
#define ENABLE_PLDM_PASS_COMPONENT_CHECK
#define ENABLE_MCTP_GET_MSG_TYPE_SUPPORT_DEBUG_LOG
#define ENABLE_PLDM_GET_FW_PARAM_DEBUG_LOG

#define ENABLE_LX6301
#define ENABLE_U50SU4P180PMDAFC
Expand Down
Loading
Loading