Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

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

Signed-off-by: Simon Glass sjg@chromium.org

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

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 7d007bb..e992e64 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -47,10 +47,6 @@ config RAMBASE hex default 0x100000

-config RAMTOP

•   hex
  

•   default 0x200000
  

config XIP_ROM_SIZE hex default ROM_SIZE --

What about mtrr.h which is still referring to CONFIG_RAMTOP?

~/work/u-boot-x86/arch/x86$ grep -nr RAMTOP * include/asm/mtrr.h:103:#if !defined(CONFIG_RAMTOP) include/asm/mtrr.h:104:# error "CONFIG_RAMTOP not defined" include/asm/mtrr.h:117:#if (CONFIG_RAMTOP & (CONFIG_RAMTOP - 1)) != 0 include/asm/mtrr.h:118:# error "CONFIG_RAMTOP must be a power of 2" Kconfig:50:config RAMTOP

Regards, Bin

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

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

Signed-off-by: Simon Glass sjg@chromium.org

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

diff --git a/tools/ifdtool.c b/tools/ifdtool.c index fe8366b..590ccc9 100644 --- a/tools/ifdtool.c +++ b/tools/ifdtool.c @@ -788,9 +788,9 @@ static int write_uboot(char *image, int size, struct input_file *uboot, fdt_strerror(data_size)); return -ENOENT; }

•           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 bmeng.cn@gmail.com

Tested on Intel Crown Bay by adjusting ROM_SIZE to 2MB

Tested-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

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 sjg@chromium.org

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

diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile index 5033d2b..62e43c0 100644 --- a/arch/x86/cpu/Makefile +++ b/arch/x86/cpu/Makefile @@ -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 diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c index cfacc05..6d06d5a 100644 --- a/arch/x86/cpu/coreboot/coreboot.c +++ b/arch/x86/cpu/coreboot/coreboot.c @@ -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 */
  

diff --git a/arch/x86/cpu/ivybridge/car.S b/arch/x86/cpu/ivybridge/car.S index dca68e4..72b22ea 100644 --- a/arch/x86/cpu/ivybridge/car.S +++ b/arch/x86/cpu/ivybridge/car.S @@ -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 diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c new file mode 100644 index 0000000..d5a825d1 --- /dev/null +++ b/arch/x86/cpu/mtrr.c @@ -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;
  

+} diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index 03d491a..15e76f6 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -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 diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h index 5f05a48..3c11740 100644 --- a/arch/x86/include/asm/mtrr.h +++ b/arch/x86/include/asm/mtrr.h @@ -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.
• • It must be called with caches disabled.
• • @do_caches: true if caches are currently on
• */

+int mtrr_commit(bool do_caches);

-#if !defined(CONFIG_RAMTOP) -# error "CONFIG_RAMTOP not defined" #endif

Can we move the removal of CONFIG_RAMTOP to the patch#2 in this series?

#if ((CONFIG_XIP_ROM_SIZE & (CONFIG_XIP_ROM_SIZE - 1)) != 0) @@ -114,8 +101,4 @@ static inline long x86_mtrr_rom_cache_var_index(void) { return -1; }

#define CACHE_ROM_BASE (((1 << 20) - (CONFIG_CACHE_ROM_SIZE >> 12)) << 12)

-#if (CONFIG_RAMTOP & (CONFIG_RAMTOP - 1)) != 0 -# error "CONFIG_RAMTOP must be a power of 2" -#endif

Ditto.

#endif

Regards, Bin

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

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 sjg@chromium.org

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

diff --git a/arch/x86/cpu/ivybridge/gma.c b/arch/x86/cpu/ivybridge/gma.c index 125021b..03b2ebd 100644 --- a/arch/x86/cpu/ivybridge/gma.c +++ b/arch/x86/cpu/ivybridge/gma.c @@ -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;
  
  

@@ -746,6 +749,11 @@ int gma_func0_init(pci_dev_t dev, struct pci_controller *hose, reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO; pci_write_config32(dev, PCI_COMMAND, reg32);

•   /* Use write-combining for the graphics memory, 256MB */
  

•   base = pci_read_bar32(hose, dev, 2);
  

•   mtrr_add_request(MTRR_TYPE_WRCOMB, base, 256 << 20);
  

To make the codes more generic, should we decode the memory size via bar instead of hardcoded 256MB?

•   mtrr_commit(true);
  

•   gtt_bar = (void *)pci_read_bar32(pci_bus_to_hose(0), dev, 0);
    debug("GT bar %p\n", gtt_bar);
    ret = gma_pm_init_pre_vbios(gtt_bar);
  

--

Regards, Bin

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

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 sjg@chromium.org

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

diff --git a/common/board_f.c b/common/board_f.c index 98c9c72..1b65575 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -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,
  
    NULL,
  

};

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

Regards, Bin

Hi Simon,

On Fri, Jan 2, 2015 at 6:23 AM, Simon Glass sjg@chromium.org wrote:

Hi Bin,

On 30 December 2014 at 22:51, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

...

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 sjg@chromium.org

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

diff --git a/common/board_f.c b/common/board_f.c index 98c9c72..1b65575 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -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,
  
    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.

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?

Regards, Bin

Hi Simon,

On Mon, Jan 5, 2015 at 9:44 AM, Simon Glass sjg@chromium.org wrote:

Hi Bin,

On 3 January 2015 at 20:26, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Simon,

On Fri, Jan 2, 2015 at 6:23 AM, Simon Glass sjg@chromium.org wrote:

...

Hi Bin,

On 30 December 2014 at 22:51, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

...

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 sjg@chromium.org

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

diff --git a/common/board_f.c b/common/board_f.c index 98c9c72..1b65575 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -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,
  
    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.

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.

...

• 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?

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

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,

On 5 January 2015 at 10:22, Graeme Russ gruss@tss-engineering.com wrote:

Hi Simon & Bin,

On Tue, Jan 6, 2015 at 12:40 AM, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Simon,

On Mon, Jan 5, 2015 at 9:44 AM, Simon Glass sjg@chromium.org wrote:

...

Hi Bin,

On 3 January 2015 at 20:26, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Simon,

On Fri, Jan 2, 2015 at 6:23 AM, Simon Glass sjg@chromium.org wrote:

...

Hi Bin,

On 30 December 2014 at 22:51, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote: > 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 sjg@chromium.org > --- > > common/board_f.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/common/board_f.c b/common/board_f.c > index 98c9c72..1b65575 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -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, > > 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 :)

Nice to hear from you again!

...

...

...

...

...

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.

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.

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

...

...

...

• 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?

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

...

...

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?)

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.

...

...

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

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.

Regards, Simon

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

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 sjg@chromium.org

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

diff --git a/arch/x86/cpu/ivybridge/car.S b/arch/x86/cpu/ivybridge/car.S index 72b22ea..6e7e1e4 100644 --- a/arch/x86/cpu/ivybridge/car.S +++ b/arch/x86/cpu/ivybridge/car.S @@ -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
  

•   invd
  

•   /* Return to the caller */
  

•   jmp     *%ebx
  

+.Lhlt:

•   post_code(0xee)
  

•   hlt
  

•   jmp     .Lhlt
  

I don't see any codes could jump to this 4 lines above. Remove it?

mtrr_table: /* Fixed MTRRs */ .word 0x250, 0x258, 0x259 --

Regards, Bin

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

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 sjg@chromium.org

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

diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S index 125782c..8cebde1 100644 --- a/arch/x86/cpu/start.S +++ b/arch/x86/cpu/start.S @@ -205,6 +205,16 @@ board_init_f_r_trampoline: /* Setup global descriptor table so gd->xyz works */ call setup_gdt

•   /* 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?

+car_ret:

car_uninit_ret

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

--

Regards, Bin

On Wed, Dec 31, 2014 at 2:45 PM, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Sun, Dec 28, 2014 at 10:20 AM, Simon Glass sjg@chromium.org wrote:

...

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 sjg@chromium.org

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(-)

diff --git a/arch/x86/cpu/ivybridge/car.S b/arch/x86/cpu/ivybridge/car.S index 6e7e1e4..95da087 100644 --- a/arch/x86/cpu/ivybridge/car.S +++ b/arch/x86/cpu/ivybridge/car.S @@ -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

...

•   xorl    %edx, %edx
  

•   movl    $_dt_ucode_base_size, %eax
  

•   movl    (%eax), %eax
  

•   addl    $0x30, %eax
  

And here 0x30 to something like MICROCODE_HEADER_LEN.

I realized UCODE_HEADER_LEN might be better. At least shorter than microcode :-), and is consistent with the MSR_IA32_UCODE_WRITE.

[snip]

Regards, Bin