Hi Simon, On Fri, May 5, 2023 at 6:51 AM Simon Glass <s...@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 <s...@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