x86: Add support for MTRRs

We don't need this in U-Boot since we calculate it based on available memory.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/Kconfig | 4 ----
 1 file changed, 4 deletions(-)

arch/x86/Kconfig +0 -0

index 7d007bb..e992e64 100644
@@ -47,10 +47,6 @@ config RAMBASE
        hex

This currently assumes that U-Boot resides at the start of ROM. Update it to remove this assumption.

Signed-off-by Simon Glass <[email protected]>

---

 tools/ifdtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/ifdtool.c +0 -0

index fe8366b..590ccc9 100644
@@ -788,9 +788,9 @@ static int write_uboot(char *image, int size, struct input_file *uboot,
                              fdt_strerror(data_size));
                        return -ENOENT;

Quoted 16 lines · Simon Glass

This currently assumes that U-Boot resides at the start of ROM. Update it to remove this assumption.

Signed-off-by Simon Glass <[email protected]>

---

 tools/ifdtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/ifdtool.c +0 -0

index fe8366b..590ccc9 100644
@@ -788,9 +788,9 @@ static int write_uboot(char *image, int size, struct input_file *uboot,
                              fdt_strerror(data_size));
                        return -ENOENT;

tools/ifdtool.c static int write_uboot(char *image, int size, struct input_file *uboot,

Quoted 10 lines

tools/ifdtool.c +2 -3

                }
-               offset = ucode_ptr - uboot->addr;
+               offset = (uint32_t)(ucode_ptr + size);
                ptr = (void *)image + offset;
-               ptr[0] = uboot->addr + (data - image);
+               ptr[0] = (data - image) - size;
                ptr[1] = data_size;
                debug("Wrote microcode pointer at %x: addr=%x, size=%x\n",
                      ucode_ptr, ptr[0], ptr[1]);
--

Reviewed-by Bin Meng <[email protected]>

Tested on Intel Crown Bay by adjusting ROM_SIZE to 2MB

Tested-by Bin Meng <[email protected]>

This can be very slow - typically 80ms even on a fast machine since it uses the SPI flash to read the data. Add an option to display the time taken.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/pci/pci_rom.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/pci/pci_rom.c +2 -0

