[meta-xilinx] [PATCH] u-boot-xlnx_2016.07: Backport fix for memory corruption
Nathan Rossi
nathan at nathanrossi.com
Tue Dec 27 05:10:07 PST 2016
On 22 December 2016 at 01:06, Nathan Rossi <nathan at nathanrossi.com> wrote:
> Backport the dram_init* memory corruption fix, this resolves issues with
> some configurations where the dram_init function for zynq/zynqmp would
> trash non-relocated locations in memory due to the use of static
> variables before bss initialization.
Applied to master and morty also. This resolves the known issue that
was present when first branching morty.
Regards,
Nathan
>
> Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
> ---
> recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb | 7 +
> ...ory-bank-decoding-functions-for-board-set.patch | 144 +++++++++++++++++
> ...place-board-specific-with-generic-memory-.patch | 170 +++++++++++++++++++++
> ...p-Replace-board-specific-with-generic-mem.patch | 156 +++++++++++++++++++
> 4 files changed, 477 insertions(+)
> create mode 100644 recipes-bsp/u-boot/u-boot/0001-fdt-add-memory-bank-decoding-functions-for-board-set.patch
> create mode 100644 recipes-bsp/u-boot/u-boot/0002-ARM-zynq-Replace-board-specific-with-generic-memory-.patch
> create mode 100644 recipes-bsp/u-boot/u-boot/0003-ARM64-zynqmp-Replace-board-specific-with-generic-mem.patch
>
> diff --git a/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb b/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb
> index 61f1cb793b..af33a4cf86 100644
> --- a/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb
> +++ b/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb
> @@ -7,6 +7,13 @@ SRCREV = "95e11f6eb4bc07bbee12a4217c58146bddac61b2"
> PV = "v2016.07-xilinx-${XILINX_RELEASE_VERSION}+git${SRCPV}"
>
> FILESEXTRAPATHS_prepend := "${THISDIR}/u-boot-xlnx:"
> +FILESEXTRAPATHS_prepend := "${THISDIR}/u-boot:"
> +
> +SRC_URI_append = " \
> + file://0001-fdt-add-memory-bank-decoding-functions-for-board-set.patch \
> + file://0002-ARM-zynq-Replace-board-specific-with-generic-memory-.patch \
> + file://0003-ARM64-zynqmp-Replace-board-specific-with-generic-mem.patch \
> + "
>
> SRC_URI_append_kc705-microblazeel = " file://microblaze-kc705-Convert-microblaze-generic-to-k.patch"
>
> diff --git a/recipes-bsp/u-boot/u-boot/0001-fdt-add-memory-bank-decoding-functions-for-board-set.patch b/recipes-bsp/u-boot/u-boot/0001-fdt-add-memory-bank-decoding-functions-for-board-set.patch
> new file mode 100644
> index 0000000000..b7d179f84f
> --- /dev/null
> +++ b/recipes-bsp/u-boot/u-boot/0001-fdt-add-memory-bank-decoding-functions-for-board-set.patch
> @@ -0,0 +1,144 @@
> +From 623f60198b38c4fdae596038cd5956e44b6224a4 Mon Sep 17 00:00:00 2001
> +From: Nathan Rossi <nathan at nathanrossi.com>
> +Date: Mon, 19 Dec 2016 00:03:34 +1000
> +Subject: [PATCH 1/3] fdt: add memory bank decoding functions for board setup
> +
> +Add two functions for use by board implementations to decode the memory
> +banks of the /memory node so as to populate the global data with
> +ram_size and board info for memory banks.
> +
> +The fdtdec_setup_memory_size() function decodes the first memory bank
> +and sets up the gd->ram_size with the size of the memory bank. This
> +function should be called from the boards dram_init().
> +
> +The fdtdec_setup_memory_banksize() function decode the memory banks
> +(up to the CONFIG_NR_DRAM_BANKS) and populates the base address and size
> +into the gd->bd->bi_dram array of banks. This function should be called
> +from the boards dram_init_banksize().
> +
> +Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
> +Cc: Simon Glass <sjg at chromium.org>
> +Cc: Michal Simek <monstr at monstr.eu>
> +Reviewed-by: Simon Glass <sjg at chromium.org>
> +Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> +Upstream-Status: Backport
> +---
> + include/fdtdec.h | 34 ++++++++++++++++++++++++++++++++++
> + lib/fdtdec.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> + 2 files changed, 90 insertions(+)
> +
> +diff --git a/include/fdtdec.h b/include/fdtdec.h
> +index 27887c8c21..d074478f14 100644
> +--- a/include/fdtdec.h
> ++++ b/include/fdtdec.h
> +@@ -976,6 +976,40 @@ struct display_timing {
> + */
> + int fdtdec_decode_display_timing(const void *blob, int node, int index,
> + struct display_timing *config);
> ++
> ++/**
> ++ * fdtdec_setup_memory_size() - decode and setup gd->ram_size
> ++ *
> ++ * Decode the /memory 'reg' property to determine the size of the first memory
> ++ * bank, populate the global data with the size of the first bank of memory.
> ++ *
> ++ * This function should be called from a boards dram_init(). This helper
> ++ * function allows for boards to query the device tree for DRAM size instead of
> ++ * hard coding the value in the case where the memory size cannot be detected
> ++ * automatically.
> ++ *
> ++ * @return 0 if OK, -EINVAL if the /memory node or reg property is missing or
> ++ * invalid
> ++ */
> ++int fdtdec_setup_memory_size(void);
> ++
> ++/**
> ++ * fdtdec_setup_memory_banksize() - decode and populate gd->bd->bi_dram
> ++ *
> ++ * Decode the /memory 'reg' property to determine the address and size of the
> ++ * memory banks. Use this data to populate the global data board info with the
> ++ * phys address and size of memory banks.
> ++ *
> ++ * This function should be called from a boards dram_init_banksize(). This
> ++ * helper function allows for boards to query the device tree for memory bank
> ++ * information instead of hard coding the information in cases where it cannot
> ++ * be detected automatically.
> ++ *
> ++ * @return 0 if OK, -EINVAL if the /memory node or reg property is missing or
> ++ * invalid
> ++ */
> ++int fdtdec_setup_memory_banksize(void);
> ++
> + /**
> + * Set up the device tree ready for use
> + */
> +diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> +index 4e619c49a2..81f47ef2c7 100644
> +--- a/lib/fdtdec.c
> ++++ b/lib/fdtdec.c
> +@@ -1174,6 +1174,62 @@ int fdtdec_decode_display_timing(const void *blob, int parent, int index,
> + return ret;
> + }
> +
> ++int fdtdec_setup_memory_size(void)
> ++{
> ++ int ret, mem;
> ++ struct fdt_resource res;
> ++
> ++ mem = fdt_path_offset(gd->fdt_blob, "/memory");
> ++ if (mem < 0) {
> ++ debug("%s: Missing /memory node\n", __func__);
> ++ return -EINVAL;
> ++ }
> ++
> ++ ret = fdt_get_resource(gd->fdt_blob, mem, "reg", 0, &res);
> ++ if (ret != 0) {
> ++ debug("%s: Unable to decode first memory bank\n", __func__);
> ++ return -EINVAL;
> ++ }
> ++
> ++ gd->ram_size = (phys_size_t)(res.end - res.start + 1);
> ++ debug("%s: Initial DRAM size %llx\n", __func__, (u64)gd->ram_size);
> ++
> ++ return 0;
> ++}
> ++
> ++#if defined(CONFIG_NR_DRAM_BANKS)
> ++int fdtdec_setup_memory_banksize(void)
> ++{
> ++ int bank, ret, mem;
> ++ struct fdt_resource res;
> ++
> ++ mem = fdt_path_offset(gd->fdt_blob, "/memory");
> ++ if (mem < 0) {
> ++ debug("%s: Missing /memory node\n", __func__);
> ++ return -EINVAL;
> ++ }
> ++
> ++ for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> ++ ret = fdt_get_resource(gd->fdt_blob, mem, "reg", bank, &res);
> ++ if (ret == -FDT_ERR_NOTFOUND)
> ++ break;
> ++ if (ret != 0)
> ++ return -EINVAL;
> ++
> ++ gd->bd->bi_dram[bank].start = (phys_addr_t)res.start;
> ++ gd->bd->bi_dram[bank].size =
> ++ (phys_size_t)(res.end - res.start + 1);
> ++
> ++ debug("%s: DRAM Bank #%d: start = 0x%llx, size = 0x%llx\n",
> ++ __func__, bank,
> ++ (unsigned long long)gd->bd->bi_dram[bank].start,
> ++ (unsigned long long)gd->bd->bi_dram[bank].size);
> ++ }
> ++
> ++ return 0;
> ++}
> ++#endif
> ++
> + int fdtdec_setup(void)
> + {
> + #if CONFIG_IS_ENABLED(OF_CONTROL)
> +--
> +2.11.0
> +
> diff --git a/recipes-bsp/u-boot/u-boot/0002-ARM-zynq-Replace-board-specific-with-generic-memory-.patch b/recipes-bsp/u-boot/u-boot/0002-ARM-zynq-Replace-board-specific-with-generic-memory-.patch
> new file mode 100644
> index 0000000000..4180696669
> --- /dev/null
> +++ b/recipes-bsp/u-boot/u-boot/0002-ARM-zynq-Replace-board-specific-with-generic-memory-.patch
> @@ -0,0 +1,170 @@
> +From de9bf1b591a80ef8fce8cad5c3d5a1139d136a77 Mon Sep 17 00:00:00 2001
> +From: Nathan Rossi <nathan at nathanrossi.com>
> +Date: Mon, 19 Dec 2016 00:03:34 +1000
> +Subject: [PATCH 2/3] ARM: zynq: Replace board specific with generic memory
> + bank decoding
> +
> +The dram_init and dram_init_banksize functions were using a board
> +specific implementation for decoding the memory banks from the fdt. This
> +board specific implementation uses a static variable 'tmp' which makes
> +these functions unsafe for execution from within the board_init_f
> +context.
> +
> +This unsafe use of a static variable was causing a specific bug when
> +using the zynq_zybo configuration, U-Boot would generate the following
> +error during image load. This was caused due to dram_init overwriting
> +the relocations for the 'image' variable within the do_bootm function.
> +Out of coincidence the un-initialized memory has a compression type
> +which is the same as the value for the relocation type R_ARM_RELATIVE.
> +
> + Uncompressing Invalid Image ... Unimplemented compression type 23
> +
> +It should be noted that this is just one way the issue could surface,
> +other cases my not be observed in normal boot flow. Depending on the
> +size of various sections, and location of relocations within __rel_dyn
> +and the compiler/linker the outcome of this bug can differ greatly.
> +
> +This change makes the dram_init* functions use a generic implementation
> +of decoding and populating memory bank and size data.
> +
> +Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
> +Fixes: 758f29d0f8 ("ARM: zynq: Support systems with more memory banks")
> +Cc: Michal Simek <monstr at monstr.eu>
> +Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> +Upstream-Status: Backport
> +---
> + board/xilinx/zynq/board.c | 112 ++--------------------------------------------
> + 1 file changed, 3 insertions(+), 109 deletions(-)
> +
> +diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> +index 2c86940957..5cd9bbf711 100644
> +--- a/board/xilinx/zynq/board.c
> ++++ b/board/xilinx/zynq/board.c
> +@@ -124,121 +124,15 @@ int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
> + }
> +
> + #if !defined(CONFIG_SYS_SDRAM_BASE) && !defined(CONFIG_SYS_SDRAM_SIZE)
> +-/*
> +- * fdt_get_reg - Fill buffer by information from DT
> +- */
> +-static phys_size_t fdt_get_reg(const void *fdt, int nodeoffset, void *buf,
> +- const u32 *cell, int n)
> +-{
> +- int i = 0, b, banks;
> +- int parent_offset = fdt_parent_offset(fdt, nodeoffset);
> +- int address_cells = fdt_address_cells(fdt, parent_offset);
> +- int size_cells = fdt_size_cells(fdt, parent_offset);
> +- char *p = buf;
> +- u64 val;
> +- u64 vals;
> +-
> +- debug("%s: addr_cells=%x, size_cell=%x, buf=%p, cell=%p\n",
> +- __func__, address_cells, size_cells, buf, cell);
> +-
> +- /* Check memory bank setup */
> +- banks = n % (address_cells + size_cells);
> +- if (banks)
> +- panic("Incorrect memory setup cells=%d, ac=%d, sc=%d\n",
> +- n, address_cells, size_cells);
> +-
> +- banks = n / (address_cells + size_cells);
> +-
> +- for (b = 0; b < banks; b++) {
> +- debug("%s: Bank #%d:\n", __func__, b);
> +- if (address_cells == 2) {
> +- val = cell[i + 1];
> +- val <<= 32;
> +- val |= cell[i];
> +- val = fdt64_to_cpu(val);
> +- debug("%s: addr64=%llx, ptr=%p, cell=%p\n",
> +- __func__, val, p, &cell[i]);
> +- *(phys_addr_t *)p = val;
> +- } else {
> +- debug("%s: addr32=%x, ptr=%p\n",
> +- __func__, fdt32_to_cpu(cell[i]), p);
> +- *(phys_addr_t *)p = fdt32_to_cpu(cell[i]);
> +- }
> +- p += sizeof(phys_addr_t);
> +- i += address_cells;
> +-
> +- debug("%s: pa=%p, i=%x, size=%zu\n", __func__, p, i,
> +- sizeof(phys_addr_t));
> +-
> +- if (size_cells == 2) {
> +- vals = cell[i + 1];
> +- vals <<= 32;
> +- vals |= cell[i];
> +- vals = fdt64_to_cpu(vals);
> +-
> +- debug("%s: size64=%llx, ptr=%p, cell=%p\n",
> +- __func__, vals, p, &cell[i]);
> +- *(phys_size_t *)p = vals;
> +- } else {
> +- debug("%s: size32=%x, ptr=%p\n",
> +- __func__, fdt32_to_cpu(cell[i]), p);
> +- *(phys_size_t *)p = fdt32_to_cpu(cell[i]);
> +- }
> +- p += sizeof(phys_size_t);
> +- i += size_cells;
> +-
> +- debug("%s: ps=%p, i=%x, size=%zu\n",
> +- __func__, p, i, sizeof(phys_size_t));
> +- }
> +-
> +- /* Return the first address size */
> +- return *(phys_size_t *)((char *)buf + sizeof(phys_addr_t));
> +-}
> +-
> +-#define FDT_REG_SIZE sizeof(u32)
> +-/* Temp location for sharing data for storing */
> +-/* Up to 64-bit address + 64-bit size */
> +-static u8 tmp[CONFIG_NR_DRAM_BANKS * 16];
> +-
> + void dram_init_banksize(void)
> + {
> +- int bank;
> +-
> +- memcpy(&gd->bd->bi_dram[0], &tmp, sizeof(tmp));
> +-
> +- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +- debug("Bank #%d: start %llx\n", bank,
> +- (unsigned long long)gd->bd->bi_dram[bank].start);
> +- debug("Bank #%d: size %llx\n", bank,
> +- (unsigned long long)gd->bd->bi_dram[bank].size);
> +- }
> ++ fdtdec_setup_memory_banksize();
> + }
> +
> + int dram_init(void)
> + {
> +- int node, len;
> +- const void *blob = gd->fdt_blob;
> +- const u32 *cell;
> +-
> +- memset(&tmp, 0, sizeof(tmp));
> +-
> +- /* find or create "/memory" node. */
> +- node = fdt_subnode_offset(blob, 0, "memory");
> +- if (node < 0) {
> +- printf("%s: Can't get memory node\n", __func__);
> +- return node;
> +- }
> +-
> +- /* Get pointer to cells and lenght of it */
> +- cell = fdt_getprop(blob, node, "reg", &len);
> +- if (!cell) {
> +- printf("%s: Can't get reg property\n", __func__);
> +- return -1;
> +- }
> +-
> +- gd->ram_size = fdt_get_reg(blob, node, &tmp, cell, len / FDT_REG_SIZE);
> +-
> +- debug("%s: Initial DRAM size %llx\n", __func__, (u64)gd->ram_size);
> ++ if (fdtdec_setup_memory_size() != 0)
> ++ return -EINVAL;
> +
> + zynq_ddrc_init();
> +
> +--
> +2.11.0
> +
> diff --git a/recipes-bsp/u-boot/u-boot/0003-ARM64-zynqmp-Replace-board-specific-with-generic-mem.patch b/recipes-bsp/u-boot/u-boot/0003-ARM64-zynqmp-Replace-board-specific-with-generic-mem.patch
> new file mode 100644
> index 0000000000..2e6e9e31b1
> --- /dev/null
> +++ b/recipes-bsp/u-boot/u-boot/0003-ARM64-zynqmp-Replace-board-specific-with-generic-mem.patch
> @@ -0,0 +1,156 @@
> +From 950f86ca38325c9ae7874895d2cdbdda5496e712 Mon Sep 17 00:00:00 2001
> +From: Nathan Rossi <nathan at nathanrossi.com>
> +Date: Mon, 19 Dec 2016 00:03:34 +1000
> +Subject: [PATCH 3/3] ARM64: zynqmp: Replace board specific with generic memory
> + bank decoding
> +
> +The dram_init and dram_init_banksize functions were using a board
> +specific implementation for decoding the memory banks from the fdt. This
> +board specific implementation uses a static variable 'tmp' which makes
> +these functions unsafe for execution from within the board_init_f
> +context.
> +
> +This change makes the dram_init* functions use a generic implementation
> +of decoding and populating memory bank and size data.
> +
> +Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
> +Fixes: 8d59d7f63b ("ARM64: zynqmp: Read RAM information from DT")
> +Cc: Michal Simek <michal.simek at xilinx.com>
> +Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> +Upstream-Status: Backport
> +---
> + board/xilinx/zynqmp/zynqmp.c | 112 ++-----------------------------------------
> + 1 file changed, 3 insertions(+), 109 deletions(-)
> +
> +diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> +index a23c38acd9..4e5871b76a 100644
> +--- a/board/xilinx/zynqmp/zynqmp.c
> ++++ b/board/xilinx/zynqmp/zynqmp.c
> +@@ -180,121 +180,15 @@ int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
> + }
> +
> + #if !defined(CONFIG_SYS_SDRAM_BASE) && !defined(CONFIG_SYS_SDRAM_SIZE)
> +-/*
> +- * fdt_get_reg - Fill buffer by information from DT
> +- */
> +-static phys_size_t fdt_get_reg(const void *fdt, int nodeoffset, void *buf,
> +- const u32 *cell, int n)
> +-{
> +- int i = 0, b, banks;
> +- int parent_offset = fdt_parent_offset(fdt, nodeoffset);
> +- int address_cells = fdt_address_cells(fdt, parent_offset);
> +- int size_cells = fdt_size_cells(fdt, parent_offset);
> +- char *p = buf;
> +- u64 val;
> +- u64 vals;
> +-
> +- debug("%s: addr_cells=%x, size_cell=%x, buf=%p, cell=%p\n",
> +- __func__, address_cells, size_cells, buf, cell);
> +-
> +- /* Check memory bank setup */
> +- banks = n % (address_cells + size_cells);
> +- if (banks)
> +- panic("Incorrect memory setup cells=%d, ac=%d, sc=%d\n",
> +- n, address_cells, size_cells);
> +-
> +- banks = n / (address_cells + size_cells);
> +-
> +- for (b = 0; b < banks; b++) {
> +- debug("%s: Bank #%d:\n", __func__, b);
> +- if (address_cells == 2) {
> +- val = cell[i + 1];
> +- val <<= 32;
> +- val |= cell[i];
> +- val = fdt64_to_cpu(val);
> +- debug("%s: addr64=%llx, ptr=%p, cell=%p\n",
> +- __func__, val, p, &cell[i]);
> +- *(phys_addr_t *)p = val;
> +- } else {
> +- debug("%s: addr32=%x, ptr=%p\n",
> +- __func__, fdt32_to_cpu(cell[i]), p);
> +- *(phys_addr_t *)p = fdt32_to_cpu(cell[i]);
> +- }
> +- p += sizeof(phys_addr_t);
> +- i += address_cells;
> +-
> +- debug("%s: pa=%p, i=%x, size=%zu\n", __func__, p, i,
> +- sizeof(phys_addr_t));
> +-
> +- if (size_cells == 2) {
> +- vals = cell[i + 1];
> +- vals <<= 32;
> +- vals |= cell[i];
> +- vals = fdt64_to_cpu(vals);
> +-
> +- debug("%s: size64=%llx, ptr=%p, cell=%p\n",
> +- __func__, vals, p, &cell[i]);
> +- *(phys_size_t *)p = vals;
> +- } else {
> +- debug("%s: size32=%x, ptr=%p\n",
> +- __func__, fdt32_to_cpu(cell[i]), p);
> +- *(phys_size_t *)p = fdt32_to_cpu(cell[i]);
> +- }
> +- p += sizeof(phys_size_t);
> +- i += size_cells;
> +-
> +- debug("%s: ps=%p, i=%x, size=%zu\n",
> +- __func__, p, i, sizeof(phys_size_t));
> +- }
> +-
> +- /* Return the first address size */
> +- return *(phys_size_t *)((char *)buf + sizeof(phys_addr_t));
> +-}
> +-
> +-#define FDT_REG_SIZE sizeof(u32)
> +-/* Temp location for sharing data for storing */
> +-/* Up to 64-bit address + 64-bit size */
> +-static u8 tmp[CONFIG_NR_DRAM_BANKS * 16];
> +-
> + void dram_init_banksize(void)
> + {
> +- int bank;
> +-
> +- memcpy(&gd->bd->bi_dram[0], &tmp, sizeof(tmp));
> +-
> +- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +- debug("Bank #%d: start %llx\n", bank,
> +- (unsigned long long)gd->bd->bi_dram[bank].start);
> +- debug("Bank #%d: size %llx\n", bank,
> +- (unsigned long long)gd->bd->bi_dram[bank].size);
> +- }
> ++ fdtdec_setup_memory_banksize();
> + }
> +
> + int dram_init(void)
> + {
> +- int node, len;
> +- const void *blob = gd->fdt_blob;
> +- const u32 *cell;
> +-
> +- memset(&tmp, 0, sizeof(tmp));
> +-
> +- /* find or create "/memory" node. */
> +- node = fdt_subnode_offset(blob, 0, "memory");
> +- if (node < 0) {
> +- printf("%s: Can't get memory node\n", __func__);
> +- return node;
> +- }
> +-
> +- /* Get pointer to cells and lenght of it */
> +- cell = fdt_getprop(blob, node, "reg", &len);
> +- if (!cell) {
> +- printf("%s: Can't get reg property\n", __func__);
> +- return -1;
> +- }
> +-
> +- gd->ram_size = fdt_get_reg(blob, node, &tmp, cell, len / FDT_REG_SIZE);
> +-
> +- debug("%s: Initial DRAM size %llx\n", __func__, (u64)gd->ram_size);
> ++ if (fdtdec_setup_memory_size() != 0)
> ++ return -EINVAL;
> +
> + return 0;
> + }
> +--
> +2.11.0
> +
> --
> 2.11.0
>
More information about the meta-xilinx
mailing list