[PATCH v3] x86: Change tesing logic of mtrr commit

[Bin Meng]

Hi Simon,

On Mon, Jul 31, 2023 at 9:56 PM Simon Glass <sjg at chromium.org> wrote:
>
> From: Bin Meng <bmeng.cn at gmail.com>
>
> On Coral U-Boot SPL programs some MTRRs and FSPv2 in U-Boot proper
> needs to program MTRRs too. With current testing logic of mtrr
> commit in init_cache_f_r(), the mtrr commit is skipped which won't
> work as the queued mtrr requests include setup for DRAM regions.
>
> Change the logic to allow such configuration.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> Tweak to put back CONFIG_FSP_VERSION2 at top:
> Signed-off-by: Simon Glass <sjg at chromium.org>
>
> ---
>
> Changes in v3:
> - Allow committing MTRRs with FSP_VERSION2
>
> Changes in v2:
> - new patch: "x86: Change tesing logic of mtrr commit"
>
>  arch/x86/lib/init_helpers.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c
> index f33194045f9e..60a2707dcf1b 100644
> --- a/arch/x86/lib/init_helpers.c
> +++ b/arch/x86/lib/init_helpers.c
> @@ -21,8 +21,7 @@ int init_cache_f_r(void)
>         /*
>          * Supported configurations:
>          *
> -        * booting from slimbootloader - in that case the MTRRs are already set
> -        *      up
> +        * booting from slimbootloader - MTRRs are already set up
>          * booting with FSPv1 - MTRRs are already set up
>          * booting with FSPv2 - MTRRs must be set here
>          * booting from coreboot - in this case there is no SPL, so we set up
> @@ -30,8 +29,7 @@ int init_cache_f_r(void)
>          * Note: if there is an SPL, then it has already set up MTRRs so we
>          *      don't need to do that here
>          */
> -       do_mtrr &= !IS_ENABLED(CONFIG_SPL) &&
> -               !IS_ENABLED(CONFIG_FSP_VERSION1) &&
> +       do_mtrr &= !IS_ENABLED(CONFIG_FSP_VERSION1) &&

The drop of IS_ENABLED(CONFIG_SPL) here will cause:

1. SPL programs the MTRR
2. U-Boot proper programs the MTRR again, which is not we want

My patch covers such scenario, no?

>                 !IS_ENABLED(CONFIG_SYS_SLIMBOOTLOADER);
>
>         if (do_mtrr) {

Regards,
Bin