[M] Change in coreboot[master]: soc/amd/common/block/lpc/spi_dma: Leverage CBFS_CACHE when using SPI DMA
Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth, Tim Van Patten, Fred Reitberger, Felix Held.
Karthik Ramasubramanian 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/b4328c60_92538aab 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.
Yes, your understanding is correct. From our use-case, the only time we encounter this scenario is when the code could not perform mem_pool_alloc successfully. In that scenario, it does not hurt to call mem_pool_free since it will return early.
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.
mem_pool library supports nested allocation up to the depth of 2(last_alloc, second_to_last_alloc). Beyond that, you can still allocate. But the library will start losing track of buffers already allocated.
How much space is left in the CBFS cache with this change?
We start with CBFS_CACHE of 256 KiB. Also we are enabling it with this change. Based on inspection of one of Raul's comments earlier, there seems to be a leak of 86 KB. Even with the leak we are still able to accommodate all our use-cases. These are the files where the leak is observed and can be investigated as follow-up CLs.
[DEBUG] FMAP: area RO_VPD found @ 800000 (16384 bytes) [SPEW ] spi_dma_readat_dma: start: dest: 0x02181600, source: 0x800000, size: 16384 -- [INFO ] CBFS: Found 'pci1002,1506.rom' @0x93500 size 0xae00 in mcache @0xb877c380 [SPEW ] spi_dma_readat_dma: start: dest: 0x02187600, source: 0xb3580, size: 44544 -- [INFO ] CBFS: Found 'cpu_microcode_8a00.bin' @0x9e380 size 0xc80 in mcache @0xb877c400 [SPEW ] spi_dma_readat_dma: start: dest: 0x02192400, source: 0xbe400, size: 3200 -- [INFO ] CBFS: Found 'fallback/dsdt.aml' @0x5f680 size 0x4527 in mcache @0xb877c200 [SPEW ] spi_dma_readat_dma: start: dest: 0x02193080, source: 0x7f700, size: 17703 -- [INFO ] Couldn't obtain OEM name from CBI [SPEW ] spi_dma_readat_dma: start: dest: 0x021975c0, source: 0x61e000, size: 4096 -- [INFO ] CBFS: Found 'fallback/payload' @0xc4a80 size 0x2811c in mcache @0xb877c6d8 [SPEW ] spi_dma_readat_dma: start: dest: 0x021985c0, source: 0xe4b00, size: 164124
-
Karthik Ramasubramanian (Code Review)