[Skiboot] [PATCH 06/17] libflash/libffs: Don't require 'part' size to be known by callers
Cyril Bur
cyril.bur at au1.ibm.com
Fri Jul 21 16:35:57 AEST 2017
Currently the FFS header/TOC generation code requires that consumers
know the size of their TOC beforehand. While this may be advantageous in
some circumstances if there are known limitations of other software. It
should not be a requirement.
Knowing the size of the FFS header/TOC partially breaks the abstraction
since it would require consumers of the library to be aware of/have some
idea of the on flash structure and size.
Future work may introduce functions to force sizes but the default
behaviour should be to calculate it behind the scenes.
This patch also addresses an off by one issue in checking for TOC
overflow.
Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
external/ffspart/ffspart.c | 10 ++-----
external/ffspart/test/files/03-tiny-pnor.in | 8 +++---
external/ffspart/test/files/03-tiny-pnor.out | Bin 2560 -> 2560 bytes
.../ffspart/test/files/03.1-tiny-pnor-backup.in | 8 +++---
.../ffspart/test/files/03.1-tiny-pnor-backup.out | Bin 2864 -> 3840 bytes
external/ffspart/test/files/04-tiny-pnor2.out | Bin 2560 -> 2560 bytes
external/ffspart/test/results/05-hdr-overlap.err | 4 +--
external/ffspart/test/results/05-hdr-overlap.out | 1 -
.../test/results/05.1-hdr-overlap-backup.err | 4 +--
external/ffspart/test/tests/03.1-tiny-pnor-backup | 2 +-
libflash/ffs.h | 1 +
libflash/libffs.c | 32 +++++++++++++--------
libflash/libffs.h | 2 +-
13 files changed, 37 insertions(+), 35 deletions(-)
diff --git a/external/ffspart/ffspart.c b/external/ffspart/ffspart.c
index 50b5861e..77034775 100644
--- a/external/ffspart/ffspart.c
+++ b/external/ffspart/ffspart.c
@@ -174,17 +174,11 @@ int main(int argc, char *argv[])
goto out;
}
- /*
- * TODO: Rethink, is ffspart providing a variable TOC size useful?
- * Use 1 block for the size of the partition table...
- */
- rc = ffs_hdr_new(block_size, block_size, block_count / sides, &new_hdr);
+ rc = ffs_hdr_new(block_size, block_count / sides, &new_hdr);
if (rc) {
if (rc == FFS_ERR_BAD_SIZE) {
/* Well this check is a tad redudant now */
- fprintf(stderr, "Bad size parametres passed to libffs: "
- "size must be a multiple of block_size\n"
- "size (%u), block_size (%u) \n", block_size, block_size);
+ fprintf(stderr, "Bad parametres passed to libffs\n");
} else {
fprintf(stderr, "Error %d initialising new TOC\n", rc);
}
diff --git a/external/ffspart/test/files/03-tiny-pnor.in b/external/ffspart/test/files/03-tiny-pnor.in
index 517dc47b..3c02b029 100644
--- a/external/ffspart/test/files/03-tiny-pnor.in
+++ b/external/ffspart/test/files/03-tiny-pnor.in
@@ -1,4 +1,4 @@
-ONE,0x00000300,0x00000100,EV,/dev/zero
-TWO,0x00000400,0x00000100,EF,/dev/zero
-THREE,0x00000500,0x00000100,EF,/dev/zero
-FOUR,0x00000600,0x00000100,EF,/dev/zero
+ONE,0x00400,0x00000100,EV,/dev/zero
+TWO,0x00500,0x00000100,EF,/dev/zero
+THREE,0x600,0x00000100,EF,/dev/zero
+FOUR,0x0700,0x00000100,EF,/dev/zero
diff --git a/external/ffspart/test/files/03-tiny-pnor.out b/external/ffspart/test/files/03-tiny-pnor.out
index 32e998d677a7abd4b7249d7957824d430e33dc64..e00fa5c0ed65090888945e913e264ae73c7d64cc 100644
GIT binary patch
literal 2560
zcmWG=3<_ajU|<AdW*}|=Vpa&3feXk+0RfJ|HwzMrN>BtL9OnN(0A#|<0n#Ajs0;e%
z^>zFExuP4&0u*3`+Rp at HLli(M6x9u2526T>z%kfp{WB!oA6*A9yx{hOJdM|W1_5;K
z#Pe70F#H+f5#)*%0&Gb3gTn%1B6j~%V*k74Iy>C_Lxa#<%#LI~I4o#o|IP*4>p}56
X3PwX<Gz3ONU^E0qLtr!nC<_4q|HpMy
literal 2560
zcmWG=3<_ajU|@ve1|ZD};WBUm*(e~uG5B^tVo?c-AcVvC9|(X<kU7i{8pNb7=%3fq
z>F?)?ZYbD3sQpYpNvOrx>~CObK-WzI-(aKlzmRZ$bR8@}0hs at xeg`SRZa;$nx_09E
zt9Kaw2=NGVMGFC7e8KGpy9Ht*_3eMRTxYkNe`pYzi`kIu2fKw<_U~Mvy$%%5qhK at y
UMnhmU1V%$(Gz3ONfU*z(0L5)}P5=M^
diff --git a/external/ffspart/test/files/03.1-tiny-pnor-backup.in b/external/ffspart/test/files/03.1-tiny-pnor-backup.in
index 517dc47b..b5527503 100644
--- a/external/ffspart/test/files/03.1-tiny-pnor-backup.in
+++ b/external/ffspart/test/files/03.1-tiny-pnor-backup.in
@@ -1,4 +1,4 @@
-ONE,0x00000300,0x00000100,EV,/dev/zero
-TWO,0x00000400,0x00000100,EF,/dev/zero
-THREE,0x00000500,0x00000100,EF,/dev/zero
-FOUR,0x00000600,0x00000100,EF,/dev/zero
+ONE,0x00400,0x100,EV,/dev/zero
+TWO,0x00500,0x100,EF,/dev/zero
+THREE,0x600,0x100,EF,/dev/zero
+FOUR,0x0700,0x100,EF,/dev/zero
diff --git a/external/ffspart/test/files/03.1-tiny-pnor-backup.out b/external/ffspart/test/files/03.1-tiny-pnor-backup.out
index 85c23e32deb6405f51643baa523621d554b377cf..e173e9e75451d5359d97aca9e16e411bd255f0eb 100644
GIT binary patch
literal 3840
zcmWG=3<_ajU|<Ad79ef_Vm1hufgi|50RfJ|HwqGqN>BtL9G3q;0Azy9VTRBk<ERVz
z=XLh@`?;bU3bKe1YCjW*4N(B4P*gX7J%}Pi0>@yZ_0N!Se{>zJKmnNjP``teV7H$^
z09`xr{M9=Qe};GjxuS&tFuvgSgTn%1BK7Tmw_In3n}28!nv2<y><5Pht?b{qKzqHD
zqqBEtKzsls*@A8229mJ!2M!B}{nSr?EB9=E14?J3U^E0qLtr!nMnhmU1V%$(Xomo_
gApvSEff@`AKs?(11qK2(FxvhdZT}AK+>Ysb0KP^AFaQ7m
literal 2864
zcmWG=3<_ajU|@ve1|ZD_;WBUm*(e~uG5AJ7Vo?c-AcVvC9|(X<kU7i{8pNb7=%3fq
z>F?)?ZYbD3sQpYpNvOrx>~CObK-WzI-(aKlzmRZ$bR8@}0hs at xeg`SRZa;$nx_09E
zt9Kaw2=NGVMGFB|kY!N+gWUo#k^1((TduR)%|A2<&Beg<0=FOR7FyZAbAk3cCr4-R
z(17>=NU{an#DQc#*e$s2hscmiuH3Wj6)2oX!DtAKhQMeD42KYa7Nnrc0)!iYc(ncp
N1_CxPTK|vM{{XxyIGg|g
diff --git a/external/ffspart/test/files/04-tiny-pnor2.out b/external/ffspart/test/files/04-tiny-pnor2.out
index dad94b8c6f15b8fea8d779a09ed4edc8a4f1b4f1..394edf0411267503a24caf3bb25d4b4b0a0327e6 100644
GIT binary patch
delta 61
zcmZn=X%OKFa107zU|?VbV&;iF%8WNB8i<Q9{|5q?D1>I7nCLyRfQ_YpUSIdd4FQ}0
DrJN5%
delta 61
zcmZn=X%OKFa107zU|?Vb;fXxTjJGEmh>I}(2Lh-lGlXWGnCLyRfQ_YpUQg%74FQ}0
Dr4|o8
diff --git a/external/ffspart/test/results/05-hdr-overlap.err b/external/ffspart/test/results/05-hdr-overlap.err
index c0ab238a..55f07c6c 100644
--- a/external/ffspart/test/results/05-hdr-overlap.err
+++ b/external/ffspart/test/results/05-hdr-overlap.err
@@ -1,2 +1,2 @@
-Adding partition 'FOUR' would cause partition 'ONE' at 0x00000200 to overlap with the header
-Couldn't add entry 'FOUR' 0x00000500 for 0x00000100
+Adding partition 'THREE' would cause partition 'ONE' at 0x00000200 to overlap with the header
+Couldn't add entry 'THREE' 0x00000400 for 0x00000100
diff --git a/external/ffspart/test/results/05-hdr-overlap.out b/external/ffspart/test/results/05-hdr-overlap.out
index 96136976..dbcdcb1d 100644
--- a/external/ffspart/test/results/05-hdr-overlap.out
+++ b/external/ffspart/test/results/05-hdr-overlap.out
@@ -1,5 +1,4 @@
Adding 'ONE' 0x00000200, 0x00000100
Adding 'TWO' 0x00000300, 0x00000100
Adding 'THREE' 0x00000400, 0x00000100
-Adding 'FOUR' 0x00000500, 0x00000100
Freeing hdr
diff --git a/external/ffspart/test/results/05.1-hdr-overlap-backup.err b/external/ffspart/test/results/05.1-hdr-overlap-backup.err
index 2adbf79a..55f07c6c 100644
--- a/external/ffspart/test/results/05.1-hdr-overlap-backup.err
+++ b/external/ffspart/test/results/05.1-hdr-overlap-backup.err
@@ -1,2 +1,2 @@
-Adding partition 'BACKUP_PART' would cause partition 'ONE' at 0x00000200 to overlap with the header
-Failed to create backup part
+Adding partition 'THREE' would cause partition 'ONE' at 0x00000200 to overlap with the header
+Couldn't add entry 'THREE' 0x00000400 for 0x00000100
diff --git a/external/ffspart/test/tests/03.1-tiny-pnor-backup b/external/ffspart/test/tests/03.1-tiny-pnor-backup
index a622ca67..3065c864 100644
--- a/external/ffspart/test/tests/03.1-tiny-pnor-backup
+++ b/external/ffspart/test/tests/03.1-tiny-pnor-backup
@@ -2,7 +2,7 @@
touch $DATA_DIR/$CUR_TEST.gen
-run_binary "./ffspart" "-b -s 0x100 -c 10 -i $DATA_DIR/$CUR_TEST.in -p $DATA_DIR/$CUR_TEST.gen"
+run_binary "./ffspart" "-b -s 0x100 -c 15 -i $DATA_DIR/$CUR_TEST.in -p $DATA_DIR/$CUR_TEST.gen"
if [ "$?" -ne 0 ] ; then
fail_test
fi
diff --git a/libflash/ffs.h b/libflash/ffs.h
index 6ba77136..18722538 100644
--- a/libflash/ffs.h
+++ b/libflash/ffs.h
@@ -213,6 +213,7 @@ struct ffs_hdr {
uint32_t size;
uint32_t block_size;
uint32_t block_count;
+ struct ffs_entry *part;
struct ffs_entry *backup;
struct ffs_hdr *side;
struct list_head entries;
diff --git a/libflash/libffs.c b/libflash/libffs.c
index 4f9509b6..5a5d432d 100644
--- a/libflash/libffs.c
+++ b/libflash/libffs.c
@@ -490,7 +490,7 @@ static int __ffs_entry_add(struct ffs_hdr *hdr, struct ffs_entry *entry)
count++;
}
- if (count * sizeof(struct __ffs_entry) +
+ if ((count + 1) * sizeof(struct __ffs_entry) +
sizeof(struct __ffs_hdr) > smallest_base) {
fprintf(stderr, "Adding partition '%s' would cause partition '%s' at "
"0x%08x to overlap with the header\n", entry->name, smallest_name,
@@ -569,15 +569,19 @@ int ffs_hdr_create_backup(struct ffs_hdr *hdr)
{
struct ffs_entry *ent;
struct ffs_entry *backup;
+ uint32_t hdr_size, flash_end;
int rc = 0;
+
ent = list_tail(&hdr->entries, struct ffs_entry, list);
if (!ent) {
return FLASH_ERR_PARM_ERROR;
}
- rc = ffs_entry_new("BACKUP_PART",
- hdr->base + (hdr->block_size * (hdr->block_count - 1 )) - hdr->size,
- hdr->size, &backup);
+ /* Why can't I think of a better way*/
+ hdr_size = ffs_hdr_raw_size(ffs_num_entries(hdr) + 1) + hdr->block_size;
+ hdr_size = (hdr_size / hdr->block_size) * hdr->block_size;
+ flash_end = hdr->base + (hdr->block_size * hdr->block_count);
+ rc = ffs_entry_new("BACKUP_PART", flash_end - hdr_size, hdr_size, &backup);
if (rc)
return rc;
@@ -604,7 +608,7 @@ int ffs_hdr_add_side(struct ffs_hdr *hdr)
if (hdr->side)
return FLASH_ERR_PARM_ERROR;
- rc = ffs_hdr_new(hdr->size, hdr->block_size, hdr->block_count, &hdr->side);
+ rc = ffs_hdr_new(hdr->block_size, hdr->block_count, &hdr->side);
if (rc)
return rc;
@@ -649,9 +653,15 @@ int ffs_hdr_finalise(struct blocklevel_device *bl, struct ffs_hdr *hdr)
*/
memset(real_hdr, 0, sizeof(*real_hdr));
+ hdr->part->size = ffs_hdr_raw_size(num_entries) + hdr->block_size;
+ /*
+ * So actual is in bytes. ffs_entry_to_flash() won't do the
+ * block_size division that we're relying on
+ */
+ hdr->part->actual = (hdr->part->size / hdr->block_size) * hdr->block_size;
real_hdr->magic = cpu_to_be32(FFS_MAGIC);
real_hdr->version = cpu_to_be32(hdr->version);
- real_hdr->size = cpu_to_be32(hdr->size / hdr->block_size);
+ real_hdr->size = cpu_to_be32(hdr->part->size / hdr->block_size);
real_hdr->entry_size = cpu_to_be32(sizeof(struct __ffs_entry));
real_hdr->entry_count = cpu_to_be32(num_entries);
real_hdr->block_size = cpu_to_be32(hdr->block_size);
@@ -740,30 +750,28 @@ int ffs_entry_new(const char *name, uint32_t base, uint32_t size, struct ffs_ent
return 0;
}
-int ffs_hdr_new(uint32_t size, uint32_t block_size, uint32_t block_count, struct ffs_hdr **r)
+int ffs_hdr_new(uint32_t block_size, uint32_t block_count, struct ffs_hdr **r)
{
struct ffs_hdr *ret;
struct ffs_entry *part_table;
int rc;
- if (size % block_size || size > block_size * block_count)
- return FFS_ERR_BAD_SIZE;
-
ret = calloc(1, sizeof(*ret));
if (!ret)
return FLASH_ERR_MALLOC_FAILED;
ret->version = FFS_VERSION_1;
- ret->size = size;
ret->block_size = block_size;
ret->block_count = block_count;
list_head_init(&ret->entries);
- rc = ffs_entry_new("part", 0, size, &part_table);
+ /* Don't know how big it will be, ffs_hdr_finalise() will fix */
+ rc = ffs_entry_new("part", 0, 0, &part_table);
if (rc) {
free(ret);
return rc;
}
+ ret->part = part_table;
part_table->pid = FFS_PID_TOPLEVEL;
part_table->type = FFS_TYPE_PARTITION;
diff --git a/libflash/libffs.h b/libflash/libffs.h
index a86c698f..a0f65a05 100644
--- a/libflash/libffs.h
+++ b/libflash/libffs.h
@@ -133,7 +133,7 @@ struct ffs_entry *ffs_entry_get(struct ffs_handle *ffs, uint32_t index);
int ffs_update_act_size(struct ffs_handle *ffs, uint32_t part_idx,
uint32_t act_size);
-int ffs_hdr_new(uint32_t size, uint32_t block_size, uint32_t block_count,
+int ffs_hdr_new(uint32_t block_size, uint32_t block_count,
struct ffs_hdr **r);
int ffs_hdr_add_side(struct ffs_hdr *hdr);
--
2.13.3
More information about the Skiboot
mailing list