[PATCH v3 0/2] x86: Various fixes to MTRR and FSP codes
At present Intel Crown Bay does not boot. This was caused by various regression issues introduced when supporting FSP2, and some flaws in MTRR codes.
With this series, U-Boot boot again on Intel Crown Bay board.
Changes in v3: - Add the missing header file include
Changes in v2: - Keep programing MTRR to FSP2 only - Move board_final_cleanup() to fsp2 directory
Bin Meng (2): x86: fsp: Don't program MTRR for DRAM for FSP1 x86: fsp: Only FSP2 has INIT_PHASE_END_FIRMWARE
arch/x86/lib/fsp/fsp_common.c | 16 ---------------- arch/x86/lib/fsp/fsp_dram.c | 27 +++++++++++++++++++++++---- arch/x86/lib/fsp2/fsp_common.c | 17 +++++++++++++++++ 3 files changed, 40 insertions(+), 20 deletions(-)
There are several outstanding issues as to why this does not apply to FSP1:
* For FSP1, the system memory and reserved memory used by FSP are already programmed in the MTRR by FSP. * The 'mtrr_top' mistakenly includes TSEG memory range that has the same RES_MEM_RESERVED resource type. Its address is programmed and reported by FSP to be near the top of 4 GiB space, which is not what we want for SDRAM. * The call to mtrr_add_request() is not guaranteed to have its size to be exactly the power of 2. This causes reserved bits of the IA32_MTRR_PHYSMASK register to be written which generates #GP.
For FSP2, it seems this is necessary as without this, U-Boot boot process on Chromebook Coral goes very slowly.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
(no changes since v2)
Changes in v2: - Keep programing MTRR to FSP2 only
arch/x86/lib/fsp/fsp_dram.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/arch/x86/lib/fsp/fsp_dram.c b/arch/x86/lib/fsp/fsp_dram.c index 8ad9aeedac..2bd408d0c5 100644 --- a/arch/x86/lib/fsp/fsp_dram.c +++ b/arch/x86/lib/fsp/fsp_dram.c @@ -48,12 +48,28 @@ int dram_init_banksize(void) phys_addr_t mtrr_top; phys_addr_t low_end; uint bank; + bool update_mtrr; + + /* + * For FSP1, the system memory and reserved memory used by FSP are + * already programmed in the MTRR by FSP. Also it is observed that + * FSP on Intel Queensbay platform reports the TSEG memory range + * that has the same RES_MEM_RESERVED resource type whose address + * is programmed by FSP to be near the top of 4 GiB space, which is + * not what we want for DRAM. + * + * However it seems FSP2's behavior is different. We need to add the + * DRAM range in MTRR otherwise the boot process goes very slowly, + * which was observed on Chrromebook Coral with FSP2. + */ + update_mtrr = CONFIG_IS_ENABLED(FSP_VERSION2);
if (!ll_boot_init()) { gd->bd->bi_dram[0].start = 0; gd->bd->bi_dram[0].size = gd->ram_size;
- mtrr_add_request(MTRR_TYPE_WRBACK, 0, gd->ram_size); + if (update_mtrr) + mtrr_add_request(MTRR_TYPE_WRBACK, 0, gd->ram_size); return 0; }
@@ -76,8 +92,10 @@ int dram_init_banksize(void) } else { gd->bd->bi_dram[bank].start = res_desc->phys_start; gd->bd->bi_dram[bank].size = res_desc->len; - mtrr_add_request(MTRR_TYPE_WRBACK, res_desc->phys_start, - res_desc->len); + if (update_mtrr) + mtrr_add_request(MTRR_TYPE_WRBACK, + res_desc->phys_start, + res_desc->len); log_debug("ram %llx %llx\n", gd->bd->bi_dram[bank].start, gd->bd->bi_dram[bank].size); @@ -92,7 +110,8 @@ int dram_init_banksize(void) * Set up an MTRR to the top of low, reserved memory. This is necessary * for graphics to run at full speed in U-Boot. */ - mtrr_add_request(MTRR_TYPE_WRBACK, 0, mtrr_top); + if (update_mtrr) + mtrr_add_request(MTRR_TYPE_WRBACK, 0, mtrr_top);
return 0; }
On Mon, 2 Aug 2021 at 03:45, Bin Meng bmeng.cn@gmail.com wrote:
There are several outstanding issues as to why this does not apply to FSP1:
- For FSP1, the system memory and reserved memory used by FSP are already programmed in the MTRR by FSP.
- The 'mtrr_top' mistakenly includes TSEG memory range that has the same RES_MEM_RESERVED resource type. Its address is programmed and reported by FSP to be near the top of 4 GiB space, which is not what we want for SDRAM.
- The call to mtrr_add_request() is not guaranteed to have its size to be exactly the power of 2. This causes reserved bits of the IA32_MTRR_PHYSMASK register to be written which generates #GP.
For FSP2, it seems this is necessary as without this, U-Boot boot process on Chromebook Coral goes very slowly.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
(no changes since v2)
Changes in v2:
- Keep programing MTRR to FSP2 only
arch/x86/lib/fsp/fsp_dram.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org Tested on chromebook_coral, chromebook_samus, chromebook_link, minnowmax Tested-by: Simon Glass sjg@chromium.org
On Mon, Aug 2, 2021 at 10:45 PM Simon Glass sjg@chromium.org wrote:
On Mon, 2 Aug 2021 at 03:45, Bin Meng bmeng.cn@gmail.com wrote:
There are several outstanding issues as to why this does not apply to FSP1:
- For FSP1, the system memory and reserved memory used by FSP are already programmed in the MTRR by FSP.
- The 'mtrr_top' mistakenly includes TSEG memory range that has the same RES_MEM_RESERVED resource type. Its address is programmed and reported by FSP to be near the top of 4 GiB space, which is not what we want for SDRAM.
- The call to mtrr_add_request() is not guaranteed to have its size to be exactly the power of 2. This causes reserved bits of the IA32_MTRR_PHYSMASK register to be written which generates #GP.
For FSP2, it seems this is necessary as without this, U-Boot boot process on Chromebook Coral goes very slowly.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
(no changes since v2)
Changes in v2:
- Keep programing MTRR to FSP2 only
arch/x86/lib/fsp/fsp_dram.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org Tested on chromebook_coral, chromebook_samus, chromebook_link, minnowmax Tested-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!
For FSP1, there is no such INIT_PHASE_END_FIRMWARE.
Move board_final_cleanup() to fsp2 directory.
Fixes: 7c73cea44290 ("x86: Notify the FSP of the 'end firmware' event") Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v3: - Add the missing header file include
Changes in v2: - Move board_final_cleanup() to fsp2 directory
arch/x86/lib/fsp/fsp_common.c | 16 ---------------- arch/x86/lib/fsp2/fsp_common.c | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c index 6365b0a50a..82f7d3ab5f 100644 --- a/arch/x86/lib/fsp/fsp_common.c +++ b/arch/x86/lib/fsp/fsp_common.c @@ -61,22 +61,6 @@ void board_final_init(void) debug("OK\n"); }
-void board_final_cleanup(void) -{ - u32 status; - - /* TODO(sjg@chromium.org): This causes Linux to crash */ - return; - - /* call into FspNotify */ - debug("Calling into FSP (notify phase INIT_PHASE_END_FIRMWARE): "); - status = fsp_notify(NULL, INIT_PHASE_END_FIRMWARE); - if (status) - debug("fail, error code %x\n", status); - else - debug("OK\n"); -} - int fsp_save_s3_stack(void) { struct udevice *dev; diff --git a/arch/x86/lib/fsp2/fsp_common.c b/arch/x86/lib/fsp2/fsp_common.c index f69456e43a..20c3f6406a 100644 --- a/arch/x86/lib/fsp2/fsp_common.c +++ b/arch/x86/lib/fsp2/fsp_common.c @@ -6,8 +6,25 @@
#include <common.h> #include <init.h> +#include <asm/fsp/fsp_support.h>
int arch_fsp_init(void) { return 0; } + +void board_final_cleanup(void) +{ + u32 status; + + /* TODO(sjg@chromium.org): This causes Linux to crash */ + return; + + /* call into FspNotify */ + debug("Calling into FSP (notify phase INIT_PHASE_END_FIRMWARE): "); + status = fsp_notify(NULL, INIT_PHASE_END_FIRMWARE); + if (status) + debug("fail, error code %x\n", status); + else + debug("OK\n"); +}
On Mon, 2 Aug 2021 at 03:45, Bin Meng bmeng.cn@gmail.com wrote:
For FSP1, there is no such INIT_PHASE_END_FIRMWARE.
Move board_final_cleanup() to fsp2 directory.
Fixes: 7c73cea44290 ("x86: Notify the FSP of the 'end firmware' event") Signed-off-by: Bin Meng bmeng.cn@gmail.com
Changes in v3:
- Add the missing header file include
Changes in v2:
- Move board_final_cleanup() to fsp2 directory
arch/x86/lib/fsp/fsp_common.c | 16 ---------------- arch/x86/lib/fsp2/fsp_common.c | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org Tested on chromebook_coral, chromebook_samus, chromebook_link, minnowmax Tested-by: Simon Glass sjg@chromium.org
On Mon, Aug 2, 2021 at 10:45 PM Simon Glass sjg@chromium.org wrote:
On Mon, 2 Aug 2021 at 03:45, Bin Meng bmeng.cn@gmail.com wrote:
For FSP1, there is no such INIT_PHASE_END_FIRMWARE.
Move board_final_cleanup() to fsp2 directory.
Fixes: 7c73cea44290 ("x86: Notify the FSP of the 'end firmware' event") Signed-off-by: Bin Meng bmeng.cn@gmail.com
Changes in v3:
- Add the missing header file include
Changes in v2:
- Move board_final_cleanup() to fsp2 directory
arch/x86/lib/fsp/fsp_common.c | 16 ---------------- arch/x86/lib/fsp2/fsp_common.c | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org Tested on chromebook_coral, chromebook_samus, chromebook_link, minnowmax Tested-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!
participants (2)
-
Bin Meng -
Simon Glass