flashrom

flashrom@flashrom.org

7 participants
9163 discussions

Re: [flashrom] [PATCH] Add an internal DMI decoder.
by Stefan Tauner 12 Sep '13

12 Sep '13

On Wed, 28 Aug 2013 18:16:12 +0200 Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> wrote: > Previously we had to rely on dmidecode to decode the DMI/SMBIOS table. > This patch integrates a DMI decoder into flashrom. The old behavior of calling > dmidecode can be brought back by using CONFIG_INTERNAL_DMI=no. > > Significant portions of this patch were taken from dmidecode, hence add its > authors to the copyright notice (dmidecode is also GPL2+). We do a few things > differently though. First of all we do more bounds checking to prevent accessing > unmapped memory. We do not support disovery via EFI (yet), but memory > scanning only. We handle the chassis-type lock bit correctly which dmidecode > did not for a long while. > > The API to the rest of flashrom remains stable, but the output changes slightly. > To share as much code as possible (which actually is not much), i have added > dmi_fill methods that get called in dmi_init. They are reponsible to fill the > dmi_strings array and setting the is_laptop variable. After it is called, dmi_init > prints all dmi_strings. Previously the strings were printed in the order they were > discovered, followed by the chassis-type, which is now output earlier (in dmi_fill). > > Because DJGPP does not support strnlen a simple implementation was added > for it. > > This is still only available on x86; actually it is not even compiled in for other > architectures at all anymore. > > Signed-off-by: Sean Nelson<audiohacked(a)gmail.com> > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > Tested-by: Maciej Pijanka <maciej.pijanka(a)gmail.com> > Tested-by: Idwer Vollering <vidwer(a)gmail.com> > Acked-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Committed in r1735 after testing it with FreeDOS too. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

Re: [flashrom] [PATCH] DOS refinements.
by Stefan Tauner 12 Sep '13

12 Sep '13

On Fri, 6 Sep 2013 11:24:40 +0200 Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> wrote: > This allows to use the DOS library trees stored in a user-specified directory. > > I have mirrored the needed patches, sources and binaries (the latter > are properly licensed to allow that) in the flashrom wiki, so use those > URLs instead of the original sources. > > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> self-acked and committed in r1734. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

[commit] r1735 - trunk
by repository service 12 Sep '13

12 Sep '13

Author: stefanct Date: Thu Sep 12 01:35:03 2013 New Revision: 1735 URL: http://flashrom.org/trac/flashrom/changeset/1735 Log: Add an internal DMI decoder. Previously we had to rely on dmidecode to decode the DMI/SMBIOS table. This patch integrates a DMI decoder into flashrom. The old behavior of calling dmidecode can be brought back by using CONFIG_INTERNAL_DMI=no. Significant portions of this patch were taken from dmidecode, hence add its authors to the copyright notice (dmidecode is also GPL2+). We do a few things differently though. First of all we do more bounds checking to prevent accessing unmapped memory. We do not support disovery via EFI (yet), but memory scanning only. We handle the chassis-type lock bit correctly which dmidecode did not for a long while. The API to the rest of flashrom remains stable, but the output changes slightly. To share as much code as possible (which actually is not much), i have added dmi_fill methods that get called in dmi_init. They are reponsible to fill the dmi_strings array and setting the is_laptop variable. After it is called, dmi_init prints all dmi_strings. Previously the strings were printed in the order they were discovered, followed by the chassis-type, which is now output earlier (in dmi_fill). Because DJGPP does not support strnlen a simple implementation was added for it. This is still only available on x86; actually it is not even compiled in for other architectures at all anymore. Signed-off-by: Sean Nelson<audiohacked(a)gmail.com> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Tested-by: Maciej Pijanka <maciej.pijanka(a)gmail.com> Tested-by: Idwer Vollering <vidwer(a)gmail.com> Acked-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Modified: trunk/Makefile trunk/board_enable.c trunk/dmi.c trunk/physmap.c trunk/programmer.h Modified: trunk/Makefile ============================================================================== --- trunk/Makefile Thu Sep 12 01:34:57 2013 (r1734) +++ trunk/Makefile Thu Sep 12 01:35:03 2013 (r1735) @@ -440,6 +440,23 @@ endif ############################################################################### +# Handle CONFIG_* variables that depend on others set (and verified) above. + +# The external DMI decoder (dmidecode) does not work in libpayload. Bail out if the internal one got disabled. +ifeq ($(TARGET_OS), libpayload) +ifeq ($(CONFIG_INTERNAL), yes) +ifeq ($(CONFIG_INTERNAL_DMI), no) +UNSUPPORTED_FEATURES += CONFIG_INTERNAL_DMI=no +else +override CONFIG_INTERNAL_DMI = yes +endif +endif +endif + +# Use internal DMI/SMBIOS decoder by default instead of relying on dmidecode. +CONFIG_INTERNAL_DMI ?= yes + +############################################################################### # Programmer drivers and programmer support infrastructure. # Depending on the CONFIG_* variables set and verified above we set compiler flags and parameters below. @@ -447,10 +464,13 @@ ifeq ($(CONFIG_INTERNAL), yes) FEATURE_CFLAGS += -D'CONFIG_INTERNAL=1' -PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o dmi.o internal.o +PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o internal.o ifeq ($(ARCH), x86) PROGRAMMER_OBJS += it87spi.o it85spi.o sb600spi.o amd_imc.o wbsio_spi.o mcp6x_spi.o -PROGRAMMER_OBJS += ichspi.o ich_descriptors.o +PROGRAMMER_OBJS += ichspi.o ich_descriptors.o dmi.o +ifeq ($(CONFIG_INTERNAL_DMI), yes) +FEATURE_CFLAGS += -D'CONFIG_INTERNAL_DMI=1' +endif else endif NEED_PCI := yes Modified: trunk/board_enable.c ============================================================================== --- trunk/board_enable.c Thu Sep 12 01:34:57 2013 (r1734) +++ trunk/board_enable.c Thu Sep 12 01:35:03 2013 (r1735) @@ -2560,6 +2560,7 @@ } } +#if defined(__i386__) || defined(__x86_64__) if (board->dmi_pattern) { if (!has_dmi_support) { msg_pwarn("Warning: Can't autodetect %s %s, DMI info unavailable.\n", @@ -2572,7 +2573,7 @@ continue; } } - +#endif // defined(__i386__) || defined(__x86_64__) return board; } Modified: trunk/dmi.c ============================================================================== --- trunk/dmi.c Thu Sep 12 01:34:57 2013 (r1734) +++ trunk/dmi.c Thu Sep 12 01:35:03 2013 (r1735) @@ -1,7 +1,10 @@ /* * This file is part of the flashrom project. * + * Copyright (C) 2000-2002 Alan Cox <alan(a)redhat.com> + * Copyright (C) 2002-2010 Jean Delvare <khali(a)linux-fr.org> * Copyright (C) 2009,2010 Michael Karcher + * Copyright (C) 2011-2013 Stefan Tauner * * 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 @@ -26,33 +29,28 @@ #include "flash.h" #include "programmer.h" -int has_dmi_support = 0; - -#if STANDALONE +#if defined(__i386__) || defined(__x86_64__) -/* Stub to indicate missing DMI functionality. - * has_dmi_support is 0 by default, so nothing to do here. - * Because dmidecode is not available on all systems, the goal is to implement - * the DMI subset we need directly in this file. - */ -void dmi_init(void) -{ -} +/* Enable SMBIOS decoding. Currently legacy DMI decoding is enough. */ +#define SM_SUPPORT 0 -int dmi_match(const char *pattern) -{ - return 0; -} +/* Strings longer than 4096 in DMI are just insane. */ +#define DMI_MAX_ANSWER_LEN 4096 -#else /* STANDALONE */ +int has_dmi_support = 0; -static const char *dmidecode_names[] = { - "system-manufacturer", - "system-product-name", - "system-version", - "baseboard-manufacturer", - "baseboard-product-name", - "baseboard-version", +static struct { + const char *const keyword; + const uint8_t type; + const uint8_t offset; + char *value; +} dmi_strings[] = { + { "system-manufacturer", 1, 0x04, NULL }, + { "system-product-name", 1, 0x05, NULL }, + { "system-version", 1, 0x06, NULL }, + { "baseboard-manufacturer", 2, 0x04, NULL }, + { "baseboard-product-name", 2, 0x05, NULL }, + { "baseboard-version", 2, 0x06, NULL }, }; /* This list is used to identify supposed laptops. The is_laptop field has the @@ -66,9 +64,9 @@ * The types below are the most common ones. */ static const struct { - unsigned char type; - unsigned char is_laptop; - const char *name; + uint8_t type; + uint8_t is_laptop; + char *name; } dmi_chassis_types[] = { {0x01, 2, "Other"}, {0x02, 2, "Unknown"}, @@ -86,20 +84,206 @@ {0x18, 0, "Sealed-case PC"}, /* used by Supermicro (X8SIE) */ }; -#define DMI_COMMAND_LEN_MAX 260 -static const char *dmidecode_command = "dmidecode"; +#if CONFIG_INTERNAL_DMI == 1 +#ifdef __DJGPP__ /* There is no strnlen in DJGPP. FIXME: Move this to a common utility file. */ +size_t strnlen(const char *str, size_t n) +{ + size_t i; + for (i = 0; i < n && str[i] != '\0'; i++) + ; + return i; +} +#endif -static char *dmistrings[ARRAY_SIZE(dmidecode_names)]; +static bool dmi_checksum(const uint8_t * const buf, size_t len) +{ + uint8_t sum = 0; + size_t a; -/* Strings longer than 4096 in DMI are just insane. */ -#define DMI_MAX_ANSWER_LEN 4096 + for (a = 0; a < len; a++) + sum += buf[a]; + return (sum == 0); +} + +static char *dmi_string(uint8_t *buf, uint8_t string_id, uint8_t *limit) +{ + size_t i, len; + + if (string_id == 0) + return "Not Specified"; + + while (string_id > 1 && string_id--) { + if (buf > limit) { + msg_perr("DMI table is broken (string portion out of bounds)!\n"); + return "<OUT OF BOUNDS>"; + } + buf += strnlen((char *)buf, limit - buf) + 1; + } + + if (!*buf) /* as long as the current byte we're on isn't null */ + return "<BAD INDEX>"; + + len = strnlen((char *)buf, limit - buf); + if (len > DMI_MAX_ANSWER_LEN) + len = DMI_MAX_ANSWER_LEN; + + /* fix junk bytes in the string */ + for (i = 0; i < len; i++) + if (buf[i] < 32 || buf[i] >= 127) + buf[i] = ' '; + + return (char *)buf; +} + +static void dmi_chassis_type(uint8_t code) +{ + int i; + code &= 0x7f; /* bits 6:0 are chassis type, 7th bit is the lock bit */ + is_laptop = 2; + for (i = 0; i < ARRAY_SIZE(dmi_chassis_types); i++) { + if (code == dmi_chassis_types[i].type) { + msg_pdbg("DMI string chassis-type: \"%s\"\n", dmi_chassis_types[i].name); + is_laptop = dmi_chassis_types[i].is_laptop; + break; + } + } +} + +static void dmi_table(uint32_t base, uint16_t len, uint16_t num) +{ + int i = 0, j = 0; + + uint8_t *dmi_table_mem = physmap_round("DMI Table", base, len); + if (dmi_table_mem == NULL) { + msg_perr("Unable to access DMI Table\n"); + return; + } + + uint8_t *data = dmi_table_mem; + uint8_t *limit = dmi_table_mem + len; + + /* SMBIOS structure header is always 4 B long and contains: + * - uint8_t type; // see dmi_chassis_types's type + * - uint8_t length; // data section w/ header w/o strings + * - uint16_t handle; + */ + while (i < num && data + 4 <= limit) { + /* - If a short entry is found (less than 4 bytes), not only it + * is invalid, but we cannot reliably locate the next entry. + * - If the length value indicates that this structure spreads + * accross the table border, something is fishy too. + * Better stop at this point, and let the user know his/her + * table is broken. + */ + if (data[1] < 4 || data + data[1] > limit) { + msg_perr("DMI table is broken (bogus header)!\n"); + break; + } + + if(data[0] == 3) { + if (data + 5 <= limit) + dmi_chassis_type(data[5]); + /* else the table is broken, but laptop detection is optional, hence continue. */ + } else + for (j = 0; j < ARRAY_SIZE(dmi_strings); j++) { + uint8_t offset = dmi_strings[j].offset; + uint8_t type = dmi_strings[j].type; + + if (data[0] != type) + continue; + + if (data[1] <= offset || data+offset > limit) { + msg_perr("DMI table is broken (offset out of bounds)!\n"); + goto out; + } + + /* Table will be unmapped, hence fill the struct with duplicated strings. */ + dmi_strings[j].value = strdup(dmi_string(data + data[1], data[offset], limit)); + } + /* Find next structure by skipping data and string sections */ + data += data[1]; + while (data + 1 <= limit) { + if (data[0] == 0 && data[1] == 0) + break; + data++; + } + data += 2; + i++; + } +out: + physunmap(dmi_table_mem, len); +} + +#if SM_SUPPORT +static int smbios_decode(uint8_t *buf) +{ + /* TODO: other checks mentioned in the conformance guidelines? */ + if (!dmi_checksum(buf, buf[0x05]) || + (memcmp(buf + 0x10, "_DMI_", 5) != 0) || + !dmi_checksum(buf + 0x10, 0x0F)) + return 0; + + dmi_table(mmio_readl(buf + 0x18), mmio_readw(buf + 0x16), mmio_readw(buf + 0x1C)); + + return 1; +} +#endif + +static int legacy_decode(uint8_t *buf) +{ + if (!dmi_checksum(buf, 0x0F)) + return 1; + + dmi_table(mmio_readl(buf + 0x08), mmio_readw(buf + 0x06), mmio_readw(buf + 0x0C)); + + return 0; +} + +int dmi_fill(void) +{ + size_t fp; + uint8_t *dmi_mem; + int ret = 1; + + msg_pdbg("Using Internal DMI decoder.\n"); + /* There are two ways specified to gain access to the SMBIOS table: + * - EFI's configuration table contains a pointer to the SMBIOS table. On linux it can be obtained from + * sysfs. EFI's SMBIOS GUID is: {0xeb9d2d31,0x2d88,0x11d3,0x9a,0x16,0x0,0x90,0x27,0x3f,0xc1,0x4d} + * - Scanning physical memory address range 0x000F0000h to 0x000FFFFF for the anchor-string(s). */ + dmi_mem = physmap_try_ro("DMI", 0xF0000, 0x10000); + if (dmi_mem == NULL) + return ret; + + for (fp = 0; fp <= 0xFFF0; fp += 16) { +#if SM_SUPPORT + if (memcmp(dmi_mem + fp, "_SM_", 4) == 0 && fp <= 0xFFE0) { + if (smbios_decode(dmi_mem + fp)) // FIXME: length check + goto out; + } else +#endif + if (memcmp(dmi_mem + fp, "_DMI_", 5) == 0) + if (legacy_decode(dmi_mem + fp) == 0) { + ret = 0; + goto out; + } + } + msg_pinfo("No DMI table found.\n"); +out: + physunmap(dmi_mem, 0x10000); + return ret; +} + +#else /* CONFIG_INTERNAL_DMI */ + +#define DMI_COMMAND_LEN_MAX 300 +static const char *dmidecode_command = "dmidecode"; static char *get_dmi_string(const char *string_name) { FILE *dmidecode_pipe; char *result; char answerbuf[DMI_MAX_ANSWER_LEN]; - char commandline[DMI_COMMAND_LEN_MAX + 40]; + char commandline[DMI_COMMAND_LEN_MAX]; snprintf(commandline, sizeof(commandline), "%s -s %s", dmidecode_command, string_name); @@ -138,46 +322,31 @@ /* Chomp trailing newline. */ if (answerbuf[0] != 0 && answerbuf[strlen(answerbuf) - 1] == '\n') answerbuf[strlen(answerbuf) - 1] = 0; - msg_pdbg("DMI string %s: \"%s\"\n", string_name, answerbuf); result = strdup(answerbuf); - if (!result) + if (result == NULL) msg_pwarn("Warning: Out of memory - DMI support fails"); return result; } -static int dmi_shutdown(void *data) -{ - int i; - for (i = 0; i < ARRAY_SIZE(dmistrings); i++) { - free(dmistrings[i]); - dmistrings[i] = NULL; - } - return 0; -} - -void dmi_init(void) +int dmi_fill(void) { int i; char *chassis_type; - if (register_shutdown(dmi_shutdown, NULL)) - return; - - has_dmi_support = 1; - for (i = 0; i < ARRAY_SIZE(dmidecode_names); i++) { - dmistrings[i] = get_dmi_string(dmidecode_names[i]); - if (!dmistrings[i]) { - has_dmi_support = 0; - return; - } + msg_pdbg("Using External DMI decoder.\n"); + for (i = 0; i < ARRAY_SIZE(dmi_strings); i++) { + dmi_strings[i].value = get_dmi_string(dmi_strings[i].keyword); + if (dmi_strings[i].value == NULL) + return 1; } chassis_type = get_dmi_string("chassis-type"); if (chassis_type == NULL) - return; + return 0; /* chassis-type handling is optional anyway */ + msg_pdbg("DMI string chassis-type: \"%s\"\n", chassis_type); is_laptop = 2; for (i = 0; i < ARRAY_SIZE(dmi_chassis_types); i++) { if (strcasecmp(chassis_type, dmi_chassis_types[i].name) == 0) { @@ -185,6 +354,33 @@ break; } } + free(chassis_type); + return 0; +} + +#endif /* CONFIG_INTERNAL_DMI */ + +static int dmi_shutdown(void *data) +{ + int i; + for (i = 0; i < ARRAY_SIZE(dmi_strings); i++) { + free(dmi_strings[i].value); + dmi_strings[i].value = NULL; + } + return 0; +} + +void dmi_init(void) +{ + /* Register shutdown function before we allocate anything. */ + if (register_shutdown(dmi_shutdown, NULL)) { + msg_pwarn("Warning: Could not register DMI shutdown function - continuing without DMI info.\n"); + return; + } + + /* dmi_fill fills the dmi_strings array, and if possible sets the global is_laptop variable. */ + if (dmi_fill() != 0) + return; switch (is_laptop) { case 1: @@ -194,7 +390,13 @@ msg_pdbg("DMI chassis-type is not specific enough.\n"); break; } - free(chassis_type); + + has_dmi_support = 1; + int i; + for (i = 0; i < ARRAY_SIZE(dmi_strings); i++) { + msg_pdbg("DMI string %s: \"%s\"\n", dmi_strings[i].keyword, + (dmi_strings[i].value == NULL) ? "" : dmi_strings[i].value); + } } /** @@ -252,11 +454,11 @@ if (!has_dmi_support) return 0; - for (i = 0; i < ARRAY_SIZE(dmidecode_names); i++) - if (dmi_compare(dmistrings[i], pattern)) + for (i = 0; i < ARRAY_SIZE(dmi_strings); i++) + if (dmi_compare(dmi_strings[i].value, pattern)) return 1; return 0; } -#endif /* STANDALONE */ +#endif // defined(__i386__) || defined(__x86_64__) Modified: trunk/physmap.c ============================================================================== --- trunk/physmap.c Thu Sep 12 01:34:57 2013 (r1734) +++ trunk/physmap.c Thu Sep 12 01:35:03 2013 (r1735) @@ -328,6 +328,11 @@ return physmap_common(descr, phys_addr, len, PHYSM_NOFAIL, PHYSM_RW, PHYSM_CLEANUP, PHYSM_ROUND); } +void *physmap_round(const char *descr, uintptr_t phys_addr, size_t len) +{ + return physmap_common(descr, phys_addr, len, PHYSM_NOFAIL, PHYSM_RW, PHYSM_NOCLEANUP, PHYSM_ROUND); +} + void *physmap_try_ro(const char *descr, uintptr_t phys_addr, size_t len) { return physmap_common(descr, phys_addr, len, PHYSM_MAYFAIL, PHYSM_RO, PHYSM_NOCLEANUP, PHYSM_EXACT); Modified: trunk/programmer.h ============================================================================== --- trunk/programmer.h Thu Sep 12 01:34:57 2013 (r1734) +++ trunk/programmer.h Thu Sep 12 01:35:03 2013 (r1735) @@ -277,6 +277,7 @@ /* physmap.c */ void *physmap(const char *descr, uintptr_t phys_addr, size_t len); void *rphysmap(const char *descr, uintptr_t phys_addr, size_t len); +void *physmap_round(const char *descr, uintptr_t phys_addr, size_t len); void *physmap_try_ro(const char *descr, uintptr_t phys_addr, size_t len); void physunmap(void *virt_addr, size_t len); #if CONFIG_INTERNAL == 1 @@ -288,9 +289,11 @@ int cb_check_image(uint8_t *bios, int size); /* dmi.c */ +#if defined(__i386__) || defined(__x86_64__) extern int has_dmi_support; void dmi_init(void); int dmi_match(const char *pattern); +#endif // defined(__i386__) || defined(__x86_64__) /* internal.c */ struct superio {

