May 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: Add option to read ROM layout from IFD
by Arthur Heymans (Code Review) 04 May '17

04 May '17

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/17953 ) Change subject: Add option to read ROM layout from IFD ...................................................................... Patch Set 13: here is the output https://paste.flashrom.org/view.php?id=3039 -- To view, visit https://review.coreboot.org/17953 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Gerrit-PatchSet: 13 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Add option to read ROM layout from IFD
by Arthur Heymans (Code Review) 04 May '17

04 May '17

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/17953 ) Change subject: Add option to read ROM layout from IFD ...................................................................... Patch Set 13: (1 comment) https://review.coreboot.org/#/c/17953/13/ich_descriptors.c File ich_descriptors.c: PS13, Line 948: min(desc.content.NR + 1, ARRAY_SIZE(regions)) > Um, no... `j` is used to index `layout->entries` and thus definitely What I have is a Flash_Descriptor, a BIOS, and a GbE and no ME. content.NR == 2 so it won't get to GbE... -- To view, visit https://review.coreboot.org/17953 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Gerrit-PatchSet: 13 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Add option to read ROM layout from IFD
by Nico Huber (Code Review) 04 May '17

04 May '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/17953 ) Change subject: Add option to read ROM layout from IFD ...................................................................... Patch Set 13: (1 comment) > Using 'Flash_Descriptor' works, but still invalid for GbE which > after some debugging seems to be absent from the layout struct. Please paste a full log (taken with `-o logfile`). Which descrip- tor generation is it (in the flash)? https://review.coreboot.org/#/c/17953/13/ich_descriptors.c File ich_descriptors.c: PS13, Line 948: min(desc.content.NR + 1, ARRAY_SIZE(regions)) > This seems to cause the problem. would i < ARRAY_SIZE(regions) && j < desc. Um, no... `j` is used to index `layout->entries` and thus definitely must not rely on `desc` (only). The code looks correct (for pre-Skylake descriptors). Maybe the issue is somewhere else? I might have broken something in read_ich_ descriptors_from_dump()... -- To view, visit https://review.coreboot.org/17953 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Gerrit-PatchSet: 13 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: ich_descriptors: Fix more odd +1's
by Nico Huber (Code Review) 04 May '17

04 May '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19044 ) Change subject: ich_descriptors: Fix more odd +1's ...................................................................... Patch Set 3: Code-Review-1 (3 comments) Sorry, it's still confusing... https://review.coreboot.org/#/c/19044/3/ich_descriptors.c File ich_descriptors.c: PS3, Line 620: Just realized that I killed this whitespace by accident. PS3, Line 619: sizeof(desc->north.STRPs) / 4 Isn't it just ARRAY_SIZE()? PS3, Line 619: + 1 This seems off, too. -- To view, visit https://review.coreboot.org/19044 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa5455c999e90ff9121aed29f542d71ac9ca2b1c Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: ich_descriptors: Fix range checks for dumps
by Nico Huber (Code Review) 04 May '17

04 May '17

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/19031 ) Change subject: ich_descriptors: Fix range checks for dumps ...................................................................... ich_descriptors: Fix range checks for dumps These explicit off-by-one calculations were... off-by-one. Change-Id: If57c92ba28f91c4d72123ef0cfd2d9d5ac0a0656 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/19031 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> --- M ich_descriptors.c 1 file changed, 5 insertions(+), 5 deletions(-) Approvals: David Hendricks: Looks good to me, approved build bot (Jenkins): Verified diff --git a/ich_descriptors.c b/ich_descriptors.c index 90f70ee..ec75a8a 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -724,7 +724,7 @@ } /* map */ - if (len < (4 + pch_bug_offset) * 4 - 1) + if (len < (4 + pch_bug_offset) * 4) return ICH_RET_OOB; desc->content.FLVALSIG = dump[0 + pch_bug_offset]; desc->content.FLMAP0 = dump[1 + pch_bug_offset]; @@ -732,14 +732,14 @@ desc->content.FLMAP2 = dump[3 + pch_bug_offset]; /* component */ - if (len < (getFCBA(&desc->content) + 3 * 4 - 1)) + if (len < getFCBA(&desc->content) + 3 * 4) return ICH_RET_OOB; desc->component.FLCOMP = dump[(getFCBA(&desc->content) >> 2) + 0]; desc->component.FLILL = dump[(getFCBA(&desc->content) >> 2) + 1]; desc->component.FLPB = dump[(getFCBA(&desc->content) >> 2) + 2]; /* region */ - if (len < (getFRBA(&desc->content) + 5 * 4 - 1)) + if (len < getFRBA(&desc->content) + 5 * 4) return ICH_RET_OOB; desc->region.FLREGs[0] = dump[(getFRBA(&desc->content) >> 2) + 0]; desc->region.FLREGs[1] = dump[(getFRBA(&desc->content) >> 2) + 1]; @@ -748,7 +748,7 @@ desc->region.FLREGs[4] = dump[(getFRBA(&desc->content) >> 2) + 4]; /* master */ - if (len < (getFMBA(&desc->content) + 3 * 4 - 1)) + if (len < getFMBA(&desc->content) + 3 * 4) return ICH_RET_OOB; desc->master.FLMSTR1 = dump[(getFMBA(&desc->content) >> 2) + 0]; desc->master.FLMSTR2 = dump[(getFMBA(&desc->content) >> 2) + 1]; @@ -763,7 +763,7 @@ * the maximum of 255 gives us 127.5 SPI components(!?) 8 bytes each. A * check ensures that the maximum offset actually accessed is available. */ - if (len < (getVTBA(&desc->upper) + (desc->upper.VTL / 2 * 8) - 1)) + if (len < getVTBA(&desc->upper) + (desc->upper.VTL / 2 * 8)) return ICH_RET_OOB; for (i = 0; i < desc->upper.VTL/2; i++) { -- To view, visit https://review.coreboot.org/19031 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: If57c92ba28f91c4d72123ef0cfd2d9d5ac0a0656 Gerrit-PatchSet: 5 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Remove undefined behavior
by Patrick Georgi (Code Review) 04 May '17

04 May '17

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/18792 ) Change subject: Remove undefined behavior ...................................................................... Remove undefined behavior Per clang-3.9, the compiler fails on #define ...defined(...) statements as they're undefined behavior (apparently with different behavior between gcc/clang and msvc, too). See clang's cfe repo commit r258128 for details. Change-Id: I82b6235e11b425fae45eebbe06b08f81c5bdbb98 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> Reviewed-on: https://review.coreboot.org/18792 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M hwaccess.c M platform.h 2 files changed, 30 insertions(+), 6 deletions(-) Approvals: Stefan Reinauer: Looks good to me, approved build bot (Jenkins): Verified diff --git a/hwaccess.c b/hwaccess.c index aede7e3..2bf6f42 100644 --- a/hwaccess.c +++ b/hwaccess.c @@ -37,9 +37,21 @@ #error "Unknown operating system" #endif -#define USE_IOPL (IS_LINUX || IS_MACOSX || defined(__NetBSD__) || defined(__OpenBSD__)) -#define USE_DEV_IO (defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)) -#define USE_IOPERM (defined(__gnu_hurd__)) +#if IS_LINUX || IS_MACOSX || defined(__NetBSD__) || defined(__OpenBSD__) +#define USE_IOPL 1 +#else +#define USE_IOPL 0 +#endif +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__) +#define USE_DEV_IO 1 +#else +#define USE_DEV_IO 0 +#endif +#if defined(__gnu_hurd__) +#define USE_IOPERM 1 +#else +#define USE_IOPERM 0 +#endif #if USE_IOPERM #include <sys/io.h> diff --git a/platform.h b/platform.h index c5a52ef..b2fdcd0 100644 --- a/platform.h +++ b/platform.h @@ -25,9 +25,21 @@ #define __PLATFORM_H__ 1 // Helper defines for operating systems -#define IS_LINUX (defined(__gnu_linux__) || defined(__linux__)) -#define IS_MACOSX (defined(__APPLE__) && defined(__MACH__)) /* yes, both. */ -#define IS_WINDOWS (defined(_WIN32) || defined(_WIN64) || defined(__WIN32__) || defined(__WINDOWS__)) +#if defined(__gnu_linux__) || defined(__linux__) +#define IS_LINUX 1 +#else +#define IS_LINUX 0 +#endif +#if defined(__APPLE__) && defined(__MACH__) /* yes, both. */ +#define IS_MACOSX 1 +#else +#define IS_MACOSX 0 +#endif +#if defined(_WIN32) || defined(_WIN64) || defined(__WIN32__) || defined(__WINDOWS__) +#define IS_WINDOWS 1 +#else +#define IS_WINDOWS 0 +#endif // Likewise for target architectures #if defined (__i386__) || defined (__x86_64__) || defined(__amd64__) -- To view, visit https://review.coreboot.org/18792 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I82b6235e11b425fae45eebbe06b08f81c5bdbb98 Gerrit-PatchSet: 3 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: Add option to read ROM layout from IFD
by Arthur Heymans (Code Review) 04 May '17

04 May '17

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/17953 ) Change subject: Add option to read ROM layout from IFD ...................................................................... Patch Set 13: (1 comment) https://review.coreboot.org/#/c/17953/13/ich_descriptors.c File ich_descriptors.c: PS13, Line 948: min(desc.content.NR + 1, ARRAY_SIZE(regions)) This seems to cause the problem. would i < ARRAY_SIZE(regions) && j < desc.content.NR + 1 what it should be? -- To view, visit https://review.coreboot.org/17953 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Gerrit-PatchSet: 13 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: Add option to read ROM layout from IFD
by Arthur Heymans (Code Review) 04 May '17

04 May '17

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/17953 ) Change subject: Add option to read ROM layout from IFD ...................................................................... Patch Set 13: Using 'Flash_Descriptor' works, but still invalid for GbE which after some debugging seems to be absent from the layout struct. -- To view, visit https://review.coreboot.org/17953 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Gerrit-PatchSet: 13 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Add functions to read/erase/write/verify by layout
by Arthur Heymans (Code Review) 04 May '17

04 May '17

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/17945 ) Change subject: Add functions to read/erase/write/verify by layout ...................................................................... Patch Set 10: Code-Review+1 Tested with read and verify. -- To view, visit https://review.coreboot.org/17945 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6194cea4c4c430e0cf9d586052508a865b09c86 Gerrit-PatchSet: 10 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Add option to read ROM layout from IFD
by Arthur Heymans (Code Review) 04 May '17

04 May '17

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/17953 ) Change subject: Add option to read ROM layout from IFD ...................................................................... Patch Set 13: Works fine with with --ifd -i BIOS, but with GbE and Descr. it says Invalid region selected. Is this intended? (both GbE and IFD are unlocked) -- To view, visit https://review.coreboot.org/17953 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifafff2bf6d5c5e62283416b3269723f81fdc0fa3 Gerrit-PatchSet: 13 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2017