Patch set 8:Code-Review +1
7 comments:
nit: maybe a short comment explaining what this does?
Done
Patch Set #5, Line 130: return ENV_CACHE_AS_RAM || !ENV_X86;
No, there are different special cases. […]
Just to confirm, we are using the word "accessible" in two different senses:
(1) symbols accessible: symbols are known to the linker
(2) memory accessible: a read32() to the address does not hang the CPU
This function is concerned with (1), thus we don't need to worry about the specifics of (2) and can leave that to the user to select MIGRATE_WORKBUF if necessary.
Your example about the second half of SRAM on Mediatek SoCs helps clarify. Would that be useful to put in the HELP text for MIGRATE_WORKBUF?
File src/security/vboot/common.c:
Patch Set #5, Line 145: wd_cbmem = cbmem_add(CBMEM_ID_VBOOT_WORKBUF, vb2_working_data_size());
Oh, well... […]
Done
File src/security/vboot/common.c:
Patch Set #7, Line 140: * vb2_working_data from SRAM/CAR into CBMEM.
nit: maybe mention the part where it's not necessary on all platforms
Done
Patch Set #7, Line 145: const struct vb2_working_data *wd = vb2_get_working_data(1);
nit: "wd_preram" for clarity?
Done
Patch Set #7, Line 146: struct vb2_working_data *wd_cbmem = vb2_get_working_data(0);
I... guess... this works? But it still seems somewhat weird to me, honestly. […]
Yeah, fair enough. We end up with a duplicate cbmem_add call this way, but I think you're right that vb2_migrate_working_data not calling vb2_get_working_data at all is the best route.
Patch Set #7, Line 147: printk(BIOS_INFO, "VBOOT: copying vb2_working_data to CBMEM...\n");
I'm not sure if this is really interesting enough to print. At least make it DEBUG, not INFO.
Done
To view, visit change 31329. To unsubscribe, or for help writing mail filters, visit settings.