1 0

[commit] r1734 - trunk
by repository service 12 Sep '13

12 Sep '13

Author: stefanct Date: Thu Sep 12 01:34:57 2013 New Revision: 1734 URL: http://flashrom.org/trac/flashrom/changeset/1734 Log: DOS refinements. This allows to use the DOS library trees stored in a user-specified directory. I have mirrored the needed patches, sources and binaries (the latter are properly licensed to allow that) in the flashrom wiki, so use those URLs instead of the original sources. Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Acked-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Modified: trunk/Makefile trunk/README Modified: trunk/Makefile ============================================================================== --- trunk/Makefile Tue Sep 10 12:42:48 2013 (r1733) +++ trunk/Makefile Thu Sep 12 01:34:57 2013 (r1734) @@ -39,6 +39,7 @@ EXPORTDIR ?= . AR ?= ar RANLIB ?= ranlib +DOSLIBS_BASE ?= .. # The following parameter changes the default programmer that will be used if there is no -p/--programmer # argument given when running flashrom. The predefined setting does not enable any default so that every # user has to declare the programmer he wants to use on every run. The rationale for this to be not set @@ -99,11 +100,12 @@ ifeq ($(TARGET_OS), DOS) EXEC_SUFFIX := .exe -CPPFLAGS += -I../libgetopt +CPPFLAGS += -I$(DOSLIBS_BASE)/libgetopt # DJGPP has odd uint*_t definitions which cause lots of format string warnings. CFLAGS += -Wno-format # FIXME Check if we can achieve the same effect with -L../libgetopt -lgetopt -LIBS += ../libgetopt/libgetopt.a +LIBS += -lgetopt +LDFLAGS += -L$(DOSLIBS_BASE)/libgetopt/ # Bus Pirate, Serprog and PonyProg are not supported under DOS (missing serial support). ifeq ($(CONFIG_BUSPIRATE_SPI), yes) UNSUPPORTED_FEATURES += CONFIG_BUSPIRATE_SPI=yes @@ -611,9 +613,9 @@ PCILIBS += -l$(shell uname -p) else ifeq ($(TARGET_OS), DOS) -# FIXME There needs to be a better way to do this -CPPFLAGS += -I../libpci/include -PCILIBS += ../libpci/lib/libpci.a +CPPFLAGS += -I$(DOSLIBS_BASE)/libpci/include +LDFLAGS += -L$(DOSLIBS_BASE)/libpci/lib/ +PCILIBS += -lpci else PCILIBS += -lpci ifeq ($(TARGET_OS), OpenBSD) Modified: trunk/README ============================================================================== --- trunk/README Tue Sep 10 12:42:48 2013 (r1733) +++ trunk/README Thu Sep 12 01:34:57 2013 (r1734) @@ -105,20 +105,24 @@ djcrx-2.04pre_20090725-13ap.i386.rpm The cross toolchain packages for your distribution may have slightly different names (look for packages named *djgpp*). - Download pciutils 3.1.5 and apply http://assembler.cz/flashrom/pciutils.patch - Download and compile http://assembler.cz/flashrom/libgetopt/ + + You will need the following library source trees containing their compiled + static libraries either in the parent directory of the flashrom source or + specify the base folder on compile time with the DOSLIBS_BASE parameter. + The default as described above is equal to calling + 'make djgpp-dos DOSLIBS_BASE=..' + + To get and build said libraries... + Download pciutils 3.1.5 and apply http://flashrom.org/File:Pciutils.patch.gz Compile pciutils, see README.DJGPP for instructions. + Download and compile http://flashrom.org/File:Libgetopt.tar.gz Enter the flashrom directory. - ../libpci should contain pciutils source and binaries. - ../libgetopt should contain getopt.a from libgetopt. Run either (change settings where appropriate) make CC=i586-pc-msdosdjgpp-gcc STRIP=i586-pc-msdosdjgpp-strip or (above settings hardcoded) make djgpp-dos - You might have to add WARNERROR=no to the make command line. - To run flashrom.exe, download and unpack - http://homer.rice.edu/~sandmann/cwsdpmi/csdpmi7b.zip and make sure - CWSDPMI.EXE is in the current directory. + To run flashrom.exe, download http://flashrom.org/File:Csdpmi7b.zip and + unpack CWSDPMI.EXE into the current directory or one in PATH. To cross-compile on Linux for Windows:

