Avoid using hardcoded values in MRC cache code

The MRC cache code, as implemented, in some cases uses configuration
settings for MRC cache region, and in some cases - the values read
from FMAP. These do not necessarily match, the code should use FMAP
across the board.

This change also refactors mrccache.c to limit number of iterations
through the cache area and number of fmap area searches.

Change-Id: Idb9cb70ead4baa3601aa244afc326d5be0d06446
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: http://review.coreboot.org/1788
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
This commit is contained in:
Vadim Bendebury 2012-10-15 14:30:28 -07:00 committed by Ronald G. Minnich
parent 08067ba9cb
commit ad67791382
3 changed files with 124 additions and 112 deletions

View File

@ -32,7 +32,13 @@
#include <vendorcode/google/chromeos/fmap.h> #include <vendorcode/google/chromeos/fmap.h>
#endif #endif
struct mrc_data_container *next_mrc_block(struct mrc_data_container *mrc_cache) /* convert a pointer to flash area into the offset inside the flash */
static inline u32 to_flash_offset(void *p) {
return ((u32)p + CONFIG_ROM_SIZE);
}
static struct mrc_data_container *next_mrc_block(
struct mrc_data_container *mrc_cache)
{ {
/* MRC data blocks are aligned within the region */ /* MRC data blocks are aligned within the region */
u32 mrc_size = sizeof(*mrc_cache) + mrc_cache->mrc_data_size; u32 mrc_size = sizeof(*mrc_cache) + mrc_cache->mrc_data_size;
@ -46,7 +52,7 @@ struct mrc_data_container *next_mrc_block(struct mrc_data_container *mrc_cache)
return (struct mrc_data_container *)region_ptr; return (struct mrc_data_container *)region_ptr;
} }
int is_mrc_cache(struct mrc_data_container *mrc_cache) static int is_mrc_cache(struct mrc_data_container *mrc_cache)
{ {
return (!!mrc_cache) && (mrc_cache->mrc_signature == MRC_DATA_SIGNATURE); return (!!mrc_cache) && (mrc_cache->mrc_signature == MRC_DATA_SIGNATURE);
} }
@ -57,7 +63,7 @@ int is_mrc_cache(struct mrc_data_container *mrc_cache)
* - Have each mainboard Kconfig supply a hard-coded offset * - Have each mainboard Kconfig supply a hard-coded offset
* - Use CBFS * - Use CBFS
*/ */
u32 get_mrc_cache_region(struct mrc_data_container **mrc_region_ptr) static u32 get_mrc_cache_region(struct mrc_data_container **mrc_region_ptr)
{ {
u32 region_size; u32 region_size;
#if CONFIG_CHROMEOS #if CONFIG_CHROMEOS
@ -71,73 +77,31 @@ u32 get_mrc_cache_region(struct mrc_data_container **mrc_region_ptr)
return region_size; return region_size;
} }
/* find the first empty field in the MRC cache area. If there's none, return /*
* the first region. By testing for emptiness caller can detect if flash * Find the largest index block in the MRC cache. Return NULL if non is
* needs to be erased. * found.
*
* FIXME: that interface is crap
*/ */
struct mrc_data_container *find_next_mrc_cache(void) static struct mrc_data_container *find_current_mrc_cache_local
(struct mrc_data_container *mrc_cache, u32 region_size)
{ {
u32 region_end;
u32 entry_id = 0; u32 entry_id = 0;
struct mrc_data_container *mrc_cache = NULL; struct mrc_data_container *mrc_next = mrc_cache;
u32 region_size = get_mrc_cache_region(&mrc_cache);
void *mrc_region = (void*)mrc_cache;
if (mrc_cache == NULL) { region_end = (u32) mrc_cache + region_size;
printk(BIOS_ERR, "%s: could not find mrc cache area\n", __func__);
return NULL;
}
/* Search for the first empty entry in the region */
while (is_mrc_cache(mrc_cache)) {
entry_id++;
mrc_cache = next_mrc_block(mrc_cache);
/* If we exceed the defined area, move to front */
if ((void*)mrc_cache >= (void*)(mrc_region + region_size)) {
mrc_cache = (struct mrc_data_container *)mrc_region;
break;
}
}
printk(BIOS_DEBUG, "picked entry %u from cache block when looking for empty block\n", entry_id);
return mrc_cache;
}
struct mrc_data_container *find_current_mrc_cache(void)
{
u32 entry_id = 0;
struct mrc_data_container *mrc_next, *mrc_cache = NULL;
u32 region_size = get_mrc_cache_region(&mrc_cache);
void *mrc_region = (void*)mrc_cache;
mrc_next = mrc_cache;
if (mrc_cache == NULL) {
printk(BIOS_ERR, "%s: could not find mrc cache area\n", __func__);
return NULL;
}
if (mrc_cache->mrc_data_size == -1UL) {
printk(BIOS_ERR, "%s: MRC cache not initialized?\n", __func__);
/* return non-initialized cache, so we can discern this
* from having no cache area at all
*/
return mrc_cache;
}
/* Search for the last filled entry in the region */ /* Search for the last filled entry in the region */
while (is_mrc_cache(mrc_next)) { while (is_mrc_cache(mrc_next)) {
entry_id++; entry_id++;
mrc_cache = mrc_next; mrc_cache = mrc_next;
mrc_next = next_mrc_block(mrc_cache); mrc_next = next_mrc_block(mrc_next);
/* Stay in the mrc data region */ if ((u32)mrc_next >= region_end) {
if ((void*)mrc_next >= (void*)(mrc_region + region_size)) /* Stay in the MRC data region */
break; break;
}
} }
entry_id--;
if (entry_id == -1) { if (entry_id == 0) {
printk(BIOS_ERR, "%s: No valid MRC cache found.\n", __func__); printk(BIOS_ERR, "%s: No valid MRC cache found.\n", __func__);
return NULL; return NULL;
} }
@ -150,7 +114,8 @@ struct mrc_data_container *find_current_mrc_cache(void)
return NULL; return NULL;
} }
printk(BIOS_DEBUG, "picked entry %u from cache block\n", entry_id); printk(BIOS_DEBUG, "%s: picked entry %u from cache block\n", __func__,
entry_id - 1);
return mrc_cache; return mrc_cache;
} }
@ -159,10 +124,42 @@ struct mrc_data_container *find_current_mrc_cache(void)
* Also unknown if writing flash from XIP-flash code is a good idea * Also unknown if writing flash from XIP-flash code is a good idea
*/ */
#if !defined(__PRE_RAM__) #if !defined(__PRE_RAM__)
/* find the first empty block in the MRC cache area.
* If there's none, return NULL.
*
* @mrc_cache_base - base address of the MRC cache area
* @mrc_cache - current entry (for which we need to find next)
* @region_size - total size of the MRC cache area
*/
static struct mrc_data_container *find_next_mrc_cache
(struct mrc_data_container *mrc_cache_base,
struct mrc_data_container *mrc_cache,
u32 region_size)
{
u32 region_end = (u32) mrc_cache_base + region_size;
mrc_cache = next_mrc_block(mrc_cache);
if ((u32)mrc_cache >= region_end) {
/* Crossed the boundary */
mrc_cache = NULL;
printk(BIOS_DEBUG, "%s: no available entries found\n",
__func__);
} else {
printk(BIOS_DEBUG,
"%s: picked next entry from cache block at %p\n",
__func__, mrc_cache);
}
return mrc_cache;
}
void update_mrc_cache(void) void update_mrc_cache(void)
{ {
printk(BIOS_DEBUG, "Updating MRC cache data.\n"); printk(BIOS_DEBUG, "Updating MRC cache data.\n");
struct mrc_data_container *current = cbmem_find(CBMEM_ID_MRCDATA); struct mrc_data_container *current = cbmem_find(CBMEM_ID_MRCDATA);
struct mrc_data_container *cache, *cache_base;
u32 cache_size;
if (!current) { if (!current) {
printk(BIOS_ERR, "No MRC cache in cbmem. Can't update flash.\n"); printk(BIOS_ERR, "No MRC cache in cbmem. Can't update flash.\n");
return; return;
@ -172,17 +169,20 @@ void update_mrc_cache(void)
return; return;
} }
cache_size = get_mrc_cache_region(&cache_base);
if (cache_base == NULL) {
printk(BIOS_ERR, "%s: could not find MRC cache area\n",
__func__);
return;
}
/* /*
* we need to: * we need to:
*/ */
// 0. compare MRC data to last mrc-cache block (exit if same) // 0. compare MRC data to last mrc-cache block (exit if same)
struct mrc_data_container *cache; cache = find_current_mrc_cache_local(cache_base, cache_size);
if ((cache = find_current_mrc_cache()) == NULL) {
printk(BIOS_DEBUG, "Failure looking for current last block.\n");
return;
}
if ((cache->mrc_data_size == current->mrc_data_size) && if (cache && (cache->mrc_data_size == current->mrc_data_size) &&
(memcmp(cache, current, cache->mrc_data_size) == 0)) { (memcmp(cache, current, cache->mrc_data_size) == 0)) {
printk(BIOS_DEBUG, printk(BIOS_DEBUG,
"MRC data in flash is up to date. No update.\n"); "MRC data in flash is up to date. No update.\n");
@ -198,22 +198,48 @@ void update_mrc_cache(void)
} }
// 2. look up the first unused block // 2. look up the first unused block
cache = find_next_mrc_cache(); if (cache)
if (!cache) { cache = find_next_mrc_cache(cache_base, cache, cache_size);
printk(BIOS_DEBUG, "Could not find MRC cache area\n");
return;
}
// 3. if no such place exists, erase entire mrc-cache range & use block 0 /*
if (cache->mrc_data_size != -1) { * 3. if no such place exists, erase entire mrc-cache range & use
printk(BIOS_DEBUG, "We need to erase the MRC cache region\n"); * block 0. First time around the erase is not needed, but this is a
flash->erase(flash, CONFIG_MRC_CACHE_LOCATION, CONFIG_MRC_CACHE_SIZE); * small overhead for simpler code.
/* we know we can start at the beginning again */ */
get_mrc_cache_region(&cache); if (!cache) {
printk(BIOS_DEBUG,
"Need to erase the MRC cache region of %d bytes at %p\n",
cache_size, cache_base);
flash->erase(flash, to_flash_offset(cache_base), cache_size);
/* we will start at the beginning again */
cache = cache_base;
} }
// 4. write mrc data with flash->write() // 4. write mrc data with flash->write()
printk(BIOS_DEBUG, "Finally: write MRC cache update to flash\n"); printk(BIOS_DEBUG, "Finally: write MRC cache update to flash at %p\n",
flash->write(flash, (u32)(((void*)cache)-CONFIG_MRC_CACHE_BASE), current->mrc_data_size + sizeof(*current), current); cache);
flash->write(flash, to_flash_offset(cache),
current->mrc_data_size + sizeof(*current), current);
} }
#endif #endif
struct mrc_data_container *find_current_mrc_cache(void)
{
struct mrc_data_container *cache_base;
u32 cache_size;
cache_size = get_mrc_cache_region(&cache_base);
if (cache_base == NULL) {
printk(BIOS_ERR, "%s: could not find MRC cache area\n",
__func__);
return NULL;
}
/*
* we need to:
*/
// 0. compare MRC data to last mrc-cache block (exit if same)
return find_current_mrc_cache_local(cache_base, cache_size);
}

