[flashrom] [PATCH] ichspi: add ICH/PCH flash descriptor decoding via FDOC/FDOD

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu Sep 15 09:07:20 CEST 2011


Am 15.09.2011 00:53 schrieb Stefan Tauner:
> based on the work of Matthias 'mazzoo' Wenzel this patch adds pretty
> printing of those ICH/PCH flash descriptor sections that are
> cached/mapped by the chipset (and which are therefore reachable via
> FDOC/FDOD registers).
>
> this includes the following:
> - content section:
>     describes the image and some generic properties (number of
>     sections, offset of sections, PCH/ICH and MCH/PROC strap
>     offsets and lengths)
> - component section:
>     identify the different SPI flash chips and their capabilities.
> - region section
>     similarly to a partition table this describes the different regions.
>     the content of FLREG* is derived from this section.
> - master section
>     defines SPI master (host, ME, GbE) access rights of the
>     individual regions. the content of PR* is derived from this section.
>
> this is only a part of the data included in the descriptor. other
> information can be retrieved from a complete binary dump of the
> descriptor region only.
>
> this patch also adds macros and pretty printing for "Vendor Specific
> Component Capabilities" registers: there are two of them: lower and
> upper. they describe the properties of the address space divided by
> FPBA (which allows to use multiple flash chips or partitions with
> different properties). the properties of all supported flash chips
> (together with their RDIDs) are stored in the same format in table
> in a descriptor section (which is used by the ME apparently). a
> later patch will use the macros outside of ichspi.c which is the
> reason why the prettyprinting function and the register bit macros
> are not defined in ichspi.c but ich_descriptors.h (else they would
> be moved in the follow-up patch).
>
> because this patch relies on (compiler) implementation-specific
> layouting of bit-fields, it checks for correct layout before taking
> any action on runtime.
>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>

Get it in!
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
with small comments.


> this is the version without the compile-time checks as requested by carldani.
>
>  Makefile          |    3 +-
>  ich_descriptors.c |  286 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ich_descriptors.h |  259 ++++++++++++++++++++++++++++++++++++++++++++++++
>  ichspi.c          |   58 +++++++++--
>  4 files changed, 594 insertions(+), 12 deletions(-)
>  create mode 100644 ich_descriptors.c
>  create mode 100644 ich_descriptors.h
>
> diff --git a/Makefile b/Makefile
> index 6407e6b..4a8dc2c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -349,7 +349,8 @@ 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
>  ifeq ($(ARCH),"x86")
> -PROGRAMMER_OBJS += it87spi.o it85spi.o ichspi.o sb600spi.o wbsio_spi.o mcp6x_spi.o
> +PROGRAMMER_OBJS += it87spi.o it85spi.o sb600spi.o wbsio_spi.o mcp6x_spi.o
> +PROGRAMMER_OBJS += ichspi.o ich_descriptors.o
>  else
>  endif
>  NEED_PCI := yes
> diff --git a/ich_descriptors.c b/ich_descriptors.c
> new file mode 100644
> index 0000000..4ab2602
> --- /dev/null
> +++ b/ich_descriptors.c
> @@ -0,0 +1,286 @@
> +/*
> + * This file is part of the flashrom project.
> + *
> + * Copyright (c) 2010  Matthias Wenzel <bios at mazzoo dot de>
> + * Copyright (c) 2011  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
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> + */
> +
> +#if defined(__i386__) || defined(__x86_64__)
> +
> +#include "ich_descriptors.h"
> +#include "flash.h" /* for msg_* */
> +#include "programmer.h"
> +
> +void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity)
> +{
> +	print(verbosity, "BES=0x%x, ",	(reg_val & VSCC_BES)  >> VSCC_BES_OFF);
> +	print(verbosity, "WG=%d, ",	(reg_val & VSCC_WG)   >> VSCC_WG_OFF);
> +	print(verbosity, "WSR=%d, ",	(reg_val & VSCC_WSR)  >> VSCC_WSR_OFF);
> +	print(verbosity, "WEWS=%d, ",	(reg_val & VSCC_WEWS) >> VSCC_WEWS_OFF);
> +	print(verbosity, "EO=0x%x, ",	(reg_val & VSCC_EO)   >> VSCC_EO_OFF);
> +	print(verbosity, "VCL=%d\n",	(reg_val & VSCC_VCL)  >> VSCC_VCL_OFF);
> +}
> +
> +#define getFCBA(cont)	(((cont)->FLMAP0 <<  4) & 0x00000ff0)
> +#define getFRBA(cont)	(((cont)->FLMAP0 >> 12) & 0x00000ff0)
> +#define getFMBA(cont)	(((cont)->FLMAP1 <<  4) & 0x00000ff0)
> +#define getFISBA(cont)	(((cont)->FLMAP1 >> 12) & 0x00000ff0)
> +#define getFMSBA(cont)	(((cont)->FLMAP2 <<  4) & 0x00000ff0)
> +
> +void prettyprint_ich_descriptors(enum ich_chipset cs, const struct ich_descriptors *desc)
> +{
> +	prettyprint_ich_descriptor_content(&desc->content);
> +	prettyprint_ich_descriptor_component(desc);
> +	prettyprint_ich_descriptor_region(desc);
> +	prettyprint_ich_descriptor_master(&desc->master);
> +}
> +
> +void prettyprint_ich_descriptor_content(const struct ich_desc_content *cont)
> +{
> +	msg_pdbg2("=== Content Section ===\n");
> +	msg_pdbg2("FLVALSIG 0x%08x\n", cont->FLVALSIG);
> +	msg_pdbg2("FLMAP0   0x%08x\n", cont->FLMAP0);
> +	msg_pdbg2("FLMAP1   0x%08x\n", cont->FLMAP1);
> +	msg_pdbg2("FLMAP2   0x%08x\n", cont->FLMAP2);
> +	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("\n");
> +}
> +
> +void prettyprint_ich_descriptor_component(const struct ich_descriptors *desc)
> +{
> +	static const char * const str_freq[8] = {

maybe freq_name or freq_str?


> +		"20 MHz",	/* 000 */
> +		"33 MHz",	/* 001 */
> +		"reserved",	/* 010 */
> +		"reserved",	/* 011 */
> +		"50 MHz",	/* 100 */
> +		"reserved",	/* 101 */
> +		"reserved",	/* 110 */
> +		"reserved"	/* 111 */
> +	};
> +	static const char * const str_mem[8] = {

Maybe size_name or size_str?


> +		"512 kB",	/* 000 */
> +		"  1 MB",	/* 001 */
> +		"  2 MB",	/* 010 */
> +		"  4 MB",	/* 011 */
> +		"  8 MB",	/* 100 */
> +		" 16 MB",	/* 101 */
> +		"reserved",	/* 110 */
> +		"reserved",	/* 111 */
> +	};
> +
> +	msg_pdbg2("=== Component Section ===\n");
> +	msg_pdbg2("FLCOMP   0x%08x\n", desc->component.FLCOMP);
> +	msg_pdbg2("FLILL    0x%08x\n", desc->component.FLILL );
> +	msg_pdbg2("\n");
> +
> +	msg_pdbg2("--- Details ---\n");
> +	msg_pdbg2("Component 1 density:           %s\n",
> +		  str_mem[desc->component.comp1_density]);
> +	if (desc->content.NC)
> +		msg_pdbg2("Component 2 density:           %s\n",
> +			  str_mem[desc->component.comp2_density]);
> +	else
> +		msg_pdbg2("Component 2 is not used.\n");
> +	msg_pdbg2("Read Clock Frequency:           %s\n",
> +		  str_freq[desc->component.freq_read]);
> +	msg_pdbg2("Read ID and Status Clock Freq.: %s\n",
> +		  str_freq[desc->component.freq_read_id]);
> +	msg_pdbg2("Write and Erase Clock Freq.:    %s\n",
> +		  str_freq[desc->component.freq_write]);
> +	msg_pdbg2("Fast Read is %ssupported.\n",
> +		  desc->component.fastread ? "" : "not ");


Not something you have to change, but maybe we should store that
information about fast read capability somewhere.


> +	if (desc->component.fastread)
> +		msg_pdbg2("Fast Read Clock Frequency:      %s\n",
> +			  str_freq[desc->component.freq_fastread]);
> +	if (desc->component.FLILL == 0)
> +		msg_pdbg2("No forbidden opcodes.\n");
> +	else {
> +		msg_pdbg2("Invalid instruction 0:          0x%02x\n",
> +			  desc->component.invalid_instr0);
> +		msg_pdbg2("Invalid instruction 1:          0x%02x\n",
> +			  desc->component.invalid_instr1);
> +		msg_pdbg2("Invalid instruction 2:          0x%02x\n",
> +			  desc->component.invalid_instr2);
> +		msg_pdbg2("Invalid instruction 3:          0x%02x\n",
> +			  desc->component.invalid_instr3);
> +	}
> +	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."
> +	};
> +	if (i >= 5) {
> +		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]);
> +	if (base > limit)
> +		msg_pdbg2("is unused.\n");
> +	else
> +		msg_pdbg2("0x%08x - 0x%08x\n", base, limit | 0x0fff);
> +}
> +
> +void prettyprint_ich_descriptor_region(const struct ich_descriptors *desc)
> +{
> +	uint8_t i;
> +	uint8_t nr = desc->content.NR + 1;
> +	msg_pdbg2("=== Region Section ===\n");
> +	if (nr >= 5) {
> +		msg_pdbg2("%s: number of regions too high (%d).\n", __func__,
> +			 nr);
> +		return;
> +	}
> +	for (i = 0; i <= nr; i++)
> +		msg_pdbg2("FLREG%d   0x%08x\n", i, desc->region.FLREGs[i]);
> +	msg_pdbg2("\n");
> +
> +	msg_pdbg2("--- Details ---\n");
> +	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)
> +{
> +	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);
> +	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':' ');
> +	msg_pdbg2("\n");
> +}
> +
> +static uint32_t read_descriptor_reg(uint8_t section, uint16_t offset, void *spibar)
> +{
> +	uint32_t control = 0;
> +	control |= (section << FDOC_FDSS_OFF) & FDOC_FDSS;
> +	control |= (offset << FDOC_FDSI_OFF) & FDOC_FDSI;
> +	mmio_le_writel(control, spibar + ICH9_REG_FDOC);
> +	return mmio_le_readl(spibar + ICH9_REG_FDOD);
> +}
> +
> +int read_ich_descriptors_via_fdo(void *spibar, struct ich_descriptors *desc)
> +{
> +	uint8_t i;
> +	uint8_t nr;
> +	struct ich_desc_region *r = &desc->region;
> +
> +	/* Test if bit-fields are working as expected */
> +	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) {
> +		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);
> +		return RET_ERR;
> +	}

Hm. Having the block above in a separate function would be nice, but if
you add a short
/* FIXME: Replace this with dynamic bitfield fixup */
comment, that would be totally OK for me (and the pending patch queue
would be a lot shorter).


> +
> +	msg_pdbg2("Reading flash descriptors "
> +		 "mapped by the chipset via FDOC/FDOD...");
> +	/* content section */
> +	desc->content.FLVALSIG	= read_descriptor_reg(0, 0, spibar);
> +	desc->content.FLMAP0	= read_descriptor_reg(0, 1, spibar);
> +	desc->content.FLMAP1	= read_descriptor_reg(0, 2, spibar);
> +	desc->content.FLMAP2	= read_descriptor_reg(0, 3, spibar);
> +
> +	/* component section */
> +	desc->component.FLCOMP	= read_descriptor_reg(1, 0, spibar);
> +	desc->component.FLILL	= read_descriptor_reg(1, 1, spibar);
> +	desc->component.FLPB	= read_descriptor_reg(1, 2, spibar);
> +
> +	/* region section */
> +	nr = desc->content.NR + 1;
> +	if (nr >= 5) {
> +		msg_pdbg2("%s: number of regions too high (%d) - failed\n",
> +			  __func__, nr);
> +		return RET_ERR;
> +	}
> +	for (i = 0; i <= nr; i++)
> +		desc->region.FLREGs[i] = read_descriptor_reg(2, i, spibar);
> +
> +	/* master section */
> +	desc->master.FLMSTR1 = read_descriptor_reg(3, 0, spibar);
> +	desc->master.FLMSTR2 = read_descriptor_reg(3, 1, spibar);
> +	desc->master.FLMSTR3 = read_descriptor_reg(3, 2, 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.

Interesting. So Intel removed that capability on ICH9 and later?


> +	 */
> +
> +	msg_pdbg2(" done.\n");
> +	return RET_OK;
> +}
> +#endif /* defined(__i386__) || defined(__x86_64__) */
> diff --git a/ich_descriptors.h b/ich_descriptors.h
> new file mode 100644
> index 0000000..098d3cd
> --- /dev/null
> +++ b/ich_descriptors.h
> @@ -0,0 +1,259 @@
> +/*
> + * This file is part of the flashrom project.
> + *
> + * Copyright (c) 2010  Matthias Wenzel <bios at mazzoo dot de>
> + * Copyright (c) 2011  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
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> + */
> +
> +#if defined(__i386__) || defined(__x86_64__)
> +#ifndef __ICH_DESCRIPTORS_H__
> +#define __ICH_DESCRIPTORS_H__ 1
> +
> +#include <stdint.h>
> +
> +#define RET_OK		0
> +#define RET_ERR		-1
> +#define RET_WARN	-2
> +#define RET_PARAM	-3
> +#define RET_OOB		-4