1 0

Supermicro server BIOS update question
by Ben Schwartz 12 Sep '13

12 Sep '13

Hello! I came across your page today in search of an easy way to update BIOS on ~800 Supermicro servers around the world at different datacenters. I have used the asulnx tool in the past for another board but it doesn't seem to work for these. I saw in your supported hardware page that the Supermicro X8SIE-F and X9SCM-F boards are not supported under flashrom utility because: 113. ↑<http://www.flashrom.org/Supported_hardware#boards_ref113> Requires unlocking the ME although the registers are set up correctly by the descriptor/BIOS already (tested with swseq and hwseq)." What exactly does this reason mean and do you know of another BIOS update utility that will work for these motherboards that can be run from Linux (centos5-6)? Thanks! -Ben

1 0

Re: [flashrom] BCOM WinNET P680 patch
by Stefan Tauner 10 Sep '13

10 Sep '13

On Sun, 8 Sep 2013 22:39:11 -0400 Corey Osgood <corey.osgood(a)gmail.com> wrote: > > On Sun, Sep 8, 2013 at 10:24 PM, Corey Osgood <corey.osgood(a)gmail.com>wrote: > > > >> This patch replaces Alex Mauer's previous patch for this board. Tested to > >> read, erase, and write on 2 different boards, both with AMIC A29040BL flash > >> chips, using both stock BIOS and coreboot images. Patch marks the AMIC chip > >> as tested at the same time. > >> > >> Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com> Thanks Corey! I was not aware of that patch from 2008(!). Acked-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> and committed in r1733 after some minor changes. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

