[coreboot-gerrit] [M] Change in coreboot[master]: soc/amd/common/block/lpc/spi_dma: Leverage CBFS_CACHE when using SPI DMA

[Tim Van Patten (Code Review)]

Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth, Fred Reitberger, Karthik Ramasubramanian, Felix Held.

Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74334 )

Change subject: soc/amd/common/block/lpc/spi_dma: Leverage CBFS_CACHE when using SPI DMA
......................................................................

Patch Set 6:

(1 comment)

File src/soc/amd/common/block/lpc/spi_dma.c:

https://review.coreboot.org/c/coreboot/+/74334/comment/735e7215_83bef414 
PS6, Line 242: 		mem_pool_free(&cbfs_cache, mapping);
This will attempt to free `&mdev->base[offset]` if `mem_pool_alloc()` failed initially. However, looking at the implementation of `mem_pool_free()`, `mp->last_alloc` will not point to that address, so it will return early and do nothing.

Looking at the implementation of `mem_pool.c`, it expects cached blocks to be returned in the order they are allocated. Otherwise, the memory is lost forever, which seems like the most likely case without careful consideration of allocations/frees.

How much space is left in the CBFS cache with this change? Do we expect the blocks to be freed immediately and recycled, or for the cache buffers to be consumed for the duration of Coreboot's execution?