Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35402 )
Change subject: soc/intel/common/block/cse: Add boot partition related APIs ......................................................................
Patch Set 15:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35402/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35402/14//COMMIT_MSG@17 PS14, Line 17: definations definition
https://review.coreboot.org/c/coreboot/+/35402/14/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse_bp.c:
https://review.coreboot.org/c/coreboot/+/35402/14/src/soc/intel/common/block... PS14, Line 59: static struct cse_boot_partition_info cse_bp_info; : static bool cse_bp_init = false; Can we group them into one construct? something like
struct bp_info { struct get_boot_partition_info_resp info_resp; bool initialized; }; static struct bp_info cse_bp_info;
Then use the same in getting the response in receive function.
if (!heci_send_receive(&info_req, sizeof(cse_bp_info.info_resp), &cse_bp_info.info_resp, &resp_size))
https://review.coreboot.org/c/coreboot/+/35402/14/src/soc/intel/common/block... PS14, Line 125: memcpy((void *)&cse_bp_info, (void *)&info_resp.bp_info, : sizeof(struct cse_boot_partition_info)); we can avoid this memcpy, with the change mentioned at line 59-60.
https://review.coreboot.org/c/coreboot/+/35402/14/src/soc/intel/common/block... PS14, Line 128: return 1; initialize = false at the beginning of this function. just set the initialized to true here, if there are no errors.
https://review.coreboot.org/c/coreboot/+/35402/14/src/soc/intel/common/block... PS14, Line 135: static uint8_t cse_init_bp_info(bool force_cse_init) : { : if (cse_bp_init == false || force_cse_init == true) { : if (!cse_update_boot_partition_info()) { : printk(BIOS_ERR, "Boot partition update fail\n"); : return 0; : } : cse_print_boot_partition_info(); : } : cse_bp_init = true; : return 1; : } The only place force_cse_init is true is when you try to set_next_boot_partition. you could just initialize the cse_bp_info everytime you switch the partition. This function may be eliminated, if you insist on having it it could just check if the cse_bp_info initialized or not and take appropriate action.
https://review.coreboot.org/c/coreboot/+/35402/14/src/soc/intel/common/block... PS14, Line 214: if (!cse_init_bp_info(false)) : return 0; you can just check for the information is initialized or not, instead of forcing or not forcing the update.