[PATCH v3 12/17] x86: Return mtrr_add_request() to its old purpose

[Bin Meng]

Hi Simon,

On Fri, May 5, 2023 at 6:51 AM Simon Glass <sjg at chromium.org> wrote:
>
> This function used to be for adding a list of requests to be actioned on
> relocation. Revert it back to this purpose, to avoid problems with boards
> which need control of their MTRRs (i.e. those which don't use FSP).
>
> The mtrr_set_next_var() function is available when the next free
> variable-MTRR must be set, so this can be used instead.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Fixes: 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed..")
> Fixes: 596bd0589ad ("x86: mtrr: Do not clear the unused ones..")
> ---
>
> Changes in v3:
> - Fix invalid commit IDs obtained from another branch
>
>  arch/x86/cpu/mtrr.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c
> index e69dfb552b1..c174dd9b3ad 100644
> --- a/arch/x86/cpu/mtrr.c
> +++ b/arch/x86/cpu/mtrr.c
> @@ -156,8 +156,12 @@ int mtrr_commit(bool do_caches)
>         debug("open done\n");
>         qsort(req, gd->arch.mtrr_req_count, sizeof(*req), h_comp_mtrr);
>         for (i = 0; i < gd->arch.mtrr_req_count; i++, req++)
> -               mtrr_set_next_var(req->type, req->start, req->size);
> +               set_var_mtrr(i, req->type, req->start, req->size);

This change actually reverts 3bcd6cf89ef ("x86: mtrr: Skip MSRs that
were already programmed.."), but as 3bcd6cf89ef commit message says:

    At present mtrr_commit() programs the MTRR MSRs starting from
    index 0, which may overwrite MSRs that were already programmed
    by previous boot stage or FSP.

So this change will cause regression.

>
> +       /* Clear the ones that are unused */
> +       debug("clear\n");
> +       for (; i < mtrr_get_var_count(); i++)
> +               wrmsrl(MTRR_PHYS_MASK_MSR(i), 0);

This change actually reverts 596bd0589ad ("x86: mtrr: Do not clear the
unused ones.."), which will also create regression unfortunately..

>         debug("close\n");
>         mtrr_close(&state, do_caches);
>         debug("mtrr done\n");
> --

Regards,
Bin