* [U-Boot] [PATCH] tools/env: fix environment alignment tests for block devices
@ 2016-11-18 10:38 Max Krummenacher
2016-11-18 13:26 ` Andreas Fenkart
0 siblings, 1 reply; 3+ messages in thread
From: Max Krummenacher @ 2016-11-18 10:38 UTC (permalink / raw)
To: u-boot
commit 183923d3e412500bdc597d1745e2fb6f7f679ec7 enforces that the
environment must start at an erase block boundary.
For block devices the sample fw_env.config does not mandate a erase block size
for block devices. A missing setting defaults to the full env size.
Depending on the environment location the alignment check now errors out for
perfectly legal settings.
Fix this by defaulting to the standard blocksize of 0x200 for environments
stored in a block device.
That keeps the fw_env.config files for block devices working even with that
new check.
Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
---
tools/env/fw_env.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 3dc0d53..f2126f8 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -1335,10 +1335,25 @@ static int check_device_config(int dev)
goto err;
}
DEVTYPE(dev) = mtdinfo.type;
+ if (DEVESIZE(dev) == -1) {
+ /* Assume the erase size is the same as the env-size */
+ DEVESIZE(dev) = ENVSIZE(dev);
+ /* Assume enough env sectors to cover the environment */
+ ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
+ DEVESIZE(dev);
+ }
} else {
uint64_t size;
DEVTYPE(dev) = MTD_ABSENT;
+ if (DEVESIZE(dev) == -1) {
+ /* Assume the erase size to be 512 bytes */
+ DEVESIZE(dev) = 0x200;
+ /* Assume enough env sectors to cover the environment */
+ ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
+ DEVESIZE(dev);
+ }
+
/*
* Check for negative offsets, treat it as backwards offset
* from the end of the block device
@@ -1467,10 +1482,9 @@ static int get_config (char *fname)
DEVNAME(i) = devname;
if (rc < 4)
- /* Assume the erase size is the same as the env-size */
- DEVESIZE(i) = ENVSIZE(i);
-
- if (rc < 5)
+ /* Fixup later depending on DEVTYPE */
+ DEVESIZE(i) = -1;
+ else if (rc < 5)
/* Assume enough env sectors to cover the environment */
ENVSECTORS (i) = (ENVSIZE(i) + DEVESIZE(i) - 1) / DEVESIZE(i);
--
2.5.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* [U-Boot] [PATCH] tools/env: fix environment alignment tests for block devices
2016-11-18 10:38 [U-Boot] [PATCH] tools/env: fix environment alignment tests for block devices Max Krummenacher
@ 2016-11-18 13:26 ` Andreas Fenkart
2016-11-19 12:52 ` Max Krummenacher
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Fenkart @ 2016-11-18 13:26 UTC (permalink / raw)
To: u-boot
2016-11-18 11:38 GMT+01:00 Max Krummenacher <max.oss.09@gmail.com>:
> commit 183923d3e412500bdc597d1745e2fb6f7f679ec7 enforces that the
> environment must start at an erase block boundary.
>
> For block devices the sample fw_env.config does not mandate a erase block size
> for block devices. A missing setting defaults to the full env size.
>
> Depending on the environment location the alignment check now errors out for
> perfectly legal settings.
>
> Fix this by defaulting to the standard blocksize of 0x200 for environments
> stored in a block device.
> That keeps the fw_env.config files for block devices working even with that
> new check.
>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> ---
>
> tools/env/fw_env.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 3dc0d53..f2126f8 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -1335,10 +1335,25 @@ static int check_device_config(int dev)
> goto err;
> }
> DEVTYPE(dev) = mtdinfo.type;
There is 'erasesize' in that struct, not sure how reliable that is and
if we shall rely on it. Any experiences on this?
In general we should take that and in case it is wrong the user has to
overwrite it in fw_env.config
# mtdinfo /dev/mtd5
mtd5
Name: spi32766.0
Type: nor
Eraseblock size: 65536 bytes, 64.0 KiB
Amount of eraseblocks: 8 (524288 bytes, 512.0 KiB)
Minimum input/output unit size: 1 byte
Sub-page size: 1 byte
Character device major/minor: 90:10
Bad blocks are allowed: false
Device is writable: true
> + if (DEVESIZE(dev) == -1) {
> + /* Assume the erase size is the same as the env-size */
> + DEVESIZE(dev) = ENVSIZE(dev);
> + /* Assume enough env sectors to cover the environment */
> + ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
> + DEVESIZE(dev);
DIV_ROUND_UP
> + }
> } else {
> uint64_t size;
> DEVTYPE(dev) = MTD_ABSENT;
>
> + if (DEVESIZE(dev) == -1) {
> + /* Assume the erase size to be 512 bytes */
> + DEVESIZE(dev) = 0x200;
> + /* Assume enough env sectors to cover the environment */
> + ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
> + DEVESIZE(dev);
DIV_ROUND_UP
> + }
> +
> /*
> * Check for negative offsets, treat it as backwards offset
> * from the end of the block device
> @@ -1467,10 +1482,9 @@ static int get_config (char *fname)
> DEVNAME(i) = devname;
>
> if (rc < 4)
> - /* Assume the erase size is the same as the env-size */
> - DEVESIZE(i) = ENVSIZE(i);
> -
> - if (rc < 5)
> + /* Fixup later depending on DEVTYPE */
> + DEVESIZE(i) = -1;
I think we can rely on '0', since that is invalid already.
> + else if (rc < 5)
> /* Assume enough env sectors to cover the environment */
> ENVSECTORS (i) = (ENVSIZE(i) + DEVESIZE(i) - 1) / DEVESIZE(i);
I guess the !defined(CONFIG_FILE) has the same problem. Let's just
ignore the cases rc < 4,5 here and handle them all in check_config.
>
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [U-Boot] [PATCH] tools/env: fix environment alignment tests for block devices
2016-11-18 13:26 ` Andreas Fenkart
@ 2016-11-19 12:52 ` Max Krummenacher
0 siblings, 0 replies; 3+ messages in thread
From: Max Krummenacher @ 2016-11-19 12:52 UTC (permalink / raw)
To: u-boot
Hi Andreas
Am Freitag, den 18.11.2016, 14:26 +0100 schrieb Andreas Fenkart:
> 2016-11-18 11:38 GMT+01:00 Max Krummenacher <max.oss.09@gmail.com>:
> > commit 183923d3e412500bdc597d1745e2fb6f7f679ec7 enforces that the
> > environment must start at an erase block boundary.
> >
> > For block devices the sample fw_env.config does not mandate a erase block size
> > for block devices. A missing setting defaults to the full env size.
> >
> > Depending on the environment location the alignment check now errors out for
> > perfectly legal settings.
> >
> > Fix this by defaulting to the standard blocksize of 0x200 for environments
> > stored in a block device.
> > That keeps the fw_env.config files for block devices working even with that
> > new check.
> >
> > Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> > ---
> >
> > tools/env/fw_env.c | 22 ++++++++++++++++++----
> > 1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> > index 3dc0d53..f2126f8 100644
> > --- a/tools/env/fw_env.c
> > +++ b/tools/env/fw_env.c
> > @@ -1335,10 +1335,25 @@ static int check_device_config(int dev)
> > goto err;
> > }
> > DEVTYPE(dev) = mtdinfo.type;
>
> There is 'erasesize' in that struct, not sure how reliable that is and
> if we shall rely on it. Any experiences on this?
> In general we should take that and in case it is wrong the user has to
> overwrite it in fw_env.config
>
> # mtdinfo /dev/mtd5
> mtd5
> Name: spi32766.0
> Type: nor
> Eraseblock size: 65536 bytes, 64.0 KiB
> Amount of eraseblocks: 8 (524288 bytes, 512.0 KiB)
> Minimum input/output unit size: 1 byte
> Sub-page size: 1 byte
> Character device major/minor: 90:10
> Bad blocks are allowed: false
> Device is writable: true
>
That is exactly the bug I try to fix.
(e)MMC/SD cards are not MTD devices, the concept of an erase block does
not exist for these devices:
# mtdinfo /dev/mmcblk0
libmtd: error!: "/dev/mmcblk0" is not a character device
mtdinfo: error!: cannot get information about MTD device "/dev/mmcblk0"
error 22 (Invalid argument)
> > + if (DEVESIZE(dev) == -1) {
> > + /* Assume the erase size is the same as the env-size */
> > + DEVESIZE(dev) = ENVSIZE(dev);
> > + /* Assume enough env sectors to cover the environment */
> > + ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
> > + DEVESIZE(dev);
>
> DIV_ROUND_UP
I will try if we are allowed to include the relevant header in this
userspace application.
Ups, someone defined that macro inside the file already, will use it.
>
> > + }
> > } else {
> > uint64_t size;
> > DEVTYPE(dev) = MTD_ABSENT;
> >
> > + if (DEVESIZE(dev) == -1) {
> > + /* Assume the erase size to be 512 bytes */
> > + DEVESIZE(dev) = 0x200;
> > + /* Assume enough env sectors to cover the environment */
> > + ENVSECTORS(dev) = (ENVSIZE(dev) + DEVESIZE(dev) - 1) /
> > + DEVESIZE(dev);
>
> DIV_ROUND_UP
Ok.
>
> > + }
> > +
> > /*
> > * Check for negative offsets, treat it as backwards offset
> > * from the end of the block device
> > @@ -1467,10 +1482,9 @@ static int get_config (char *fname)
> > DEVNAME(i) = devname;
> >
> > if (rc < 4)
> > - /* Assume the erase size is the same as the env-size */
> > - DEVESIZE(i) = ENVSIZE(i);
> > -
> > - if (rc < 5)
> > + /* Fixup later depending on DEVTYPE */
> > + DEVESIZE(i) = -1;
>
> I think we can rely on '0', since that is invalid already.
Actually that uncovered a bug I introduced. The fixup has to come
before the test 'if (DEVOFFSET(dev) % DEVESIZE(dev) != 0)' which
uses the values.
I will change to 0, aka rely on global variables defaulting to 0.
>
>
> > + else if (rc < 5)
> > /* Assume enough env sectors to cover the environment */
> > ENVSECTORS (i) = (ENVSIZE(i) + DEVESIZE(i) - 1) / DEVESIZE(i);
>
> I guess the !defined(CONFIG_FILE) has the same problem. Let's just
> ignore the cases rc < 4,5 here and handle them all in check_config.
I will remove the default handling in both cases in favor of handling this later
in check_device_config().
Will send a V2 shortly.
Regards
Max
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-19 12:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-18 10:38 [U-Boot] [PATCH] tools/env: fix environment alignment tests for block devices Max Krummenacher
2016-11-18 13:26 ` Andreas Fenkart
2016-11-19 12:52 ` Max Krummenacher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox