- flashrom-gerrit - mail.coreboot.org

Change in flashrom[stable]: Enable continuous SPI reads
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Hello Urja Rannikko, I'd like you to do a code review. Please visit https://review.coreboot.org/21813 to review the following change. Change subject: Enable continuous SPI reads ...................................................................... Enable continuous SPI reads Previous unnecessary page-by-page reading is repurposed to read by big naturally aligned areas (now chip size limited to 16MB for future-proofing of 4 byte addressed multi-die chips) and serprog hack for continuous reads is removed. Original-Change-Id: Iadf909c9216578b1c5dacd4c4991bb436e32edc9 Original-Reviewed-on: https://review.coreboot.org/20223 Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I8fdf4621847b1973e5d7344b369d63da5c62e16f Signed-off-by: Urja Rannikko <urjaman(a)gmail.com> --- M serprog.c M spi25.c 2 files changed, 15 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/21813/1 diff --git a/serprog.c b/serprog.c index 98aac83..25c9944 100644 --- a/serprog.c +++ b/serprog.c @@ -303,15 +303,13 @@ unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); -static int serprog_spi_read(struct flashctx *flash, uint8_t *buf, - unsigned int start, unsigned int len); static struct spi_master spi_master_serprog = { .type = SPI_CONTROLLER_SERPROG, .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = serprog_spi_send_command, .multicommand = default_spi_send_multicommand, - .read = serprog_spi_read, + .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, }; @@ -931,25 +929,6 @@ readarr); free(parmbuf); return ret; -} - -/* FIXME: This function is optimized so that it does not split each transaction - * into chip page_size long blocks unnecessarily like spi_read_chunked. This has - * the advantage that it is much faster for most chips, but breaks those with - * non-continuous reads. When spi_read_chunked is fixed this method can be removed. */ -static int serprog_spi_read(struct flashctx *flash, uint8_t *buf, - unsigned int start, unsigned int len) -{ - unsigned int i, cur_len; - const unsigned int max_read = spi_master_serprog.max_data_read; - for (i = 0; i < len; i += cur_len) { - int ret; - cur_len = min(max_read, (len - i)); - ret = spi_nbyte_read(flash, start + i, buf + i, cur_len); - if (ret) - return ret; - } - return 0; } void *serprog_map(const char *descr, uintptr_t phys_addr, size_t len) diff --git a/spi25.c b/spi25.c index af4b6db..76242be 100644 --- a/spi25.c +++ b/spi25.c @@ -940,30 +940,31 @@ /* * Read a part of the flash chip. * FIXME: Use the chunk code from Michael Karcher instead. - * Each page is read separately in chunks with a maximum size of chunksize. + * Each naturally aligned area is read separately in chunks with a maximum size of chunksize. */ int spi_read_chunked(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len, unsigned int chunksize) { int rc = 0; unsigned int i, j, starthere, lenhere, toread; - unsigned int page_size = flash->chip->page_size; + /* Limit for multi-die 4-byte-addressing chips. */ + unsigned int area_size = min(flash->chip->total_size * 1024, 16 * 1024 * 1024); /* Warning: This loop has a very unusual condition and body. - * The loop needs to go through each page with at least one affected - * byte. The lowest page number is (start / page_size) since that - * division rounds down. The highest page number we want is the page + * The loop needs to go through each area with at least one affected + * byte. The lowest area number is (start / area_size) since that + * division rounds down. The highest area number we want is the area * where the last byte of the range lives. That last byte has the - * address (start + len - 1), thus the highest page number is - * (start + len - 1) / page_size. Since we want to include that last - * page as well, the loop condition uses <=. + * address (start + len - 1), thus the highest area number is + * (start + len - 1) / area_size. Since we want to include that last + * area as well, the loop condition uses <=. */ - for (i = start / page_size; i <= (start + len - 1) / page_size; i++) { - /* Byte position of the first byte in the range in this page. */ + for (i = start / area_size; i <= (start + len - 1) / area_size; i++) { + /* Byte position of the first byte in the range in this area. */ /* starthere is an offset to the base address of the chip. */ - starthere = max(start, i * page_size); - /* Length of bytes in the range in this page. */ - lenhere = min(start + len, (i + 1) * page_size) - starthere; + starthere = max(start, i * area_size); + /* Length of bytes in the range in this area. */ + lenhere = min(start + len, (i + 1) * area_size) - starthere; for (j = 0; j < lenhere; j += chunksize) { toread = min(chunksize, lenhere - j); rc = spi_nbyte_read(flash, starthere + j, buf + starthere - start + j, toread); -- To view, visit https://review.coreboot.org/21813 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I8fdf4621847b1973e5d7344b369d63da5c62e16f Gerrit-Change-Number: 21813 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com>

1 0