Argl... If you add a comment
/* FIXME: Replace with generic return codes */
that's OK for me now.


> +
> +#define ICH9_REG_FDOC		0xB0	/* 32 Bits Flash Descriptor Observability Control */
> +					/* 0-1: reserved */
> +#define FDOC_FDSI_OFF		2	/* 2-11: Flash Descriptor Section Index */
> +#define FDOC_FDSI		(0x3f << FDOC_FDSI_OFF)
> +#define FDOC_FDSS_OFF		12	/* 12-14: Flash Descriptor Section Select */
> +#define FDOC_FDSS		(0x3 << FDOC_FDSS_OFF)
> +					/* 15-31: reserved */
> +
> +#define ICH9_REG_FDOD		0xB4	/* 32 Bits Flash Descriptor Observability Data */
> +
> +/* Field locations and semantics for LVSCC, UVSCC and related words in the flash
> + * descriptor are equal therefore they all share the same macros below. */
> +#define VSCC_BES_OFF		0	/* 0-1: Block/Sector Erase Size */
> +#define VSCC_BES			(0x3 << VSCC_BES_OFF)
> +#define VSCC_WG_OFF		2	/* 2: Write Granularity */
> +#define VSCC_WG				(0x1 << VSCC_WG_OFF)
> +#define VSCC_WSR_OFF		3	/* 3: Write Status Required */
> +#define VSCC_WSR			(0x1 << VSCC_WSR_OFF)
> +#define VSCC_WEWS_OFF		4	/* 4: Write Enable on Write Status */
> +#define VSCC_WEWS			(0x1 << VSCC_WEWS_OFF)
> +					/* 5-7: reserved */
> +#define VSCC_EO_OFF		8	/* 8-15: Erase Opcode */
> +#define VSCC_EO				(0xff << VSCC_EO_OFF)
> +					/* 16-22: reserved */
> +#define VSCC_VCL_OFF		23	/* 23: Vendor Component Lock */
> +#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)

Are you sure all users of ICH_FREG_{BASE,LIMIT} have been adjusted to
the new definition, especially in ichspi.c and elsewhere?


> +
> +/* Used to select the right descriptor printing function.
> + * Currently only ICH8 and Ibex Peak are supported.
> + */
> +enum ich_chipset {
> +	CHIPSET_ICH_UNKNOWN,
> +	CHIPSET_ICH7 = 7,
> +	CHIPSET_ICH8,
> +	CHIPSET_ICH9,
> +	CHIPSET_ICH10,
> +	CHIPSET_5_SERIES_IBEX_PEAK,
> +	CHIPSET_6_SERIES_COUGAR_POINT,
> +	CHIPSET_7_SERIES_PANTHER_POINT
> +};
> +
> +void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity);
> +
> +struct ich_desc_content {
> +	uint32_t FLVALSIG;	/* 0x00 */
> +	union {			/* 0x04 */
> +		uint32_t FLMAP0;
> +		struct {
> +			uint32_t FCBA	:8, /* Flash Component Base Address */
> +				 NC	:2, /* Number Of Components */
> +					:6,
> +				 FRBA	:8, /* Flash Region Base Address */
> +				 NR	:3, /* Number Of Regions */
> +					:5;
> +		};
> +	};
> +	union {			/* 0x08 */
> +		uint32_t FLMAP1;
> +		struct {
> +			uint32_t FMBA	:8, /* Flash Master Base Address */
> +				 NM	:3, /* Number Of Masters */
> +					:5,
> +				 FISBA	:8, /* Flash ICH Strap Base Address */
> +				 ISL	:8; /* ICH Strap Length */
> +		};
> +	};
> +	union {			/* 0x0c */
> +		uint32_t FLMAP2;
> +		struct {
> +			uint32_t FMSBA	:8, /* Flash (G)MCH Strap Base Addr. */
> +				 MSL	:8, /* MCH Strap Length */
> +					:16;
> +		};
> +	};
> +};
> +
> +struct ich_desc_component {
> +	union {			/* 0x00 */
> +		uint32_t FLCOMP; /* Flash Components Register */
> +		struct {
> +			uint32_t comp1_density	:3,
> +				 comp2_density	:3,
> +						:11,
> +				 freq_read	:3,
> +				 fastread	:1,
> +				 freq_fastread	:3,
> +				 freq_write	:3,
> +				 freq_read_id	:3,
> +						:2;
> +		};
> +	};
> +	union {			/* 0x04 */
> +		uint32_t FLILL; /* Flash Invalid Instructions Register */
> +		struct {
> +			uint32_t invalid_instr0	:8,
> +				 invalid_instr1	:8,
> +				 invalid_instr2	:8,
> +				 invalid_instr3	:8;
> +		};
> +	};
> +	union {			/* 0x08 */
> +		uint32_t FLPB; /* Flash Partition Boundary Register */
> +		struct {
> +			uint32_t FPBA	:13, /* Flash Partition Boundary Addr */
> +					:19;
> +		};
> +	};
> +};
> +
> +struct ich_desc_region {
> +	union {
> +		uint32_t FLREGs[5];
> +		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;
> +			};
> +		};
> +	};
> +};
> +
> +struct ich_desc_master {
> +	union {
> +		uint32_t FLMSTR1;
> +		struct {
> +			uint32_t BIOS_req_ID	:16,
> +				 BIOS_descr_r	:1,
> +				 BIOS_BIOS_r	:1,
> +				 BIOS_ME_r	:1,
> +				 BIOS_GbE_r	:1,
> +				 BIOS_plat_r	:1,
> +						:3,
> +				 BIOS_descr_w	:1,
> +				 BIOS_BIOS_w	:1,
> +				 BIOS_ME_w	:1,
> +				 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,
> +				 ME_ME_r	:1,
> +				 ME_GbE_r	:1,
> +				 ME_plat_r	:1,
> +						:3,
> +				 ME_descr_w	:1,
> +				 ME_BIOS_w	:1,
> +				 ME_ME_w	:1,
> +				 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,
> +				 GbE_ME_r	:1,
> +				 GbE_GbE_r	:1,
> +				 GbE_plat_r	:1,
> +						:3,
> +				 GbE_descr_w	:1,
> +				 GbE_BIOS_w	:1,
> +				 GbE_ME_w	:1,
> +				 GbE_GbE_w	:1,
> +				 GbE_plat_w	:1,
> +						:3;
> +		};
> +	};
> +};

