[U-Boot] [PATCH v3 042/108] x86: Correct mrccache find_next_mrc_cache() calculation

[Bin Meng]

Hi Simon,

On Mon, Oct 21, 2019 at 11:40 AM Simon Glass <sjg at 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 apollolake 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 at chromium.org>
> ---
>
> Changes in v3:
> - Add an extra size parameter to the find_next_mrc_cache() function
>
> Changes in v2: None
>
>  arch/x86/lib/mrccache.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/lib/mrccache.c b/arch/x86/lib/mrccache.c
> index 33bb52039bd..7292d1fe070 100644
> --- a/arch/x86/lib/mrccache.c
> +++ b/arch/x86/lib/mrccache.c
> @@ -80,21 +80,26 @@ 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
>   *
>   * @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) {
> +       cache = next_mrc_block(prev);
> +       if ((ulong)cache + mrc_block_size(prev->data_size) > end_addr) {

It takes me another several minutes to understand this, as I was
confused by the v2 patch :)

So in v2 we agreed to add some comments here, as we assume the
data_size will be the same.

>                 /* Crossed the boundary */
>                 cache = NULL;
>                 debug("%s: no available entries found\n", __func__);
> @@ -131,7 +136,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
> --

Regards,
Bin