Attention is currently required from: Arthur Heymans, Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Nico Huber, Tarun.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77886?usp=email )
Change subject: drivers/intel/gma/opregion: Use CBFS cache to load VBT ......................................................................
Patch Set 33:
(1 comment)
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/77886/comment/5f9b5829_f49247a7 : PS4, Line 27: cbfs_map
If it's like the generic heap, why have it separate?
Historical reasons, and the fact that it needs to be available in pre-RAM stages. If you're asking whether we should combine it with the ramstage heap into a single malloc() API for all stages, then... maybe. There are a few gotchas (e.g. it needs special alignment requirements on a few platforms) but it could certainly be done.
There might also be some concern that exposing a general malloc() API in pre-RAM stages could lead to it being abused too widely and cause out-of-memory edge cases. As long as it's only for files, there are less edge case combinations to test. There could also be the concern that if more code uses this, the chance that a file allocation can still clean up after itself gets lower (because the allocator is only able to free the last two allocations), so that might be a good reason to keep the heaps separate.
Anyway, probably not a question to decide on this CL.
We eventually copy the VBT to CBMEM anyway. So there shouldn't be a need to map twice.
I'm not really familiar with this code but it looks like this is an exported function that has a bunch of callers outside this file (e.g. 10 just via `vbt_get()`). It's possible that something could be cleaned up and reuse the CBMEM allocation there, but probably not a small task.