March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: pony_spi.c: Fix memory leak in function pony_init_spi
by Light (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Light. Hello build bot (Jenkins), Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62724 to look at the new patch set (#8). Change subject: pony_spi.c: Fix memory leak in function pony_init_spi ...................................................................... pony_spi.c: Fix memory leak in function pony_init_spi I found the issue by running scan-build. Memory leak was caused as data variable wasn't deallocated in some error cases where the function returned without deallocating it. After making the changes the issue was no longer appeared in scan-build. Change-Id: I7910db94f63693e7f131836d4963e88cfdbec301 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M pony_spi.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/62724/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/62724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7910db94f63693e7f131836d4963e88cfdbec301 Gerrit-Change-Number: 62724 Gerrit-PatchSet: 8 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Martin L Roth <martinroth(a)google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Subrata Banik (Code Review) 14 Mar '22

14 Mar '22

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... ichspi: Add support for Meteor Lake This patch adds Meteor Lake support into flashrom. BUG=b:224325352 TEST=Flashrom is able to detect MTL SPI DID and show chipset name as below: > flashrom --flash-name .... Found chipset "Intel Meteor Lake-P/M". .... Note: Meteor Lake doesn't have PCH die hence, dropped `_POINT` postfix from the chipset name. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 --- M chipset_enable.c M ich_descriptors.c M ichspi.c M programmer.h M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 36 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/62783/1 diff --git a/chipset_enable.c b/chipset_enable.c index ac9377b..fbe4700 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -606,6 +606,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_ELKHART_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: @@ -711,6 +712,7 @@ break; case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: boot_straps = boot_straps_pch500; break; case CHIPSET_APOLLO_LAKE: @@ -744,6 +746,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: @@ -1002,6 +1005,11 @@ return enable_flash_pch100_or_c620(dev, name, 0x1f, 5, CHIPSET_600_SERIES_ALDER_POINT); } +static int enable_flash_mtl(struct pci_dev *const dev, const char *const name) +{ + return enable_flash_pch100_or_c620(dev, name, 0x1f, 5, CHIPSET_METEOR_LAKE); +} + static int enable_flash_mcc(struct pci_dev *const dev, const char *const name) { return enable_flash_pch100_or_c620(dev, name, 0x1f, 5, CHIPSET_ELKHART_LAKE); @@ -2151,6 +2159,7 @@ {0x8086, 0x438b, B_S, DEP, "Intel", "HM570", enable_flash_pch500}, {0x8086, 0x54a4, B_S, DEP, "Intel", "Alder Lake-N", enable_flash_pch600}, {0x8086, 0x51a4, B_S, DEP, "Intel", "Alder Lake-P", enable_flash_pch600}, + {0x8086, 0x7e23, B_S, DEP, "Intel", "Meteor Lake-P/M", enable_flash_mtl}, #endif {0}, }; diff --git a/ich_descriptors.c b/ich_descriptors.c index 256224d..c0560d6 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -47,6 +47,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_ELKHART_LAKE: return 16; case CHIPSET_100_SERIES_SUNRISE_POINT: @@ -74,6 +75,7 @@ case CHIPSET_C620_SERIES_LEWISBURG: case CHIPSET_APOLLO_LAKE: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: if (cont->NM <= MAX_NUM_MASTERS) @@ -209,6 +211,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: { @@ -304,6 +307,7 @@ return freq_str[2][value]; case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: return freq_str[3][value]; case CHIPSET_ELKHART_LAKE: return freq_str[4][value]; @@ -351,6 +355,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: @@ -476,7 +481,8 @@ cs == CHIPSET_300_SERIES_CANNON_POINT || cs == CHIPSET_400_SERIES_COMET_POINT || cs == CHIPSET_500_SERIES_TIGER_POINT || - cs == CHIPSET_600_SERIES_ALDER_POINT) { + cs == CHIPSET_600_SERIES_ALDER_POINT || + cs == CHIPSET_METEOR_LAKE) { const char *const master_names[] = { "BIOS", "ME", "GbE", "unknown", "EC", }; @@ -1039,8 +1045,12 @@ return CHIPSET_500_SERIES_TIGER_POINT; if (content->CSSL == 0x14) return CHIPSET_600_SERIES_ALDER_POINT; - if (content->CSSL == 0x03) - return CHIPSET_ELKHART_LAKE; + if (content->CSSL == 0x03) { + if (content->CSSO == 0x70) + return CHIPSET_METEOR_LAKE; + else + return CHIPSET_ELKHART_LAKE; + } msg_pwarn("Unknown flash descriptor, assuming 500 series compatibility.\n"); return CHIPSET_500_SERIES_TIGER_POINT; } @@ -1063,6 +1073,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: /* `freq_read` was repurposed, so can't check on it any more. */ @@ -1220,6 +1231,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: @@ -1260,6 +1272,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: diff --git a/ichspi.c b/ichspi.c index 9f45ec2..1ffb8e8 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1799,6 +1799,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: @@ -1836,6 +1837,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: @@ -1895,6 +1897,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: @@ -1973,6 +1976,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_BAYTRAIL: @@ -2010,6 +2014,7 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: + case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_ELKHART_LAKE: @@ -2053,8 +2058,9 @@ if (ich_spi_mode == ich_auto && (ich_gen == CHIPSET_APOLLO_LAKE || ich_gen == CHIPSET_GEMINI_LAKE || - ich_gen == CHIPSET_ELKHART_LAKE)) { - msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini/Elkhart Lake.\n"); + ich_gen == CHIPSET_ELKHART_LAKE || + ich_gen == CHIPSET_METEOR_LAKE)) { + msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini/Elkhart/Meteor Lake.\n"); ich_spi_mode = ich_hwseq; } diff --git a/programmer.h b/programmer.h index 6a0c3fa..e751079 100644 --- a/programmer.h +++ b/programmer.h @@ -354,6 +354,7 @@ CHIPSET_400_SERIES_COMET_POINT, CHIPSET_500_SERIES_TIGER_POINT, CHIPSET_600_SERIES_ALDER_POINT, + CHIPSET_METEOR_LAKE, CHIPSET_APOLLO_LAKE, CHIPSET_GEMINI_LAKE, CHIPSET_ELKHART_LAKE, diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 532a1fb..dacc199 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -237,6 +237,8 @@ cs = CHIPSET_500_SERIES_TIGER_POINT; else if (strcmp(csn, "600") == 0) cs = CHIPSET_600_SERIES_ALDER_POINT; + else if (strcmp(csn, "meteor") == 0) + cs = CHIPSET_METEOR_LAKE; else if (strcmp(csn, "apollo") == 0) cs = CHIPSET_APOLLO_LAKE; else if (strcmp(csn, "gemini") == 0) -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: board_enable.c: Fix dead assignment
by Anastasia Klimchuk (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Keith Hui, Light. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 ) Change subject: board_enable.c: Fix dead assignment ...................................................................... Patch Set 3: (1 comment) Patchset: PS1: > I don't this change should affect the logic in any way but still wondering why were there 2 assignme […] Maybe INB has some side-effects, for example doing something and then the assignment happens. Keith, Idwer, maybe you know about it? Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/62761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc Gerrit-Change-Number: 62761 Gerrit-PatchSet: 3 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 08:53:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Anastasia Klimchuk (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Angel Pons, Light. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62747 ) Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc ...................................................................... Patch Set 5: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62747/comment/80514137_676b34e3 PS5, Line 13: So allocate them using calloc to initialize them to : zeroes or if allocating using malloc separately initialize them using a loop. Maybe separate this by two new lines, so that it will be easier to read? Patchset: PS5: Angel, I have something in memory that you had a patch at some point to replace malloc with calloc? I don't remember what happened with that patch :) But looks like it was a good idea. -- To view, visit https://review.coreboot.org/c/flashrom/+/62747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Gerrit-Change-Number: 62747 Gerrit-PatchSet: 5 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 08:47:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: serprog.c: Avoid calling memcpy with NULL pointer arguments
by Anastasia Klimchuk (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Paul Menzel, Angel Pons, Light. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62726 ) Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments ...................................................................... Patch Set 9: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62726 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b Gerrit-Change-Number: 62726 Gerrit-PatchSet: 9 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 08:34:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom.c: Fix unintialized value passed to function
by Anastasia Klimchuk (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Paul Menzel, Angel Pons, Light. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62725 ) Change subject: libflashrom.c: Fix unintialized value passed to function ...................................................................... Patch Set 12: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62725/comment/41d1fe6f_e01d1cf6 PS12, Line 15: Signed-off-by: aarya <aarya.chaumal(a)gmail.com> You need to remove this line, you have Signed-off-by tag already just above. In general, you need real name in Signed-off-by so first line is correct. -- To view, visit https://review.coreboot.org/c/flashrom/+/62725 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d Gerrit-Change-Number: 62725 Gerrit-PatchSet: 12 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 08:26:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pony_spi.c: Fix memory leak in function pony_init_spi
by Anastasia Klimchuk (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Light. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62724 ) Change subject: pony_spi.c: Fix memory leak in function pony_init_spi ...................................................................... Patch Set 7: Code-Review+1 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62724/comment/f304e532_cb8eef74 PS3, Line 11: ii > Done This seems to be still in the latest patchset? Still 2 ii in "deallocatiing". Commit Message: https://review.coreboot.org/c/flashrom/+/62724/comment/a7b6b27f_b5c169ec PS7, Line 9: Memory leaked was caused I think "Memory leak was caused" sounds better (memory leak instead of memory leaked). Patchset: PS7: I have just 2 comments about commit message. -- To view, visit https://review.coreboot.org/c/flashrom/+/62724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7910db94f63693e7f131836d4963e88cfdbec301 Gerrit-Change-Number: 62724 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Martin L Roth <martinroth(a)google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 08:23:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests/: Factor out all link-time wrapped func
by Edward O'Callaghan (Code Review) 14 Mar '22

14 Mar '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62777 ) Change subject: tests/: Factor out all link-time wrapped func ...................................................................... tests/: Factor out all link-time wrapped func Situate all the link-time wrapped functions into their own object file. This makes it easier to grok test code and keep all related harness supports together. BUG=b:217629892,b:215255210 TEST=`ninja test`. Change-Id: Ic5e18c9390c064a3112caa7f5778569ae478552d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/meson.build M tests/tests.c A tests/wraps.c 3 files changed, 325 insertions(+), 305 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/62777/1 diff --git a/tests/meson.build b/tests/meson.build index 610d1c6..0a90ed6 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -23,6 +23,7 @@ 'lifecycle.c', 'layout.c', 'chip.c', + 'wraps.c', ] mocks = [ diff --git a/tests/tests.c b/tests/tests.c index 21cc1ed..3a3d381 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -14,318 +14,13 @@ */ #include <include/test.h> -#include "io_mock.h" #include "tests.h" -#include <stdio.h> -#include <string.h> -#include <stdint.h> - void *not_null(void) { return (void *)NON_ZERO; } -/* Workaround for https://github.com/clibs/cmocka/issues/17 */ -char *__wrap_strdup(const char *s) -{ - size_t len = strlen(s) + 1; - void *new = malloc(len); - if (new == NULL) - return NULL; - return (char *)memcpy(new, s, len); -} - -void __wrap_physunmap(void *virt_addr, size_t len) -{ - LOG_ME; -} - -void *__wrap_physmap(const char *descr, uintptr_t phys_addr, size_t len) -{ - LOG_ME; - return NULL; -} - -struct pci_dev mock_pci_dev = { - .device_id = NON_ZERO, -}; - -struct pci_dev *__wrap_pcidev_init(void *devs, int bar) -{ - LOG_ME; - return &mock_pci_dev; -} - -uintptr_t __wrap_pcidev_readbar(void *dev, int bar) -{ - LOG_ME; - return NON_ZERO; -} - -void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data) -{ - LOG_ME; -} - -uint8_t __wrap_sio_read(uint16_t port, uint8_t reg) -{ - LOG_ME; - return (uint8_t)mock(); -} - -int __wrap_open(const char *pathname, int flags) -{ - LOG_ME; - if (get_io() && get_io()->open) - return get_io()->open(get_io()->state, pathname, flags); - return NON_ZERO; -} - -int __wrap_open64(const char *pathname, int flags) -{ - LOG_ME; - if (get_io() && get_io()->open) - return get_io()->open(get_io()->state, pathname, flags); - return NON_ZERO; -} - -int __wrap_ioctl(int fd, unsigned long int request, ...) -{ - LOG_ME; - if (get_io() && get_io()->ioctl) { - va_list args; - int out; - va_start(args, request); - out = get_io()->ioctl(get_io()->state, fd, request, args); - va_end(args); - return out; - } - return 0; -} - -int __wrap_write(int fd, const void *buf, size_t sz) -{ - LOG_ME; - if (get_io() && get_io()->write) - return get_io()->write(get_io()->state, fd, buf, sz); - return sz; -} - -int __wrap_read(int fd, void *buf, size_t sz) -{ - LOG_ME; - if (get_io() && get_io()->read) - return get_io()->read(get_io()->state, fd, buf, sz); - return sz; -} - -FILE *__wrap_fopen(const char *pathname, const char *mode) -{ - LOG_ME; - if (get_io() && get_io()->fopen) - return get_io()->fopen(get_io()->state, pathname, mode); - return not_null(); -} - -FILE *__wrap_fopen64(const char *pathname, const char *mode) -{ - LOG_ME; - if (get_io() && get_io()->fopen) - return get_io()->fopen(get_io()->state, pathname, mode); - return not_null(); -} - -FILE *__wrap_fdopen(int fd, const char *mode) -{ - LOG_ME; - return not_null(); -} - -int __wrap_stat(const char *path, void *buf) -{ - LOG_ME; - return 0; -} - -int __wrap_stat64(const char *path, void *buf) -{ - LOG_ME; - return 0; -} - -int __wrap___xstat(const char *path, void *buf) -{ - LOG_ME; - return 0; -} - -int __wrap___xstat64(const char *path, void *buf) -{ - LOG_ME; - return 0; -} - -int __wrap_fstat(int fd, void *buf) -{ - LOG_ME; - return 0; -} - -int __wrap_fstat64(int fd, void *buf) -{ - LOG_ME; - return 0; -} - -int __wrap___fxstat(int fd, void *buf) -{ - LOG_ME; - return 0; -} - -int __wrap___fxstat64(int fd, void *buf) -{ - LOG_ME; - return 0; -} - -char *__wrap_fgets(char *buf, int len, FILE *fp) -{ - LOG_ME; - if (get_io() && get_io()->fgets) - return get_io()->fgets(get_io()->state, buf, len, fp); - return NULL; -} - -size_t __wrap_fread(void *ptr, size_t size, size_t nmemb, FILE *fp) -{ - LOG_ME; - if (get_io() && get_io()->fread) - return get_io()->fread(get_io()->state, ptr, size, nmemb, fp); - return nmemb; -} - -size_t __wrap_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp) -{ - LOG_ME; - return nmemb; -} - -int __wrap_fflush(FILE *fp) -{ - LOG_ME; - return 0; -} - -int __wrap_fileno(FILE *fp) -{ - LOG_ME; - return NON_ZERO; -} - -int __wrap_fsync(int fd) -{ - LOG_ME; - return 0; -} - -int __wrap_setvbuf(FILE *fp, char *buf, int type, size_t size) -{ - LOG_ME; - return 0; -} - -int __wrap_fprintf(FILE *fp, const char *fmt, ...) -{ - LOG_ME; - if (get_io() && get_io()->fprintf) { - va_list args; - int out; - va_start(args, fmt); - out = get_io()->fprintf(get_io()->state, fp, fmt, args); - va_end(args); - return out; - } - return 0; -} - -int __wrap_fclose(FILE *fp) -{ - LOG_ME; - if (get_io() && get_io()->fclose) - return get_io()->fclose(get_io()->state, fp); - return 0; -} - -int __wrap_feof(FILE *fp) -{ - /* LOG_ME; */ - return 0; -} - -int __wrap_ferror(FILE *fp) -{ - /* LOG_ME; */ - return 0; -} -void __wrap_clearerr(FILE *fp) -{ - /* LOG_ME; */ - return; -} - -int __wrap_rget_io_perms(void) -{ - LOG_ME; - return 0; -} - -void __wrap_test_outb(unsigned char value, unsigned short port) -{ - /* LOG_ME; */ - if (get_io() && get_io()->outb) - get_io()->outb(get_io()->state, value, port); -} - -unsigned char __wrap_test_inb(unsigned short port) -{ - /* LOG_ME; */ - if (get_io() && get_io()->inb) - return get_io()->inb(get_io()->state, port); - return 0; -} - -void __wrap_test_outw(unsigned short value, unsigned short port) -{ - /* LOG_ME; */ - if (get_io() && get_io()->outw) - get_io()->outw(get_io()->state, value, port); -} - -unsigned short __wrap_test_inw(unsigned short port) -{ - /* LOG_ME; */ - if (get_io() && get_io()->inw) - return get_io()->inw(get_io()->state, port); - return 0; -} - -void __wrap_test_outl(unsigned int value, unsigned short port) -{ - /* LOG_ME; */ - if (get_io() && get_io()->outl) - get_io()->outl(get_io()->state, value, port); -} - -unsigned int __wrap_test_inl(unsigned short port) -{ - /* LOG_ME; */ - if (get_io() && get_io()->inl) - return get_io()->inl(get_io()->state, port); - return 0; -} - int main(void) { int ret = 0; diff --git a/tests/wraps.c b/tests/wraps.c new file mode 100644 index 0000000..f3fe343 --- /dev/null +++ b/tests/wraps.c @@ -0,0 +1,324 @@ +/* + * This file is part of the flashrom project. + * + * Copyright 2021-2022 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <include/test.h> + +#include <stdio.h> +#include <string.h> +#include <stdint.h> + +#include "io_mock.h" + + +/* Workaround for https://github.com/clibs/cmocka/issues/17 */ +char *__wrap_strdup(const char *s) +{ + size_t len = strlen(s) + 1; + void *new = malloc(len); + if (new == NULL) + return NULL; + return (char *)memcpy(new, s, len); +} + +void __wrap_physunmap(void *virt_addr, size_t len) +{ + LOG_ME; +} + +void *__wrap_physmap(const char *descr, uintptr_t phys_addr, size_t len) +{ + LOG_ME; + return NULL; +} + +struct pci_dev mock_pci_dev = { + .device_id = NON_ZERO, +}; + +struct pci_dev *__wrap_pcidev_init(void *devs, int bar) +{ + LOG_ME; + return &mock_pci_dev; +} + +uintptr_t __wrap_pcidev_readbar(void *dev, int bar) +{ + LOG_ME; + return NON_ZERO; +} + +void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data) +{ + LOG_ME; +} + +uint8_t __wrap_sio_read(uint16_t port, uint8_t reg) +{ + LOG_ME; + return (uint8_t)mock(); +} + +int __wrap_open(const char *pathname, int flags) +{ + LOG_ME; + if (get_io() && get_io()->open) + return get_io()->open(get_io()->state, pathname, flags); + return NON_ZERO; +} + +int __wrap_open64(const char *pathname, int flags) +{ + LOG_ME; + if (get_io() && get_io()->open) + return get_io()->open(get_io()->state, pathname, flags); + return NON_ZERO; +} + +int __wrap_ioctl(int fd, unsigned long int request, ...) +{ + LOG_ME; + if (get_io() && get_io()->ioctl) { + va_list args; + int out; + va_start(args, request); + out = get_io()->ioctl(get_io()->state, fd, request, args); + va_end(args); + return out; + } + return 0; +} + +int __wrap_write(int fd, const void *buf, size_t sz) +{ + LOG_ME; + if (get_io() && get_io()->write) + return get_io()->write(get_io()->state, fd, buf, sz); + return sz; +} + +int __wrap_read(int fd, void *buf, size_t sz) +{ + LOG_ME; + if (get_io() && get_io()->read) + return get_io()->read(get_io()->state, fd, buf, sz); + return sz; +} + +FILE *__wrap_fopen(const char *pathname, const char *mode) +{ + LOG_ME; + if (get_io() && get_io()->fopen) + return get_io()->fopen(get_io()->state, pathname, mode); + return not_null(); +} + +FILE *__wrap_fopen64(const char *pathname, const char *mode) +{ + LOG_ME; + if (get_io() && get_io()->fopen) + return get_io()->fopen(get_io()->state, pathname, mode); + return not_null(); +} + +FILE *__wrap_fdopen(int fd, const char *mode) +{ + LOG_ME; + return not_null(); +} + +int __wrap_stat(const char *path, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap_stat64(const char *path, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap___xstat(const char *path, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap___xstat64(const char *path, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap_fstat(int fd, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap_fstat64(int fd, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap___fxstat(int fd, void *buf) +{ + LOG_ME; + return 0; +} + +int __wrap___fxstat64(int fd, void *buf) +{ + LOG_ME; + return 0; +} + +char *__wrap_fgets(char *buf, int len, FILE *fp) +{ + LOG_ME; + if (get_io() && get_io()->fgets) + return get_io()->fgets(get_io()->state, buf, len, fp); + return NULL; +} + +size_t __wrap_fread(void *ptr, size_t size, size_t nmemb, FILE *fp) +{ + LOG_ME; + if (get_io() && get_io()->fread) + return get_io()->fread(get_io()->state, ptr, size, nmemb, fp); + return nmemb; +} + +size_t __wrap_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp) +{ + LOG_ME; + return nmemb; +} + +int __wrap_fflush(FILE *fp) +{ + LOG_ME; + return 0; +} + +int __wrap_fileno(FILE *fp) +{ + LOG_ME; + return NON_ZERO; +} + +int __wrap_fsync(int fd) +{ + LOG_ME; + return 0; +} + +int __wrap_setvbuf(FILE *fp, char *buf, int type, size_t size) +{ + LOG_ME; + return 0; +} + +int __wrap_fprintf(FILE *fp, const char *fmt, ...) +{ + LOG_ME; + if (get_io() && get_io()->fprintf) { + va_list args; + int out; + va_start(args, fmt); + out = get_io()->fprintf(get_io()->state, fp, fmt, args); + va_end(args); + return out; + } + return 0; +} + +int __wrap_fclose(FILE *fp) +{ + LOG_ME; + if (get_io() && get_io()->fclose) + return get_io()->fclose(get_io()->state, fp); + return 0; +} + +int __wrap_feof(FILE *fp) +{ + /* LOG_ME; */ + return 0; +} + +int __wrap_ferror(FILE *fp) +{ + /* LOG_ME; */ + return 0; +} + +void __wrap_clearerr(FILE *fp) +{ + /* LOG_ME; */ + return; +} + +int __wrap_rget_io_perms(void) +{ + LOG_ME; + return 0; +} + +void __wrap_test_outb(unsigned char value, unsigned short port) +{ + /* LOG_ME; */ + if (get_io() && get_io()->outb) + get_io()->outb(get_io()->state, value, port); +} + +unsigned char __wrap_test_inb(unsigned short port) +{ + /* LOG_ME; */ + if (get_io() && get_io()->inb) + return get_io()->inb(get_io()->state, port); + return 0; +} + +void __wrap_test_outw(unsigned short value, unsigned short port) +{ + /* LOG_ME; */ + if (get_io() && get_io()->outw) + get_io()->outw(get_io()->state, value, port); +} + +unsigned short __wrap_test_inw(unsigned short port) +{ + /* LOG_ME; */ + if (get_io() && get_io()->inw) + return get_io()->inw(get_io()->state, port); + return 0; +} + +void __wrap_test_outl(unsigned int value, unsigned short port) +{ + /* LOG_ME; */ + if (get_io() && get_io()->outl) + get_io()->outl(get_io()->state, value, port); +} + +unsigned int __wrap_test_inl(unsigned short port) +{ + /* LOG_ME; */ + if (get_io() && get_io()->inl) + return get_io()->inl(get_io()->state, port); + return 0; +} -- To view, visit https://review.coreboot.org/c/flashrom/+/62777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5e18c9390c064a3112caa7f5778569ae478552d Gerrit-Change-Number: 62777 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tests/chips.c: Mock out open() for lock file
by Edward O'Callaghan (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62324 ) Change subject: tests/chips.c: Mock out open() for lock file ...................................................................... Patch Set 4: (1 comment) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/62324/comment/52967d81_68a5c88f PS3, Line 189: #define DEFAULT_MOCK_FD 0xC0FE > Whatever will be end result in CB:62319 ideally it can be reused here Cleaned up now put consolidating this into a new wraps.c object introduced in commit#1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62324 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I527a266233648b0eef11a89108e82d0a008eeb8d Gerrit-Change-Number: 62324 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 07:01:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests/linux_spi: Validate param file path
by Edward O'Callaghan (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62319 ) Change subject: tests/linux_spi: Validate param file path ...................................................................... Patch Set 5: (2 comments) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/62319/comment/c7c33198_5c5390a8 PS4, Line 41: default_io_mock_state > Ok, maybe multipath is not great name if it has a meaning already, let's think of other name. […] I have rebased with 'fallback' instead. I would be inclined to move all the wrap implementations in tests.c to wraps.c but I didn't do that in this patch series yet. https://review.coreboot.org/c/flashrom/+/62319/comment/2004c2d6_2c17fae9 PS4, Line 43: 2 > Yes, great idea to have it as define! Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62319 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5d24c65f291c53a35509fea5d2f5b3fdb51c306 Gerrit-Change-Number: 62319 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 06:59:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022