View File

@ -230,10 +230,6 @@ struct mrc_data_container {
u8 mrc_data[0]; // Variable size, platform/run time dependent. u8 mrc_data[0]; // Variable size, platform/run time dependent.
} __attribute__ ((packed)); } __attribute__ ((packed));
struct mrc_data_container *next_mrc_block(struct mrc_data_container *mrc_cache);
int is_mrc_cache(struct mrc_data_container *mrc_cache);
u32 get_mrc_cache_region(struct mrc_data_container **mrc_region_ptr);
struct mrc_data_container *find_next_mrc_cache(void);
struct mrc_data_container *find_current_mrc_cache(void); struct mrc_data_container *find_current_mrc_cache(void);
#if !defined(__PRE_RAM__) #if !defined(__PRE_RAM__)
void update_mrc_cache(void); void update_mrc_cache(void);

View File

@ -23,44 +23,32 @@
#include <console/console.h> #include <console/console.h>
#include "fmap.h" #include "fmap.h"
static int fmap_try_find(void *fmap)
{
if (!memcmp(fmap, FMAP_SIGNATURE,
sizeof(FMAP_SIGNATURE)-1))
return 1;
return 0;
}
/* Find FMAP data structure in ROM. /* Find FMAP data structure in ROM.
* See http://code.google.com/p/flashmap/ for more information on FMAP. * See http://code.google.com/p/flashmap/ for more information on FMAP.
*/ */
const struct fmap *fmap_find(void) const struct fmap *fmap_find(void)
{ {
const struct fmap *fmap = NULL;
/* FIXME: Get rid of the hard codes. The "easy" way would be to /* FIXME: Get rid of the hard codes. The "easy" way would be to
* do a binary search, but since ROM accesses are slow, we don't * do a binary search, but since ROM accesses are slow, we don't
* want to spend a lot of time looking for the FMAP. An elegant * want to spend a lot of time looking for the FMAP. An elegant
* solution would be to store a pointer to the FMAP in the CBFS * solution would be to store a pointer to the FMAP in the CBFS
* master header; that would require some more changes to cbfstool * master header; that would require some more changes to cbfstool
* and possibly cros_bundle_firmware. * and possibly cros_bundle_firmware.
* FIXME: Use CONFIG_ROMSIZE instead of CONFIG_MRC_CACHE_BASE
* (and get rid of CONFIG_MRC_CACHE_BASE), once we are building
* coreboot images with ME firmware etc built in instead of just
* the CBFS part.
*/ */
if (fmap_try_find((void *)CONFIG_MRC_CACHE_BASE +
CONFIG_FLASHMAP_OFFSET)) /* wrapping around 0x100000000 */
fmap = (const struct fmap *)(CONFIG_MRC_CACHE_BASE + const struct fmap *fmap = (void *)
CONFIG_FLASHMAP_OFFSET); (CONFIG_FLASHMAP_OFFSET - CONFIG_ROM_SIZE);
if (fmap) {
printk(BIOS_DEBUG, "FMAP: Found \"%s\" version %d.%d at %p.\n", if (memcmp(fmap, FMAP_SIGNATURE, sizeof(FMAP_SIGNATURE)-1)) {
fmap->name, fmap->ver_major, fmap->ver_minor, fmap); printk(BIOS_DEBUG, "No FMAP found at %p.\n", fmap);
printk(BIOS_DEBUG, "FMAP: base = %llx size = %x #areas = %d\n", return NULL;
(unsigned long long)fmap->base, fmap->size, }
fmap->nareas);
} else printk(BIOS_DEBUG, "FMAP: Found \"%s\" version %d.%d at %p.\n",
printk(BIOS_DEBUG, "No FMAP found.\n"); fmap->name, fmap->ver_major, fmap->ver_minor, fmap);
printk(BIOS_DEBUG, "FMAP: base = %llx size = %x #areas = %d\n",
(unsigned long long)fmap->base, fmap->size, fmap->nareas);
return fmap; return fmap;
} }
@ -114,13 +102,15 @@ int find_fmap_entry(const char name[], void **pointer)
*/ */
if (fmap->base) { if (fmap->base) {
base = (void *)(unsigned long)fmap->base; base = (void *)(unsigned long)fmap->base;
printk(BIOS_DEBUG, "FMAP: %s base at %p\n", name, base);
} else { } else {
base = (void *)(0 - CONFIG_ROM_SIZE);
printk(BIOS_WARNING, "FMAP: No valid base address, using" printk(BIOS_WARNING, "FMAP: No valid base address, using"
" 0x%08x\n", CONFIG_MRC_CACHE_BASE); " 0x%p\n", base);
base = (void *)CONFIG_MRC_CACHE_BASE;
} }
*pointer = base + area->offset; *pointer = (void*) ((u32)base + area->offset);
printk(BIOS_DEBUG, "FMAP: %s at %p (offset %x)\n",
name, *pointer, area->offset);
return area->size; return area->size;
} }