Hi Bin,
On Thu, 10 Oct 2019 at 00:23, Bin Meng <bmeng...@gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Sep 25, 2019 at 10:59 PM Simon Glass <s...@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 <s...@chromium.org>
> > ---
> >
> > arch/x86/lib/mrccache.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/lib/mrccache.c b/arch/x86/lib/mrccache.c
> > index 33bb52039bd..e286bdf1b30 100644
> > --- a/arch/x86/lib/mrccache.c
> > +++ b/arch/x86/lib/mrccache.c
> > @@ -86,15 +86,16 @@ struct mrc_data_container *mrccache_find_current(struct
> > mrc_region *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)
> > {
> > + 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) {
>
> This does not look good to me. Why adding the "next" cache position to
> "prev" cache size? It should add the "next" cache size.
Yes, although here we assume they are the same. Should I add a comment?
Alternatively I could pass in the size that the caller wants for the new item?
>
> I agree there is an issue in missing check of boundary, but the check
> should not happen here, but mrccache_update(), before writing to SPI
> flash.
OK, so passing in the size would be best, I suspect.
Perhaps rename the function to find_next_mrc_cache_pos()?
>
> > /* Crossed the boundary */
> > cache = NULL;
> > debug("%s: no available entries found\n", __func__);
> > --
>
Regards,
SImon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
