[U-Boot] env: mmc: Fix misaligned buffer address when saving envvars to FAT

Message ID 1526543381-30905-1-git-send-email-vipul.kumar@xilinx.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series
  • [U-Boot] env: mmc: Fix misaligned buffer address when saving envvars to FAT
Related show

Commit Message

Vipul Kumar May 17, 2018, 7:49 a.m.
From: Gary Mussar <gmussar@ciena.com>

When doing a u-boot saveenv with the environment in FAT we see the
following warning:

ZynqMP> saveenv
Saving Environment to FAT...
writing uboot.env
FAT: Misaligned buffer address (000000007deb9b60)
done

This can be eliminated by aligning the environment to an appropriate
boundary.

Signed-off-by: Gary Mussar <gmussar@ciena.com>
Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
---
 include/environment.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Comments

Alex Kiernan May 17, 2018, 8:01 a.m. | #1
On Thu, May 17, 2018 at 8:46 AM Vipul Kumar <vipul.kumar@xilinx.com> wrote:

> From: Gary Mussar <gmussar@ciena.com>

> When doing a u-boot saveenv with the environment in FAT we see the
> following warning:

> ZynqMP> saveenv
> Saving Environment to FAT...
> writing uboot.env
> FAT: Misaligned buffer address (000000007deb9b60)
> done

> This can be eliminated by aligning the environment to an appropriate
> boundary.


Are you still seeing after this commit?

cda87ec Fix misaligned buffer in env_fat_save

> Signed-off-by: Gary Mussar <gmussar@ciena.com>
> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
> ---
>   include/environment.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

> diff --git a/include/environment.h b/include/environment.h
> index 50c62c5..d6c530d 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -150,7 +150,11 @@ typedef struct environment_s {
>          unsigned char   flags;          /* active/obsolete flags        */
>   #endif
>          unsigned char   data[ENV_SIZE]; /* Environment data             */
> -} env_t;
> +} env_t
> +#ifdef ARCH_DMA_MINALIGN
> +__aligned(ARCH_DMA_MINALIGN)
> +#endif
> +;

>   #ifdef ENV_IS_EMBEDDED
>   extern env_t environment;
> --
> 2.7.4

> This email and any attachments are intended for the sole use of the named
recipient(s) and contain(s) confidential information that may be
proprietary, privileged or copyrighted under applicable law. If you are not
the intended recipient, do not read, copy, or forward this email message or
any attachments. Delete this email message and any attachments immediately.
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Vipul Kumar May 17, 2018, 9:06 a.m. | #2
Hi,

> -----Original Message-----
> From: Alex Kiernan [mailto:alex.kiernan@gmail.com]
> Sent: Thursday, May 17, 2018 1:31 PM
> To: Vipul Kumar <vipulk@xilinx.com>
> Cc: u-boot <u-boot@lists.denx.de>; gmussar@ciena.com; Michal Simek
> <michal.simek@xilinx.com>; Siva Durga Prasad Paladugu
> <sivadur@xilinx.com>
> Subject: Re: [U-Boot] [UBOOT PATCH] env: mmc: Fix misaligned buffer
> address when saving envvars to FAT
> 
> On Thu, May 17, 2018 at 8:46 AM Vipul Kumar <vipul.kumar@xilinx.com>
> wrote:
> 
> > From: Gary Mussar <gmussar@ciena.com>
> 
> > When doing a u-boot saveenv with the environment in FAT we see the
> > following warning:
> 
> > ZynqMP> saveenv
> > Saving Environment to FAT...
> > writing uboot.env
> > FAT: Misaligned buffer address (000000007deb9b60) done
> 
> > This can be eliminated by aligning the environment to an appropriate
> > boundary.
> 
> 
> Are you still seeing after this commit?

With cda87ec commit, it's working fine. I didn’t test with this commit earlier. So, there is no need of this patch.

Regards,
Vipul

> 
> cda87ec Fix misaligned buffer in env_fat_save
> 
> > Signed-off-by: Gary Mussar <gmussar@ciena.com>
> > Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
> > ---
> >   include/environment.h | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> > diff --git a/include/environment.h b/include/environment.h index
> > 50c62c5..d6c530d 100644
> > --- a/include/environment.h
> > +++ b/include/environment.h
> > @@ -150,7 +150,11 @@ typedef struct environment_s {
> >          unsigned char   flags;          /* active/obsolete flags        */
> >   #endif
> >          unsigned char   data[ENV_SIZE]; /* Environment data             */
> > -} env_t;
> > +} env_t
> > +#ifdef ARCH_DMA_MINALIGN
> > +__aligned(ARCH_DMA_MINALIGN)
> > +#endif
> > +;
> 
> >   #ifdef ENV_IS_EMBEDDED
> >   extern env_t environment;
> > --
> > 2.7.4
> 
> > This email and any attachments are intended for the sole use of the
> > named
> recipient(s) and contain(s) confidential information that may be proprietary,
> privileged or copyrighted under applicable law. If you are not the intended
> recipient, do not read, copy, or forward this email message or any
> attachments. Delete this email message and any attachments immediately.
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> 
> 
> 
> --
> Alex Kiernan
Tom Rini May 27, 2018, 9:24 p.m. | #3
On Thu, May 17, 2018 at 01:19:41PM +0530, Vipul Kumar wrote:

> From: Gary Mussar <gmussar@ciena.com>
> 
> When doing a u-boot saveenv with the environment in FAT we see the
> following warning:
> 
> ZynqMP> saveenv
> Saving Environment to FAT...
> writing uboot.env
> FAT: Misaligned buffer address (000000007deb9b60)
> done
> 
> This can be eliminated by aligning the environment to an appropriate
> boundary.
> 
> Signed-off-by: Gary Mussar <gmussar@ciena.com>
> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com>
> ---
>  include/environment.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --
> 2.7.4
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> diff --git a/include/environment.h b/include/environment.h
> index 50c62c5..d6c530d 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -150,7 +150,11 @@ typedef struct environment_s {
>         unsigned char   flags;          /* active/obsolete flags        */
>  #endif
>         unsigned char   data[ENV_SIZE]; /* Environment data             */
> -} env_t;
> +} env_t
> +#ifdef ARCH_DMA_MINALIGN
> +__aligned(ARCH_DMA_MINALIGN)
> +#endif
> +;
> 
>  #ifdef ENV_IS_EMBEDDED
>  extern env_t environment;

This needs some more work.  The first problem is that ARCH_DMA_MINALIGN
is defined for all platforms.  But it might not be defined in all uses,
for example M5485BFE and other cases of embedded in the binary
environment.  And if we drop the check for ARCH_DMA_MINALIGN existing we
run into problems with needing to add __align to include/compiler.h.

My guess is that what we should do here is that we have another case in
env/fat.c (there's already one) where a variable needs to be __aligned
properly instead.  Can you look down that path?  Thanks!

Patch

diff --git a/include/environment.h b/include/environment.h
index 50c62c5..d6c530d 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -150,7 +150,11 @@  typedef struct environment_s {
        unsigned char   flags;          /* active/obsolete flags        */
 #endif
        unsigned char   data[ENV_SIZE]; /* Environment data             */
-} env_t;
+} env_t
+#ifdef ARCH_DMA_MINALIGN
+__aligned(ARCH_DMA_MINALIGN)
+#endif
+;

 #ifdef ENV_IS_EMBEDDED
 extern env_t environment;