| Message ID | 1324290964-14096-1-git-send-email-jonas.gorski@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2011-12-19 at 11:36 +0100, Jonas Gorski wrote: > The CFE boot loader on BCM63XX platforms assumes itself and the NVRAM > partition to be 64 KiB (or erase block sized, if larger). > Ensure this assumption is also met when creating the partitions to > prevent accidential erasure of CFE or NVRAM. > > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> If someone creates a partition smaller than 64 KiB, then why it is better to silently make it 64 KiB (and thus doing not what the user asked to do and possibly confusing him), rather than returning an error or just printing a warning?
On 19 December 2011 11:56, Artem Bityutskiy <dedekind1@gmail.com> wrote: > On Mon, 2011-12-19 at 11:36 +0100, Jonas Gorski wrote: >> The CFE boot loader on BCM63XX platforms assumes itself and the NVRAM >> partition to be 64 KiB (or erase block sized, if larger). >> Ensure this assumption is also met when creating the partitions to >> prevent accidential erasure of CFE or NVRAM. >> >> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> > > If someone creates a partition smaller than 64 KiB, then why it is > better to silently make it 64 KiB (and thus doing not what the user > asked to do and possibly confusing him), rather than returning an error > or just printing a warning? This adjustment to 64 KiB is only done for the CFE and NVRAM partitions, not for the rootfs or kernel partitions. The CFE and NVRAM lengths/offsets are defined in the CFE boot loader at build time and fixed, so to change them you would need to build and flash your own CFE (and the sources are not public, so you also need to be a Broadcom customer). I have yet to see a device where this was done, so this is currently just a theoretical possibility. Also you can't create/modify partitions arbitrarily as there is no partition table, just a fixed image header format (which the CFE parses on boot). So the only two partitions changeable from the outside are the kernel and rootfs partitions, which are defined in the image tag, which always resides at the beginning of the first erase block after the CFE. Changing the CFE length would result in a changed offset of the image tag, leading to a "wrong" image tag being read (or in case after patch 5, a warning that the image tag is likely corrupt, and no rootfs/kernel partitions created). Of course everything is done under the assumption the boot loader is CFE (there are a few devices with RedBoot out there), but the parser already bails out if no CFE is detected. Hope that clears things up. Jonas
On Mon, 2011-12-19 at 11:36 +0100, Jonas Gorski wrote: > The CFE boot loader on BCM63XX platforms assumes itself and the NVRAM > partition to be 64 KiB (or erase block sized, if larger). > Ensure this assumption is also met when creating the partitions to > prevent accidential erasure of CFE or NVRAM. > > Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> Updated, thanks!
diff --git a/drivers/mtd/bcm63xxpart.c b/drivers/mtd/bcm63xxpart.c index 9933b34..23f6201 100644 --- a/drivers/mtd/bcm63xxpart.c +++ b/drivers/mtd/bcm63xxpart.c @@ -36,6 +36,9 @@ #define BCM63XX_EXTENDED_SIZE 0xBFC00000 /* Extended flash address */ +#define BCM63XX_MIN_CFE_SIZE 0x10000 /* always at least 64K */ +#define BCM63XX_MIN_NVRAM_SIZE 0x10000 /* always at least 64K */ + #define BCM63XX_CFE_MAGIC_OFFSET 0x4e0 static int bcm63xx_detect_cfe(struct mtd_info *master) @@ -74,6 +77,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, size_t retlen; unsigned int rootfsaddr, kerneladdr, spareaddr; unsigned int rootfslen, kernellen, sparelen, totallen; + unsigned int cfelen, nvramlen; int namelen = 0; int i; char *boardid; @@ -82,14 +86,18 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, if (bcm63xx_detect_cfe(master)) return -EINVAL; + cfelen = max_t(uint32_t, master->erasesize, BCM63XX_MIN_CFE_SIZE); + nvramlen = max_t(uint32_t, master->erasesize, BCM63XX_MIN_NVRAM_SIZE); + /* Allocate memory for buffer */ buf = vmalloc(sizeof(struct bcm_tag)); if (!buf) return -ENOMEM; /* Get the tag */ - ret = master->read(master, master->erasesize, sizeof(struct bcm_tag), - &retlen, (void *)buf); + ret = master->read(master, cfelen, sizeof(struct bcm_tag), &retlen, + (void *)buf); + if (retlen != sizeof(struct bcm_tag)) { vfree(buf); return -EIO; @@ -106,8 +114,8 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, kerneladdr = kerneladdr - BCM63XX_EXTENDED_SIZE; rootfsaddr = kerneladdr + kernellen; - spareaddr = roundup(totallen, master->erasesize) + master->erasesize; - sparelen = master->size - spareaddr - master->erasesize; + spareaddr = roundup(totallen, master->erasesize) + cfelen; + sparelen = master->size - spareaddr - nvramlen; rootfslen = spareaddr - rootfsaddr; /* Determine number of partitions */ @@ -131,7 +139,7 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, /* Start building partition list */ parts[curpart].name = "CFE"; parts[curpart].offset = 0; - parts[curpart].size = master->erasesize; + parts[curpart].size = cfelen; curpart++; if (kernellen > 0) { @@ -151,8 +159,8 @@ static int bcm63xx_parse_cfe_partitions(struct mtd_info *master, } parts[curpart].name = "nvram"; - parts[curpart].offset = master->size - master->erasesize; - parts[curpart].size = master->erasesize; + parts[curpart].offset = master->size - nvramlen; + parts[curpart].size = nvramlen; /* Global partition "linux" to make easy firmware upgrade */ curpart++;
The CFE boot loader on BCM63XX platforms assumes itself and the NVRAM partition to be 64 KiB (or erase block sized, if larger). Ensure this assumption is also met when creating the partitions to prevent accidential erasure of CFE or NVRAM. Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com> --- Changes V1 -> V2: Clarified the need for the minimum size. Hope this one's better :) drivers/mtd/bcm63xxpart.c | 22 +++++++++++++++------- 1 files changed, 15 insertions(+), 7 deletions(-)