* [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number()
@ 2015-03-20 11:51 Thierry Reding
2015-03-20 11:51 ` [U-Boot] [PATCH 2/3] fdt: Fix fdtdec_get_addr_size() for 64-bit Thierry Reding
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Thierry Reding @ 2015-03-20 11:51 UTC (permalink / raw)
To: u-boot
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;
--
2.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/3] fdt: Fix fdtdec_get_addr_size() for 64-bit
2015-03-20 11:51 [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number() Thierry Reding
@ 2015-03-20 11:51 ` Thierry Reding
2015-03-23 23:20 ` Simon Glass
2015-03-20 11:51 ` [U-Boot] [PATCH 3/3] fdt: Fix fdtdec_get_pci_addr() " Thierry Reding
2015-03-23 23:16 ` [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number() Simon Glass
2 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2015-03-20 11:51 UTC (permalink / raw)
To: u-boot
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,
--
2.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 3/3] fdt: Fix fdtdec_get_pci_addr() for 64-bit
2015-03-20 11:51 [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number() Thierry Reding
2015-03-20 11:51 ` [U-Boot] [PATCH 2/3] fdt: Fix fdtdec_get_addr_size() for 64-bit Thierry Reding
@ 2015-03-20 11:51 ` Thierry Reding
2015-03-23 23:20 ` Simon Glass
2015-03-23 23:16 ` [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number() Simon Glass
2 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2015-03-20 11:51 UTC (permalink / raw)
To: u-boot
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 +
--
2.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number()
2015-03-20 11:51 [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number() Thierry Reding
2015-03-20 11:51 ` [U-Boot] [PATCH 2/3] fdt: Fix fdtdec_get_addr_size() for 64-bit Thierry Reding
2015-03-20 11:51 ` [U-Boot] [PATCH 3/3] fdt: Fix fdtdec_get_pci_addr() " Thierry Reding
@ 2015-03-23 23:16 ` Simon Glass
2 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2015-03-23 23:16 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/3] fdt: Fix fdtdec_get_addr_size() for 64-bit
2015-03-20 11:51 ` [U-Boot] [PATCH 2/3] fdt: Fix fdtdec_get_addr_size() for 64-bit Thierry Reding
@ 2015-03-23 23:20 ` Simon Glass
0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2015-03-23 23:20 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 3/3] fdt: Fix fdtdec_get_pci_addr() for 64-bit
2015-03-20 11:51 ` [U-Boot] [PATCH 3/3] fdt: Fix fdtdec_get_pci_addr() " Thierry Reding
@ 2015-03-23 23:20 ` Simon Glass
0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2015-03-23 23:20 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-23 23:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-20 11:51 [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number() Thierry Reding
2015-03-20 11:51 ` [U-Boot] [PATCH 2/3] fdt: Fix fdtdec_get_addr_size() for 64-bit Thierry Reding
2015-03-23 23:20 ` Simon Glass
2015-03-20 11:51 ` [U-Boot] [PATCH 3/3] fdt: Fix fdtdec_get_pci_addr() " Thierry Reding
2015-03-23 23:20 ` Simon Glass
2015-03-23 23:16 ` [U-Boot] [PATCH 1/3] fdt: Export fdtdec_get_number() Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.