Change in flashrom[stable]: udelay: Use clock_gettime() if available and precise
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21812 Change subject: udelay: Use clock_gettime() if available and precise ...................................................................... udelay: Use clock_gettime() if available and precise Instead of calibrating our busy loop against a coarse clock, check if a precise clock is available and loop against that. The former is unre- liable by definition on any modern system that may dynamically reclock the processor. v2: Apparently _POSIX_MONOTONIC_CLOCK being defined only means that the library knows about CLOCK_MONOTONIC. So check for its support at runtime and fall back to CLOCK_REALTIME if it's missing. TEST=Manually added a 10s loop and compared to real time. Run on Linux RPi3, Linux x86 and my original use case Linux in VirtualBox (Linux host). Original-Change-Id: I85ad359823875237ada9cd027af3017d62e9a235 Original-Reviewed-on: https://review.coreboot.org/19391 Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Original-Reviewed-by: Philippe Mathieu-Daudé <f4bug(a)amsat.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I08df071915b282e829214610958e4512f5d71b01 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile M udelay.c 2 files changed, 78 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/21812/1 diff --git a/Makefile b/Makefile index 44aa24c..6806e14 100644 --- a/Makefile +++ b/Makefile @@ -1016,6 +1016,9 @@ # We could use PULLED_IN_LIBS, but that would be ugly. FEATURE_LIBS += $(call debug_shell,grep -q "NEEDLIBZ := yes" .libdeps && printf "%s" "-lz") +FEATURE_CFLAGS += $(call debug_shell,grep -q "CLOCK_GETTIME := yes" .features && printf "%s" "-D'HAVE_CLOCK_GETTIME=1'") +FEATURE_LIBS += $(call debug_shell,grep -q "CLOCK_GETTIME := yes" .features && printf "%s" "-lrt") + LIBFLASHROM_OBJS = $(CHIP_OBJS) $(PROGRAMMER_OBJS) $(LIB_OBJS) OBJS = $(CLI_OBJS) $(LIBFLASHROM_OBJS) @@ -1312,6 +1315,18 @@ endef export LINUX_I2C_TEST +define CLOCK_GETTIME_TEST +#include <time.h> + +int main(int argc, char **argv) +{ + struct timespec res; + clock_gettime(CLOCK_REALTIME, &res); + return 0; +} +endef +export CLOCK_GETTIME_TEST + features: compiler @echo "FEATURES := yes" > .features.tmp ifneq ($(NEED_LIBFTDI), ) @@ -1354,6 +1369,13 @@ @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \ ( echo "found."; echo "UTSNAME := yes" >> .features.tmp ) || \ ( echo "not found."; echo "UTSNAME := no" >> .features.tmp ) } 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) + @printf "Checking for clock_gettime support... " | tee -a $(BUILD_DETAILS_FILE) + @echo "$$CLOCK_GETTIME_TEST" >.featuretest.c + @printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -lrt .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE) + @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -lrt .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \ + ( echo "found."; echo "CLOCK_GETTIME := yes" >>.features.tmp ) || \ + ( echo "not found."; echo "CLOCK_GETTIME := no" >>.features.tmp ) } \ + 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) @$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features @rm -f .featuretest.c .featuretest$(EXEC_SUFFIX) diff --git a/udelay.c b/udelay.c index 7c6961d..3a44c16 100644 --- a/udelay.c +++ b/udelay.c @@ -21,12 +21,63 @@ #ifndef __LIBPAYLOAD__ +#include <stdbool.h> #include <unistd.h> +#include <errno.h> #include <time.h> #include <sys/time.h> #include <stdlib.h> #include <limits.h> #include "flash.h" + +static bool use_clock_gettime = false; + +#if HAVE_CLOCK_GETTIME == 1 + +#ifdef _POSIX_MONOTONIC_CLOCK +static clockid_t clock_id = CLOCK_MONOTONIC; +#else +static clockid_t clock_id = CLOCK_REALTIME; +#endif + +static void clock_usec_delay(int usecs) +{ + struct timespec now; + clock_gettime(clock_id, &now); + + const long end_nsec = now.tv_nsec + usecs * 1000L; + const struct timespec end = { + end_nsec / (1000 * 1000 * 1000) + now.tv_sec, + end_nsec % (1000 * 1000 * 1000) + }; + do { + clock_gettime(clock_id, &now); + } while (now.tv_sec < end.tv_sec || (now.tv_sec == end.tv_sec && now.tv_nsec < end.tv_nsec)); +} + +static int clock_check_res(void) +{ + struct timespec res; + if (!clock_getres(clock_id, &res)) { + if (res.tv_sec == 0 && res.tv_nsec <= 100) { + msg_pinfo("Using clock_gettime for delay loops (clk_id: %d, resolution: %ldns).\n", + (int)clock_id, res.tv_nsec); + use_clock_gettime = true; + return 1; + } + } else if (clock_id != CLOCK_REALTIME && errno == EINVAL) { + /* Try again with CLOCK_REALTIME. */ + clock_id = CLOCK_REALTIME; + return clock_check_res(); + } + return 0; +} +#else + +static inline void clock_usec_delay(int usecs) {} +static inline int clock_check_res(void) { return 0; } + +#endif /* HAVE_CLOCK_GETTIME == 1 */ /* loops per microsecond */ static unsigned long micro = 1; @@ -87,6 +138,9 @@ void myusec_calibrate_delay(void) { + if (clock_check_res()) + return; + unsigned long count = 1000; unsigned long timeusec, resolution; int i, tries = 0; @@ -189,6 +243,8 @@ /* If the delay is >1 s, use internal_sleep because timing does not need to be so precise. */ if (usecs > 1000000) { internal_sleep(usecs); + } else if (use_clock_gettime) { + clock_usec_delay(usecs); } else { myusec_delay(usecs); } -- To view, visit https://review.coreboot.org/21812 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I08df071915b282e829214610958e4512f5d71b01 Gerrit-Change-Number: 21812 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: Move register decodes into enable_flash_ich_handle_gcs()
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21785 ) Change subject: Move register decodes into enable_flash_ich_handle_gcs() ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/580/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/525/ : SUCCESS -- To view, visit https://review.coreboot.org/21785 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: Ia4b77af1b135ec0caee9405684f18f6314715722 Gerrit-Change-Number: 21785 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:48:55 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: ich_descriptors: Pretty print an assumed chipset
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21811 Change subject: ich_descriptors: Pretty print an assumed chipset ...................................................................... ich_descriptors: Pretty print an assumed chipset Original-Change-Id: Id28cb3abc45c6e7f4c4accfc019579c7448c45d7 Original-Reviewed-on: https://review.coreboot.org/20247 Original-Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Original-Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I031356e253569712045770bbec2be1d451336894 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ich_descriptors.c M ich_descriptors.h 2 files changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/21811/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index a2aa940..a2f8edf 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -90,6 +90,21 @@ #define getFISBA(cont) (((cont)->FLMAP1 >> 12) & 0x00000ff0) #define getFMSBA(cont) (((cont)->FLMAP2 << 4) & 0x00000ff0) +void prettyprint_ich_chipset(enum ich_chipset cs) +{ + static const char *const chipset_names[] = { + "Unknown ICH", "ICH8", "ICH9", "ICH10", + "5 series Ibex Peak", "6 series Cougar Point", "7 series Panther Point", + "8 series Lynx Point", "Baytrail", "8 series Lynx Point LP", "8 series Wellsburg", + "9 series Wildcat Point", "9 series Wildcat Point LP", "100 series Sunrise Point", + }; + if (cs < CHIPSET_ICH8 || cs - CHIPSET_ICH8 + 1 >= ARRAY_SIZE(chipset_names)) + cs = 0; + else + cs = cs - CHIPSET_ICH8 + 1; + msg_pdbg2("Assuming chipset '%s'.\n", chipset_names[cs]); +} + void prettyprint_ich_descriptors(enum ich_chipset cs, const struct ich_descriptors *desc) { prettyprint_ich_descriptor_content(cs, &desc->content); @@ -881,8 +896,10 @@ desc->component.FLILL = dump[(getFCBA(&desc->content) >> 2) + 1]; desc->component.FLPB = dump[(getFCBA(&desc->content) >> 2) + 2]; - if (*cs == CHIPSET_ICH_UNKNOWN) + if (*cs == CHIPSET_ICH_UNKNOWN) { *cs = guess_ich_chipset(&desc->content, &desc->component); + prettyprint_ich_chipset(*cs); + } /* region */ const ssize_t nr = ich_number_of_regions(*cs, &desc->content); diff --git a/ich_descriptors.h b/ich_descriptors.h index bccea27..8c313d1 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -567,6 +567,7 @@ 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); +void prettyprint_ich_chipset(enum ich_chipset cs); void prettyprint_ich_descriptors(enum ich_chipset cs, const struct ich_descriptors *desc); void prettyprint_ich_descriptor_content(enum ich_chipset cs, const struct ich_desc_content *content); -- To view, visit https://review.coreboot.org/21811 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I031356e253569712045770bbec2be1d451336894 Gerrit-Change-Number: 21811 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: Make image parameter of cb_check_image const
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21783 ) Change subject: Make image parameter of cb_check_image const ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/579/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/524/ : SUCCESS -- To view, visit https://review.coreboot.org/21783 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: I605386b6e903acfce85b19ad1e029e62e88df839 Gerrit-Change-Number: 21783 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:48:53 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: chipset_enable: Set 100 series chipsets to NT
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21810 Change subject: chipset_enable: Set 100 series chipsets to NT ...................................................................... chipset_enable: Set 100 series chipsets to NT Original-Change-Id: I9376a0c180b7e73751fbd3c8c37b693d358cbfb8 Original-Reviewed-on: https://review.coreboot.org/19047 Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I2635d01245856136cf4dc8d723378160b1978004 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M chipset_enable.c 1 file changed, 30 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/21810/1 diff --git a/chipset_enable.c b/chipset_enable.c index a85ae21..20d2662 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1880,36 +1880,36 @@ {0x8086, 0x9cc7, NT, "Intel", "Broadwell Y Premium", enable_flash_pch9_lp}, {0x8086, 0x9cc9, NT, "Intel", "Broadwell Y Base", enable_flash_pch9_lp}, {0x8086, 0x9ccb, NT, "Intel", "Broadwell H", enable_flash_pch9}, - {0x8086, 0x9d41, BAD, "Intel", "Skylake / Kaby Lake Sample", enable_flash_pch100}, - {0x8086, 0x9d43, BAD, "Intel", "Skylake U Base", enable_flash_pch100}, - {0x8086, 0x9d46, BAD, "Intel", "Skylake Y Premium", enable_flash_pch100}, - {0x8086, 0x9d48, BAD, "Intel", "Skylake U Premium", enable_flash_pch100}, - {0x8086, 0x9d4b, BAD, "Intel", "Kaby Lake Y w/ iHDCP2.2 Prem.", enable_flash_pch100}, - {0x8086, 0x9d4e, BAD, "Intel", "Kaby Lake U w/ iHDCP2.2 Prem.", enable_flash_pch100}, - {0x8086, 0x9d50, BAD, "Intel", "Kaby Lake U w/ iHDCP2.2 Base", enable_flash_pch100}, - {0x8086, 0x9d51, BAD, "Intel", "Kabe Lake w/ iHDCP2.2 Sample", enable_flash_pch100}, - {0x8086, 0x9d53, BAD, "Intel", "Kaby Lake U Base", enable_flash_pch100}, - {0x8086, 0x9d56, BAD, "Intel", "Kaby Lake Y Premium", enable_flash_pch100}, - {0x8086, 0x9d58, BAD, "Intel", "Kaby Lake U Premium", enable_flash_pch100}, - {0x8086, 0xa141, BAD, "Intel", "Sunrise Point Desktop Sample", enable_flash_pch100}, - {0x8086, 0xa142, BAD, "Intel", "Sunrise Point Unknown Sample", enable_flash_pch100}, - {0x8086, 0xa143, BAD, "Intel", "H110", enable_flash_pch100}, - {0x8086, 0xa144, BAD, "Intel", "H170", enable_flash_pch100}, - {0x8086, 0xa145, BAD, "Intel", "Z170", enable_flash_pch100}, - {0x8086, 0xa146, BAD, "Intel", "Q170", enable_flash_pch100}, - {0x8086, 0xa147, BAD, "Intel", "Q150", enable_flash_pch100}, - {0x8086, 0xa148, BAD, "Intel", "B150", enable_flash_pch100}, - {0x8086, 0xa149, BAD, "Intel", "C236", enable_flash_pch100}, - {0x8086, 0xa14a, BAD, "Intel", "C232", enable_flash_pch100}, - {0x8086, 0xa14b, BAD, "Intel", "Sunrise Point Server Sample", enable_flash_pch100}, - {0x8086, 0xa14d, BAD, "Intel", "QM170", enable_flash_pch100}, - {0x8086, 0xa14e, BAD, "Intel", "HM170", enable_flash_pch100}, - {0x8086, 0xa150, BAD, "Intel", "CM236", enable_flash_pch100}, - {0x8086, 0xa151, BAD, "Intel", "QMS180", enable_flash_pch100}, - {0x8086, 0xa152, BAD, "Intel", "HM175", enable_flash_pch100}, - {0x8086, 0xa153, BAD, "Intel", "QM175", enable_flash_pch100}, - {0x8086, 0xa154, BAD, "Intel", "CM238", enable_flash_pch100}, - {0x8086, 0xa155, BAD, "Intel", "QMU185", enable_flash_pch100}, + {0x8086, 0x9d41, NT, "Intel", "Skylake / Kaby Lake Sample", enable_flash_pch100}, + {0x8086, 0x9d43, NT, "Intel", "Skylake U Base", enable_flash_pch100}, + {0x8086, 0x9d46, NT, "Intel", "Skylake Y Premium", enable_flash_pch100}, + {0x8086, 0x9d48, NT, "Intel", "Skylake U Premium", enable_flash_pch100}, + {0x8086, 0x9d4b, NT, "Intel", "Kaby Lake Y w/ iHDCP2.2 Prem.", enable_flash_pch100}, + {0x8086, 0x9d4e, NT, "Intel", "Kaby Lake U w/ iHDCP2.2 Prem.", enable_flash_pch100}, + {0x8086, 0x9d50, NT, "Intel", "Kaby Lake U w/ iHDCP2.2 Base", enable_flash_pch100}, + {0x8086, 0x9d51, NT, "Intel", "Kabe Lake w/ iHDCP2.2 Sample", enable_flash_pch100}, + {0x8086, 0x9d53, NT, "Intel", "Kaby Lake U Base", enable_flash_pch100}, + {0x8086, 0x9d56, NT, "Intel", "Kaby Lake Y Premium", enable_flash_pch100}, + {0x8086, 0x9d58, NT, "Intel", "Kaby Lake U Premium", enable_flash_pch100}, + {0x8086, 0xa141, NT, "Intel", "Sunrise Point Desktop Sample", enable_flash_pch100}, + {0x8086, 0xa142, NT, "Intel", "Sunrise Point Unknown Sample", enable_flash_pch100}, + {0x8086, 0xa143, NT, "Intel", "H110", enable_flash_pch100}, + {0x8086, 0xa144, NT, "Intel", "H170", enable_flash_pch100}, + {0x8086, 0xa145, NT, "Intel", "Z170", enable_flash_pch100}, + {0x8086, 0xa146, NT, "Intel", "Q170", enable_flash_pch100}, + {0x8086, 0xa147, NT, "Intel", "Q150", enable_flash_pch100}, + {0x8086, 0xa148, NT, "Intel", "B150", enable_flash_pch100}, + {0x8086, 0xa149, NT, "Intel", "C236", enable_flash_pch100}, + {0x8086, 0xa14a, NT, "Intel", "C232", enable_flash_pch100}, + {0x8086, 0xa14b, NT, "Intel", "Sunrise Point Server Sample", enable_flash_pch100}, + {0x8086, 0xa14d, NT, "Intel", "QM170", enable_flash_pch100}, + {0x8086, 0xa14e, NT, "Intel", "HM170", enable_flash_pch100}, + {0x8086, 0xa150, NT, "Intel", "CM236", enable_flash_pch100}, + {0x8086, 0xa151, NT, "Intel", "QMS180", enable_flash_pch100}, + {0x8086, 0xa152, NT, "Intel", "HM175", enable_flash_pch100}, + {0x8086, 0xa153, NT, "Intel", "QM175", enable_flash_pch100}, + {0x8086, 0xa154, NT, "Intel", "CM238", enable_flash_pch100}, + {0x8086, 0xa155, NT, "Intel", "QMU185", enable_flash_pch100}, #endif {0}, }; -- To view, visit https://review.coreboot.org/21810 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I2635d01245856136cf4dc8d723378160b1978004 Gerrit-Change-Number: 21810 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: Remove undefined behavior
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21782 ) Change subject: Remove undefined behavior ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/577/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/522/ : SUCCESS -- To view, visit https://review.coreboot.org/21782 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: Ib8bb468622fa158d3bf5ef797049ecb549cc1f40 Gerrit-Change-Number: 21782 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:48:52 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: ich_descriptors: Update for Intel Skylake
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21809 Change subject: ich_descriptors: Update for Intel Skylake ...................................................................... ich_descriptors: Update for Intel Skylake Interpretation of component clocks changed. Also more regions and more masters are supported now. The number of regions (NR) is now static per chipset (10 in the 100 Series case) and not coded into the descriptor any more. v2: o Use guess_ich_chipset() for read_ich_descriptors_from_dump(). o Update region extraction in `ich_descriptors_tool`. TEST=Run `ich_descriptors_tool` over a 100 Series dump and checked that output looks sane. Run `ich_descriptors_tool` over dumps of five different older systems (1 x Sandy Bridge, 3 x Ivy Bridge, 1 x Haswell). Beside whitespace changes, regions not accounted by `NR` are not printed any more. Original-Change-Id: Idd60a857d1ecffcb2e437af21134d9de44dcceb8 Original-Reviewed-on: https://review.coreboot.org/18973 Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I34ac844dd414db6f8957763846cbfba75301bae5 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ich_descriptors.c M ich_descriptors.h M util/ich_descriptors_tool/ich_descriptors_tool.c 3 files changed, 265 insertions(+), 163 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/21809/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 4c18dfa..a2aa940 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -42,6 +42,36 @@ #define min(a, b) (((a) < (b)) ? (a) : (b)) #endif +ssize_t ich_number_of_regions(const enum ich_chipset cs, const struct ich_desc_content *const cont) +{ + switch (cs) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + return 10; + case CHIPSET_9_SERIES_WILDCAT_POINT_LP: + case CHIPSET_9_SERIES_WILDCAT_POINT: + case CHIPSET_8_SERIES_LYNX_POINT_LP: + case CHIPSET_8_SERIES_LYNX_POINT: + case CHIPSET_8_SERIES_WELLSBURG: + if (cont->NR <= 6) + return cont->NR + 1; + else + return -1; + default: + if (cont->NR <= 4) + return cont->NR + 1; + else + return -1; + } +} + +ssize_t ich_number_of_masters(const enum ich_chipset cs, const struct ich_desc_content *const cont) +{ + if (cont->NM < MAX_NUM_MASTERS) + return cont->NM + 1; + else + return -1; +} + void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity, bool print_vcl) { print(verbosity, "BES=0x%x, ", (reg_val & VSCC_BES) >> VSCC_BES_OFF); @@ -62,10 +92,10 @@ void prettyprint_ich_descriptors(enum ich_chipset cs, const struct ich_descriptors *desc) { - prettyprint_ich_descriptor_content(&desc->content); + prettyprint_ich_descriptor_content(cs, &desc->content); prettyprint_ich_descriptor_component(cs, desc); - prettyprint_ich_descriptor_region(desc); - prettyprint_ich_descriptor_master(&desc->master); + prettyprint_ich_descriptor_region(cs, desc); + prettyprint_ich_descriptor_master(cs, desc); #ifdef ICH_DESCRIPTORS_FROM_DUMP_ONLY if (cs >= CHIPSET_ICH8) { prettyprint_ich_descriptor_upper_map(&desc->upper); @@ -74,7 +104,7 @@ #endif /* ICH_DESCRIPTORS_FROM_DUMP_ONLY */ } -void prettyprint_ich_descriptor_content(const struct ich_desc_content *cont) +void prettyprint_ich_descriptor_content(enum ich_chipset cs, const struct ich_desc_content *cont) { msg_pdbg2("=== Content Section ===\n"); msg_pdbg2("FLVALSIG 0x%08x\n", cont->FLVALSIG); @@ -84,16 +114,16 @@ msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); - msg_pdbg2("NR (Number of Regions): %5d\n", cont->NR + 1); - msg_pdbg2("FRBA (Flash Region Base Address): 0x%03x\n", getFRBA(cont)); - msg_pdbg2("NC (Number of Components): %5d\n", cont->NC + 1); - msg_pdbg2("FCBA (Flash Component Base Address): 0x%03x\n", getFCBA(cont)); - msg_pdbg2("ISL (ICH/PCH Strap Length): %5d\n", cont->ISL); - msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH Strap Base Address): 0x%03x\n", getFISBA(cont)); - msg_pdbg2("NM (Number of Masters): %5d\n", cont->NM + 1); - msg_pdbg2("FMBA (Flash Master Base Address): 0x%03x\n", getFMBA(cont)); - msg_pdbg2("MSL/PSL (MCH/PROC Strap Length): %5d\n", cont->MSL); - msg_pdbg2("FMSBA (Flash MCH/PROC Strap Base Address): 0x%03x\n", getFMSBA(cont)); + msg_pdbg2("NR (Number of Regions): %5zd\n", ich_number_of_regions(cs, cont)); + msg_pdbg2("FRBA (Flash Region Base Address): 0x%03x\n", getFRBA(cont)); + msg_pdbg2("NC (Number of Components): %5d\n", cont->NC + 1); + msg_pdbg2("FCBA (Flash Component Base Address): 0x%03x\n", getFCBA(cont)); + msg_pdbg2("ISL (ICH/PCH Strap Length): %5d\n", cont->ISL); + msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH Strap Base Address): 0x%03x\n", getFISBA(cont)); + msg_pdbg2("NM (Number of Masters): %5zd\n", ich_number_of_masters(cs, cont)); + msg_pdbg2("FMBA (Flash Master Base Address): 0x%03x\n", getFMBA(cont)); + msg_pdbg2("MSL/PSL (MCH/PROC Strap Length): %5d\n", cont->MSL); + msg_pdbg2("FMSBA (Flash MCH/PROC Strap Base Address): 0x%03x\n", getFMSBA(cont)); msg_pdbg2("\n"); } @@ -140,7 +170,8 @@ case CHIPSET_8_SERIES_LYNX_POINT_LP: case CHIPSET_8_SERIES_WELLSBURG: case CHIPSET_9_SERIES_WILDCAT_POINT: - case CHIPSET_9_SERIES_WILDCAT_POINT_LP: { + case CHIPSET_9_SERIES_WILDCAT_POINT_LP: + case CHIPSET_100_SERIES_SUNRISE_POINT: { uint8_t size_enc; if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; @@ -159,7 +190,7 @@ static const char *pprint_freq(enum ich_chipset cs, uint8_t value) { - static const char * const freq_str[8] = { + static const char *const freq_str[2][8] = { { "20 MHz", /* 000 */ "33 MHz", /* 001 */ "reserved", /* 010 */ @@ -168,7 +199,16 @@ "reserved", /* 101 */ "reserved", /* 110 */ "reserved" /* 111 */ - }; + }, { + "reserved", /* 000 */ + "reserved", /* 001 */ + "48 MHz", /* 010 */ + "reserved", /* 011 */ + "30 MHz", /* 100 */ + "reserved", /* 101 */ + "17 MHz", /* 110 */ + "reserved" /* 111 */ + } }; switch (cs) { case CHIPSET_ICH8: @@ -185,7 +225,9 @@ case CHIPSET_8_SERIES_WELLSBURG: case CHIPSET_9_SERIES_WILDCAT_POINT: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: - return freq_str[value]; + return freq_str[0][value]; + case CHIPSET_100_SERIES_SUNRISE_POINT: + return freq_str[1][value]; case CHIPSET_ICH_UNKNOWN: default: return "unknown"; @@ -198,6 +240,8 @@ msg_pdbg2("=== Component Section ===\n"); msg_pdbg2("FLCOMP 0x%08x\n", desc->component.FLCOMP); msg_pdbg2("FLILL 0x%08x\n", desc->component.FLILL ); + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) + msg_pdbg2("FLILL1 0x%08x\n", desc->component.FLILL1); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); @@ -216,9 +260,10 @@ if (cs > CHIPSET_6_SERIES_COUGAR_POINT) msg_pdbg2("Dual Output Fast Read Support: %sabled\n", desc->component.modes.dual_output ? "dis" : "en"); - if (desc->component.FLILL == 0) + if (desc->component.FLILL == 0 && + (cs != CHIPSET_100_SERIES_SUNRISE_POINT || desc->component.FLILL1 == 0)) { msg_pdbg2("No forbidden opcodes.\n"); - else { + } else { msg_pdbg2("Invalid instruction 0: 0x%02x\n", desc->component.invalid_instr0); msg_pdbg2("Invalid instruction 1: 0x%02x\n", @@ -227,76 +272,116 @@ desc->component.invalid_instr2); msg_pdbg2("Invalid instruction 3: 0x%02x\n", desc->component.invalid_instr3); + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + msg_pdbg2("Invalid instruction 4: 0x%02x\n", + desc->component.invalid_instr4); + msg_pdbg2("Invalid instruction 5: 0x%02x\n", + desc->component.invalid_instr5); + msg_pdbg2("Invalid instruction 6: 0x%02x\n", + desc->component.invalid_instr6); + msg_pdbg2("Invalid instruction 7: 0x%02x\n", + desc->component.invalid_instr7); + } } msg_pdbg2("\n"); } static void pprint_freg(const struct ich_desc_region *reg, uint32_t i) { - static const char *const region_names[5] = { - "Descr.", "BIOS", "ME", "GbE", "Platf." + static const char *const region_names[] = { + "Descr.", "BIOS", "ME", "GbE", "Platf.", "unknown", "unknown", "unknown", + "EC", "unknown", }; - if (i >= 5) { + if (i >= ARRAY_SIZE(region_names)) { msg_pdbg2("%s: region index too high.\n", __func__); return; } uint32_t base = ICH_FREG_BASE(reg->FLREGs[i]); uint32_t limit = ICH_FREG_LIMIT(reg->FLREGs[i]); - msg_pdbg2("Region %d (%-6s) ", i, region_names[i]); + msg_pdbg2("Region %d (%-7s) ", i, region_names[i]); if (base > limit) msg_pdbg2("is unused.\n"); else msg_pdbg2("0x%08x - 0x%08x\n", base, limit); } -void prettyprint_ich_descriptor_region(const struct ich_descriptors *desc) +void prettyprint_ich_descriptor_region(const enum ich_chipset cs, const struct ich_descriptors *const desc) { - uint8_t i; - uint8_t nr = desc->content.NR + 1; + size_t i; + const ssize_t nr = ich_number_of_regions(cs, &desc->content); msg_pdbg2("=== Region Section ===\n"); - if (nr > 5) { + if (nr < 0) { msg_pdbg2("%s: number of regions too high (%d).\n", __func__, - nr); + desc->content.NR + 1); return; } - for (i = 0; i < 5; i++) - msg_pdbg2("FLREG%d 0x%08x\n", i, desc->region.FLREGs[i]); + for (i = 0; i < nr; i++) + msg_pdbg2("FLREG%zu 0x%08x\n", i, desc->region.FLREGs[i]); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); - for (i = 0; i < 5; i++) + for (i = 0; i < nr; i++) pprint_freg(&desc->region, i); msg_pdbg2("\n"); } -void prettyprint_ich_descriptor_master(const struct ich_desc_master *mstr) +void prettyprint_ich_descriptor_master(const enum ich_chipset cs, const struct ich_descriptors *const desc) { + size_t i; + const ssize_t nm = ich_number_of_masters(cs, &desc->content); msg_pdbg2("=== Master Section ===\n"); - msg_pdbg2("FLMSTR1 0x%08x\n", mstr->FLMSTR1); - msg_pdbg2("FLMSTR2 0x%08x\n", mstr->FLMSTR2); - msg_pdbg2("FLMSTR3 0x%08x\n", mstr->FLMSTR3); + if (nm < 0) { + msg_pdbg2("%s: number of masters too high (%d).\n", __func__, + desc->content.NM + 1); + return; + } + for (i = 0; i < nm; i++) + msg_pdbg2("FLMSTR%zu 0x%08x\n", i + 1, desc->master.FLMSTRs[i]); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); - msg_pdbg2(" Descr. BIOS ME GbE Platf.\n"); - msg_pdbg2("BIOS %c%c %c%c %c%c %c%c %c%c\n", - (mstr->BIOS_descr_r) ?'r':' ', (mstr->BIOS_descr_w) ?'w':' ', - (mstr->BIOS_BIOS_r) ?'r':' ', (mstr->BIOS_BIOS_w) ?'w':' ', - (mstr->BIOS_ME_r) ?'r':' ', (mstr->BIOS_ME_w) ?'w':' ', - (mstr->BIOS_GbE_r) ?'r':' ', (mstr->BIOS_GbE_w) ?'w':' ', - (mstr->BIOS_plat_r) ?'r':' ', (mstr->BIOS_plat_w) ?'w':' '); - msg_pdbg2("ME %c%c %c%c %c%c %c%c %c%c\n", - (mstr->ME_descr_r) ?'r':' ', (mstr->ME_descr_w) ?'w':' ', - (mstr->ME_BIOS_r) ?'r':' ', (mstr->ME_BIOS_w) ?'w':' ', - (mstr->ME_ME_r) ?'r':' ', (mstr->ME_ME_w) ?'w':' ', - (mstr->ME_GbE_r) ?'r':' ', (mstr->ME_GbE_w) ?'w':' ', - (mstr->ME_plat_r) ?'r':' ', (mstr->ME_plat_w) ?'w':' '); - msg_pdbg2("GbE %c%c %c%c %c%c %c%c %c%c\n", - (mstr->GbE_descr_r) ?'r':' ', (mstr->GbE_descr_w) ?'w':' ', - (mstr->GbE_BIOS_r) ?'r':' ', (mstr->GbE_BIOS_w) ?'w':' ', - (mstr->GbE_ME_r) ?'r':' ', (mstr->GbE_ME_w) ?'w':' ', - (mstr->GbE_GbE_r) ?'r':' ', (mstr->GbE_GbE_w) ?'w':' ', - (mstr->GbE_plat_r) ?'r':' ', (mstr->GbE_plat_w) ?'w':' '); + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + const char *const master_names[] = { + "BIOS", "ME", "GbE", "unknown", "EC", + }; + if (nm >= ARRAY_SIZE(master_names)) { + msg_pdbg2("%s: number of masters too high (%d).\n", __func__, + desc->content.NM + 1); + return; + } + + msg_pdbg2(" FD BIOS ME GbE Pltf Reg5 Reg6 Reg7 EC Reg9\n"); + for (i = 0; i < nm; i++) { + size_t j; + msg_pdbg2("%-4s", master_names[i]); + for (j = 0; j < 10; j++) + msg_pdbg2(" %c%c ", + desc->master.mstr[i].read & (1 << j) ? 'r' : ' ', + desc->master.mstr[i].write & (1 << j) ? 'w' : ' '); + msg_pdbg2("\n"); + } + } else { + const struct ich_desc_master *const mstr = &desc->master; + msg_pdbg2(" Descr. BIOS ME GbE Platf.\n"); + msg_pdbg2("BIOS %c%c %c%c %c%c %c%c %c%c\n", + (mstr->BIOS_descr_r) ?'r':' ', (mstr->BIOS_descr_w) ?'w':' ', + (mstr->BIOS_BIOS_r) ?'r':' ', (mstr->BIOS_BIOS_w) ?'w':' ', + (mstr->BIOS_ME_r) ?'r':' ', (mstr->BIOS_ME_w) ?'w':' ', + (mstr->BIOS_GbE_r) ?'r':' ', (mstr->BIOS_GbE_w) ?'w':' ', + (mstr->BIOS_plat_r) ?'r':' ', (mstr->BIOS_plat_w) ?'w':' '); + msg_pdbg2("ME %c%c %c%c %c%c %c%c %c%c\n", + (mstr->ME_descr_r) ?'r':' ', (mstr->ME_descr_w) ?'w':' ', + (mstr->ME_BIOS_r) ?'r':' ', (mstr->ME_BIOS_w) ?'w':' ', + (mstr->ME_ME_r) ?'r':' ', (mstr->ME_ME_w) ?'w':' ', + (mstr->ME_GbE_r) ?'r':' ', (mstr->ME_GbE_w) ?'w':' ', + (mstr->ME_plat_r) ?'r':' ', (mstr->ME_plat_w) ?'w':' '); + msg_pdbg2("GbE %c%c %c%c %c%c %c%c %c%c\n", + (mstr->GbE_descr_r) ?'r':' ', (mstr->GbE_descr_w) ?'w':' ', + (mstr->GbE_BIOS_r) ?'r':' ', (mstr->GbE_BIOS_w) ?'w':' ', + (mstr->GbE_ME_r) ?'r':' ', (mstr->GbE_ME_w) ?'w':' ', + (mstr->GbE_GbE_r) ?'r':' ', (mstr->GbE_GbE_w) ?'w':' ', + (mstr->GbE_plat_r) ?'r':' ', (mstr->GbE_plat_w) ?'w':' '); + } msg_pdbg2("\n"); } @@ -739,8 +824,8 @@ * only valid value since. Skylake is currently the most important dis- * tinction because of the dropped number of regions field (NR). */ -enum ich_chipset guess_ich_chipset(const struct ich_desc_content *const content, - const struct ich_desc_component *const component) +static enum ich_chipset guess_ich_chipset(const struct ich_desc_content *const content, + const struct ich_desc_component *const component) { const enum ich_chipset guess = guess_ich_chipset_from_content(content); @@ -765,9 +850,10 @@ } /* len is the length of dump in bytes */ -int read_ich_descriptors_from_dump(const uint32_t *dump, unsigned int len, struct ich_descriptors *desc) +int read_ich_descriptors_from_dump(const uint32_t *const dump, const size_t len, + enum ich_chipset *const cs, struct ich_descriptors *const desc) { - unsigned int i, max_count; + size_t i, max_count; uint8_t pch_bug_offset = 0; if (dump == NULL || desc == NULL) @@ -795,21 +881,22 @@ desc->component.FLILL = dump[(getFCBA(&desc->content) >> 2) + 1]; desc->component.FLPB = dump[(getFCBA(&desc->content) >> 2) + 2]; + if (*cs == CHIPSET_ICH_UNKNOWN) + *cs = guess_ich_chipset(&desc->content, &desc->component); + /* region */ - if (len < getFRBA(&desc->content) + 5 * 4) + const ssize_t nr = ich_number_of_regions(*cs, &desc->content); + if (nr < 0 || len < getFRBA(&desc->content) + nr * 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]; - desc->region.FLREGs[2] = dump[(getFRBA(&desc->content) >> 2) + 2]; - desc->region.FLREGs[3] = dump[(getFRBA(&desc->content) >> 2) + 3]; - desc->region.FLREGs[4] = dump[(getFRBA(&desc->content) >> 2) + 4]; + for (i = 0; i < nr; i++) + desc->region.FLREGs[i] = dump[(getFRBA(&desc->content) >> 2) + i]; /* master */ - if (len < getFMBA(&desc->content) + 3 * 4) + const ssize_t nm = ich_number_of_masters(*cs, &desc->content); + if (nm < 0 || len < getFMBA(&desc->content) + nm * 4) return ICH_RET_OOB; - desc->master.FLMSTR1 = dump[(getFMBA(&desc->content) >> 2) + 0]; - desc->master.FLMSTR2 = dump[(getFMBA(&desc->content) >> 2) + 1]; - desc->master.FLMSTR3 = dump[(getFMBA(&desc->content) >> 2) + 2]; + for (i = 0; i < nm; i++) + desc->master.FLMSTRs[i] = dump[(getFMBA(&desc->content) >> 2) + i]; /* upper map */ desc->upper.FLUMAP1 = dump[(UPPER_MAP_OFFSET >> 2) + 0]; @@ -929,8 +1016,7 @@ int read_ich_descriptors_via_fdo(enum ich_chipset cs, void *spibar, struct ich_descriptors *desc) { - uint8_t i; - uint8_t nr; + size_t i; struct ich_desc_region *r = &desc->region; /* Test if bit-fields are working as expected. @@ -938,20 +1024,20 @@ */ for (i = 0; i < 4; i++) desc->region.FLREGs[i] = 0x5A << (i * 8); - if (r->reg0_base != 0x005A || r->reg0_limit != 0x0000 || - r->reg1_base != 0x1A00 || r->reg1_limit != 0x0000 || - r->reg2_base != 0x0000 || r->reg2_limit != 0x005A || - r->reg3_base != 0x0000 || r->reg3_limit != 0x1A00) { + if (r->old_reg[0].base != 0x005A || r->old_reg[0].limit != 0x0000 || + r->old_reg[1].base != 0x1A00 || r->old_reg[1].limit != 0x0000 || + r->old_reg[2].base != 0x0000 || r->old_reg[2].limit != 0x005A || + r->old_reg[3].base != 0x0000 || r->old_reg[3].limit != 0x1A00) { msg_pdbg("The combination of compiler and CPU architecture used" "does not lay out bit-fields as expected, sorry.\n"); - msg_pspew("r->reg0_base = 0x%04X (0x005A)\n", r->reg0_base); - msg_pspew("r->reg0_limit = 0x%04X (0x0000)\n", r->reg0_limit); - msg_pspew("r->reg1_base = 0x%04X (0x1A00)\n", r->reg1_base); - msg_pspew("r->reg1_limit = 0x%04X (0x0000)\n", r->reg1_limit); - msg_pspew("r->reg2_base = 0x%04X (0x0000)\n", r->reg2_base); - msg_pspew("r->reg2_limit = 0x%04X (0x005A)\n", r->reg2_limit); - msg_pspew("r->reg3_base = 0x%04X (0x0000)\n", r->reg3_base); - msg_pspew("r->reg3_limit = 0x%04X (0x1A00)\n", r->reg3_limit); + msg_pspew("r->old_reg[0].base = 0x%04X (0x005A)\n", r->old_reg[0].base); + msg_pspew("r->old_reg[0].limit = 0x%04X (0x0000)\n", r->old_reg[0].limit); + msg_pspew("r->old_reg[1].base = 0x%04X (0x1A00)\n", r->old_reg[1].base); + msg_pspew("r->old_reg[1].limit = 0x%04X (0x0000)\n", r->old_reg[1].limit); + msg_pspew("r->old_reg[2].base = 0x%04X (0x0000)\n", r->old_reg[2].base); + msg_pspew("r->old_reg[2].limit = 0x%04X (0x005A)\n", r->old_reg[2].limit); + msg_pspew("r->old_reg[3].base = 0x%04X (0x0000)\n", r->old_reg[3].base); + msg_pspew("r->old_reg[3].limit = 0x%04X (0x1A00)\n", r->old_reg[3].limit); return ICH_RET_ERR; } @@ -968,19 +1054,24 @@ desc->component.FLPB = read_descriptor_reg(cs, 1, 2, spibar); /* region section */ - nr = desc->content.NR + 1; - if (nr > 5) { + const ssize_t nr = ich_number_of_regions(cs, &desc->content); + if (nr < 0) { msg_pdbg2("%s: number of regions too high (%d) - failed\n", - __func__, nr); + __func__, desc->content.NR + 1); return ICH_RET_ERR; } - for (i = 0; i < 5; i++) + for (i = 0; i < nr; i++) desc->region.FLREGs[i] = read_descriptor_reg(cs, 2, i, spibar); /* master section */ - desc->master.FLMSTR1 = read_descriptor_reg(cs, 3, 0, spibar); - desc->master.FLMSTR2 = read_descriptor_reg(cs, 3, 1, spibar); - desc->master.FLMSTR3 = read_descriptor_reg(cs, 3, 2, spibar); + const ssize_t nm = ich_number_of_masters(cs, &desc->content); + if (nm < 0) { + msg_pdbg2("%s: number of masters too high (%d) - failed\n", + __func__, desc->content.NM + 1); + return ICH_RET_ERR; + } + for (i = 0; i < nm; i++) + desc->master.FLMSTRs[i] = read_descriptor_reg(cs, 3, i, spibar); /* Accessing the strap section via FDOC/D is only possible on ICH8 and * reading the upper map is impossible on all chipsets, so don't bother. @@ -1003,16 +1094,19 @@ */ int layout_from_ich_descriptors(struct ich_layout *const layout, const void *const dump, const size_t len) { - static const char *regions[] = { "fd", "bios", "me", "gbe", "pd" }; + static const char *const regions[] = { + "fd", "bios", "me", "gbe", "pd", "reg5", "reg6", "reg7", "ec", "reg9" + }; struct ich_descriptors desc; - if (read_ich_descriptors_from_dump(dump, len, &desc)) + enum ich_chipset cs = CHIPSET_ICH_UNKNOWN; + if (read_ich_descriptors_from_dump(dump, len, &cs, &desc)) return 1; memset(layout, 0x00, sizeof(*layout)); - size_t i, j; - for (i = 0, j = 0; i < min(desc.content.NR + 1, ARRAY_SIZE(regions)); ++i) { + ssize_t i, j; + for (i = 0, j = 0; i < min(ich_number_of_regions(cs, &desc.content), ARRAY_SIZE(regions)); ++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) diff --git a/ich_descriptors.h b/ich_descriptors.h index 920e098..bccea27 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -62,8 +62,8 @@ #define VSCC_VCL (0x1 << VSCC_VCL_OFF) /* 24-31: reserved */ -#define ICH_FREG_BASE(flreg) (((flreg) << 12) & 0x01fff000) -#define ICH_FREG_LIMIT(flreg) ((((flreg) >> 4) & 0x01fff000) | 0x00000fff) +#define ICH_FREG_BASE(flreg) (((flreg) << 12) & 0x07fff000) +#define ICH_FREG_LIMIT(flreg) ((((flreg) >> 4) & 0x07fff000) | 0x00000fff) void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity, bool print_vcl); @@ -76,7 +76,7 @@ NC :2, /* Number Of Components */ :6, FRBA :8, /* Flash Region Base Address */ - NR :3, /* Number Of Regions */ + NR :3, /* Number Of Regions (reserved from Skylake on) */ :5; }; }; @@ -116,7 +116,8 @@ * Patsburg: 50 30 5 0:2, 3:5 * Panther Point/7 50 30 5 0:2, 3:5 * Lynx Point/8: 50 30 7 0:3, 4:7 - * Wildcat Point/9: 50 ?? (multi I/O) ? ?:?, ?:? + * Wildcat Point/9: 50 30 (multi I/O) 7 0:3, 4:7 + * Sunrise Point/100: 48 30 7 0:3, 4:7 */ struct { uint32_t :17, @@ -149,55 +150,49 @@ }; }; union { /* 0x08 */ - uint32_t FLPB; /* Flash Partition Boundary Register */ + uint32_t FLPB; /* Flash Partition Boundary Register, until Panther Point/7 */ struct { uint32_t FPBA :13, /* Flash Partition Boundary Addr */ :19; }; - }; -}; - -struct ich_desc_region { - union { - uint32_t FLREGs[5]; + uint32_t FLILL1; /* Flash Invalid Instructions Register, new since Sunrise Point/100 */ struct { - struct { /* FLREG0 Flash Descriptor */ - uint32_t reg0_base :13, - :3, - reg0_limit :13, - :3; - }; - struct { /* FLREG1 BIOS */ - uint32_t reg1_base :13, - :3, - reg1_limit :13, - :3; - }; - struct { /* FLREG2 ME */ - uint32_t reg2_base :13, - :3, - reg2_limit :13, - :3; - }; - struct { /* FLREG3 GbE */ - uint32_t reg3_base :13, - :3, - reg3_limit :13, - :3; - }; - struct { /* FLREG4 Platform */ - uint32_t reg4_base :13, - :3, - reg4_limit :13, - :3; - }; + uint32_t invalid_instr4 :8, + invalid_instr5 :8, + invalid_instr6 :8, + invalid_instr7 :8; }; }; }; +#define MAX_NUM_FLREGS 10 /* 16 on unsupported Lewisburg PCH */ +struct ich_desc_region { + /* + * Number of entries and width differ on various generations: + * + * Chipset/Generation #FLREGs width (bits) + * ICH8 .. Panther Point/7 5 13 + * Lynx Point/8 .. Wildcat Point/9 7 15 + * Sunrise Point/100 .. 10 15 + */ + union { + uint32_t FLREGs[MAX_NUM_FLREGS]; /* Flash Descriptor Regions */ + + /* only used for bit-field check */ + struct { + uint32_t base :13, + :3, + limit :13, + :3; + } old_reg[MAX_NUM_FLREGS]; + }; +}; + +#define MAX_NUM_MASTERS 5 /* 6 on unsupported Lewisburg PCH */ struct ich_desc_master { union { - uint32_t FLMSTR1; + uint32_t FLMSTRs[MAX_NUM_MASTERS]; /* Flash Masters */ + /* For pre-Skylake platforms */ struct { uint32_t BIOS_req_ID :16, BIOS_descr_r :1, @@ -212,11 +207,6 @@ BIOS_GbE_w :1, BIOS_plat_w :1, :3; - }; - }; - union { - uint32_t FLMSTR2; - struct { uint32_t ME_req_ID :16, ME_descr_r :1, ME_BIOS_r :1, @@ -230,11 +220,6 @@ ME_GbE_w :1, ME_plat_w :1, :3; - }; - }; - union { - uint32_t FLMSTR3; - struct { uint32_t GbE_req_ID :16, GbE_descr_r :1, GbE_BIOS_r :1, @@ -249,6 +234,12 @@ GbE_plat_w :1, :3; }; + /* From Skylake on */ + struct { + uint32_t :8, + read :12, + write :12; + } mstr[MAX_NUM_MASTERS]; }; }; @@ -573,16 +564,19 @@ struct ich_desc_upper_map upper; }; +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); + void prettyprint_ich_descriptors(enum ich_chipset cs, const struct ich_descriptors *desc); -void prettyprint_ich_descriptor_content(const struct ich_desc_content *content); +void prettyprint_ich_descriptor_content(enum ich_chipset cs, const struct ich_desc_content *content); void prettyprint_ich_descriptor_component(enum ich_chipset cs, const struct ich_descriptors *desc); -void prettyprint_ich_descriptor_region(const struct ich_descriptors *desc); -void prettyprint_ich_descriptor_master(const struct ich_desc_master *master); +void prettyprint_ich_descriptor_region(enum ich_chipset cs, const struct ich_descriptors *desc); +void prettyprint_ich_descriptor_master(enum ich_chipset cs, const struct ich_descriptors *desc); void prettyprint_ich_descriptor_upper_map(const struct ich_desc_upper_map *umap); void prettyprint_ich_descriptor_straps(enum ich_chipset cs, const struct ich_descriptors *desc); -int read_ich_descriptors_from_dump(const uint32_t *dump, unsigned int len, struct ich_descriptors *desc); +int read_ich_descriptors_from_dump(const uint32_t *dump, size_t len, enum ich_chipset *cs, struct ich_descriptors *desc); 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); diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 7dfa5c0..82c74bd 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -40,15 +40,18 @@ #include <sys/mman.h> #endif -static void dump_file(const char *prefix, const uint32_t *dump, unsigned int len, struct ich_desc_region *reg, unsigned int i) +static const char *const region_names[] = { + "Descriptor", "BIOS", "ME", "GbE", "Platform", + "Region5", "Region6", "Region7", "EC", "Region9", +}; + +static void dump_file(const char *prefix, const uint32_t *dump, unsigned int len, + const struct ich_desc_region *const reg, unsigned int i) { int ret; char *fn; const char *reg_name; uint32_t file_len; - const char *const region_names[5] = { - "Descriptor", "BIOS", "ME", "GbE", "Platform" - }; uint32_t base = ICH_FREG_BASE(reg->FLREGs[i]); uint32_t limit = ICH_FREG_LIMIT(reg->FLREGs[i]); @@ -94,12 +97,19 @@ close(fh); } -void dump_files(const char *name, const uint32_t *buf, unsigned int len, struct ich_desc_region *reg) +int min(int a, int b) { - unsigned int i; + return (a < b) ? a : b; +} + +static void dump_files(const char *name, const uint32_t *buf, unsigned int len, + const enum ich_chipset cs, const struct ich_descriptors *const desc) +{ + ssize_t i; + const ssize_t nr = min(ich_number_of_regions(cs, &desc->content), ARRAY_SIZE(region_names)); printf("=== Dumping region files ===\n"); - for (i = 0; i < 5; i++) - dump_file(name, buf, len, reg, i); + for (i = 0; i < nr; i++) + dump_file(name, buf, len, &desc->region, i); printf("\n"); } @@ -123,6 +133,7 @@ "\t- \"7\" or \"panther\" for Intel's 7 series chipsets.\n" "\t- \"8\" or \"lynx\" for Intel's 8 series chipsets.\n" "\t- \"9\" or \"wildcat\" for Intel's 9 series chipsets.\n" +"\t- \"100\" or \"sunrise\" for Intel's 100 series chipsets.\n" "If '-d' is specified some regions such as the BIOS image as seen by the CPU or\n" "the GbE blob that is required to initialize the GbE are also dumped to files.\n", argv[0], argv[0]); @@ -208,9 +219,12 @@ else if ((strcmp(csn, "9") == 0) || (strcmp(csn, "wildcat") == 0)) cs = CHIPSET_9_SERIES_WILDCAT_POINT; + else if ((strcmp(csn, "100") == 0) || + (strcmp(csn, "sunrise") == 0)) + cs = CHIPSET_100_SERIES_SUNRISE_POINT; } - ret = read_ich_descriptors_from_dump(buf, len, &desc); + ret = read_ich_descriptors_from_dump(buf, len, &cs, &desc); switch (ret) { case ICH_RET_OK: break; @@ -228,15 +242,15 @@ prettyprint_ich_descriptors(cs, &desc); - pMAC = (uint8_t *) &buf[ICH_FREG_BASE(desc.region.reg3_base) >> 2]; - if (len >= ICH_FREG_BASE(desc.region.reg3_base) + 6 && pMAC[0] != 0xff) + pMAC = (uint8_t *) &buf[ICH_FREG_BASE(desc.region.FLREGs[3]) >> 2]; + if (len >= ICH_FREG_BASE(desc.region.FLREGs[3]) + 6 && pMAC[0] != 0xff) printf("The MAC address might be at offset 0x%x: " "%02x:%02x:%02x:%02x:%02x:%02x\n", - ICH_FREG_BASE(desc.region.reg3_base), + ICH_FREG_BASE(desc.region.FLREGs[3]), pMAC[0], pMAC[1], pMAC[2], pMAC[3], pMAC[4], pMAC[5]); if (dump == 1) - dump_files(fn, buf, len, &desc.region); + dump_files(fn, buf, len, cs, &desc); return 0; } -- To view, visit https://review.coreboot.org/21809 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I34ac844dd414db6f8957763846cbfba75301bae5 Gerrit-Change-Number: 21809 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: ich_descriptors: Add function to guess chipset version
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21808 Change subject: ich_descriptors: Add function to guess chipset version ...................................................................... ich_descriptors: Add function to guess chipset version Add guess_ich_chipset() that takes fields from a descriptor dump and returns the lowest possible chipset version. Intel did several incompatible changes to the descriptor through the years. However, they forgot to add a version number. So we have to apply some heuristics to detect the chipset version in case of exter- nal flashing. Original-Change-Id: Ie1736663dc33801b19d3e695c072c61a6c6345a2 Original-Reviewed-on: https://review.coreboot.org/20246 Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I3968901a9e82a34356e8c5edb14354b82bf27305 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ich_descriptors.c M ich_descriptors.h 2 files changed, 66 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/21808/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 3e53ec9..4c18dfa 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -702,6 +702,68 @@ msg_pdbg2("\n"); } +/* + * Guesses a minimum chipset version based on the maximum number of + * soft straps per generation. + */ +static enum ich_chipset guess_ich_chipset_from_content(const struct ich_desc_content *const content) +{ + if (content->ICCRIBA == 0x00) { + if (content->MSL == 0 && content->ISL <= 2) + return CHIPSET_ICH8; + else if (content->ISL <= 2) + return CHIPSET_ICH9; + else if (content->ISL <= 10) + return CHIPSET_ICH10; + else if (content->ISL <= 16) + return CHIPSET_5_SERIES_IBEX_PEAK; + msg_pwarn("Peculiar firmware descriptor, assuming Ibex Peak compatibility.\n"); + return CHIPSET_5_SERIES_IBEX_PEAK; + } else if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) { + if (content->MSL == 0 && content->ISL <= 17) + return CHIPSET_BAYTRAIL; + else if (content->MSL <= 1 && content->ISL <= 18) + return CHIPSET_6_SERIES_COUGAR_POINT; + else if (content->MSL <= 1 && content->ISL <= 21) + return CHIPSET_8_SERIES_LYNX_POINT; + msg_pwarn("Peculiar firmware descriptor, assuming Wildcat Point compatibility.\n"); + return CHIPSET_9_SERIES_WILDCAT_POINT; + } else { + return CHIPSET_100_SERIES_SUNRISE_POINT; + } +} + +/* + * As an additional measure, we check the read frequency like `ifdtool`. + * The frequency value 6 (17MHz) was reserved before Skylake and is the + * only valid value since. Skylake is currently the most important dis- + * tinction because of the dropped number of regions field (NR). + */ +enum ich_chipset guess_ich_chipset(const struct ich_desc_content *const content, + const struct ich_desc_component *const component) +{ + const enum ich_chipset guess = guess_ich_chipset_from_content(content); + + if (component->modes.freq_read == 6) { + if (guess != CHIPSET_100_SERIES_SUNRISE_POINT) + msg_pwarn("\nThe firmware descriptor has the read frequency set to 17MHz. However,\n" + "it doesn't look like a Skylake/Sunrise Point compatible descriptor.\n" + "Please report this message, the output of `ich_descriptors_tool` for\n" + "your descriptor and the output of `lspci -nn` to flashrom(a)flashrom.org\n\n"); + return CHIPSET_100_SERIES_SUNRISE_POINT; + } else { + if (guess == CHIPSET_100_SERIES_SUNRISE_POINT) { + msg_pwarn("\nThe firmware descriptor looks like a Skylake/Sunrise Point descriptor.\n" + "However, the read frequency isn't set to 17MHz (the only valid value).\n" + "Please report this message, the output of `ich_descriptors_tool` for\n" + "your descriptor and the output of `lspci -nn` to flashrom(a)flashrom.org\n\n"); + return CHIPSET_9_SERIES_WILDCAT_POINT; + } + } + + return guess; +} + /* len is the length of dump in bytes */ int read_ich_descriptors_from_dump(const uint32_t *dump, unsigned int len, struct ich_descriptors *desc) { diff --git a/ich_descriptors.h b/ich_descriptors.h index ec85e0c..920e098 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -93,9 +93,10 @@ union { /* 0x0c */ uint32_t FLMAP2; struct { - uint32_t FMSBA :8, /* Flash (G)MCH Strap Base Addr. */ - MSL :8, /* MCH Strap Length */ - :16; + uint32_t FMSBA :8, /* Flash (G)MCH Strap Base Addr. */ + MSL :8, /* MCH Strap Length */ + ICCRIBA :8, /* ICC Reg. Init Base Addr. (new since Sandy Bridge) */ + RIL :8; /* Register Init Length (new since Hawell) */ }; }; }; -- To view, visit https://review.coreboot.org/21808 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I3968901a9e82a34356e8c5edb14354b82bf27305 Gerrit-Change-Number: 21808 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: ich_descriptors: Draw +0xfff into ICH_FREG_LIMIT()
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21807 Change subject: ich_descriptors: Draw +0xfff into ICH_FREG_LIMIT() ...................................................................... ich_descriptors: Draw +0xfff into ICH_FREG_LIMIT() The condition `base > limit` is still valid since `base` is always at least 4096 greater than `limit` in this case. Original-Change-Id: I11ac0a50b3f32f47879e7cfb7a26068cd0572ede Original-Reviewed-on: https://review.coreboot.org/19046 Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: Idfa1a13292e1dcd5e92bb2dbfe11392693952698 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ich_descriptors.c M ich_descriptors.h M ichspi.c M util/ich_descriptors_tool/ich_descriptors_tool.c 4 files changed, 6 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/21807/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 1fc8835..3e53ec9 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -246,7 +246,7 @@ if (base > limit) msg_pdbg2("is unused.\n"); else - msg_pdbg2("0x%08x - 0x%08x\n", base, limit | 0x0fff); + msg_pdbg2("0x%08x - 0x%08x\n", base, limit); } void prettyprint_ich_descriptor_region(const struct ich_descriptors *desc) @@ -952,7 +952,7 @@ size_t i, j; for (i = 0, j = 0; i < min(desc.content.NR + 1, ARRAY_SIZE(regions)); ++i) { const chipoff_t base = ICH_FREG_BASE(desc.region.FLREGs[i]); - const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]) + 0xfff; + const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]); if (limit <= base) continue; layout->entries[j].start = base; diff --git a/ich_descriptors.h b/ich_descriptors.h index 831341a..ec85e0c 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -63,7 +63,7 @@ /* 24-31: reserved */ #define ICH_FREG_BASE(flreg) (((flreg) << 12) & 0x01fff000) -#define ICH_FREG_LIMIT(flreg) (((flreg) >> 4) & 0x01fff000) +#define ICH_FREG_LIMIT(flreg) ((((flreg) >> 4) & 0x01fff000) | 0x00000fff) void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity, bool print_vcl); diff --git a/ichspi.c b/ichspi.c index ecf0c42..5f3e394 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1566,14 +1566,12 @@ msg_pdbg("0x%02X: 0x%08x ", offset, freg); if (rwperms == 0x3) { msg_pdbg("FREG%i: %s region (0x%08x-0x%08x) is %s.\n", i, - region_name, base, (limit | 0x0fff), - access_names[rwperms]); + region_name, base, limit, access_names[rwperms]); return 0; } msg_pwarn("FREG%i: Warning: %s region (0x%08x-0x%08x) is %s.\n", i, - region_name, base, (limit | 0x0fff), - access_names[rwperms]); + region_name, base, limit, access_names[rwperms]); return 1; } @@ -1608,7 +1606,7 @@ msg_pdbg("0x%02X: 0x%08x ", off, pr); msg_pwarn("%sPR%u: Warning: 0x%08x-0x%08x is %s.\n", prefix, i, ICH_FREG_BASE(pr), - ICH_FREG_LIMIT(pr) | 0x0fff, access_names[rwperms]); + ICH_FREG_LIMIT(pr), access_names[rwperms]); return 1; } diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 250abf0..7dfa5c0 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -58,7 +58,6 @@ return; } - limit = limit | 0x0fff; file_len = limit + 1 - base; if (base + file_len > len) { printf("The %s region is spanning 0x%08x-0x%08x, but it is " -- To view, visit https://review.coreboot.org/21807 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Idfa1a13292e1dcd5e92bb2dbfe11392693952698 Gerrit-Change-Number: 21807 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit