diff mbox series

[v6,029/102] x86: Correct mrccache find_next_mrc_cache() calculation

Message ID 20191206213936.v6.29.I73b4396e3d9bb2ef5dd5d745e4f7df6652047567@changeid
State Accepted
Commit 0ad9b6a98836b8109d46ba32e6a4a40beb058388
Delegated to: Bin Meng
Headers show
Series x86: Add initial support for apollolake | expand

Commit Message

Simon Glass Dec. 7, 2019, 4:42 a.m. UTC 
This should take account of the end of the new cache record since a record
cannot extend beyond the end of the flash region. This problem was not
seen before due to the alignment of the relatively small amount of MRC
data.

But with Apollo Lake the MRC data is about 45KB, even if most of it is
zeroes.

Fix this bug and update the parameter name to be less confusing.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---

Changes in v6: None
Changes in v5: None
Changes in v4:
- Add comments about MRC-cache records being the same size
- apollolake -> Apollo Lake

Changes in v3:
- Add an extra size parameter to the find_next_mrc_cache() function

Changes in v2: None

 arch/x86/lib/mrccache.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Bin Meng Dec. 8, 2019, 2:53 a.m. UTC | #1 
On Sat, Dec 7, 2019 at 12:47 PM Simon Glass <sjg@chromium.org> wrote:
>
> This should take account of the end of the new cache record since a record
> cannot extend beyond the end of the flash region. This problem was not
> seen before due to the alignment of the relatively small amount of MRC
> data.
>
> But with Apollo Lake the MRC data is about 45KB, even if most of it is
> zeroes.
>
> Fix this bug and update the parameter name to be less confusing.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
> Changes in v6: None
> Changes in v5: None
> Changes in v4:
> - Add comments about MRC-cache records being the same size
> - apollolake -> Apollo Lake
>
> Changes in v3:
> - Add an extra size parameter to the find_next_mrc_cache() function
>
> Changes in v2: None
>
>  arch/x86/lib/mrccache.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>

applied to u-boot-x86/next, thanks!
diff mbox series

Patch

diff --git a/arch/x86/lib/mrccache.c b/arch/x86/lib/mrccache.c
index 33bb52039b..9d56685d36 100644
--- a/arch/x86/lib/mrccache.c
+++ b/arch/x86/lib/mrccache.c
@@ -80,21 +80,31 @@  struct mrc_data_container *mrccache_find_current(struct mrc_region *entry)
 /**
  * find_next_mrc_cache() - get next cache entry
  *
+ * This moves to the next cache entry in the region, making sure it has enough
+ * space to hold data of size @data_size.
+ *
  * @entry:	MRC cache flash area
  * @cache:	Entry to start from
+ * @data_size:	Required data size of the new entry. Note that we assume that
+ *	all cache entries are the same size
  *
  * @return next cache entry if found, NULL if we got to the end
  */
 static struct mrc_data_container *find_next_mrc_cache(struct mrc_region *entry,
-		struct mrc_data_container *cache)
+		struct mrc_data_container *prev, int data_size)
 {
+	struct mrc_data_container *cache;
 	ulong base_addr, end_addr;
 
 	base_addr = entry->base + entry->offset;
 	end_addr = base_addr + entry->length;
 
-	cache = next_mrc_block(cache);
-	if ((ulong)cache >= end_addr) {
+	/*
+	 * We assume that all cache entries are the same size, but let's use
+	 * data_size here for clarity.
+	 */
+	cache = next_mrc_block(prev);
+	if ((ulong)cache + mrc_block_size(data_size) > end_addr) {
 		/* Crossed the boundary */
 		cache = NULL;
 		debug("%s: no available entries found\n", __func__);
@@ -131,7 +141,7 @@  int mrccache_update(struct udevice *sf, struct mrc_region *entry,
 
 	/* Move to the next block, which will be the first unused block */
 	if (cache)
-		cache = find_next_mrc_cache(entry, cache);
+		cache = find_next_mrc_cache(entry, cache, cur->data_size);
 
 	/*
 	 * If we have got to the end, erase the entire mrc-cache area and start