[PATCH 1/5] x86: fsp: Use mtrr_set_next_var() for graphics memory

[Simon Glass]

Hi Bin,

On Fri, 21 Jul 2023 at 10:12, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> At present this uses mtrr_add_request() & mtrr_commit() combination
> to program the MTRR for graphics memory. This usage has two major
> issues as below:
>
> - mtrr_commit() will re-initialize all MTRR registers from index 0,
>   using the settings previously added by mtrr_add_request() and saved
>   in gd->arch.mtrr_req[], which won't cause any issue but is unnecessary
> - The way such combination works is based on the assumption that U-Boot
>   has full control with MTRR programming (e.g.: U-Boot without any blob
>   that does all low-level initialization on its own, or using FSP2 which
>   does not touch MTRR), but this is not the case with FSP. FSP programs
>   some MTRRs during its execution but U-Boot does not have the settings
>   saved in gd->arch.mtrr_req[] and when doing mtrr_commit() it will
>   corrupt what was already programmed previously.
>
> Correct this to use mtrr_set_next_var() instead.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
>  arch/x86/lib/fsp/fsp_graphics.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Thanks for looking into this. The series works fine on link. I suspect
it will be find on samus too, but I cannot test right now. Sadly
minnowmax is also dead right now but I hope to fix it soon. I don't
expect any problems there.

However, for coral, this first patch breaks the mtrrs. With master we get:

=> mtrr
CPU 0:
Reg Valid Write-type   Base   ||        Mask   ||        Size   ||
0   Y     Back         00000000fef00000 0000007ffff80000 0000000000080000
1   Y     Back         00000000fef80000 0000007ffffc0000 0000000000040000
2   Y     Back         0000000000000000 0000007f80000000 0000000080000000
3   Y     Combine      00000000b0000000 0000007ff0000000 0000000010000000
4   Y     Back         0000000100000000 0000007f80000000 0000000080000000
5   N     Uncacheable  0000000000000000 0000000000000000 0000008000000000
6   N     Uncacheable  0000000000000000 0000000000000000 0000008000000000
7   N     Uncacheable  0000000000000000 0000000000000000 0000008000000000
8   N     Uncacheable  0000000000000000 0000000000000000 0000008000000000
9   N     Uncacheable  0000000000000000 0000000000000000 0000008000000000

with this patch on coral we get:

=> mtrr
CPU 0:
Reg Valid Write-type   Base   ||        Mask   ||        Size   ||
0   Y     Back         00000000fef00000 0000007ffff80000 0000000000080000
1   Y     Back         00000000fef80000 0000007ffffc0000 0000000000040000
2   Y     Combine      00000000b0000000 0000007ff0000000 0000000010000000
3   N     Uncacheable  0000000000000000 0000000000000000 0000008000000000

At present coral expects to handle the MTRRs itself, and it seems that
perhaps the APL FSPv2 does not? Do we need a new Kconfig for dealing
with FSPv2 perhaps?

Regards,
Simon