mail.coreboot.org
Sign In
Sign Up
Sign In
Sign Up
Manage this list
×
Keyboard Shortcuts
Thread View
j
: Next unread message
k
: Previous unread message
j a
: Jump to all threads
j l
: Jump to MailingList overview
2024
May
April
March
February
January
2023
December
November
October
September
August
July
June
May
April
March
February
January
2022
December
November
October
September
August
July
June
May
April
March
February
January
2021
December
November
October
September
August
July
June
May
April
March
February
January
2020
December
November
October
September
August
July
June
May
April
March
February
January
2019
December
November
October
September
August
July
June
May
April
March
February
January
2018
December
November
October
September
August
July
June
May
April
March
February
January
2017
December
November
October
September
August
July
June
May
April
March
List overview
Download
flashrom-gerrit
June 2021
----- 2024 -----
May 2024
April 2024
March 2024
February 2024
January 2024
----- 2023 -----
December 2023
November 2023
October 2023
September 2023
August 2023
July 2023
June 2023
May 2023
April 2023
March 2023
February 2023
January 2023
----- 2022 -----
December 2022
November 2022
October 2022
September 2022
August 2022
July 2022
June 2022
May 2022
April 2022
March 2022
February 2022
January 2022
----- 2021 -----
December 2021
November 2021
October 2021
September 2021
August 2021
July 2021
June 2021
May 2021
April 2021
March 2021
February 2021
January 2021
----- 2020 -----
December 2020
November 2020
October 2020
September 2020
August 2020
July 2020
June 2020
May 2020
April 2020
March 2020
February 2020
January 2020
----- 2019 -----
December 2019
November 2019
October 2019
September 2019
August 2019
July 2019
June 2019
May 2019
April 2019
March 2019
February 2019
January 2019
----- 2018 -----
December 2018
November 2018
October 2018
September 2018
August 2018
July 2018
June 2018
May 2018
April 2018
March 2018
February 2018
January 2018
----- 2017 -----
December 2017
November 2017
October 2017
September 2017
August 2017
July 2017
June 2017
May 2017
April 2017
March 2017
flashrom-gerrit@flashrom.org
1 participants
874 discussions
Start a n
N
ew thread
Change in flashrom[master]: layout: Rework normalize_romentries() API
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/54285
) Change subject: layout: Rework normalize_romentries() API ...................................................................... layout: Rework normalize_romentries() API Rename it to layout_sanity_checks() as that is what it does and let it work on the currently active layout instead of the global layout. Change-Id: Ifae3480d4bd68c939c291f05734544e93f00306c Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/54285
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flash.h M flashrom.c M layout.c M layout.h 4 files changed, 5 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/flash.h b/flash.h index 3912e73..c8c0f9d 100644 --- a/flash.h +++ b/flash.h @@ -420,7 +420,6 @@ /* layout.c */ int register_include_arg(struct layout_include_args **args, const char *arg); int read_romlayout(const char *name); -int normalize_romentries(const struct flashctx *flash); void layout_cleanup(struct layout_include_args **args); /* spi.c */ diff --git a/flashrom.c b/flashrom.c index b20e3e1..6cc899f 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1865,7 +1865,7 @@ return 1; } - if (flash->layout == get_global_layout() && normalize_romentries(flash)) { + if (layout_sanity_checks(flash)) { msg_cerr("Requested regions can not be handled. Aborting.\n"); return 1; } diff --git a/layout.c b/layout.c index 9df0035..e4f1681 100644 --- a/layout.c +++ b/layout.c @@ -298,11 +298,10 @@ flashrom_layout_release(layout); } -/* Validate and - if needed - normalize layout entries. */ -int normalize_romentries(const struct flashctx *flash) +int layout_sanity_checks(const struct flashrom_flashctx *const flash) { - struct flashrom_layout *const layout = get_global_layout(); - chipsize_t total_size = flash->chip->total_size * 1024; + const struct flashrom_layout *const layout = get_layout(flash); + const chipsize_t total_size = flash->chip->total_size * 1024; int ret = 0; const struct romentry *entry = NULL; diff --git a/layout.h b/layout.h index 808c516..f5fbc50 100644 --- a/layout.h +++ b/layout.h @@ -62,5 +62,6 @@ const struct romentry *layout_next(const struct flashrom_layout *, const struct romentry *); int included_regions_overlap(const struct flashrom_layout *const flashrom_layout); void prepare_layout_for_extraction(struct flashrom_flashctx *flash); +int layout_sanity_checks(const struct flashrom_flashctx *); #endif /* !__LAYOUT_H__ */ -- To view, visit
https://review.coreboot.org/c/flashrom/+/54285
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifae3480d4bd68c939c291f05734544e93f00306c Gerrit-Change-Number: 54285 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
Change in flashrom[master]: libflashrom: Avoid using the global layout
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/54284
) Change subject: libflashrom: Avoid using the global layout ...................................................................... libflashrom: Avoid using the global layout We used to borrow the global layout from the CLI here. Create a dynamically allocated one instead that doesn't need special treatment. Change-Id: Ic48c9e73a3d00782f638f6ff41b620910b24ab6f Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/54284
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M libflashrom.c 1 file changed, 5 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/libflashrom.c b/libflashrom.c index d0f94a9..1ecf650 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -502,15 +502,17 @@ int i; char name[FMAP_STRLEN + 1]; const struct fmap_area *area; - struct flashrom_layout *l = get_global_layout(); + struct flashrom_layout *l; - if (!fmap || !l) + if (!fmap || flashrom_layout_new(&l)) return 1; for (i = 0, area = fmap->areas; i < fmap->nareas; i++, area++) { snprintf(name, sizeof(name), "%s", area->name); - if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name)) + if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name)) { + flashrom_layout_release(l); return 1; + } } *layout = l; -- To view, visit
https://review.coreboot.org/c/flashrom/+/54284
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic48c9e73a3d00782f638f6ff41b620910b24ab6f Gerrit-Change-Number: 54284 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
Change in flashrom[master]: libflashrom: Free `chip` instance upon flashctx teardown
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/33546
) Change subject: libflashrom: Free `chip` instance upon flashctx teardown ...................................................................... libflashrom: Free `chip` instance upon flashctx teardown Change-Id: I761d7e167a43e5bf08b5b3d269b0a476e3d343c5 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/33546
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M libflashrom.c 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/libflashrom.c b/libflashrom.c index cb0d470..d0f94a9 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -331,6 +331,7 @@ /* We found one chip, now check that there is no second match. */ if (probe_flash(®istered_masters[i], flash_idx + 1, &second_flashctx, 0) != -1) { flashrom_layout_release(second_flashctx.default_layout); + free(second_flashctx.chip); ret = 3; break; } @@ -362,6 +363,7 @@ void flashrom_flash_release(struct flashrom_flashctx *const flashctx) { flashrom_layout_release(flashctx->default_layout); + free(flashctx->chip); free(flashctx); } -- To view, visit
https://review.coreboot.org/c/flashrom/+/33546
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I761d7e167a43e5bf08b5b3d269b0a476e3d343c5 Gerrit-Change-Number: 33546 Gerrit-PatchSet: 10 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
Change in flashrom[master]: layout: Make `struct layout_include_args` private to `layout.c`
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/33545
) Change subject: layout: Make `struct layout_include_args` private to `layout.c` ...................................................................... layout: Make `struct layout_include_args` private to `layout.c` Change-Id: Icbfee68e85429fe41db1cad6b99f25e9f30cd672 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/33545
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M layout.c M layout.h 2 files changed, 7 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/layout.c b/layout.c index e71f6a4..9df0035 100644 --- a/layout.c +++ b/layout.c @@ -29,6 +29,12 @@ struct romentry *head; }; +struct layout_include_args { + char *name; + char *file; + struct layout_include_args *next; +}; + static struct flashrom_layout *global_layout; struct flashrom_layout *get_global_layout(void) diff --git a/layout.h b/layout.h index bf183a6..808c516 100644 --- a/layout.h +++ b/layout.h @@ -47,11 +47,7 @@ struct flashrom_layout; -struct layout_include_args { - char *name; - char *file; - struct layout_include_args *next; -}; +struct layout_include_args; struct flashrom_flashctx; struct flashrom_layout *get_global_layout(void); -- To view, visit
https://review.coreboot.org/c/flashrom/+/33545
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icbfee68e85429fe41db1cad6b99f25e9f30cd672 Gerrit-Change-Number: 33545 Gerrit-PatchSet: 10 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
Change in ...flashrom[master]: layout: Drop `count` parameter of flashrom_layout_empty()
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Hello Angel Pons, Arthur Heymans, David Hendricks, Thomas Heijligen, I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/33544
to review the following change. Change subject: layout: Drop `count` parameter of flashrom_layout_empty() ...................................................................... layout: Drop `count` parameter of flashrom_layout_empty() Change-Id: I22c180c9971068b1ae101845ce88484c6842b852 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flashrom.c M ich_descriptors.c M layout.c M libflashrom.h 4 files changed, 5 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/33544/1 diff --git a/flashrom.c b/flashrom.c index cb91cef..eeaa067 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1332,7 +1332,7 @@ return -1; /* Fill default layout covering the whole chip. */ - if (flashrom_layout_empty(&flash->default_layout, 1) || + if (flashrom_layout_empty(&flash->default_layout) || flashrom_layout_add_region(flash->default_layout, 0, flash->chip->total_size * 1024 - 1, "complete flash") || flashrom_layout_include_region(flash->default_layout, "complete flash")) diff --git a/ich_descriptors.c b/ich_descriptors.c index 8383801..1d35cca 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -1170,7 +1170,7 @@ if (read_ich_descriptors_from_dump(dump, len, &cs, &desc)) return 1; - if (flashrom_layout_empty(layout, ARRAY_SIZE(regions))) + if (flashrom_layout_empty(layout)) return 2; ssize_t i; diff --git a/layout.c b/layout.c index 3560a18..9d633bb 100644 --- a/layout.c +++ b/layout.c @@ -33,7 +33,7 @@ struct flashrom_layout *get_global_layout(void) { if (!global_layout) - flashrom_layout_empty(&global_layout, 0); + flashrom_layout_empty(&global_layout); return global_layout; } @@ -264,12 +264,11 @@ * @brief Create a new, empty layout. * * @param layout Pointer to returned layout reference. - * @param count Number of layout entries to allocate. * * @return 0 on success, * 1 if out of memory. */ -int flashrom_layout_empty(struct flashrom_layout **const layout, const unsigned int count) +int flashrom_layout_empty(struct flashrom_layout **layout) { *layout = malloc(sizeof(**layout)); if (!*layout) { diff --git a/libflashrom.h b/libflashrom.h index 90f3e2c..785217e 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -62,7 +62,7 @@ int flashrom_image_verify(struct flashrom_flashctx *, const void *buffer, size_t buffer_len); struct flashrom_layout; -int flashrom_layout_empty(struct flashrom_layout **layout, unsigned int count); +int flashrom_layout_empty(struct flashrom_layout **); int flashrom_layout_read_from_ifd(struct flashrom_layout **, struct flashrom_flashctx *, const void *dump, size_t len); int flashrom_layout_read_fmap_from_rom(struct flashrom_layout **, struct flashrom_flashctx *, off_t offset, size_t length); -- To view, visit
https://review.coreboot.org/c/flashrom/+/33544
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I22c180c9971068b1ae101845ce88484c6842b852 Gerrit-Change-Number: 33544 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange
5
10
0
0
Change in flashrom[master]: layout: Use linked list for `struct romentry`
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/33521
) Change subject: layout: Use linked list for `struct romentry` ...................................................................... layout: Use linked list for `struct romentry` This gets rid of the entry limit and hopefully makes future layout handling easier. We start by making `struct flashrom_layout` private to `layout.c`. Change-Id: I60a0aa1007ebcd5eb401db116f835d129b3e9732 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/33521
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M layout.c M layout.h M libflashrom.c 3 files changed, 51 insertions(+), 71 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/layout.c b/layout.c index 11df15c..8047a93 100644 --- a/layout.c +++ b/layout.c @@ -25,12 +25,17 @@ #include "programmer.h" #include "layout.h" -static struct romentry entries[MAX_ROMLAYOUT]; -static struct flashrom_layout global_layout = { entries, MAX_ROMLAYOUT, 0 }; +struct flashrom_layout { + struct romentry *head; +}; + +static struct flashrom_layout *global_layout; struct flashrom_layout *get_global_layout(void) { - return &global_layout; + if (!global_layout) + flashrom_layout_new(&global_layout, 0); + return global_layout; } const struct flashrom_layout *get_default_layout(const struct flashrom_flashctx *const flashctx) @@ -40,7 +45,7 @@ const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx) { - if (flashctx->layout && flashctx->layout->num_entries) + if (flashctx->layout) return flashctx->layout; else return get_default_layout(flashctx); @@ -49,16 +54,7 @@ static struct romentry *mutable_layout_next( const struct flashrom_layout *const layout, struct romentry *iterator) { - const struct romentry *const end = layout->entries + layout->num_entries; - - if (iterator) - ++iterator; - else - iterator = &layout->entries[0]; - - if (iterator < end) - return iterator; - return NULL; + return iterator ? iterator->next : layout->head; } static struct romentry *_layout_entry_by_name( @@ -91,11 +87,6 @@ while (!feof(romlayout)) { char *tstr1, *tstr2; - if (layout->num_entries >= layout->capacity) { - msg_gerr("Maximum number of ROM images (%zu) in layout " - "file reached.\n", layout->capacity); - goto _close_ret; - } if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, tempname)) continue; #if 0 @@ -186,7 +177,7 @@ /* returns -1 if an entry is not found, 0 if found. */ static int find_romentry(struct flashrom_layout *const l, char *name, char *file) { - if (l->num_entries == 0) + if (!l->head) return -1; msg_gspew("Looking for region \"%s\"... ", name); @@ -222,7 +213,7 @@ return 0; /* User has specified an include argument, but no layout is loaded. */ - if (l->num_entries == 0) { + if (!l->head) { msg_gerr("Region requested (with -i \"%s\"), " "but no layout data is available.\n", args->name); @@ -287,7 +278,6 @@ void layout_cleanup(struct layout_include_args **args) { struct flashrom_layout *const layout = get_global_layout(); - unsigned int i; struct layout_include_args *tmp; while (*args) { @@ -298,12 +288,8 @@ *args = tmp; } - for (i = 0; i < layout->num_entries; i++) { - free(layout->entries[i].name); - free(layout->entries[i].file); - layout->entries[i].included = false; - } - layout->num_entries = 0; + global_layout = NULL; + flashrom_layout_release(layout); } /* Validate and - if needed - normalize layout entries. */ @@ -380,7 +366,7 @@ const struct romentry *layout_next( const struct flashrom_layout *const layout, const struct romentry *iterator) { - return mutable_layout_next(layout, (struct romentry *)iterator); + return iterator ? iterator->next : layout->head; } /** @@ -399,17 +385,13 @@ */ int flashrom_layout_new(struct flashrom_layout **const layout, const unsigned int count) { - *layout = malloc(sizeof(**layout) + count * sizeof(struct romentry)); + *layout = malloc(sizeof(**layout)); if (!*layout) { msg_gerr("Error creating layout: %s\n", strerror(errno)); return 1; } - const struct flashrom_layout tmp = { - .entries = (void *)((char *)*layout + sizeof(**layout)), - .capacity = count, - .num_entries = 0, - }; + const struct flashrom_layout tmp = { 0 }; **layout = tmp; return 0; } @@ -423,32 +405,36 @@ * @param name Name of the region. * * @return 0 on success, - * 1 if out of memory, - * 2 if the layout is full already. + * 1 if out of memory. */ int flashrom_layout_add_region( struct flashrom_layout *const layout, const size_t start, const size_t end, const char *const name) { - if (layout->num_entries >= layout->capacity) { - msg_gerr("Error adding layout entry: No space left\n"); - return 2; - } + struct romentry *const entry = malloc(sizeof(*entry)); + if (!entry) + goto _err_ret; - struct romentry *const entry = &layout->entries[layout->num_entries]; - entry->start = start; - entry->end = end; - entry->included = false; - entry->name = strdup(name); - entry->file = NULL; - if (!entry->name) { - msg_gerr("Error adding layout entry: %s\n", strerror(errno)); - return 1; - } + const struct romentry tmp = { + .next = layout->head, + .start = start, + .end = end, + .included = false, + .name = strdup(name), + .file = NULL, + }; + *entry = tmp; + if (!entry->name) + goto _err_ret; msg_gdbg("Added layout entry %08zx - %08zx named %s\n", start, end, name); - ++layout->num_entries; + layout->head = entry; return 0; + +_err_ret: + msg_gerr("Error adding layout entry: %s\n", strerror(errno)); + free(entry); + return 1; } /** @@ -472,14 +458,18 @@ */ void flashrom_layout_release(struct flashrom_layout *const layout) { - unsigned int i; - - if (!layout || layout == get_global_layout()) + if (layout == global_layout) return; - for (i = 0; i < layout->num_entries; ++i) { - free(layout->entries[i].name); - free(layout->entries[i].file); + if (!layout) + return; + + while (layout->head) { + struct romentry *const entry = layout->head; + layout->head = entry->next; + free(entry->file); + free(entry->name); + free(entry); } free(layout); } diff --git a/layout.h b/layout.h index 49c1a13..bf183a6 100644 --- a/layout.h +++ b/layout.h @@ -36,6 +36,8 @@ #define MAX_ROMLAYOUT 128 struct romentry { + struct romentry *next; + chipoff_t start; chipoff_t end; bool included; @@ -43,14 +45,7 @@ char *file; }; -struct flashrom_layout { - /* entries store the entries specified in a layout file and associated run-time data */ - struct romentry *entries; - /* the maximum number of entries */ - size_t capacity; - /* the number of successfully parsed entries */ - size_t num_entries; -}; +struct flashrom_layout; struct layout_include_args { char *name; diff --git a/libflashrom.c b/libflashrom.c index dd5cb00..cb0d470 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -505,11 +505,6 @@ if (!fmap || !l) return 1; - if (l->num_entries + fmap->nareas > l->capacity) { - msg_gerr("Cannot add fmap entries to layout - Too many entries.\n"); - return 1; - } - for (i = 0, area = fmap->areas; i < fmap->nareas; i++, area++) { snprintf(name, sizeof(name), "%s", area->name); if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name)) -- To view, visit
https://review.coreboot.org/c/flashrom/+/33521
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I60a0aa1007ebcd5eb401db116f835d129b3e9732 Gerrit-Change-Number: 33521 Gerrit-PatchSet: 11 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
Change in flashrom[master]: Pass layout directly to verify_by_layout()
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/33520
) Change subject: Pass layout directly to verify_by_layout() ...................................................................... Pass layout directly to verify_by_layout() It used the current layout from the flash context, before. This made it necessary to replace the pointer on-the-fly. Passing the layout directly, works without that stunt. Change-Id: Id496deec85c18bdfe968df6a798b626eb9cfbed5 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/33520
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashrom.c 1 file changed, 11 insertions(+), 11 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 7c91fa1..34cbdac 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1513,16 +1513,18 @@ * contents will be compared. * * @param flashctx Flash context to be used. + * @param layout Flash layout information. * @param curcontents A buffer of full chip size to read current chip contents into. * @param newcontents The new image to compare to. * @return 0 on success, * 1 if reading failed, * 3 if the contents don't match. */ -static int verify_by_layout(struct flashctx *const flashctx, - void *const curcontents, const uint8_t *const newcontents) +static int verify_by_layout( + struct flashctx *const flashctx, + const struct flashrom_layout *const layout, + void *const curcontents, const uint8_t *const newcontents) { - const struct flashrom_layout *const layout = get_layout(flashctx); const struct romentry *entry = NULL; while ((entry = layout_next_included(layout, entry))) { @@ -2029,6 +2031,8 @@ const size_t flash_size = flashctx->chip->total_size * 1024; const bool verify_all = flashctx->flags.verify_whole_chip; const bool verify = flashctx->flags.verify_after_write; + const struct flashrom_layout *const verify_layout = + verify_all ? get_default_layout(flashctx) : get_layout(flashctx); if (buffer_len != flash_size) return 4; @@ -2117,19 +2121,14 @@ /* Verify only if we actually changed something. */ if (verify && !all_skipped) { - const struct flashrom_layout *const layout_bak = flashctx->layout; - msg_cinfo("Verifying flash... "); /* Work around chips which need some time to calm down. */ programmer_delay(1000*1000); - if (verify_all) { + if (verify_all) combine_image_by_layout(flashctx, newcontents, oldcontents); - flashctx->layout = NULL; - } - ret = verify_by_layout(flashctx, curcontents, newcontents); - flashctx->layout = layout_bak; + ret = verify_by_layout(flashctx, verify_layout, curcontents, newcontents); /* If we tried to write, and verification now fails, we might have an emergency situation. */ if (ret) @@ -2165,6 +2164,7 @@ */ int flashrom_image_verify(struct flashctx *const flashctx, const void *const buffer, const size_t buffer_len) { + const struct flashrom_layout *const layout = get_layout(flashctx); const size_t flash_size = flashctx->chip->total_size * 1024; if (buffer_len != flash_size) @@ -2183,7 +2183,7 @@ goto _free_ret; msg_cinfo("Verifying flash... "); - ret = verify_by_layout(flashctx, curcontents, newcontents); + ret = verify_by_layout(flashctx, layout, curcontents, newcontents); if (!ret) msg_cinfo("VERIFIED.\n"); -- To view, visit
https://review.coreboot.org/c/flashrom/+/33520
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id496deec85c18bdfe968df6a798b626eb9cfbed5 Gerrit-Change-Number: 33520 Gerrit-PatchSet: 11 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
Change in flashrom[master]: layout: Introduce get_default_layout()
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/33519
) Change subject: layout: Introduce get_default_layout() ...................................................................... layout: Introduce get_default_layout() Containing an included, full-flash-chip sized default region. This allows us to query the default layout specifically, also if an additional layout is attached to the flash context. Change-Id: Ia343e9775ec5bdc3fea5cdb6b347298515996e34 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/33519
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M layout.c M layout.h 2 files changed, 8 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/layout.c b/layout.c index ede476a..11df15c 100644 --- a/layout.c +++ b/layout.c @@ -33,12 +33,17 @@ return &global_layout; } +const struct flashrom_layout *get_default_layout(const struct flashrom_flashctx *const flashctx) +{ + return flashctx->default_layout; +} + const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx) { if (flashctx->layout && flashctx->layout->num_entries) return flashctx->layout; else - return flashctx->default_layout; + return get_default_layout(flashctx); } static struct romentry *mutable_layout_next( diff --git a/layout.h b/layout.h index c05d1e7..49c1a13 100644 --- a/layout.h +++ b/layout.h @@ -58,8 +58,9 @@ struct layout_include_args *next; }; -struct flashrom_layout *get_global_layout(void); struct flashrom_flashctx; +struct flashrom_layout *get_global_layout(void); +const struct flashrom_layout *get_default_layout(const struct flashrom_flashctx *); const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx); int get_region_range(struct flashrom_layout *const l, const char *name, -- To view, visit
https://review.coreboot.org/c/flashrom/+/33519
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia343e9775ec5bdc3fea5cdb6b347298515996e34 Gerrit-Change-Number: 33519 Gerrit-PatchSet: 10 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
Change in ...flashrom[master]: layout: Introduce flashrom_layout_empty()
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Hello Angel Pons, Arthur Heymans, David Hendricks, Thomas Heijligen, I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/33543
to review the following change. Change subject: layout: Introduce flashrom_layout_empty() ...................................................................... layout: Introduce flashrom_layout_empty() It initializes an empty layout. Currently the maximum number of entries has to be specified, which will vanisch once we use dynamic allocation. Change-Id: I2ae7246493ff592e631cce924777925c7825e398 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M cli_classic.c M flash.h M flashrom.c M ich_descriptors.c M ich_descriptors.h M layout.c M libflashrom.c M libflashrom.h 8 files changed, 54 insertions(+), 37 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/33543/1 diff --git a/cli_classic.c b/cli_classic.c index 2e07612..a98be47 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -673,8 +673,10 @@ out_shutdown: programmer_shutdown(); out: - for (i = 0; i < chipcount; i++) + for (i = 0; i < chipcount; i++) { + flashrom_layout_release(flashes[i].default_layout); free(flashes[i].chip); + } layout_cleanup(&include_args); free(filename); diff --git a/flash.h b/flash.h index 5bbfa0a..84de7a7 100644 --- a/flash.h +++ b/flash.h @@ -252,7 +252,7 @@ chipaddr virtual_registers; struct registered_master *mst; const struct flashrom_layout *layout; - struct single_layout fallback_layout; + struct flashrom_layout *default_layout; struct { bool force; bool force_boardmismatch; diff --git a/flashrom.c b/flashrom.c index e355731..3436a59 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1331,15 +1331,12 @@ if (!flash->chip) return -1; - /* Fill fallback layout covering the whole chip. */ - struct single_layout *const fallback = &flash->fallback_layout; - fallback->base.entries = &fallback->entry; - fallback->base.max_entries = 1; - fallback->base.num_entries = 0; - if (flashrom_layout_add_region(&fallback->base, + /* Fill default layout covering the whole chip. */ + if (flashrom_layout_empty(&flash->default_layout, 1) || + flashrom_layout_add_region(flash->default_layout, 0, flash->chip->total_size * 1024 - 1, "complete flash") || - flashrom_layout_include_region(&fallback->base, "complete flash")) - return -1; + flashrom_layout_include_region(flash->default_layout, "complete flash")) + return -1; tmp = flashbuses_to_text(flash->chip->bustype); msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found", diff --git a/ich_descriptors.c b/ich_descriptors.c index b2c5d4c..8383801 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -1156,7 +1156,9 @@ * 1 if the descriptor couldn't be parsed, * 2 when out of memory. */ -int layout_from_ich_descriptors(struct ich_layout *const layout, const void *const dump, const size_t len) +int layout_from_ich_descriptors( + struct flashrom_layout **const layout, + const void *const dump, const size_t len) { static const char *const regions[] = { "fd", "bios", "me", "gbe", "pd", "reg5", "bios2", "reg7", "ec", "reg9", "ie", @@ -1168,10 +1170,8 @@ if (read_ich_descriptors_from_dump(dump, len, &cs, &desc)) return 1; - memset(layout, 0x00, sizeof(*layout)); - layout->base.entries = layout->entries; - layout->base.max_entries = ARRAY_SIZE(layout->entries); - layout->base.num_entries = 0; + if (flashrom_layout_empty(layout, ARRAY_SIZE(regions))) + return 2; ssize_t i; for (i = 0; i < min(ich_number_of_regions(cs, &desc.content), ARRAY_SIZE(regions)); ++i) { @@ -1179,8 +1179,11 @@ const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]); if (limit <= base) continue; - if (flashrom_layout_add_region(&layout->base, base, limit, regions[i])) + if (flashrom_layout_add_region(*layout, base, limit, regions[i])) { + flashrom_layout_release(*layout); + *layout = NULL; return 2; + } } return 0; } diff --git a/ich_descriptors.h b/ich_descriptors.h index 4225286..498af21 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -562,11 +562,6 @@ struct ich_desc_upper_map upper; }; -struct ich_layout { - struct flashrom_layout base; - struct romentry entries[MAX_NUM_FLREGS]; -}; - ssize_t ich_number_of_regions(enum ich_chipset cs, const struct ich_desc_content *content); ssize_t ich_number_of_masters(enum ich_chipset cs, const struct ich_desc_content *content); @@ -585,6 +580,6 @@ int read_ich_descriptors_via_fdo(enum ich_chipset cs, void *spibar, struct ich_descriptors *desc); int getFCBA_component_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx); -int layout_from_ich_descriptors(struct ich_layout *, const void *dump, size_t len); +int layout_from_ich_descriptors(struct flashrom_layout **, const void *dump, size_t len); #endif /* __ICH_DESCRIPTORS_H__ */ diff --git a/layout.c b/layout.c index 0978a67..8198f17 100644 --- a/layout.c +++ b/layout.c @@ -37,7 +37,7 @@ if (flashctx->layout && flashctx->layout->num_entries) return flashctx->layout; else - return &flashctx->fallback_layout.base; + return flashctx->default_layout; } #ifndef __LIBPAYLOAD__ @@ -270,6 +270,29 @@ */ /** + * @brief Create a new, empty layout. + * + * @param layout Pointer to returned layout reference. + * @param count Number of layout entries to allocate. + * + * @return 0 on success, + * 1 if out of memory. + */ +int flashrom_layout_empty(struct flashrom_layout **const layout, const unsigned int count) +{ + *layout = malloc(sizeof(**layout) + count * sizeof(struct romentry)); + if (!*layout) { + msg_gerr("Error creating layout: %s\n", strerror(errno)); + return 1; + } + + (*layout)->entries = (void *)((char *)*layout + sizeof(**layout)); + (*layout)->max_entries = count; + (*layout)->num_entries = 0; + return 0; +} + +/** * @brief Add another region to an existing layout. * * @param layout The existing layout. diff --git a/libflashrom.c b/libflashrom.c index aa43e65..d891944 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -205,13 +205,14 @@ ret = 0; /* We found one chip, now check that there is no second match. */ if (probe_flash(®istered_masters[i], flash_idx + 1, &second_flashctx, 0) != -1) { + flashrom_layout_release(second_flashctx.default_layout); ret = 3; break; } } } if (ret) { - free(*flashctx); + flashrom_flash_release(*flashctx); *flashctx = NULL; } return ret; @@ -235,6 +236,7 @@ */ void flashrom_flash_release(struct flashrom_flashctx *const flashctx) { + flashrom_layout_release(flashctx->default_layout); free(flashctx); } @@ -310,16 +312,10 @@ #ifndef __FLASHROM_LITTLE_ENDIAN__ return 6; #else - struct ich_layout dump_layout; + struct flashrom_layout *dump_layout, *chip_layout; int ret = 1; void *const desc = malloc(0x1000); - struct ich_layout *const chip_layout = malloc(sizeof(*chip_layout)); - if (!desc || !chip_layout) { - msg_gerr("Out of memory!\n"); - goto _free_ret; - } - if (prepare_flash_access(flashctx, true, false, false, false)) goto _free_ret; @@ -332,7 +328,7 @@ } msg_cinfo("done.\n"); - if (layout_from_ich_descriptors(chip_layout, desc, 0x1000)) { + if (layout_from_ich_descriptors(&chip_layout, desc, 0x1000)) { msg_cerr("Couldn't parse the descriptor!\n"); ret = 3; goto _finalize_ret; @@ -345,11 +341,11 @@ goto _finalize_ret; } - const struct romentry *chip_entry = layout_next(&chip_layout->base, NULL); - const struct romentry *dump_entry = layout_next(&dump_layout.base, NULL); + const struct romentry *chip_entry = layout_next(chip_layout, NULL); + const struct romentry *dump_entry = layout_next(dump_layout, NULL); while (chip_entry && dump_entry && !memcmp(chip_entry, dump_entry, sizeof(*chip_entry))) { - chip_entry = layout_next(&chip_layout->base, chip_entry); - dump_entry = layout_next(&dump_layout.base, dump_entry); + chip_entry = layout_next(chip_layout, chip_entry); + dump_entry = layout_next(dump_layout, dump_entry); } if (chip_entry || dump_entry) { msg_cerr("Descriptors don't match!\n"); @@ -365,7 +361,7 @@ finalize_flash_access(flashctx); _free_ret: if (ret) - free(chip_layout); + flashrom_layout_release(chip_layout); free(desc); return ret; #endif diff --git a/libflashrom.h b/libflashrom.h index 2b84bd4..90f3e2c 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -62,6 +62,7 @@ int flashrom_image_verify(struct flashrom_flashctx *, const void *buffer, size_t buffer_len); struct flashrom_layout; +int flashrom_layout_empty(struct flashrom_layout **layout, unsigned int count); int flashrom_layout_read_from_ifd(struct flashrom_layout **, struct flashrom_flashctx *, const void *dump, size_t len); int flashrom_layout_read_fmap_from_rom(struct flashrom_layout **, struct flashrom_flashctx *, off_t offset, size_t length); -- To view, visit
https://review.coreboot.org/c/flashrom/+/33543
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2ae7246493ff592e631cce924777925c7825e398 Gerrit-Change-Number: 33543 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange
5
26
0
0
Change in flashrom[master]: layout: Introduce flashrom_layout_add_region()
by Nico Huber (Code Review)
26 Jun '21
26 Jun '21
Nico Huber has submitted this change. (
https://review.coreboot.org/c/flashrom/+/33517
) Change subject: layout: Introduce flashrom_layout_add_region() ...................................................................... layout: Introduce flashrom_layout_add_region() Adds a region to an existing layout, as long as there is space. Change-Id: I50d473d0d5d1fb38bd6f9ae3d7127e9ea66a94e1 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on:
https://review.coreboot.org/c/flashrom/+/33517
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashrom.c M ich_descriptors.c M layout.c M libflashrom.c M libflashrom.h 5 files changed, 55 insertions(+), 50 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 8d260cb..ffba5c4 100644 --- a/flashrom.c +++ b/flashrom.c @@ -826,15 +826,11 @@ struct single_layout *const fallback = &flash->fallback_layout; fallback->base.entries = &fallback->entry; fallback->base.capacity = 1; - fallback->base.num_entries = 1; - fallback->entry.start = 0; - fallback->entry.end = flash->chip->total_size * 1024 - 1; - fallback->entry.included = true; - fallback->entry.name = strdup("complete flash"); - if (!fallback->entry.name) { - msg_cerr("Failed to probe chip: %s\n", strerror(errno)); + fallback->base.num_entries = 0; + if (flashrom_layout_add_region(&fallback->base, + 0, flash->chip->total_size * 1024 - 1, "complete flash") || + flashrom_layout_include_region(&fallback->base, "complete flash")) return -1; - } tmp = flashbuses_to_text(flash->chip->bustype); msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found", diff --git a/ich_descriptors.c b/ich_descriptors.c index 143e392..ad1a88b 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -1278,25 +1278,20 @@ } memset(layout, 0x00, sizeof(*layout)); + layout->base.entries = layout->entries; + layout->base.capacity = ARRAY_SIZE(layout->entries); + layout->base.num_entries = 0; - ssize_t i, j; + ssize_t i; const ssize_t nr = MIN(ich_number_of_regions(cs, &desc.content), (ssize_t)ARRAY_SIZE(regions)); - for (i = 0, j = 0; i < nr; ++i) { + for (i = 0; i < nr; ++i) { const chipoff_t base = ICH_FREG_BASE(desc.region.FLREGs[i]); const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]); if (limit <= base) continue; - layout->entries[j].start = base; - layout->entries[j].end = limit; - layout->entries[j].included = false; - layout->entries[j].name = strdup(regions[i]); - if (!layout->entries[j].name) + if (flashrom_layout_add_region(&layout->base, base, limit, regions[i])) return 2; - ++j; } - layout->base.entries = layout->entries; - layout->base.capacity = ARRAY_SIZE(layout->entries); - layout->base.num_entries = j; return 0; } diff --git a/layout.c b/layout.c index 0771d42..2f56b27 100644 --- a/layout.c +++ b/layout.c @@ -73,7 +73,6 @@ struct flashrom_layout *const layout = get_global_layout(); FILE *romlayout; char tempstr[256], tempname[256]; - unsigned int i; int ret = 1; romlayout = fopen(name, "r"); @@ -106,24 +105,10 @@ msg_gerr("Error parsing layout file. Offending string: \"%s\"\n", tempstr); goto _close_ret; } - layout->entries[layout->num_entries].start = strtol(tstr1, (char **)NULL, 16); - layout->entries[layout->num_entries].end = strtol(tstr2, (char **)NULL, 16); - layout->entries[layout->num_entries].included = false; - layout->entries[layout->num_entries].file = NULL; - layout->entries[layout->num_entries].name = strdup(tempname); - if (!layout->entries[layout->num_entries].name) { - msg_gerr("Error adding layout entry: %s\n", strerror(errno)); + if (flashrom_layout_add_region(layout, + strtol(tstr1, NULL, 16), strtol(tstr2, NULL, 16), tempname)) goto _close_ret; - } - layout->num_entries++; } - - for (i = 0; i < layout->num_entries; i++) { - msg_gdbg("romlayout %08x - %08x named %s\n", - layout->entries[i].start, - layout->entries[i].end, layout->entries[i].name); - } - ret = 0; _close_ret: @@ -399,6 +384,43 @@ */ /** + * @brief Add another region to an existing layout. + * + * @param layout The existing layout. + * @param start Start address of the region. + * @param end End address (inclusive) of the region. + * @param name Name of the region. + * + * @return 0 on success, + * 1 if out of memory, + * 2 if the layout is full already. + */ +int flashrom_layout_add_region( + struct flashrom_layout *const layout, + const size_t start, const size_t end, const char *const name) +{ + if (layout->num_entries >= layout->capacity) { + msg_gerr("Error adding layout entry: No space left\n"); + return 2; + } + + struct romentry *const entry = &layout->entries[layout->num_entries]; + entry->start = start; + entry->end = end; + entry->included = false; + entry->name = strdup(name); + entry->file = NULL; + if (!entry->name) { + msg_gerr("Error adding layout entry: %s\n", strerror(errno)); + return 1; + } + + msg_gdbg("Added layout entry %08zx - %08zx named %s\n", start, end, name); + ++layout->num_entries; + return 0; +} + +/** * @brief Mark given region as included. * * @param layout The layout to alter. diff --git a/libflashrom.c b/libflashrom.c index 2a39a80..c5c4c89 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -501,6 +501,8 @@ struct flashctx *const flashctx, const struct fmap *const fmap) { int i; + char name[FMAP_STRLEN + 1]; + const struct fmap_area *area; struct flashrom_layout *l = get_global_layout(); if (!fmap || !l) @@ -511,21 +513,10 @@ return 1; } - for (i = 0; i < fmap->nareas; i++) { - l->entries[l->num_entries].start = fmap->areas[i].offset; - l->entries[l->num_entries].end = fmap->areas[i].offset + fmap->areas[i].size - 1; - l->entries[l->num_entries].included = false; - l->entries[l->num_entries].name = - strndup((const char *)fmap->areas[i].name, FMAP_STRLEN); - if (!l->entries[l->num_entries].name) { - msg_gerr("Error adding layout entry: %s\n", strerror(errno)); + for (i = 0, area = fmap->areas; i < fmap->nareas; i++, area++) { + snprintf(name, sizeof(name), "%s", area->name); + if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name)) return 1; - } - msg_gdbg("fmap %08x - %08x named %s\n", - l->entries[l->num_entries].start, - l->entries[l->num_entries].end, - l->entries[l->num_entries].name); - l->num_entries++; } *layout = l; diff --git a/libflashrom.h b/libflashrom.h index eab78ad..d04eeb6 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -111,6 +111,7 @@ struct flashrom_flashctx *, off_t offset, size_t length); int flashrom_layout_read_fmap_from_buffer(struct flashrom_layout **layout, struct flashrom_flashctx *, const uint8_t *buf, size_t len); +int flashrom_layout_add_region(struct flashrom_layout *, size_t start, size_t end, const char *name); int flashrom_layout_include_region(struct flashrom_layout *, const char *name); void flashrom_layout_release(struct flashrom_layout *); void flashrom_layout_set(struct flashrom_flashctx *, const struct flashrom_layout *); -- To view, visit
https://review.coreboot.org/c/flashrom/+/33517
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I50d473d0d5d1fb38bd6f9ae3d7127e9ea66a94e1 Gerrit-Change-Number: 33517 Gerrit-PatchSet: 10 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged
1
0
0
0
← Newer
1
...
11
12
13
14
15
16
17
...
88
Older →
Jump to page:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
Results per page:
10
25
50
100
200