On 22 December 2016 at 01:06, Nathan Rossi <[email protected]> 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 <[email protected]> > --- > 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 <[email protected]> > +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 <[email protected]> > +Cc: Simon Glass <[email protected]> > +Cc: Michal Simek <[email protected]> > +Reviewed-by: Simon Glass <[email protected]> > +Signed-off-by: Michal Simek <[email protected]> > +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 <[email protected]> > +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 <[email protected]> > +Fixes: 758f29d0f8 ("ARM: zynq: Support systems with more memory banks") > +Cc: Michal Simek <[email protected]> > +Signed-off-by: Michal Simek <[email protected]> > +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 <[email protected]> > +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 <[email protected]> > +Fixes: 8d59d7f63b ("ARM64: zynqmp: Read RAM information from DT") > +Cc: Michal Simek <[email protected]> > +Signed-off-by: Michal Simek <[email protected]> > +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 > -- _______________________________________________ meta-xilinx mailing list [email protected] https://lists.yoctoproject.org/listinfo/meta-xilinx