[commit] r1733 - trunk
by repository service 10 Sep '13

10 Sep '13

Author: stefanct Date: Tue Sep 10 12:42:48 2013 New Revision: 1733 URL: http://flashrom.org/trac/flashrom/changeset/1733 Log: Add board enable for Bcom WinNET P680. This patch replaces Alex Mauer's previous patch for this board (from 2008). Tested to read, erase, and write on 2 different boards, both with AMIC A29040BL flash chips, using both stock BIOS and coreboot images. This patch marks the AMIC chip as tested at the same time. Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com> Acked-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Modified: trunk/board_enable.c trunk/flashchips.c Modified: trunk/board_enable.c ============================================================================== --- trunk/board_enable.c Sat Aug 31 00:23:02 2013 (r1732) +++ trunk/board_enable.c Tue Sep 10 12:42:48 2013 (r1733) @@ -2378,6 +2378,7 @@ {0x8086, 0x24dd, 0x1043, 0x80a6, 0x8086, 0x2570, 0x1043, 0x8157, NULL, NULL, NULL, P3, "ASUS", "P5PE-VM", 0, OK, intel_ich_gpio21_raise}, {0x8086, 0x2443, 0x1043, 0x8027, 0x8086, 0x1130, 0x1043, 0x8027, "^CUSL2-C", NULL, NULL, P3, "ASUS", "CUSL2-C", 0, OK, intel_ich_gpio21_raise}, {0x8086, 0x2443, 0x1043, 0x8027, 0x8086, 0x1130, 0x1043, 0x8027, "^TUSL2-C", NULL, NULL, P3, "ASUS", "TUSL2-C", 0, NT, intel_ich_gpio21_raise}, + {0x1106, 0x3059, 0x1106, 0x4161, 0x1106, 0x3065, 0x1106, 0x0102, NULL, NULL, NULL, P3, "Bcom/Clientron", "WinNET P680", 0, OK, w836xx_memw_enable_2e}, {0x1106, 0x3177, 0x1106, 0x3177, 0x1106, 0x3116, 0x1106, 0x3116, "^KM266-8235$", "biostar", "m7viq", P3, "Biostar", "M7VIQ", 0, NT, w83697xx_memw_enable_2e}, {0x10b7, 0x9055, 0x1028, 0x0082, 0x8086, 0x7190, 0, 0, NULL, NULL, NULL, P3, "Dell", "OptiPlex GX1", 0, OK, intel_piix4_gpo30_lower}, {0x8086, 0x3590, 0x1028, 0x016c, 0x1000, 0x0030, 0x1028, 0x016c, NULL, NULL, NULL, P3, "Dell", "PowerEdge 1850", 0, OK, intel_ich_gpio23_raise}, Modified: trunk/flashchips.c ============================================================================== --- trunk/flashchips.c Sat Aug 31 00:23:02 2013 (r1732) +++ trunk/flashchips.c Tue Sep 10 12:42:48 2013 (r1733) @@ -1355,7 +1355,7 @@ .total_size = 512, .page_size = 64 * 1024, .feature_bits = FEATURE_ADDR_2AA | FEATURE_SHORT_RESET, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PREW, .probe = probe_jedec, .probe_timing = TIMING_ZERO, .block_erasers =

1 0

[PATCH] squash! BSD refinements.
by Stefan Tauner 10 Sep '13

10 Sep '13

Also, fix date handling in getrevision.sh to work with the various formats for invoking 'date'. This also uses svn's info --xml output instead of the regular one. Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> --- Together with my previous patch I would also like to fix the getrevision script as follows. The commit log and inline comments explain how precisely. util/getrevision.sh | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/util/getrevision.sh b/util/getrevision.sh index c05f59e..5fd83f5 100755 --- a/util/getrevision.sh +++ b/util/getrevision.sh @@ -110,15 +110,31 @@ scm_url() { timestamp() { local t + # date syntaxes are manifold: + # gnu date [-d input]... [+FORMAT] + # netbsd date [-ajnu] [-d date] [-r seconds] [+format] [[[[[[CC]yy]mm]dd]HH]MM[.SS]] + # freebsd date [-jnu] [-d dst] [-r seconds] [-f fmt date | [[[[[cc]yy]mm]dd]HH]MM[.ss]] [+format] [...] + # dragonflybsd date [-jnu] [-d dst] [-r seconds] [-f fmt date | [[[[[cc]yy]mm]dd]HH]MM[.ss]] [+format] [...] + # openbsd date [-aju] [-d dst] [-r seconds] [+format] [[[[[[cc]yy]mm]dd]HH]MM[.SS]] [...] if svn_is_file_tracked "$2" ; then if svn_has_local_changes "$2"; then t=$(date -u "$1") else # No local changes, get date of the last log record. - local last_commit_date="$(svn info "$2" | \ - grep '^Last Changed Date:' | \ - awk '{print $4" "$5" "$6}')" - t=$(date -d "${last_commit_date}" -u "$1") + local last_commit_date="$(svn info --xml "$2"| \ + sed -n -e 's/<date>$.*$<\/date>/\1/p' -e 's/\..*Z/Z/p')" + + case $(uname) in + # Most BSD dates do not support parsing date values from user input with -d but all of + # them support parsing the syntax with [[[[[[cc]yy]mm]dd]HH]MM[.ss]]. We have to + # transform the ISO8601 date first though. + NetBSD|OpenBSD|DragonFly|FreeBSD) + last_commit_date="$(echo ${last_commit_date} | \ + sed -n -e 's/$....$-$..$-$..$T$..$:$..$:$..$Z/\1\2\3\4\5\.\6/p')" + t=$(date -u -j "${last_commit_date}" "$1" 2>/dev/null);; + *) + t=$(date -u -d "${last_commit_date}" "$1" 2>/dev/null);; + esac fi elif git_is_file_tracked "$2" ; then # are there local changes? @@ -126,12 +142,22 @@ timestamp() { t=$(date -u "${1}") else # No local changes, get date of the last commit - t=$(date -d "$(git log --pretty=format:"%cD" -1 -- "$2")" -u "$1") + case $(uname) in + # Most BSD dates do not support parsing date values from user input with -d but all of + # them support parsing epoch seconds with -r. Thanks to git we can easily use that: + NetBSD|OpenBSD|DragonFly|FreeBSD) + t=$(date -u -r "$(git log --pretty=format:%ct -1 -- $2)" "$1" 2>/dev/null);; + *) + t=$(date -d "$(git log --pretty=format:%cD -1 -- $2)" -u "$1" 2>/dev/null);; + esac fi else t=$(date -u "$1") fi + if [ -z "$t" ]; then + echo "Warning: Could not determine timestamp." 2>/dev/null + fi echo "${t}" } -- Kind regards, Stefan Tauner

