[U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number()
From: Thierry Reding treding@nvidia.com
Drivers that need to parse addresses from the device tree will want to reuse this function rather than duplicating it.
Cc: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- include/fdtdec.h | 8 ++++++++ lib/fdtdec.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 5ac515d87d2d..5530ee407311 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -723,6 +723,14 @@ int fdtdec_read_fmap_entry(const void *blob, int node, const char *name, struct fmap_entry *entry);
/** + * Read a number from a device property. + * @param ptr pointer to the cells to read + * @param cells number of cells to consume + * @return The number contained in @cells cells at @ptr, in host byte order. + */ +u64 fdtdec_get_number(const fdt32_t *ptr, unsigned int cells); + +/** * Obtain an indexed resource from a device property. * * @param fdt FDT blob diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 0776c3004cbd..613cc8494a74 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -918,7 +918,7 @@ int fdtdec_read_fmap_entry(const void *blob, int node, const char *name, return 0; }
-static u64 fdtdec_get_number(const fdt32_t *ptr, unsigned int cells) +u64 fdtdec_get_number(const fdt32_t *ptr, unsigned int cells) { u64 number = 0;
From: Thierry Reding treding@nvidia.com
The current implementation of fdtdec_get_addr_size() assumes that the sizes of fdt_addr_t and fdt_size_t match the number of cells specified by the #address-cells and #size-cells properties. However, there is no reason why that needs to be the case, so the function potentially decodes address and size wrongly.
Furthermore the function casts an array of FDT cells (32-bit unsigned) to fdt_addr_t/fdt_size_t and dereferences them directly. This can cause aborts on 64-bit architectures where fdt_addr_t/fdt_size_t are 64 bits wide, because cells are only guaranteed to be aligned to 32 bits. Fix this by reading in the address and size one cell at a time.
Cc: Hanna Hawa hannah@marvell.com Cc: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- lib/fdtdec.c | 56 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 613cc8494a74..fd244440381e 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -89,29 +89,45 @@ const char *fdtdec_get_compatible(enum fdt_compat_id id) fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, const char *prop_name, fdt_size_t *sizep) { - const fdt_addr_t *cell; - int len; + const fdt32_t *ptr, *end; + int parent, na, ns, len; + fdt_addr_t addr;
debug("%s: %s: ", __func__, prop_name); - cell = fdt_getprop(blob, node, prop_name, &len); - if (cell && ((!sizep && len == sizeof(fdt_addr_t)) || - len == sizeof(fdt_addr_t) * 2)) { - fdt_addr_t addr = fdt_addr_to_cpu(*cell); - if (sizep) { - const fdt_size_t *size; - - size = (fdt_size_t *)((char *)cell + - sizeof(fdt_addr_t)); - *sizep = fdt_size_to_cpu(*size); - debug("addr=%08lx, size=%08lx\n", - (ulong)addr, (ulong)*sizep); - } else { - debug("%08lx\n", (ulong)addr); - } - return addr; + + parent = fdt_parent_offset(blob, node); + if (parent < 0) { + debug("(no parent found)\n"); + return FDT_ADDR_T_NONE; } - debug("(not found)\n"); - return FDT_ADDR_T_NONE; + + na = fdt_address_cells(blob, parent); + ns = fdt_size_cells(blob, parent); + + ptr = fdt_getprop(blob, node, prop_name, &len); + if (!ptr) { + debug("(not found)\n"); + return FDT_ADDR_T_NONE; + } + + end = ptr + len / sizeof(*ptr); + + if (ptr + na + ns > end) { + debug("(not enough data: expected %d bytes, got %d bytes)\n", + (na + ns) * 4, len); + return FDT_ADDR_T_NONE; + } + + addr = fdtdec_get_number(ptr, na); + + if (sizep) { + *sizep = fdtdec_get_number(ptr + na, ns); + debug("addr=%pa, size=%pa\n", &addr, sizep); + } else { + debug("%pa\n", &addr); + } + + return addr; }
fdt_addr_t fdtdec_get_addr(const void *blob, int node,
Hi Thierry,
On 20 March 2015 at 05:51, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
The current implementation of fdtdec_get_addr_size() assumes that the sizes of fdt_addr_t and fdt_size_t match the number of cells specified by the #address-cells and #size-cells properties. However, there is no reason why that needs to be the case, so the function potentially decodes address and size wrongly.
Furthermore the function casts an array of FDT cells (32-bit unsigned) to fdt_addr_t/fdt_size_t and dereferences them directly. This can cause aborts on 64-bit architectures where fdt_addr_t/fdt_size_t are 64 bits wide, because cells are only guaranteed to be aligned to 32 bits. Fix this by reading in the address and size one cell at a time.
Cc: Hanna Hawa hannah@marvell.com Cc: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com
lib/fdtdec.c | 56 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 613cc8494a74..fd244440381e 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -89,29 +89,45 @@ const char *fdtdec_get_compatible(enum fdt_compat_id id) fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, const char *prop_name, fdt_size_t *sizep) {
const fdt_addr_t *cell;int len;
const fdt32_t *ptr, *end;int parent, na, ns, len;fdt_addr_t addr; debug("%s: %s: ", __func__, prop_name);
cell = fdt_getprop(blob, node, prop_name, &len);if (cell && ((!sizep && len == sizeof(fdt_addr_t)) ||len == sizeof(fdt_addr_t) * 2)) {fdt_addr_t addr = fdt_addr_to_cpu(*cell);if (sizep) {const fdt_size_t *size;size = (fdt_size_t *)((char *)cell +sizeof(fdt_addr_t));*sizep = fdt_size_to_cpu(*size);debug("addr=%08lx, size=%08lx\n",(ulong)addr, (ulong)*sizep);} else {debug("%08lx\n", (ulong)addr);}return addr;
parent = fdt_parent_offset(blob, node);
This function is very slow as it must scan the whole tree. Can we instead pass in the parent node? Also, how about (in addition) a version of this function that works for devices? Like:
device_get_addr_size(struct udevice *dev, ...)
so that it can handle this for you.
if (parent < 0) {debug("(no parent found)\n");return FDT_ADDR_T_NONE; }
debug("(not found)\n");return FDT_ADDR_T_NONE;
na = fdt_address_cells(blob, parent);ns = fdt_size_cells(blob, parent);ptr = fdt_getprop(blob, node, prop_name, &len);if (!ptr) {debug("(not found)\n");return FDT_ADDR_T_NONE;}end = ptr + len / sizeof(*ptr);if (ptr + na + ns > end) {debug("(not enough data: expected %d bytes, got %d bytes)\n",(na + ns) * 4, len);return FDT_ADDR_T_NONE;}addr = fdtdec_get_number(ptr, na);if (sizep) {*sizep = fdtdec_get_number(ptr + na, ns);debug("addr=%pa, size=%pa\n", &addr, sizep);} else {debug("%pa\n", &addr);}return addr;}
fdt_addr_t fdtdec_get_addr(const void *blob, int node,
2.3.2
Regards, Simon
From: Thierry Reding treding@nvidia.com
The fdtdec_get_pci_addr() implementation uses fdt_addr_to_cpu() to read cells from an FDT blob. That is wrong because cells are always 32 bits wide, irrespective of the architecture's address bus width, which does not apply to fdt_addr_t.
Besides reading the wrong results, this can also cause aborts on 64-bit architectures that don't allow unaligned accesses to memory. Fix this by using fdt32_to_cpu() to read the cells instead.
Cc: Hanna Hawa hannah@marvell.com Cc: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com --- lib/fdtdec.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index fd244440381e..c26b06f390b8 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -162,13 +162,13 @@ int fdtdec_get_pci_addr(const void *blob, int node, enum fdt_pci_space type,
for (i = 0; i < num; i++) { debug("pci address #%d: %08lx %08lx %08lx\n", i, - (ulong)fdt_addr_to_cpu(cell[0]), - (ulong)fdt_addr_to_cpu(cell[1]), - (ulong)fdt_addr_to_cpu(cell[2])); + (ulong)fdt32_to_cpu(cell[0]), + (ulong)fdt32_to_cpu(cell[1]), + (ulong)fdt32_to_cpu(cell[2])); if ((fdt_addr_to_cpu(*cell) & type) == type) { - addr->phys_hi = fdt_addr_to_cpu(cell[0]); - addr->phys_mid = fdt_addr_to_cpu(cell[1]); - addr->phys_lo = fdt_addr_to_cpu(cell[2]); + addr->phys_hi = fdt32_to_cpu(cell[0]); + addr->phys_mid = fdt32_to_cpu(cell[1]); + addr->phys_lo = fdt32_to_cpu(cell[2]); break; } else { cell += (FDT_PCI_ADDR_CELLS +
On 20 March 2015 at 05:51, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
The fdtdec_get_pci_addr() implementation uses fdt_addr_to_cpu() to read cells from an FDT blob. That is wrong because cells are always 32 bits wide, irrespective of the architecture's address bus width, which does not apply to fdt_addr_t.
Besides reading the wrong results, this can also cause aborts on 64-bit architectures that don't allow unaligned accesses to memory. Fix this by using fdt32_to_cpu() to read the cells instead.
Cc: Hanna Hawa hannah@marvell.com Cc: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com
Acked-by: Simon Glass sjg@chromium.org
Hi Thierry,
On 20 March 2015 at 05:51, Thierry Reding thierry.reding@gmail.com wrote:
From: Thierry Reding treding@nvidia.com
Drivers that need to parse addresses from the device tree will want to reuse this function rather than duplicating it.
Cc: Simon Glass sjg@chromium.org Signed-off-by: Thierry Reding treding@nvidia.com
Please see this patch:
http://patchwork.ozlabs.org/patch/446862/
Regards, Simon
participants (2)
-
Simon Glass -
Thierry Reding