| Message ID | 20191021124912.25569-1-jjhiblot@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot] fdt: Fix alignment issue when reading 64-bits properties | expand |
Hi Jean-Jacques, On Mon, 21 Oct 2019 at 06:50, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote: > > The FDT specification [0] gives a requirement of aligning properties on > 32-bits. Make sure that the compiler is aware of this constraint when > accessing 64-bits properties. > > [0]: https://github.com/devicetree-org/devicetree-specification/blob/master/source/flattened-format.rst > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> > --- > common/fdt_support.c | 2 +- > drivers/core/ofnode.c | 2 +- > lib/fdtdec.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/fdt_support.c b/common/fdt_support.c > index baf7924ff6..d14cf7f61a 100644 > --- a/common/fdt_support.c > +++ b/common/fdt_support.c > @@ -1566,7 +1566,7 @@ static int fdt_read_prop(const fdt32_t *prop, int prop_len, int cell_off, > uint64_t *val, int cells) > { > const fdt32_t *prop32 = &prop[cell_off]; > - const fdt64_t *prop64 = (const fdt64_t *)&prop[cell_off]; > + const fdt64_t *prop64 __aligned(4) = (const fdt64_t *)&prop[cell_off]; Excuse my ignorance, but what does this actually mean? That the pointer points to something that is aligned? Regards, Simon
Hi Simon, On 22/10/2019 01:46, Simon Glass wrote: > Hi Jean-Jacques, > > On Mon, 21 Oct 2019 at 06:50, Jean-Jacques Hiblot <jjhiblot@ti.com> wrote: >> The FDT specification [0] gives a requirement of aligning properties on >> 32-bits. Make sure that the compiler is aware of this constraint when >> accessing 64-bits properties. >> >> [0]: https://github.com/devicetree-org/devicetree-specification/blob/master/source/flattened-format.rst >> >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >> --- >> common/fdt_support.c | 2 +- >> drivers/core/ofnode.c | 2 +- >> lib/fdtdec.c | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/common/fdt_support.c b/common/fdt_support.c >> index baf7924ff6..d14cf7f61a 100644 >> --- a/common/fdt_support.c >> +++ b/common/fdt_support.c >> @@ -1566,7 +1566,7 @@ static int fdt_read_prop(const fdt32_t *prop, int prop_len, int cell_off, >> uint64_t *val, int cells) >> { >> const fdt32_t *prop32 = &prop[cell_off]; >> - const fdt64_t *prop64 = (const fdt64_t *)&prop[cell_off]; >> + const fdt64_t *prop64 __aligned(4) = (const fdt64_t *)&prop[cell_off]; > Excuse my ignorance, but what does this actually mean? That the > pointer points to something that is aligned? Well it should have made the compiler assume that the address is aligned on 32-bits not 64-bits. But I see now that it doesn't. As I didn't see the issue anymore iI thought it worked, but it just was my DTB not exposing the issue anymore. Anyway I'll send a v2 with the effective changes. Thanks. JJ > Regards, > Simon >
diff --git a/common/fdt_support.c b/common/fdt_support.c index baf7924ff6..d14cf7f61a 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1566,7 +1566,7 @@ static int fdt_read_prop(const fdt32_t *prop, int prop_len, int cell_off, uint64_t *val, int cells) { const fdt32_t *prop32 = &prop[cell_off]; - const fdt64_t *prop64 = (const fdt64_t *)&prop[cell_off]; + const fdt64_t *prop64 __aligned(4) = (const fdt64_t *)&prop[cell_off]; if ((cell_off + cells) > prop_len) return -FDT_ERR_NOSPACE; diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 297f0a0c7c..6bd4235029 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -57,7 +57,7 @@ int ofnode_read_s32_default(ofnode node, const char *propname, s32 def) int ofnode_read_u64(ofnode node, const char *propname, u64 *outp) { - const fdt64_t *cell; + const fdt64_t *cell __aligned(4); int len; assert(ofnode_valid(node)); diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 17736ce665..77a54b6433 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -242,7 +242,7 @@ int fdtdec_get_pci_bar32(struct udevice *dev, struct fdt_pci_addr *addr, uint64_t fdtdec_get_uint64(const void *blob, int node, const char *prop_name, uint64_t default_val) { - const uint64_t *cell64; + const uint64_t *cell64 __aligned(4); int length; cell64 = fdt_getprop(blob, node, prop_name, &length);
The FDT specification [0] gives a requirement of aligning properties on 32-bits. Make sure that the compiler is aware of this constraint when accessing 64-bits properties. [0]: https://github.com/devicetree-org/devicetree-specification/blob/master/source/flattened-format.rst Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> --- common/fdt_support.c | 2 +- drivers/core/ofnode.c | 2 +- lib/fdtdec.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)