Sridhar Siricilla 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 16:
(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
Done
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 […]
With this approach, complete response structure will be static. It also includes the HECI header
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.
Agreed, please see my earlier response.
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. […]
Please see my earlier response.
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. […]
This function checks if it needs to update boot partition info or not. Also, updates the boot partition info based on it's input argument.
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 […]
Please see my above response.
-
Sridhar Siricilla (Code Review)