Hi Bin,
On Mon, 8 May 2023 at 08:51, Bin Meng bmeng.cn@gmail.com wrote:
On Mon, May 8, 2023 at 10:48 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, May 5, 2023 at 6:51 AM Simon Glass sjg@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@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");--
The regression mentioned above will cause Intel Crown Bay fail to boot. See https://lore.kernel.org/u-boot/20210731084529.7524-1-bmeng.cn@gmail.com/
Can that board perhaps use the other functoin to set MTRRs? It is mtrr_set_next_var()
The change in the API has broken two of the Chromebooks which is why I am trying to go back to the way it was. Does this board set up the MTRRs in FSP? If so, perhaps we need a Kconfig for that? It is not what the newer FSPs do, but perhaps we could use that behaviour for FSPv1?
Regards, Simon