I once complained about the size of one of the bitfields in this file,
but I don't remember if you fixed that. Can you verify?


> +
> +struct ich_descriptors {
> +	struct ich_desc_content content;
> +	struct ich_desc_component component;
> +	struct ich_desc_region region;
> +	struct ich_desc_master master;
> +};
> +
> +void prettyprint_ich_descriptors(enum ich_chipset, const struct ich_descriptors *desc);
> +
> +void prettyprint_ich_descriptor_content(const struct ich_desc_content *content);
> +void prettyprint_ich_descriptor_component(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);
> +
> +int read_ich_descriptors_via_fdo(void *spibar, struct ich_descriptors *desc);
> +
> +#endif /* __ICH_DESCRIPTORS_H__ */
> +#endif /* defined(__i386__) || defined(__x86_64__) */
> diff --git a/ichspi.c b/ichspi.c
> index 8b4210e..ad0b536 100644
> --- a/ichspi.c
> +++ b/ichspi.c
> @@ -29,6 +29,7 @@
>  #include "flash.h"
>  #include "programmer.h"
>  #include "spi.h"
> +#include "ich_descriptors.h"
>  
>  /* ICH9 controller register definition */
>  #define ICH9_REG_HSFS		0x04	/* 16 Bits Hardware Sequencing Flash Status */
> @@ -119,6 +120,16 @@
>  #define ICH9_REG_BBAR		0xA0	/* 32 Bits BIOS Base Address Configuration */
>  #define BBAR_MASK	0x00ffff00		/* 8-23: Bottom of System Flash */
>  
> +#define ICH8_REG_VSCC		0xC1	/* 32 Bits Vendor Specific Component Capabilities */
> +#define ICH9_REG_LVSCC		0xC4	/* 32 Bits Host Lower Vendor Specific Component Capabilities */
> +#define ICH9_REG_UVSCC		0xC8	/* 32 Bits Host Upper Vendor Specific Component Capabilities */
> +/* The individual fields of the VSCC registers are defined in the file
> + * ich_descriptors.h. The reason is that the same layout is also used in the
> + * flash descriptor to define the properties of the different flash chips
> + * supported. The BIOS (or the ME?) is responsible to populate the ICH registers
> + * with the information from the descriptor on startup depending on the actual
> + * chip(s) detected. */
> +
>  #define ICH9_REG_FPB		0xD0	/* 32 Bits Flash Partition Boundary */
>  #define FPB_FPBA_OFF		0	/* 0-12: Block/Sector Erase Size */
>  #define FPB_FPBA			(0x1FFF << FPB_FPBA_OFF)
> @@ -1129,9 +1140,6 @@ static int ich_spi_send_multicommand(struct spi_command *cmds)
>  #define ICH_BRWA(x)  ((x >>  8) & 0xff)
>  #define ICH_BRRA(x)  ((x >>  0) & 0xff)
>  
> -#define ICH_FREG_BASE(x)  ((x >>  0) & 0x1fff)
> -#define ICH_FREG_LIMIT(x) ((x >> 16) & 0x1fff)
> -
>  static void do_ich9_spi_frap(uint32_t frap, int i)
>  {
>  	static const char *const access_names[4] = {
> @@ -1158,9 +1166,8 @@ static void do_ich9_spi_frap(uint32_t frap, int i)
>  		return;
>  	}
>  
> -	msg_pdbg("0x%08x-0x%08x is %s\n",
> -		    (base << 12), (limit << 12) | 0x0fff,
> -		    access_names[rwperms]);
> +	msg_pdbg("0x%08x-0x%08x is %s\n", base, (limit | 0x0fff),
> +		 access_names[rwperms]);

Can you recheck this change? Looks odd.


>  }
>  
>  static const struct spi_programmer spi_programmer_ich7 = {
> @@ -1190,6 +1197,7 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
>  	uint8_t old, new;
>  	uint16_t spibar_offset, tmp2;
>  	uint32_t tmp;
> +	int ichspi_desc = 0;
>  
>  	switch (ich_generation) {
>  	case 7:
> @@ -1261,6 +1269,8 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
>  			msg_pinfo("WARNING: SPI Configuration Lockdown activated.\n");
>  			ichspi_lock = 1;
>  		}
> +		if (tmp2 & HSFS_FDV)
> +			ichspi_desc = 1;
>  
>  		tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFC);
>  		msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2);
> @@ -1308,12 +1318,38 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
>  			     mmio_readl(ich_spibar + ICH9_REG_OPMENU));
>  		msg_pdbg("0x9C: 0x%08x (OPMENU+4)\n",
>  			     mmio_readl(ich_spibar + ICH9_REG_OPMENU + 4));
> -		ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR);
> -		msg_pdbg("0xA0: 0x%08x (BBAR)\n",
> -			     ichspi_bbar);
> -		tmp = mmio_readl(ich_spibar + ICH9_REG_FPB);
> -		msg_pdbg("0xD0: 0x%08x (FPB)\n", tmp);
> +		if (ich_generation == 8) {
> +			tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC);
> +			msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp);
> +			msg_pdbg("VSCC: ");
> +			prettyprint_ich_reg_vscc(tmp, MSG_DEBUG);
> +		} else {
> +			ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR);
> +			msg_pdbg("0xA0: 0x%08x (BBAR)\n",
> +				     ichspi_bbar);
> +
> +			tmp = mmio_readl(ich_spibar + ICH9_REG_LVSCC);
> +			msg_pdbg("0xC4: 0x%08x (LVSCC)\n", tmp);
> +			msg_pdbg("LVSCC: ");
> +			prettyprint_ich_reg_vscc(tmp, MSG_DEBUG);
> +
> +			tmp = mmio_readl(ich_spibar + ICH9_REG_UVSCC);
> +			msg_pdbg("0xC8: 0x%08x (UVSCC)\n", tmp);
> +			msg_pdbg("UVSCC: ");
> +			prettyprint_ich_reg_vscc(tmp, MSG_DEBUG);
> +
> +			tmp = mmio_readl(ich_spibar + ICH9_REG_FPB);
> +			msg_pdbg("0xD0: 0x%08x (FPB)\n", tmp);
> +		}

Didn't see that flash descriptor override is printed anywhere... you
once described its effect in detail and that description would be really
nice to have in ichspi.c, maybe even with a flag which tells flashrom
about the effect (access control disabled or whatever it was).


>  
> +		msg_pdbg("\n");
> +		if (ichspi_desc) {
> +			struct ich_descriptors desc = {{ 0 }};
> +			if (read_ich_descriptors_via_fdo(ich_spibar, &desc) ==
> +			    RET_OK)
> +				prettyprint_ich_descriptors(CHIPSET_ICH_UNKNOWN,
> +							    &desc);
> +		}
>  		ich_init_opcodes();
>  		break;
>  	default:

Thanks a lot for reworking this patch so often!

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list