1 0

Re: [flashrom] [PATCH 2/6] sbxxx: Set SPI clock to 16.5 MHz and disable fast reads.
by Carl-Daniel U. Hailfinger 10 Sep '13

10 Sep '13

[Sorry for breaking the threading. This is a reply to http://patchwork.coreboot.org/patch/4011/ ] Stefan Tauner wrote on 2013-08-14 17:44:19 > > On Wed, 14 Aug 2013 08:56:27 +0200 > Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> wrote: > > > Am 10.08.2013 03:45 schrieb Stefan Tauner: > > > diff --git a/sb600spi.c b/sb600spi.c > > > index c9be44c..2c1d400 100644 > > > --- a/sb600spi.c > > > +++ b/sb600spi.c > > > @@ -272,6 +272,61 @@ static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, > > > return 0; > > > } > > > > > > +struct spispeed { > > > + const char *const name; > > > + const int8_t speed; > > > +}; > > > + > > > +static const struct spispeed spispeeds[] = { > > > + { "66 MHz", 0x00 }, > > > + { "33 MHz", 0x01 }, > > > + { "22 MHz", 0x02 }, > > > + { "16.5 MHz", 0x03 }, > > > +}; > > > I see struct spispeed and think it is a somewhat odd construct. The > > speed member is just the array index stored in the corresponding array > > member. Do you envision speed values which are not an array index? > > I want to merge the various frequency setting functions eventually and > Dediprog for example uses non-linear values. Hence I'd rather use this > unnecessary scheme for easier refactoring later. OK. > > > +static int set_speed(struct pci_dev *dev, const struct spispeed *spispeed) > > > +{ > > > + bool success = false; > > > + uint8_t speed = spispeed->speed; > > > + > > > + msg_pdbg("Setting SPI clock to %s (0x%x).\n", spispeed->name, speed); > > > + if (amd_gen != CHIPSET_YANGTZE) { > > > + rmmio_writeb((mmio_readb(sb600_spibar + 0xd) & ~(0x3 << 4)) | (speed << 4), sb600_spibar + 0xd); > > > + success = (speed == ((mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3)); > > > The code reads spispeed and then sets 16 MHz regardless of whether it's > > already 16 MHz. I also seem to remember that you were opposed to setting > > this speed permanently and advocated to restore it on shutdown. > > This should not be done unconditionally, right, I will change this. > But it is done (like the (un)setting of fast speed also) with the > r*-mmio functions so it should be reverted on shutdown AFAIK? Indeed, I overlooked the r* prefix. Given that a repost of part 1 of this series would be nice, I'll go out on a limb and ask you to repost this one after the changes as well. With the change you mentioned, this is Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Regards, Carl-Daniel

1 0

Re: [flashrom] [PATCH 1/6] sbxxx: Add detection for the remaining AMD chipset families.
by Carl-Daniel U. Hailfinger 10 Sep '13

10 Sep '13

[Sorry for breaking the threading. This is meant to be a reply to http://patchwork.coreboot.org/patch/4016/ ] Stefan Tauner wrote on 2013-08-14 17:35:13 > > On Wed, 14 Aug 2013 08:42:15 +0200 > Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> wrote: > > > Am 10.08.2013 03:45 schrieb Stefan Tauner: > > > Also, correct prettyprinting of the registers of the various families, > > > and abort if SpiAccessMacRomEn or SpiHostAccessRomEn prohibit full access. > > > > > > Tested reading/writing on ASRock IMB-A180, and chipset detection on > > > one of each affected generation by Chris Goodrich from Sage. > > > > > > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > > > --- > > > sb600spi.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++------------- > > > 1 file changed, 106 insertions(+), 27 deletions(-) > > > > > > diff --git a/sb600spi.c b/sb600spi.c > > > index cb7c4ac..c9be44c 100644 > > > --- a/sb600spi.c > > > +++ b/sb600spi.c > > > @@ -57,7 +57,28 @@ static enum amd_chipset amd_gen = CHIPSET_AMD_UNKNOWN; > > > static void determine_generation(struct pci_dev *dev) > > > { > > > amd_gen = CHIPSET_AMD_UNKNOWN; > > > - if (dev->device_id == 0x780e) { > > > + msg_pdbg2("Trying to determine the generation of the SPI interface... "); > > > + if (dev->device_id == 0x438d) { > > > + amd_gen = CHIPSET_SB6XX; > > > + msg_pdbg("SB6xx detected.\n"); > > > + } else if (dev->device_id == 0x439d) { > > > + struct pci_dev *smbus_dev = pci_dev_find(0x1002, 0x4385); > > > > Why do we search twice for this SMBus device, once inside > > sb600_probe_spi and once here? Can't we just pass the smbus device as > > second parameter to determine_generation? > > Yes we could and I thought about this too but came to the conclusion > that it is not worth it. Do you think otherwise? I'd really like to limit the number of times we search for PCI devices. On some machines with lots of PCI devices that can take a lot of time. That said, I also think such refactoring can be done in a later iteration, and it is also possible to have the generation detection as first command inside the AMD SBx00 SPI probe function, and let it set a static struct *smbus_dev. Your choice when/how to deal with it. Anyway, most/all of the code above was committed in r1706, so this is mostly an after-the-fact review. > > > + if (smbus_dev == NULL) > > > + return; > > > + uint8_t rev = pci_read_byte(smbus_dev, PCI_REVISION_ID); > > > + if (rev >= 0x39 && rev <= 0x3D) { > > > + amd_gen = CHIPSET_SB7XX; > > > + msg_pdbg("SB7xx/SP5100 detected.\n"); > > > + } else if (rev >= 0x40 && rev <= 0x42) { > > > + amd_gen = CHIPSET_SB89XX; > > > + msg_pdbg("SB8xx/SB9xx/Hudson-1 detected.\n"); > > > + } else { > > > + msg_pwarn("SB device found but SMBus revision 0x%02x does not match known values.\n" > > > + "Assuming SB8xx/SB9xx/Hudson-1. Please send a log to flashrom(a)flashrom.org\n", > > > + rev); > > > + amd_gen = CHIPSET_SB89XX; > > > > We could error out here as well. The next release is a few weeks away > > anyway. > > No regressions if we can easily avoid them please (I'll break enough > stuff unintentionally, rest assured :) > It is fairly safe to assume that the code path for sb89xx/hudson will > work on such devices if they exist, maybe with some wrong prints if it > is sb7xx but nothing we worried about until I provided a solution for > it in most cases :) I'm not convinced that this would likely introduce a regression. For all we know, AMD might use higher SMBus device PCI revision IDs for respins of older chipsets, and there we really want to error out instead of having flashrom deal with bogus errors. > > > + } > > > + } else if (dev->device_id == 0x780e) { > > > /* The PCI ID of the LPC bridge doesn't change between Hudson-2/3/4 and Yangtze (Kabini/Temash) > > > * although they use different SPI interfaces. */ > > > #ifdef USE_YANGTZE_HEURISTICS > > > @@ -94,7 +115,11 @@ static void determine_generation(struct pci_dev *dev) > > > "the output of lspci -nnvx, thanks!.\n", rev); > > > } > > > #endif > > > - } > > > + } else > > > + msg_pwarn("%s: Unknown LPC device %" PRIx16 ":%" PRIx16 ".\n" > > > + "Please report this to flashrom(a)flashrom.org and include this log and\n" > > > + "the output of lspci -nnvx, thanks!\n", > > > + __func__, dev->vendor_id, dev->device_id); > > > } > > > > > > static void reset_internal_fifo_pointer(void) > > > @@ -250,7 +275,7 @@ static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, > > > static int sb600_handle_imc(struct pci_dev *dev, bool amd_imc_force) > > > { > > > /* Handle IMC everywhere but sb600 which does not have one. */ > > > - if (dev->device_id == 0x438d) > > > + if (amd_gen == CHIPSET_SB6XX) > > > return 0; > > > > > > /* TODO: we should not only look at IntegratedImcPresent (LPC Dev 20, Func 3, 40h) but also at > > > @@ -334,6 +359,12 @@ int sb600_probe_spi(struct pci_dev *dev) > > > sb600_spibar += tmp & 0xfff; > > > > > > determine_generation(dev); > > > + if (amd_gen == CHIPSET_AMD_UNKNOWN) { > > > + /* FIXME: Eventually this should be a fatal error, but until we are certain to have covered all > > > + * IDs actually found in the wild, try to mimmick the previous behavior. */ > > > > Given that we're now post-0.9.7, we can make this a fatal error. > > Besides that, the current patch doesn't even set CHIPSET_AMD_UNKNOWN for > > some combinations with unknown SMBus PCI revision ID. > > Sure, this was targeted to be in 0.9.7 (and see above). > > > > + msg_pwarn("Could not determine chipset generation. Assuming SB6xx."); > > > + amd_gen = CHIPSET_SB6XX; > > > + } > > > > > > if (amd_gen == CHIPSET_YANGTZE) { > > > msg_perr("SPI on Kabini/Temash and newer chipsets are not yet supported.\n" > > > @@ -341,35 +372,83 @@ int sb600_probe_spi(struct pci_dev *dev) > > > return ERROR_NONFATAL; > > > } > > > > > > - tmp = pci_read_long(dev, 0xa0); > > > - msg_pdbg("AltSpiCSEnable=%i, SpiRomEnable=%i, " > > > - "AbortEnable=%i\n", tmp & 0x1, (tmp & 0x2) >> 1, > > > - (tmp & 0x4) >> 2); > > > - tmp = (pci_read_byte(dev, 0xba) & 0x4) >> 2; > > > - msg_pdbg("PrefetchEnSPIFromIMC=%i, ", tmp); > > > - > > > - tmp = pci_read_byte(dev, 0xbb); > > > - /* FIXME: Set bit 3,6,7 if not already set. > > > - * Set bit 5, otherwise SPI accesses are pointless in LPC mode. > > > - * See doc 42413 AMD SB700/710/750 RPR. > > > + /* Chipset support matrix for SPI Base_Addr (LPC PCI reg 0xa0) > > > + * bit 6xx 7xx/SP5100 8xx 9xx hudson1 hudson2+ yangtze > > > + * 3 rsvd <- <- ? <- ? RouteTpm2Spi > > > + * 2 rsvd AbortEnable rsvd ? <- ? <- > > > + * 1 rsvd SpiRomEnable <- ? <- ? <- > > > + * 0 rsvd AltSpiCSEnable<1> rsvd ? <- ? <- > > > > Maybe it's just me, but I have trouble parsing that table. Let's talk > > about bit 2: What you mean is clear for 6xx (rsvd), 7xx (abortenable), > > 8xx (abortenable). I'm assuming that this bit is not described for 9xx > > and hudson1, but why is the hudson2+ entry "?" instead of "<-" ? Or do > > the "?" entries for 9xx and hudson2+ have a different meaning? > > Actually it is the <- that changes meaning, if you will, and the ? are > not just "we dont know", as one obviously would to infer. > The ? marks values where we do not have a datasheet, but which we > assume to be identical to the previous generations. So maybe we should > use "<-?" instead? Other suggestions are very welcomed. > Now it should also be clear what the <- after a ? means: we have a > datasheet and it indicates the same values as the generation *before* > the question marks before it. I used this mainly for myself but I would > still like to include it in some form. Let me try to phrase this in a way I can understand: +++++++++++++ How to read this table: "nd" means we have no datasheet for this chipset generation. "ni" means the datasheet for this generation doesn't have any relevant info. "<-" means the bit/register meaning is identical to the next non-ni/non-nd chipset to the left. If a "nd" chipset is between two chipsets with identical meaning, we assume the meaning didn't change twice in between, i.e. the meaning is unchanged for the "nd" chipset. This is a tried and (so far) valid assumption. +++++++++++++ Your "?" would turn into "nd". That said, keeping your symbols is also an option. My phrasing for that scenario would be: +++++++++++++ How to read this table: "?" means we have no datasheet for this chipset generation or the datasheet doesn't have any relevant info. "<-" means the bit/register meaning is identical to the next non-"?" chipset to the left. "<-" thus never refers to a "?". If a "?" chipset is between two chipsets with identical meaning, we assume the meaning didn't change twice in between, i.e. the meaning is unchanged for the "?" chipset. This is a tried and (so far) valid assumption. +++++++++++++ > > > + * > > > + * <1> It is unknown if that was ever working. On later chipsets the CS config is at memmapped 0x1D. > > > > Heh. Let's find someone with DualBIOS on a SB7xx board. But seriously, > > I'd assume this works if it's still in the current revision of the > > register reference guide. > > The problem is (IIRC) that it is not too well defined (who would have > thought that!) but maybe I am confusing it with Fast Speed. AFAICS the meaning is well defined, except that vendors apparently didn't use it on the boards we know. A simple test would be to set that bit and check if SPI probe still returns anything. If yes, there are either two flash chips or my understanding of that bit is wrong. > > > */ > > > - msg_pdbg("PrefetchEnSPIFromHost=%i, SpiOpEnInLpcMode=%i\n", > > > - tmp & 0x1, (tmp & 0x20) >> 5); > > > - tmp = mmio_readl(sb600_spibar); > > > - /* FIXME: If SpiAccessMacRomEn or SpiHostAccessRomEn are zero on > > > - * SB700 or later, reads and writes will be corrupted. Abort in this > > > - * case. Make sure to avoid this check on SB600. > > > + if (amd_gen >= CHIPSET_SB7XX) { > > > + tmp = pci_read_long(dev, 0xa0); > > > + msg_pdbg("SpiRomEnable=%i", (tmp >> 1) & 0x1); > > > + if (amd_gen == CHIPSET_SB7XX) > > > + msg_pdbg(", AltSpiCSEnable=%i, AbortEnable=%i", tmp & 0x1, (tmp >> 2) & 0x1); > > > + > > > + tmp = pci_read_byte(dev, 0xba); > > > + msg_pdbg(", PrefetchEnSPIFromIMC=%i", (tmp & 0x4) >> 2); > > > + > > > + tmp = pci_read_byte(dev, 0xbb); > > > + /* FIXME: Set bit 3,6,7 if not already set. > > > + * Set bit 5, otherwise SPI accesses are pointless in LPC mode. > > > + * See doc 42413 AMD SB700/710/750 RPR. > > > > Are these bits required to be set even for post-SB7000 chipsets? > > That's left as an exercise to the reader. :P > 5 is reserved anywhere but on sb7x/sp5100, the other bits change > semantics in sb8xx... > > > > + */ > > > + if (amd_gen == CHIPSET_SB7XX) > > > + msg_pdbg(", SpiOpEnInLpcMode=%i", (tmp >> 5) & 0x1); > > > + msg_pdbg(", PrefetchEnSPIFromHost=%i\n", tmp & 0x1); > > > + } > > > + > > > + /* Chipset support matrix for SPI_Cntrl0 (spibar + 0x0) > > > + * bit 6xx 7xx/SP5100 8xx 9xx hudson1 hudson2+ yangtze > > > + * 17 rsvd <- <- ? <- ? <- > > > > Again the table I don't understand. Maybe you should add an explanation > > which specifies the meaning of "?". > > Given the explanation above, would you mind formulating it? IMHO it > improves such things quite a bit if another mind puts it into words > than the one inventing it. Done, see above. > > > + * 18 rsvd <- fastReadEnable<1> ? <- ? SpiReadMode[0]<1> > > > + * 19 SpiArbEnable <- <- ? <- ? <- > > > + * 20 (FifoPtrClr) <- <- ? <- ? <- > > > + * 21 (FifoPtrInc) <- <- ? <- ? IllegalAccess > > > + * 22 SpiAccessMacRomEn <- <- ? <- ? <- > > > + * 23 SpiHostAccessRomEn <- <- ? <- ? <- > > > + * 24:26 ArbWaitCount <- <- ? <- ? <- > > > + * 27 SpiBridgeDisable <- <- ? <- ? rsvd > > > + * 28 rsvd DropOneClkOnRd = SPIClkGate ? <- ? <- > > > + * 29:30 rsvd <- <- ? <- ? SpiReadMode[2:1]<1> > > > + * 31 rsvd <- SpiBusy ? <- ? <- > > > + * > > > + * <1> see handle_speed > > > > Which handle_speed? Can't find it in svn HEAD nor in this patch. > > Added later in the patch set. Do you really want me to change those > lines again in the followup patch? It would have made review easier. > I had the impression you hate that :) Yes and no. While I try to avoid changing a line multiple times in a patchset, I also try to keep each patch self-contained. > > > */ > > > - msg_pdbg("(0x%08" PRIx32 ") fastReadEnable=%u, SpiArbEnable=%i, SpiAccessMacRomEn=%i, " > > > - "SpiHostAccessRomEn=%i, ArbWaitCount=%i, " > > > - "SpiBridgeDisable=%i, DropOneClkOnRd=%i\n", > > > - tmp, (tmp >> 18) & 0x1, > > > - (tmp >> 19) & 0x1, (tmp >> 22) & 0x1, > > > - (tmp >> 23) & 0x1, (tmp >> 24) & 0x7, > > > - (tmp >> 27) & 0x1, (tmp >> 28) & 0x1); > > > + tmp = mmio_readl(sb600_spibar + 0x00); > > > + msg_pdbg("(0x%08" PRIx32 ") SpiArbEnable=%i", tmp, (tmp >> 19) & 0x1); > > > + > > > + msg_pdbg(", SpiAccessMacRomEn=%i, SpiHostAccessRomEn=%i, ArbWaitCount=%i", > > > + (tmp >> 22) & 0x1, (tmp >> 23) & 0x1, (tmp >> 24) & 0x7); > > > + > > > + if (amd_gen != CHIPSET_YANGTZE) > > > + msg_pdbg(", SpiBridgeDisable=%i", (tmp >> 27) & 0x1); > > > + > > > + switch (amd_gen) { > > > + case CHIPSET_SB7XX: > > > + msg_pdbg(", DropOneClkOnRd/SpiClkGate=%i", (tmp >> 28) & 0x1); > > > > Should we check that bit and set it to 0? I haven't yet been able to > > find a good explanation of what it does, but it seems to be a trick to > > allow normal read commands at higher speed. I could be mistaken, though. > > I would refrain from changing it if we don't really know what it does > (and we probably will never do so...). I have read the description a > few times... and it does not make a lot of sense to me. Maybe our friends at AMD/Sage can explain this? > > > + case CHIPSET_SB89XX: > > > + case CHIPSET_HUDSON234: > > > + msg_pdbg(", SpiBusy=%i", (tmp >> 31) & 0x1); > > > + default: break; > > > + } > > > + msg_pdbg("\n"); > > > + > > > + if (((tmp >> 22) & 0x1) == 0 || ((tmp >> 23) & 0x1) == 0) { > > > + msg_perr("ERROR: State of SpiAccessMacRomEn or SpiHostAccessRomEn prohibits full access.\n"); > > > + return ERROR_NONFATAL; > > > + } > > > + > > > tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3; > > > msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]); > > > > AFAICS this speed handling stuff is changing in a later patch, correct? > > Exactly because this is a good breaking point to avoid a monster > patch ;) > > Thanks for the review! Apart from the ? explanation and the fatal > error, I think this can go in soon? Yes, looks fine otherwise. Could you please forward-port this patch and repost it? I just want to make sure I didn't overlook anything serious. Otherwise, this is (with the ? explanation and the fatal error) Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Regards, Carl-Daniel

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom