[U-Boot] [PATCH 07/10] x86: fsp: Make sure HOB list is not overwritten by U-Boot

[Simon Glass]

Hi Bin,

On 11 December 2015 at 03:55, Bin Meng <bmeng.cn at gmail.com> wrote:
> Intel IvyBridge FSP seems to be buggy that it does not report memory
> used by FSP itself as reserved in the resource descriptor HOB. The
> FSP specification does not describe how resource descriptor HOBs are
> generated by the FSP to describe what memory regions. It looks newer
> FSPs like Queensbay and BayTrail do not have such issue. This causes
> U-Boot relocation overwrites the important boot service data which is
> used by FSP, and the subsequent call to fsp_notify() will fail.
>
> To resolve this, we find out the lowest memory base address allocated
> by FSP for the boot service data when walking through the HOB list in
> fsp_get_usable_lowmem_top(). Check whether the memory top address is
> below the FSP HOB list, and if not, use the lowest memory base address
> allocated by FSP as the memory top address.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
>  arch/x86/lib/fsp/fsp_support.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

Acked-by: Simon Glass <sjg at chromium.org>
Tested on link (ivybridge non-FSP)
Tested-by: Simon Glass <sjg at chromium.org>

But if this is a bug I'd prefer to have it behind an #iffdef, or with
a large comment in the code. Otherwise we might think it is an
important feature for all platforms.

>
> diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
> index 875c96a..d6bbe1a 100644
> --- a/arch/x86/lib/fsp/fsp_support.c
> +++ b/arch/x86/lib/fsp/fsp_support.c
> @@ -223,7 +223,9 @@ u32 fsp_get_usable_lowmem_top(const void *hob_list)
>  {
>         const struct hob_header *hdr;
>         struct hob_res_desc *res_desc;
> +       struct hob_mem_alloc *res_mem;
>         phys_addr_t phys_start;
> +       phys_addr_t mem_base = 0;
>         u32 top;
>
>         /* Get the HOB list for processing */
> @@ -242,9 +244,34 @@ u32 fsp_get_usable_lowmem_top(const void *hob_list)
>                                         top += (u32)(res_desc->len);
>                         }
>                 }
> +
> +               /*
> +                * Find out the lowest memory base address allocated by FSP
> +                * for the boot service data
> +                */
> +               if (hdr->type == HOB_TYPE_MEM_ALLOC) {
> +                       res_mem = (struct hob_mem_alloc *)hdr;
> +                       if (!mem_base)
> +                               mem_base = res_mem->mem_base;
> +                       if (res_mem->mem_base < mem_base)
> +                               mem_base = res_mem->mem_base;
> +               }
> +
>                 hdr = get_next_hob(hdr);
>         }
>
> +       /*
> +        * Check whether the memory top address is below the FSP HOB list.
> +        * If not, use the lowest memory base address allocated by FSP as
> +        * the memory top address. This is to prevent U-Boot relocation
> +        * overwrites the important boot service data which is used by FSP,
> +        * otherwise the subsequent call to fsp_notify() will fail.
> +        */
> +       if (top > (u32)hob_list) {
> +               debug("Adjust memory top address due to a buggy FSP\n");
> +               top = (u32)mem_base;
> +       }
> +
>         return top;
>  }
>
> --
> 1.8.2.1
>

Regards,
Simon