index 9808bb3..5ba315b 100644
@@ -156,6 +156,8 @@ int pci_rom_load(uint16_t class, struct pci_rom_header *rom_header,
 
        target = (void *)PCI_VGA_RAM_IMAGE_START;
        if (target != rom_header) {
+               ulong start = get_timer(0);
+
                debug("Copying VGA ROM Image from %p to %p, 0x%x bytes\n",
                      rom_header, target, rom_size);
                memcpy(target, rom_header, rom_size);

This takes about about 700ms on link when running natively and 900ms when running using the emulator. It is a waste of time if video is not enabled, so don't bother running the video BIOS in that case.

We could add a command to run the video BIOS later when needed, but this is not considered at present.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/ivybridge/gma.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

arch/x86/cpu/ivybridge/gma.c +0 -0

index 3d7f740..125021b 100644

The hex value is more commonly understood, so use that instead of decimal. Add a 0x prefix to avoid confusion.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/pci/pci_rom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/pci/pci_rom.c +1 -1

index 5ba315b..7d25cc9 100644
@@ -247,7 +247,7 @@ int pci_run_vga_bios(pci_dev_t dev, int (*int15_handler)(void), bool emulate)
                defined(CONFIG_FRAMEBUFFER_VESA_MODE)
        vesa_mode = CONFIG_FRAMEBUFFER_VESA_MODE;
 #endif
-       debug("Selected vesa mode %d\b", vesa_mode);
+       debug("Selected vesa mode %#x\n", vesa_mode);
        if (emulate) {
 #ifdef CONFIG_BIOSEMU
                BE_VGAInfo *info;

Memory Type Range Registers are used to tell the CPU whether memory is cacheable and if so the cache write mode to use.

Clean up the existing header file to follow style, and remove the unneeded code.

These can speed up booting so should be supported. Add these to global_data so they can be requested while booting. We will apply the changes during relocation (in a later commit).

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/Makefile              |   1 +
 arch/x86/cpu/coreboot/coreboot.c   |  22 +++--
 arch/x86/cpu/ivybridge/car.S       |  12 +--
 arch/x86/cpu/mtrr.c                |  81 ++++++++++++++++++
 arch/x86/include/asm/global_data.h |  15 ++++
 arch/x86/include/asm/mtrr.h        | 163 +++++++++++++++++--------------------
 6 files changed, 186 insertions(+), 108 deletions(-)
 create mode 100644 arch/x86/cpu/mtrr.c

arch/x86/cpu/Makefile +1 -0

index 5033d2b..62e43c0 100644
@@ -17,5 +17,6 @@ obj-$(CONFIG_NORTHBRIDGE_INTEL_SANDYBRIDGE) += ivybridge/
 obj-$(CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE) += ivybridge/
 obj-$(CONFIG_INTEL_QUEENSBAY) += queensbay/
 obj-y += lapic.o
+obj-y += mtrr.o
 obj-$(CONFIG_PCI) += pci.o
 obj-y += turbo.o

arch/x86/cpu/coreboot/coreboot.c +10 -12

index cfacc05..6d06d5a 100644
@@ -15,6 +15,7 @@
 #include <asm/cache.h>
 #include <asm/cpu.h>
 #include <asm/io.h>
+#include <asm/mtrr.h>
 #include <asm/arch/tables.h>
 #include <asm/arch/sysinfo.h>
 #include <asm/arch/timestamp.h>

@@ -64,11 +65,6 @@ int board_eth_init(bd_t *bis)
        return pci_eth_init(bis);
 }
 
-#define MTRR_TYPE_WP          5
-#define MTRRcap_MSR           0xfe
-#define MTRRphysBase_MSR(reg) (0x200 + 2 * (reg))
-#define MTRRphysMask_MSR(reg) (0x200 + 2 * (reg) + 1)
-
 void board_final_cleanup(void)
 {
        /* Un-cache the ROM so the kernel has one

@@ -77,15 +73,17 @@ void board_final_cleanup(void)
         * Coreboot should have assigned this to the
         * top available variable MTRR.
         */
-       u8 top_mtrr = (native_read_msr(MTRRcap_MSR) & 0xff) - 1;
-       u8 top_type = native_read_msr(MTRRphysBase_MSR(top_mtrr)) & 0xff;
+       u8 top_mtrr = (native_read_msr(MTRR_CAP_MSR) & 0xff) - 1;
+       u8 top_type = native_read_msr(MTRR_PHYS_BASE_MSR(top_mtrr)) & 0xff;
 
        /* Make sure this MTRR is the correct Write-Protected type */
-       if (top_type == MTRR_TYPE_WP) {
-               disable_caches();
-               wrmsrl(MTRRphysBase_MSR(top_mtrr), 0);
-               wrmsrl(MTRRphysMask_MSR(top_mtrr), 0);
-               enable_caches();
+       if (top_type == MTRR_TYPE_WRPROT) {
+               struct mtrr_state state;
+
+               mtrr_open(&state);
+               wrmsrl(MTRR_PHYS_BASE_MSR(top_mtrr), 0);
+               wrmsrl(MTRR_PHYS_MASK_MSR(top_mtrr), 0);
+               mtrr_close(&state);
        }
 
        /* Issue SMI to Coreboot to lock down ME and registers */

arch/x86/cpu/ivybridge/car.S +6 -6

index dca68e4..72b22ea 100644
@@ -61,7 +61,7 @@ clear_mtrrs:
 
        post_code(POST_CAR_MTRR)
        /* Configure the default memory type to uncacheable */
-       movl    $MTRRdefType_MSR, %ecx
+       movl    $MTRR_DEF_TYPE_MSR, %ecx
        rdmsr
        andl    $(~0x00000cff), %eax
        wrmsr

@@ -76,16 +76,16 @@ clear_mtrrs:
        post_code(POST_CAR_BASE_ADDRESS)
        /* Set Cache-as-RAM mask */
        movl    $(MTRR_PHYS_MASK_MSR(0)), %ecx
-       movl    $(~(CACHE_AS_RAM_SIZE - 1) | MTRRphysMaskValid), %eax
+       movl    $(~(CACHE_AS_RAM_SIZE - 1) | MTRR_PHYS_MASK_VALID), %eax
        movl    $CPU_PHYSMASK_HI, %edx
        wrmsr
 
        post_code(POST_CAR_MASK)
 
        /* Enable MTRR */
-       movl    $MTRRdefType_MSR, %ecx
+       movl    $MTRR_DEF_TYPE_MSR, %ecx
        rdmsr
-       orl     $MTRRdefTypeEn, %eax
+       orl     $MTRR_DEF_TYPE_EN, %eax
        wrmsr
 
        /* Enable cache (CR0.CD = 0, CR0.NW = 0) */

@@ -130,7 +130,7 @@ clear_mtrrs:
 
        movl    $MTRR_PHYS_MASK_MSR(1), %ecx
        movl    $CPU_PHYSMASK_HI, %edx
-       movl    $(~(CONFIG_XIP_ROM_SIZE - 1) | MTRRphysMaskValid), %eax
+       movl    $(~(CONFIG_XIP_ROM_SIZE - 1) | MTRR_PHYS_MASK_VALID), %eax
        wrmsr
 
        post_code(POST_CAR_ROM_CACHE)

@@ -141,7 +141,7 @@ clear_mtrrs:
        xorl    %edx, %edx
        wrmsr
        movl    $MTRR_PHYS_MASK_MSR(2), %ecx
-       movl    $(CACHE_MRC_MASK | MTRRphysMaskValid), %eax
+       movl    $(CACHE_MRC_MASK | MTRR_PHYS_MASK_VALID), %eax
        movl    $CPU_PHYSMASK_HI, %edx
        wrmsr
 #endif

arch/x86/cpu/mtrr.c new file +81 -0

new file mode 100644
index 0000000..d5a825d1
@@ -0,0 +1,81 @@
+/*
+ * (C) Copyright 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ *
+ * Memory Type Range Regsters - these are used to tell the CPU whether
+ * memory is cacheable and if so the cache write mode to use.
+ *
+ * These can speed up booting. See the mtrr command.
+ *
+ * Reference: Intel Architecture Software Developer's Manual, Volume 3:
+ * System Programming
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/msr.h>
+#include <asm/mtrr.h>
+
+/* Prepare to adjust MTRRs */
+void mtrr_open(struct mtrr_state *state)
+{
+       state->enable_cache = dcache_status();
+
+       if (state->enable_cache)
+               disable_caches();
+       state->deftype = native_read_msr(MTRR_DEF_TYPE_MSR);
+       wrmsrl(MTRR_DEF_TYPE_MSR, state->deftype & ~MTRR_DEF_TYPE_EN);
+}
+
+/* Clean up after adjusting MTRRs, and enable them */
+void mtrr_close(struct mtrr_state *state)
+{
+       wrmsrl(MTRR_DEF_TYPE_MSR, state->deftype | MTRR_DEF_TYPE_EN);
+       if (state->enable_cache)
+               enable_caches();
+}
+
+int mtrr_commit(bool do_caches)
+{
+       struct mtrr_request *req = gd->arch.mtrr_req;
+       struct mtrr_state state;
+       uint64_t mask;
+       int i;
+
+       mtrr_open(&state);
+       for (i = 0; i < gd->arch.mtrr_req_count; i++, req++) {
+               mask = ~(req->size - 1);
+               mask &= (1ULL << CONFIG_CPU_ADDR_BITS) - 1;
+               wrmsrl(MTRR_PHYS_BASE_MSR(i), req->start | req->type);
+               wrmsrl(MTRR_PHYS_MASK_MSR(i), mask | MTRR_PHYS_MASK_VALID);
+       }
+
+       /* Clear the ones that are unused */
+       for (; i < MTRR_COUNT; i++)
+               wrmsrl(MTRR_PHYS_MASK_MSR(i), 0);
+       mtrr_close(&state);
+
+       return 0;
+}
+
+int mtrr_add_request(int type, uint64_t start, uint64_t size)
+{
+       struct mtrr_request *req;
+       uint64_t mask;
+
+       if (gd->arch.mtrr_req_count == MAX_MTRR_REQUESTS)
+               return -ENOSPC;
+       req = &gd->arch.mtrr_req[gd->arch.mtrr_req_count++];
+       req->type = type;
+       req->start = start;
+       req->size = size;
+       debug("%d: type=%d, %08llx  %08llx\n", gd->arch.mtrr_req_count - 1,
+             req->type, req->start, req->size);
+       mask = ~(req->size - 1);
+       mask &= (1ULL << CONFIG_CPU_ADDR_BITS) - 1;
+       mask |= MTRR_PHYS_MASK_VALID;
+       debug("   %016llx %016llx\n", req->start | req->type, mask);
+
+       return 0;
+}

arch/x86/include/asm/global_data.h +15 -0

index 03d491a..15e76f6 100644
@@ -29,6 +29,19 @@ struct memory_info {
        struct memory_area area[CONFIG_NR_DRAM_BANKS];
 };
 
+#define MAX_MTRR_REQUESTS      8
+
+/**
+ * A request for a memory region to be set up in a particular way. These
+ * requests are processed before board_init_r() is called. They are generally
+ * optional and can be ignored with some performance impact.
+ */
+struct mtrr_request {
+       int type;               /* MTRR_TYPE_... */
+       uint64_t start;
+       uint64_t size;
+};
+
 /* Architecture-specific global data */
 struct arch_global_data {
        struct global_data *gd_addr;            /* Location of Global Data */

@@ -50,6 +63,8 @@ struct arch_global_data {
 #ifdef CONFIG_HAVE_FSP
        void    *hob_list;              /* FSP HOB list */
 #endif
+       struct mtrr_request mtrr_req[MAX_MTRR_REQUESTS];
+       int mtrr_req_count;
 };
 
 #endif

arch/x86/include/asm/mtrr.h +68 -84

index 5f05a48..3c11740 100644
@@ -9,99 +9,86 @@
 #ifndef _ASM_MTRR_H
 #define _ASM_MTRR_H
 
-/*  These are the region types  */
-#define MTRR_TYPE_UNCACHEABLE 0
-#define MTRR_TYPE_WRCOMB     1
-/*#define MTRR_TYPE_         2*/
-/*#define MTRR_TYPE_         3*/
-#define MTRR_TYPE_WRTHROUGH  4
-#define MTRR_TYPE_WRPROT     5
-#define MTRR_TYPE_WRBACK     6
-#define MTRR_NUM_TYPES       7
-
-#define MTRRcap_MSR     0x0fe
-#define MTRRdefType_MSR 0x2ff
-
-#define MTRRdefTypeEn          (1 << 11)
-#define MTRRdefTypeFixEn       (1 << 10)
-
-#define SMRRphysBase_MSR 0x1f2
-#define SMRRphysMask_MSR 0x1f3
-
-#define MTRRphysBase_MSR(reg) (0x200 + 2 * (reg))
-#define MTRRphysMask_MSR(reg) (0x200 + 2 * (reg) + 1)
-
-#define MTRRphysMaskValid      (1 << 11)
-
-#define NUM_FIXED_RANGES 88
-#define RANGES_PER_FIXED_MTRR 8
-#define MTRRfix64K_00000_MSR 0x250
-#define MTRRfix16K_80000_MSR 0x258
-#define MTRRfix16K_A0000_MSR 0x259
-#define MTRRfix4K_C0000_MSR 0x268
-#define MTRRfix4K_C8000_MSR 0x269
-#define MTRRfix4K_D0000_MSR 0x26a
-#define MTRRfix4K_D8000_MSR 0x26b
-#define MTRRfix4K_E0000_MSR 0x26c
-#define MTRRfix4K_E8000_MSR 0x26d
-#define MTRRfix4K_F0000_MSR 0x26e
-#define MTRRfix4K_F8000_MSR 0x26f
+/* MTRR region types */
+#define MTRR_TYPE_UNCACHEABLE  0
+#define MTRR_TYPE_WRCOMB       1
+#define MTRR_TYPE_WRTHROUGH    4
+#define MTRR_TYPE_WRPROT       5
+#define MTRR_TYPE_WRBACK       6
+
+#define MTRR_TYPE_COUNT                7
+
+#define MTRR_CAP_MSR           0x0fe
+#define MTRR_DEF_TYPE_MSR      0x2ff
+
+#define MTRR_DEF_TYPE_EN       (1 << 11)
+#define MTRR_DEF_TYPE_FIX_EN   (1 << 10)
+
+#define MTRR_PHYS_BASE_MSR(reg)        (0x200 + 2 * (reg))
+#define MTRR_PHYS_MASK_MSR(reg)        (0x200 + 2 * (reg) + 1)
+
+#define MTRR_PHYS_MASK_VALID   (1 << 11)
+
+#define MTRR_BASE_TYPE_MASK    0x7
+
+/* Number of MTRRs supported */
+#define MTRR_COUNT             8
 
 #if !defined(__ASSEMBLER__)
 
-/*
- * The MTRR code has some side effects that the callers should be aware for.
- * 1. The call sequence matters. x86_setup_mtrrs() calls
- *    x86_setup_fixed_mtrrs_no_enable() then enable_fixed_mtrrs() (equivalent
- *    of x86_setup_fixed_mtrrs()) then x86_setup_var_mtrrs(). If the callers
- *    want to call the components of x86_setup_mtrrs() because of other
- *    rquirements the ordering should still preserved.
- * 2. enable_fixed_mtrr() will enable both variable and fixed MTRRs because
- *    of the nature of the global MTRR enable flag. Therefore, all direct
- *    or indirect callers of enable_fixed_mtrr() should ensure that the
- *    variable MTRR MSRs do not contain bad ranges.
- * 3. If CONFIG_CACHE_ROM is selected an MTRR is allocated for enabling
- *    the caching of the ROM. However, it is set to uncacheable (UC). It
- *    is the responsiblity of the caller to enable it by calling
- *    x86_mtrr_enable_rom_caching().
+/**
+ * Information about the previous MTRR state, set up by mtrr_open()
+ *
+ * @deftype:           Previous value of MTRR_DEF_TYPE_MSR
+ * @enable_cache:      true if cache was enabled
  */
-void x86_setup_mtrrs(void);
-/*
- * x86_setup_var_mtrrs() parameters:
- * address_bits - number of physical address bits supported by cpu
- * above4gb - 2 means dynamically detect number of variable MTRRs available.
- *            non-zero means handle memory ranges above 4GiB.
- *            0 means ignore memory ranges above 4GiB
+struct mtrr_state {
+       uint64_t deftype;
+       bool enable_cache;
+};
+
+/**
+ * mtrr_open() - Prepare to adjust MTRRs
+ *
+ * Use mtrr_open() passing in a structure - this function will init it. Then
+ * when done, pass the same structure to mtrr_close() to re-enable MTRRs and
+ * possibly the cache.
+ *
+ * @state:     Empty structure to pass in to hold settings
  */
-void x86_setup_var_mtrrs(unsigned int address_bits, unsigned int above4gb);
-void enable_fixed_mtrr(void);
-void x86_setup_fixed_mtrrs(void);
-/* Set up fixed MTRRs but do not enable them. */
-void x86_setup_fixed_mtrrs_no_enable(void);
-int x86_mtrr_check(void);
-/* ROM caching can be used after variable MTRRs are set up. Beware that
- * enabling CONFIG_CACHE_ROM will eat through quite a few MTRRs based on
- * one's IO hole size and WRCOMB resources. Be sure to check the console
- * log when enabling CONFIG_CACHE_ROM or adding WRCOMB resources. Beware that
- * on CPUs with core-scoped MTRR registers such as hyperthreaded CPUs the
- * rom caching will be disabled if all threads run the MTRR code. Therefore,
- * one needs to call x86_mtrr_enable_rom_caching() after all threads of the
- * same core have run the MTRR code. */
-#if CONFIG_CACHE_ROM
-void x86_mtrr_enable_rom_caching(void);
-void x86_mtrr_disable_rom_caching(void);
-/* Return the variable range MTRR index of the ROM cache. */
-long x86_mtrr_rom_cache_var_index(void);
-#else
-static inline void x86_mtrr_enable_rom_caching(void) {}
-static inline void x86_mtrr_disable_rom_caching(void) {}
-static inline long x86_mtrr_rom_cache_var_index(void) { return -1; }
-#endif /* CONFIG_CACHE_ROM */
+void mtrr_open(struct mtrr_state *state);
 
-#endif
+/**
+ * mtrr_open() - Clean up after adjusting MTRRs, and enable them
+ *
+ * This uses the structure containing information returned from mtrr_open().
+ *
+ * @state:     Structure from mtrr_open()
+ */
+/*  */
+void mtrr_close(struct mtrr_state *state);
+
+/**
+ * mtrr_add_request() - Add a new MTRR request
+ *
+ * This adds a request for a memory region to be set up in a particular way.
+ *
+ * @type:      Requested type (MTRR_TYPE_)
+ * @start:     Start address
+ * @size:      Size
+ */
+int mtrr_add_request(int type, uint64_t start, uint64_t size);
+
+/**
+ * mtrr_commit() - set up the MTRR registers based on current requests
+ *
+ * This sets up MTRRs for the available DRAM and the requests received so far.

Set the frame buffer to write-combining. This makes it faster, although for scrolling write-through is even faster for U-Boot.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/ivybridge/gma.c | 8 ++++++++
 1 file changed, 8 insertions(+)

arch/x86/cpu/ivybridge/gma.c +3 -0

index 125021b..03b2ebd 100644
@@ -12,6 +12,7 @@
 #include <fdtdec.h>
 #include <pci_rom.h>
 #include <asm/io.h>
+#include <asm/mtrr.h>
 #include <asm/pci.h>
 #include <asm/arch/pch.h>
 #include <asm/arch/sandybridge.h>

@@ -738,6 +739,8 @@ int gma_func0_init(pci_dev_t dev, struct pci_controller *hose,
                   const void *blob, int node)
 {
        void *gtt_bar;
+       ulong start;
+       ulong base;
        u32 reg32;
        int ret;
 

For bare platforms we turn off ROM-caching before calling board_init_f_r() It is then very slow to copy U-Boot from ROM to RAM. So adjust the order so that the copying happens before we turn off ROM-caching.

Signed-off-by Simon Glass <[email protected]>

---

 common/board_f.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

common/board_f.c +5 -0

index 98c9c72..1b65575 100644
@@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
        INIT_FUNC_WATCHDOG_RESET
        reloc_fdt,
        setup_reloc,
+#ifdef CONFIG_X86
+       copy_uboot_to_ram,
+       clear_bss,
+       do_elf_reloc_fixups,
+#endif
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
        jump_to_copy,
 #endif

Hi Simon,

Quoted 34 lines · Simon Glass

For bare platforms we turn off ROM-caching before calling board_init_f_r() It is then very slow to copy U-Boot from ROM to RAM. So adjust the order so that the copying happens before we turn off ROM-caching.

Signed-off-by Simon Glass <[email protected]>

---

 common/board_f.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

common/board_f.c +5 -3

index 98c9c72..1b65575 100644
@@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
        INIT_FUNC_WATCHDOG_RESET
        reloc_fdt,
        setup_reloc,
+#ifdef CONFIG_X86
+       copy_uboot_to_ram,
+       clear_bss,
+       do_elf_reloc_fixups,
+#endif
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
        jump_to_copy,
 #endif

@@ -1042,9 +1047,6 @@ void board_init_f(ulong boot_flags)
  */
 static init_fnc_t init_sequence_f_r[] = {
        init_cache_f_r,
-       copy_uboot_to_ram,
-       clear_bss,
-       do_elf_reloc_fixups,
 

common/board_f.c void board_init_f(ulong boot_flags)

Quoted 3 lines

common/board_f.c +0 -1

        NULL,
 };
--

I don't understand. Isn't the init_cache_f_r() which turns on the cache?

Hi Bin,

Quoted 38 lines · Bin Meng

Hi Simon,

Quoted 34 lines · Simon Glass

For bare platforms we turn off ROM-caching before calling board_init_f_r() It is then very slow to copy U-Boot from ROM to RAM. So adjust the order so that the copying happens before we turn off ROM-caching.

Signed-off-by Simon Glass <[email protected]>

---

 common/board_f.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

common/board_f.c +5 -3

index 98c9c72..1b65575 100644
@@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
        INIT_FUNC_WATCHDOG_RESET
        reloc_fdt,
        setup_reloc,
+#ifdef CONFIG_X86
+       copy_uboot_to_ram,
+       clear_bss,
+       do_elf_reloc_fixups,
+#endif
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
        jump_to_copy,
 #endif

@@ -1042,9 +1047,6 @@ void board_init_f(ulong boot_flags)
  */
 static init_fnc_t init_sequence_f_r[] = {
        init_cache_f_r,
-       copy_uboot_to_ram,
-       clear_bss,
-       do_elf_reloc_fixups,
 

Quoted 6 lines

common/board_f.c void board_init_f(ulong boot_flags)

Quoted 3 lines

common/board_f.c +0 -1

        NULL,
 };
--

I don't understand. Isn't the init_cache_f_r() which turns on the cache?

Yes it turns on the cache, but turns off the ROM cache (they are different things). So this ordering is much faster. I think I might have more tuning to do though.

Hi Simon,

Quoted 48 lines · Simon Glass

Hi Bin,

Quoted 38 lines · Bin Meng

Hi Simon,

Quoted 34 lines · Simon Glass

For bare platforms we turn off ROM-caching before calling board_init_f_r() It is then very slow to copy U-Boot from ROM to RAM. So adjust the order so that the copying happens before we turn off ROM-caching.

Signed-off-by Simon Glass <[email protected]>

---

 common/board_f.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

common/board_f.c +5 -3

index 98c9c72..1b65575 100644
@@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
        INIT_FUNC_WATCHDOG_RESET
        reloc_fdt,
        setup_reloc,
+#ifdef CONFIG_X86
+       copy_uboot_to_ram,
+       clear_bss,
+       do_elf_reloc_fixups,
+#endif
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
        jump_to_copy,
 #endif

@@ -1042,9 +1047,6 @@ void board_init_f(ulong boot_flags)
  */
 static init_fnc_t init_sequence_f_r[] = {
        init_cache_f_r,
-       copy_uboot_to_ram,
-       clear_bss,
-       do_elf_reloc_fixups,
 

Quoted 6 lines

common/board_f.c void board_init_f(ulong boot_flags)

Quoted 3 lines

common/board_f.c +0 -1

        NULL,
 };
--

I don't understand. Isn't the init_cache_f_r() which turns on the cache?

Quoted 4 lines

Yes it turns on the cache, but turns off the ROM cache (they are different things). So this ordering is much faster. I think I might have more tuning to do though.

Got it. Since we moved these 3 routines from init_sequence_f_r[] to init_sequence_f[], how about we remove the whole init_sequence_f_r[] completely? If not possible, the comment block above init_sequence_f_r[] needs to be fixed?

*
* The '_f_r' sequence must, as a minimum, copy U-Boot to RAM (if
* supported). It _should_, if possible, copy global data to RAM and
* initialise the CPU caches (to speed up the relocation process) *
* NOTE: At present only x86 uses this route, but it is intended that
* all archs will move to this when generic relocation is implemented. */

So looks that we should either drop this _f_r stage, or make other arch use this _f_r?

Hi Bin,

Quoted 56 lines · Bin Meng

Hi Simon,

Quoted 48 lines · Simon Glass

Hi Bin,

Quoted 38 lines · Bin Meng

Hi Simon,

Quoted 34 lines · Simon Glass

For bare platforms we turn off ROM-caching before calling board_init_f_r() It is then very slow to copy U-Boot from ROM to RAM. So adjust the order so that the copying happens before we turn off ROM-caching.

Signed-off-by Simon Glass <[email protected]>

---

 common/board_f.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

common/board_f.c +5 -3

index 98c9c72..1b65575 100644
@@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
        INIT_FUNC_WATCHDOG_RESET
        reloc_fdt,
        setup_reloc,
+#ifdef CONFIG_X86
+       copy_uboot_to_ram,
+       clear_bss,
+       do_elf_reloc_fixups,
+#endif
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
        jump_to_copy,
 #endif

@@ -1042,9 +1047,6 @@ void board_init_f(ulong boot_flags)
  */
 static init_fnc_t init_sequence_f_r[] = {
        init_cache_f_r,
-       copy_uboot_to_ram,
-       clear_bss,
-       do_elf_reloc_fixups,
 

Quoted 6 lines

common/board_f.c void board_init_f(ulong boot_flags)

Quoted 3 lines

common/board_f.c +0 -1

        NULL,
 };
--

I don't understand. Isn't the init_cache_f_r() which turns on the cache?

Quoted 4 lines

Yes it turns on the cache, but turns off the ROM cache (they are different things). So this ordering is much faster. I think I might have more tuning to do though.

Quoted 4 lines

Got it. Since we moved these 3 routines from init_sequence_f_r[] to init_sequence_f[], how about we remove the whole init_sequence_f_r[] completely? If not possible, the comment block above init_sequence_f_r[] needs to be fixed?

I'll remove the comment.

Quoted 10 lines

*
* The '_f_r' sequence must, as a minimum, copy U-Boot to RAM (if
* supported). It _should_, if possible, copy global data to RAM and
* initialise the CPU caches (to speed up the relocation process) *
* NOTE: At present only x86 uses this route, but it is intended that
* all archs will move to this when generic relocation is implemented. */

Quoted 2 lines

So looks that we should either drop this _f_r stage, or make other arch use this _f_r?

I think we should move to generic relocation, meaning that all archs do relocation the same way with the same code. Then only arch-specific stuff is the then ELF fixup function, which adjusts a relocation site, and the code to actually change the stack pointer.

This board_init_f_r() code is part of one attempt to do this - the premise was that turning the cache on before relocating U-Boot will save time. That's true, but it would be even better to turn the cache on much earlier. With pit (Chromebook 2) we turn on the cache in SPL. So I think turning it on here is too late if we are trying to save time. Still it is a good start and if we could make it happen generally it would be nice.

See here for my original attempt, which was tied up with generic board. In the end I untied them and we got one but not the other.

http://lists.denx.de/pipermail/u-boot/2012-February/118409.html

Since then Albert has tidied up ARM start.S a lot which makes this much easier.

So that's the background. One of these days I might take another look at it if it doesn't get someone's attention.

Regards,
Simon

Hi Simon,

Quoted 13 lines · Simon Glass

Hi Bin,

Quoted 10 lines · Bin Meng

Hi Simon,

Quoted 7 lines · Simon Glass

Hi Bin,

Quoted 4 lines · Bin Meng

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass <[email protected]>

Quoted 1 line · wrote:

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

For bare platforms we turn off ROM-caching before calling

board_init_f_r()

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

It is then very slow to copy U-Boot from ROM to RAM. So adjust the

order so

Quoted 32 lines

Quoted 32 lines

Quoted 32 lines

Quoted 32 lines

Quoted 32 lines

that the copying happens before we turn off ROM-caching.

Signed-off-by Simon Glass <[email protected]>

---

 common/board_f.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

common/board_f.c +5 -3

index 98c9c72..1b65575 100644
@@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
        INIT_FUNC_WATCHDOG_RESET
        reloc_fdt,
        setup_reloc,
+#ifdef CONFIG_X86
+       copy_uboot_to_ram,
+       clear_bss,
+       do_elf_reloc_fixups,
+#endif
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
        jump_to_copy,
 #endif

@@ -1042,9 +1047,6 @@ void board_init_f(ulong boot_flags)
  */
 static init_fnc_t init_sequence_f_r[] = {
        init_cache_f_r,
-       copy_uboot_to_ram,
-       clear_bss,
-       do_elf_reloc_fixups,
 

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

common/board_f.c void board_init_f(ulong boot_flags)

Quoted 3 lines

common/board_f.c +0 -1

        NULL,
 };
--

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

I don't understand. Isn't the init_cache_f_r() which turns on the

cache?

Quoted 13 lines

Quoted 11 lines

Quoted 6 lines

Quoted 1 line

Yes it turns on the cache, but turns off the ROM cache (they are different things). So this ordering is much faster. I think I might have more tuning to do though.

Got it. Since we moved these 3 routines from init_sequence_f_r[] to init_sequence_f[], how about we remove the whole init_sequence_f_r[] completely? If not possible, the comment block above init_sequence_f_r[] needs to be fixed?

I'll remove the comment.

Quoted 11 lines

Quoted 11 lines

*
* The '_f_r' sequence must, as a minimum, copy U-Boot to RAM (if
* supported). It _should_, if possible, copy global data to RAM and
* initialise the CPU caches (to speed up the relocation process) *
* NOTE: At present only x86 uses this route, but it is intended that
* all archs will move to this when generic relocation is implemented. */

Quoted 7 lines

Quoted 2 lines

So looks that we should either drop this _f_r stage, or make other arch use this _f_r?

I think we should move to generic relocation, meaning that all archs do relocation the same way with the same code. Then only arch-specific stuff is the then ELF fixup function, which adjusts a relocation site, and the code to actually change the stack pointer.

Quoted 8 lines

This board_init_f_r() code is part of one attempt to do this - the premise was that turning the cache on before relocating U-Boot will save time. That's true, but it would be even better to turn the cache on much earlier. With pit (Chromebook 2) we turn on the cache in SPL. So I think turning it on here is too late if we are trying to save time. Still it is a good start and if we could make it happen generally it would be nice.

Quoted 6 lines

See here for my original attempt, which was tied up with generic board. In the end I untied them and we got one but not the other.

http://lists.denx.de/pipermail/u-boot/2012-February/118409.html

Quoted 1 line

Since then Albert has tidied up ARM start.S a lot which makes this much

easier.

Quoted 3 lines

So that's the background. One of these days I might take another look at it if it doesn't get someone's attention.

Quoted 3 lines

Regards,
Simon

Thanks for the background information. I will take a look. Hope we can achieve generic board support as soon as we can.

Regards,
Bin

Hi Simon & Bin,

Quoted 54 lines · Bin Meng

Hi Simon,

Quoted 50 lines · Simon Glass

Hi Bin,

Quoted 47 lines · Bin Meng

Hi Simon,

Quoted 44 lines · Simon Glass

Hi Bin,

Quoted 41 lines · Bin Meng

Hi Simon,

Quoted 36 lines · Simon Glass

For bare platforms we turn off ROM-caching before calling board_init_f_r()
It is then very slow to copy U-Boot from ROM to RAM. So adjust the order so
that the copying happens before we turn off ROM-caching.

Signed-off-by Simon Glass <[email protected]>

---

 common/board_f.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

common/board_f.c +5 -3

index 98c9c72..1b65575 100644
@@ -983,6 +983,11 @@ static init_fnc_t init_sequence_f[] = {
        INIT_FUNC_WATCHDOG_RESET
        reloc_fdt,
        setup_reloc,
+#ifdef CONFIG_X86
+       copy_uboot_to_ram,
+       clear_bss,
+       do_elf_reloc_fixups,
+#endif
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
        jump_to_copy,
 #endif

@@ -1042,9 +1047,6 @@ void board_init_f(ulong boot_flags)
  */
 static init_fnc_t init_sequence_f_r[] = {
        init_cache_f_r,
-       copy_uboot_to_ram,
-       clear_bss,
-       do_elf_reloc_fixups,
 

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

common/board_f.c void board_init_f(ulong boot_flags)

Quoted 3 lines

common/board_f.c +0 -1

        NULL,
 };
--

Wow, doesn't this bring back some memories. I've had a look over this code as it is now and it took a while to sink in. Things have moved on in the past 2 years :)

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

Quoted 6 lines

I don't understand. Isn't the init_cache_f_r() which turns on the cache?

Yes it turns on the cache, but turns off the ROM cache (they are different things). So this ordering is much faster. I think I might have more tuning to do though.

Quoted 6 lines

Quoted 6 lines

Quoted 4 lines

Got it. Since we moved these 3 routines from init_sequence_f_r[] to init_sequence_f[], how about we remove the whole init_sequence_f_r[] completely? If not possible, the comment block above init_sequence_f_r[] needs to be fixed?

I'll remove the comment.

I think init_sequence_f_r can go... but I need to have a better look at the code

If turning off the ROM cache by init_cache_f_r is the problem, then perhaps the following order would be better:

copy_uboot_to_ram,
init_cache_f_r,
clear_bss,
do_elf_reloc_fixups,

Without enabling the CPU's cache, clear_bss and do_elf_reloc_fixups will suffer.

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

*
* The '_f_r' sequence must, as a minimum, copy U-Boot to RAM (if
* supported). It _should_, if possible, copy global data to RAM and
* initialise the CPU caches (to speed up the relocation process) *
* NOTE: At present only x86 uses this route, but it is intended that
* all archs will move to this when generic relocation is implemented. */

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

So looks that we should either drop this _f_r stage, or make other arch use this _f_r?

I think we should move to generic relocation, meaning that all archs do relocation the same way with the same code. Then only arch-specific stuff is the then ELF fixup function, which adjusts a relocation site, and the code to actually change the stack pointer.

This was always my plan - have arch specific do_reloc_fixups and the rest would be generic

Quoted 9 lines

Quoted 9 lines

This board_init_f_r() code is part of one attempt to do this - the premise was that turning the cache on before relocating U-Boot will save time. That's true, but it would be even better to turn the cache on much earlier. With pit (Chromebook 2) we turn on the cache in SPL. So I think turning it on here is too late if we are trying to save time. Still it is a good start and if we could make it happen generally it would be nice.

And now you have me thinking board_init_f_r is not needed at all...

If we can setup the stack, clear BSS, copy U-Boot to RAM and perform relocation fixups while running from ROM, what is left for board_init_f_r to do?

The only thing I can think of is the caveats mentioned in the comment ('static' variables are read-only / Global Data (gd->xxx) is read/write). But aren't these true when running from ROM?

Looking at non-x86 arches, relocate_code() looks to do what copy_uboot_to_ram and clear_bss does in x86 land (not sure about do_elf_reloc_fixups - seems to be arch specific as to whether relocate_code() does this or it is done later (hence the CONFIG_NEEDS_MANUAL_RELOC #define?)

Quoted 7 lines

Quoted 7 lines

See here for my original attempt, which was tied up with generic board. In the end I untied them and we got one but not the other.

http://lists.denx.de/pipermail/u-boot/2012-February/118409.html

Ah, generic relocation... I really wish my life hadn't taken a hard-left turn when it did. We got so close.

From where I'm looking (fresh eyes - I might be missing something big) we should be able to do the ROM->RAM copy, BSS clearing, and relocation fixups in board_init_f.

When we return from board_init_f it should be a fairly simple matter of:
- Copying the contents of the Global Data structure from CAR to RAM (or
from RAM to RAM if your on a platform which initialises RAM before U-Boot)
- Set the gd pointer (reserved register) to point to the new copy
- Figure out where board_init_r is and jump to it

board_init_r should be able to do any remaining cache tweaks. If cache tweaks cannot be done while executing from RAM then they need to be done in board_init_f

I just cannot see the point of board_init_f_r any more

Nice to hear from you again!

Quoted 28 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 3 lines

I don't understand. Isn't the init_cache_f_r() which turns on the cache?

Yes it turns on the cache, but turns off the ROM cache (they are different things). So this ordering is much faster. I think I might have more tuning to do though.

Quoted 6 lines

Quoted 6 lines

Quoted 4 lines

Got it. Since we moved these 3 routines from init_sequence_f_r[] to init_sequence_f[], how about we remove the whole init_sequence_f_r[] completely? If not possible, the comment block above init_sequence_f_r[] needs to be fixed?

I'll remove the comment.

I think init_sequence_f_r can go... but I need to have a better look at the code

If turning off the ROM cache by init_cache_f_r is the problem, then perhaps the following order would be better:

   copy_uboot_to_ram,
   init_cache_f_r,
   clear_bss,
   do_elf_reloc_fixups,

Quoted 2 lines

Without enabling the CPU's cache, clear_bss and do_elf_reloc_fixups will suffer.

Better would be to have init_cache_f_r after all this I think.

Quoted 19 lines

Quoted 7 lines

Quoted 7 lines

This board_init_f_r() code is part of one attempt to do this - the premise was that turning the cache on before relocating U-Boot will save time. That's true, but it would be even better to turn the cache on much earlier. With pit (Chromebook 2) we turn on the cache in SPL. So I think turning it on here is too late if we are trying to save time. Still it is a good start and if we could make it happen generally it would be nice.

And now you have me thinking board_init_f_r is not needed at all...

If we can setup the stack, clear BSS, copy U-Boot to RAM and perform relocation fixups while running from ROM, what is left for board_init_f_r to do?

The only thing I can think of is the caveats mentioned in the comment ('static' variables are read-only / Global Data (gd->xxx) is read/write). But aren't these true when running from ROM?

Quoted 5 lines

Looking at non-x86 arches, relocate_code() looks to do what copy_uboot_to_ram and clear_bss does in x86 land (not sure about do_elf_reloc_fixups - seems to be arch specific as to whether relocate_code() does this or it is done later (hence the CONFIG_NEEDS_MANUAL_RELOC #define?)

Yes this can be unified. There is still something in there though for board_init_f_r(), at least as things are now. It just so happens on x86 that we are running from ROM and CAR so as I understand it we sort-of have the cache on before relocation. That doesn't apply for coreboot though, so there is probably an optimisation to be made somewhere.

Quoted 24 lines

Quoted 5 lines

Quoted 5 lines

See here for my original attempt, which was tied up with generic board. In the end I untied them and we got one but not the other.

http://lists.denx.de/pipermail/u-boot/2012-February/118409.html

Ah, generic relocation... I really wish my life hadn't taken a hard-left turn when it did. We got so close.

From where I'm looking (fresh eyes - I might be missing something big) we
should be able to do the ROM->RAM copy, BSS clearing, and relocation fixups in board_init_f.

When we return from board_init_f it should be a fairly simple matter of:
- Copying the contents of the Global Data structure from CAR to RAM (or
from RAM to RAM if your on a platform which initialises RAM before U-Boot)
- Set the gd pointer (reserved register) to point to the new copy
- Figure out where board_init_r is and jump to it

board_init_r should be able to do any remaining cache tweaks. If cache tweaks cannot be done while executing from RAM then they need to be done in board_init_f

Quoted 1 line

I just cannot see the point of board_init_f_r any more

Yes, it's hard to see, I'm not sure either.

Anyway I'm going to apply this patch while we figure out what further work can be done.

Hi Simon,

Quoted 3 lines · Simon Glass

Nice to hear from you again!

Thanks - Good to be back in the game

Quoted 36 lines

Quoted 34 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 3 lines

I don't understand. Isn't the init_cache_f_r() which turns on the cache?

Yes it turns on the cache, but turns off the ROM cache (they are different things). So this ordering is much faster. I think I might have more tuning to do though.

Quoted 7 lines

Quoted 7 lines

Quoted 4 lines

Got it. Since we moved these 3 routines from init_sequence_f_r[] to init_sequence_f[], how about we remove the whole init_sequence_f_r[] completely? If not possible, the comment block above init_sequence_f_r[] needs to be fixed?

I'll remove the comment.

I think init_sequence_f_r can go... but I need to have a better look at the
code

If turning off the ROM cache by init_cache_f_r is the problem, then perhaps
the following order would be better:

   copy_uboot_to_ram,
   init_cache_f_r,
   clear_bss,
   do_elf_reloc_fixups,

Without enabling the CPU's cache, clear_bss and do_elf_reloc_fixups will suffer.

Quoted 1 line

Better would be to have init_cache_f_r after all this I think.

Why so? After copying the U-Boot code from ROM to RAM, we would want RAM based operations to be as fast as possible. By this point, we have passed most of the XIP code, so XIP performance _should_ be a non-issue.

Quoted 30 lines

Quoted 23 lines

Quoted 9 lines

Quoted 9 lines

This board_init_f_r() code is part of one attempt to do this - the premise was that turning the cache on before relocating U-Boot will save time. That's true, but it would be even better to turn the cache on much earlier. With pit (Chromebook 2) we turn on the cache in SPL. So I think turning it on here is too late if we are trying to save time. Still it is a good start and if we could make it happen generally it would be nice.

And now you have me thinking board_init_f_r is not needed at all...

If we can setup the stack, clear BSS, copy U-Boot to RAM and perform relocation fixups while running from ROM, what is left for board_init_f_r to
do?

The only thing I can think of is the caveats mentioned in the comment ('static' variables are read-only / Global Data (gd->xxx) is read/write). But aren't these true when running from ROM?

Quoted 5 lines

Looking at non-x86 arches, relocate_code() looks to do what copy_uboot_to_ram and clear_bss does in x86 land (not sure about do_elf_reloc_fixups - seems to be arch specific as to whether relocate_code() does this or it is done later (hence the CONFIG_NEEDS_MANUAL_RELOC #define?)

Quoted 6 lines

Yes this can be unified. There is still something in there though for board_init_f_r(), at least as things are now. It just so happens on x86 that we are running from ROM and CAR so as I understand it we sort-of have the cache on before relocation. That doesn't apply for coreboot though, so there is probably an optimisation to be made somewhere.

U-Boot runs from RAM right from the start in coreboot correct? In which case, coreboot builds can probably do away with all the relocation code I assume coreboot can relocate U-Boot to any place in memory. Last time I looked, coreboot had some ELF processing code.

Quoted 29 lines

Quoted 29 lines

Quoted 6 lines

Quoted 6 lines

See here for my original attempt, which was tied up with generic board. In the end I untied them and we got one but not the other.

http://lists.denx.de/pipermail/u-boot/2012-February/118409.html

Ah, generic relocation... I really wish my life hadn't taken a hard-left turn when it did. We got so close.

From where I'm looking (fresh eyes - I might be missing something big)
we
should be able to do the ROM->RAM copy, BSS clearing, and relocation fixups
in board_init_f.

When we return from board_init_f it should be a fairly simple matter of:
- Copying the contents of the Global Data structure from CAR to RAM (or
from RAM to RAM if your on a platform which initialises RAM before U-Boot)
- Set the gd pointer (reserved register) to point to the new copy
- Figure out where board_init_r is and jump to it

board_init_r should be able to do any remaining cache tweaks. If cache tweaks cannot be done while executing from RAM then they need to be done in
board_init_f

Quoted 7 lines

Quoted 1 line

I just cannot see the point of board_init_f_r any more

Yes, it's hard to see, I'm not sure either.

Anyway I'm going to apply this patch while we figure out what further work can be done.

Sounds good to me.

The pity is that I don't have ANY hardware to test any of this any more, and it looks like any development I do in the foreseeable future will be on ARM. So hacking and testing x86 might be a bit of a problem

Cache-as-RAM should be turned off when we relocate since we want to run from RAM. Add a function to perform this task.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/ivybridge/car.S | 52 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

arch/x86/cpu/ivybridge/car.S +41 -0

index 72b22ea..6e7e1e4 100644
@@ -163,6 +163,58 @@ wait_for_sipi:
        /* return */
        jmp     car_init_ret
 
+.globl car_uninit
+car_uninit:
+       /* Disable cache */
+       movl    %cr0, %eax
+       orl     $X86_CR0_CD, %eax
+       movl    %eax, %cr0
+
+       /* Disable MTRRs */
+       movl    $MTRR_DEF_TYPE_MSR, %ecx
+       rdmsr
+       andl    $(~MTRR_DEF_TYPE_EN), %eax
+       wrmsr
+
+       /* Disable the no-eviction run state */
+       movl    NOEVICTMOD_MSR, %ecx
+       rdmsr
+       andl    $~2, %eax
+       wrmsr
+
+       invd
+
+       /* Disable the no-eviction mode */
+       rdmsr
+       andl    $~1, %eax
+       wrmsr
+
+#ifdef CONFIG_CACHE_MRC_BIN
+       /* Clear the MTRR that was used to cache MRC */
+       xorl    %eax, %eax
+       xorl    %edx, %edx
+       movl    $MTRR_PHYS_BASE_MSR(2), %ecx
+       wrmsr
+       movl    $MTRR_PHYS_MASK_MSR(2), %ecx
+       wrmsr
+#endif
+
+       /* Enable MTRRs */
+       movl    $MTRR_DEF_TYPE_MSR, %ecx
+       rdmsr
+       orl     $MTRR_DEF_TYPE_EN, %eax
+       wrmsr

For platforms with CAR we should disable it before relocation. Check if this function is available and call it if so.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/start.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

arch/x86/cpu/start.S +0 -0

index 125782c..8cebde1 100644
@@ -205,6 +205,16 @@ board_init_f_r_trampoline:
        /* Setup global descriptor table so gd->xyz works */
        call    setup_gdt

Hi Simon,

Quoted 17 lines · Simon Glass

For platforms with CAR we should disable it before relocation. Check if this function is available and call it if so.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/start.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

arch/x86/cpu/start.S +0 -0

index 125782c..8cebde1 100644
@@ -205,6 +205,16 @@ board_init_f_r_trampoline:
        /* Setup global descriptor table so gd->xyz works */
        call    setup_gdt
 

arch/x86/cpu/start.S board_init_f_r_trampoline: · line 216

Quoted 9 lines

arch/x86/cpu/start.S +9 -0

+       /* Set if we need to disable CAR */
+       movl    $car_uninit, %eax
+       cmpl    $0, %eax
+       jz      car_ret
+
+       /* Pass return address in ebx */
+.weak  car_uninit
+       movl    $car_ret, %ebx
+       jmp     car_uninit

Can we use 'call' here instead of jmp and %ebx as the return address?

arch/x86/cpu/start.S board_init_f_r_trampoline: · line 217

Quoted 2 lines

arch/x86/cpu/start.S +1 -0

+car_ret:

car_uninit_ret

arch/x86/cpu/start.S board_init_f_r_trampoline: · line 219

Quoted 2 lines

arch/x86/cpu/start.S +0 -0

        /* Re-enter U-Boot by calling board_init_f_r */
        call    board_init_f_r

At present the normal update (which happens much later) does not work. This seems to have something to do with the 'no eviction' mode in the CAR, or at least moving the microcode update after that causes it not to work.

For now, do an update early on so that it definitely works. Also refuse to continue unless the microcode update check (later in boot) is successful.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/ivybridge/car.S             | 14 ++++++++++++++
 arch/x86/cpu/ivybridge/cpu.c             |  2 +-
 arch/x86/cpu/ivybridge/microcode_intel.c |  9 +++++++--
 arch/x86/dts/link.dts                    |  3 ---
 4 files changed, 22 insertions(+), 6 deletions(-)

arch/x86/cpu/ivybridge/car.S +0 -0

index 6e7e1e4..95da087 100644

arch/x86/cpu/ivybridge/car.S +8 -0

+       wrmsr
+
        post_code(POST_CAR_SIPI)
        /* Zero out all fixed range and variable range MTRRs */
        movl    $mtrr_table, %esi

@@ -228,3 +236,9 @@ mtrr_table:
        .word 0x20C, 0x20D, 0x20E, 0x20F
        .word 0x210, 0x211, 0x212, 0x213
 mtrr_table_end:
+
+       .align 4
+_dt_ucode_base_size:
+       /* These next two fields are filled in by ifdtool */
+       .long   0                       /* microcode base */
+       .long   0                       /* microcode size */

arch/x86/cpu/ivybridge/cpu.c +0 -0

index 0543e06..e925310 100644

arch/x86/cpu/ivybridge/cpu.c +1 -1

-       if (ret && ret != -ENOENT && ret != -EEXIST)
+       if (ret)
                return ret;
 
        /* Enable upper 128bytes of CMOS */

arch/x86/cpu/ivybridge/microcode_intel.c +7 -2

index 0817751..c012820 100644
@@ -120,6 +120,7 @@ int microcode_update_intel(void)
        int count;
        int node;
        int ret;
+       int rev;
 
        microcode_read_cpu(&cpu);
        node = 0;

@@ -147,12 +148,16 @@ int microcode_update_intel(void)
                        skipped++;
                        continue;
                }
-               ret = microcode_read_rev();
                wrmsr(0x79, (ulong)update.data, 0);
+               rev = microcode_read_rev();
                debug("microcode: updated to revision 0x%x date=%04x-%02x-%02x\n",
-                     microcode_read_rev(), update.date_code & 0xffff,
+                     rev, update.date_code & 0xffff,
                      (update.date_code >> 24) & 0xff,
                      (update.date_code >> 16) & 0xff);
+               if (update.update_revision != rev) {
+                       printf("Microcode update failed\n");
+                       return -EFAULT;
+               }
                count++;
        } while (1);
 }

arch/x86/dts/link.dts +0 -0

index a739080..1ebc334 100644

Hi Simon,

Quoted 21 lines · Simon Glass

At present the normal update (which happens much later) does not work. This seems to have something to do with the 'no eviction' mode in the CAR, or at least moving the microcode update after that causes it not to work.

For now, do an update early on so that it definitely works. Also refuse to continue unless the microcode update check (later in boot) is successful.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/ivybridge/car.S             | 14 ++++++++++++++
 arch/x86/cpu/ivybridge/cpu.c             |  2 +-
 arch/x86/cpu/ivybridge/microcode_intel.c |  9 +++++++--
 arch/x86/dts/link.dts                    |  3 ---
 4 files changed, 22 insertions(+), 6 deletions(-)

arch/x86/cpu/ivybridge/car.S +0 -0

index 6e7e1e4..95da087 100644

arch/x86/cpu/ivybridge/car.S car_init: · line 49

Quoted 5 lines

arch/x86/cpu/ivybridge/car.S +2 -0

@@ -45,6 +45,14 @@ car_init:
        movl    $0xFEE00300, %esi
        movl    %eax, (%esi)
 
+       /* TODO: Load microcode later - the 'no eviction' mode breaks this */
+       movl    $0x79, %ecx

Replace 0x79 to MSR_IA32_UCODE_WRITE from msr-index.h

arch/x86/cpu/ivybridge/car.S car_init: · line 53

Quoted 4 lines

arch/x86/cpu/ivybridge/car.S +4 -0

+       xorl    %edx, %edx
+       movl    $_dt_ucode_base_size, %eax
+       movl    (%eax), %eax
+       addl    $0x30, %eax

And here 0x30 to something like MICROCODE_HEADER_LEN.

Hi Simon,

Quoted 21 lines · Simon Glass

At present the normal update (which happens much later) does not work. This seems to have something to do with the 'no eviction' mode in the CAR, or at least moving the microcode update after that causes it not to work.

For now, do an update early on so that it definitely works. Also refuse to continue unless the microcode update check (later in boot) is successful.

Signed-off-by Simon Glass <[email protected]>

---

 arch/x86/cpu/ivybridge/car.S             | 14 ++++++++++++++
 arch/x86/cpu/ivybridge/cpu.c             |  2 +-
 arch/x86/cpu/ivybridge/microcode_intel.c |  9 +++++++--
 arch/x86/dts/link.dts                    |  3 ---
 4 files changed, 22 insertions(+), 6 deletions(-)

arch/x86/cpu/ivybridge/car.S +0 -0

index 6e7e1e4..95da087 100644

arch/x86/cpu/ivybridge/car.S car_init: · line 49

Quoted 5 lines

arch/x86/cpu/ivybridge/car.S +2 -0

@@ -45,6 +45,14 @@ car_init:
        movl    $0xFEE00300, %esi
        movl    %eax, (%esi)
 
+       /* TODO: Load microcode later - the 'no eviction' mode breaks this */
+       movl    $0x79, %ecx

Replace 0x79 to MSR_IA32_UCODE_WRITE from msr-index.h

arch/x86/cpu/ivybridge/car.S car_init: · line 53

Quoted 4 lines

arch/x86/cpu/ivybridge/car.S +4 -0

+       xorl    %edx, %edx
+       movl    $_dt_ucode_base_size, %eax
+       movl    (%eax), %eax
+       addl    $0x30, %eax

And here 0x30 to something like MICROCODE_HEADER_LEN.

Quoted 22 lines

arch/x86/cpu/ivybridge/car.S +8 -0

+       wrmsr
+
        post_code(POST_CAR_SIPI)
        /* Zero out all fixed range and variable range MTRRs */
        movl    $mtrr_table, %esi

@@ -228,3 +236,9 @@ mtrr_table:
        .word 0x20C, 0x20D, 0x20E, 0x20F
        .word 0x210, 0x211, 0x212, 0x213
 mtrr_table_end:
+
+       .align 4
+_dt_ucode_base_size:
+       /* These next two fields are filled in by ifdtool */
+       .long   0                       /* microcode base */
+       .long   0                       /* microcode size */

arch/x86/cpu/ivybridge/cpu.c +0 -0

index 0543e06..e925310 100644
@@ -263,7 +263,7 @@ int print_cpuinfo(void)
        enable_lapic();
 

arch/x86/cpu/ivybridge/cpu.c int print_cpuinfo(void) · line 265

Quoted 1 line

arch/x86/cpu/ivybridge/cpu.c +0 -0

        ret = microcode_update_intel();

Since we already did the microcode update in car_init, why should we do this again here?

Quoted 43 lines

arch/x86/cpu/ivybridge/cpu.c +1 -1

-       if (ret && ret != -ENOENT && ret != -EEXIST)
+       if (ret)
                return ret;
 
        /* Enable upper 128bytes of CMOS */

arch/x86/cpu/ivybridge/microcode_intel.c +7 -2

index 0817751..c012820 100644
@@ -120,6 +120,7 @@ int microcode_update_intel(void)
        int count;
        int node;
        int ret;
+       int rev;
 
        microcode_read_cpu(&cpu);
        node = 0;

@@ -147,12 +148,16 @@ int microcode_update_intel(void)
                        skipped++;
                        continue;
                }
-               ret = microcode_read_rev();
                wrmsr(0x79, (ulong)update.data, 0);
+               rev = microcode_read_rev();
                debug("microcode: updated to revision 0x%x date=%04x-%02x-%02x\n",
-                     microcode_read_rev(), update.date_code & 0xffff,
+                     rev, update.date_code & 0xffff,
                      (update.date_code >> 24) & 0xff,
                      (update.date_code >> 16) & 0xff);
+               if (update.update_revision != rev) {
+                       printf("Microcode update failed\n");
+                       return -EFAULT;
+               }
                count++;
        } while (1);
 }

arch/x86/dts/link.dts +0 -0

index a739080..1ebc334 100644
@@ -214,9 +214,6 @@
 

arch/x86/dts/link.dts

Quoted 9 lines

arch/x86/dts/link.dts +0 -4

        microcode {
                update at 0 {
-#include "microcode/m12206a7_00000029.dtsi"
-               };
-               update at 1 {
 #include "microcode/m12306a9_0000001b.dtsi"
                };
        };
--

It is useful to be able to see the MTRR setup in U-Boot. Add a command to list the state of the variable MTRR registers and allow them to be changed.

common/cmd_mtrr.c new file +138 -0

@@ -0,0 +1,138 @@
+/*
+ * (C) Copyright 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/msr.h>
+#include <asm/mtrr.h>
+
+static const char *const mtrr_type_name[MTRR_TYPE_COUNT] = {
+       "Uncacheable",
+       "Combine",
+       "2",
+       "3",
+       "Through",
+       "Protect",
+       "Back",
+};
+
+static int do_mtrr_list(void)
+{
+       int i;
+
+       printf("Reg Valid Write-type   %-16s %-16s %-16s\n", "Base   ||",
+              "Mask   ||", "Size   ||");
+       for (i = 0; i < MTRR_COUNT; i++) {
+               const char *type = "Invalid";
+               uint64_t base, mask, size;
+               bool valid;
+
+               base = native_read_msr(MTRR_PHYS_BASE_MSR(i));
+               mask = native_read_msr(MTRR_PHYS_MASK_MSR(i));
+               size = ~mask & ((1ULL << CONFIG_CPU_ADDR_BITS) - 1);
+               size |= (1 << 12) - 1;
+               size += 1;
+               valid = mask & MTRR_PHYS_MASK_VALID;
+               type = mtrr_type_name[base & MTRR_BASE_TYPE_MASK];
+               printf("%d   %-5s %-12s %016llx %016llx %016llx\n", i,
+                      valid ? "Y" : "N", type, base, mask, size);
+       }
+
+       return 0;
+}
+
+static int do_mtrr_set(uint reg, int argc, char * const argv[])
+{
+       const char *typename = argv[0];
+       struct mtrr_state state;
+       uint32_t start, size;
+       uint64_t base, mask;
+       int i, type = -1;
+       bool valid;
+
+       if (argc < 3)
+               return CMD_RET_USAGE;
+       for (i = 0; i < MTRR_TYPE_COUNT; i++) {
+               if (*typename == *mtrr_type_name[i])
+                       type = i;
+       }
+       if (type == -1) {
+               printf("Invalid type name %s\n", typename);
+               return CMD_RET_USAGE;
+       }
+       start = simple_strtoul(argv[1], NULL, 16);
+       size = simple_strtoul(argv[2], NULL, 16);
+
+       base = start | type;
+       valid = native_read_msr(MTRR_PHYS_MASK_MSR(reg)) & MTRR_PHYS_MASK_VALID;
+       mask = ~((uint64_t)size - 1);
+       mask &= (1ULL << CONFIG_CPU_ADDR_BITS) - 1;
+       if (valid)
+               mask |= MTRR_PHYS_MASK_VALID;
+
+       printf("base=%llx, mask=%llx\n", base, mask);
+       mtrr_open(&state);
+       wrmsrl(MTRR_PHYS_BASE_MSR(reg), base);
+       wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask);
+       mtrr_close(&state);
+
+       return 0;
+}
+
+static int mtrr_set_valid(int reg, bool valid)
+{
+       struct mtrr_state state;
+       uint64_t mask;
+
+       mtrr_open(&state);
+       mask = native_read_msr(MTRR_PHYS_MASK_MSR(reg));
+       if (valid)
+               mask |= MTRR_PHYS_MASK_VALID;
+       else
+               mask &= ~MTRR_PHYS_MASK_VALID;
+       wrmsrl(MTRR_PHYS_MASK_MSR(reg), mask);
+       mtrr_close(&state);
+
+       return 0;
+}
+
+static int do_mtrr(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+       const char *cmd;
+       uint reg;
+
+       cmd = argv[1];
+       if (argc < 2 || *cmd == 'l')
+               return do_mtrr_list();
+       argc -= 2;
+       argv += 2;
+       if (argc <= 0)
+               return CMD_RET_USAGE;
+       reg = simple_strtoul(argv[0], NULL, 16);
+       if (reg >= MTRR_COUNT) {
+               printf("Invalid register number\n");
+               return CMD_RET_USAGE;
+       }
+       if (*cmd == 'e')
+               return mtrr_set_valid(reg, true);
+       else if (*cmd == 'd')
+               return mtrr_set_valid(reg, false);
+       else if (*cmd == 's')
+               return do_mtrr_set(reg, argc - 1, argv + 1);
+       else
+               return CMD_RET_USAGE;
+
+       return 0;
+}
+
+U_BOOT_CMD(
+       mtrr,   6,      1,      do_mtrr,
+       "Use x86 memory type range registers (32-bit only)",
+       "[list]        - list current registers\n"
+       "set <reg> <type> <start> <size>   - set a register\n"
+       "\t<type> is Uncacheable, Combine, Through, Protect, Back\n"
+       "disable <reg>      - disable a register\n"
+       "ensable <reg>      - enable a register"
+);

doc/README.x86 +0 -0

index 5fab044..6fbfb90 100644
@@ -110,9 +110,25 @@ be turned on. Not every device on the board is configured via devie tree, but
 more and more devices will be added as time goes by. Check out the directory
 arch/x86/dts/ for these device tree source files.

doc/README.x86 +5 -0

+        Atom.
+iod  - Display I/O memory
+iow  - Write I/O memory
+mtrr - List and set the Memory Type Range Registers (MTRR). These are used to
+        tell the CPU whether memory is cacheable and if so the cache write