Am 18.09.2011 02:31 schrieb Stefan Tauner:
> Move the serprog specification there and document a few things we could not
> figure out on intel platforms yet.
>
> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
Author: stefanct
Date: Mon Nov 14 00:03:30 2011
New Revision: 1465
URL: http://flashrom.org/trac/flashrom/changeset/1465
Log:
Create a directory for documentation files
Move the serprog specification there and document a few things we could not
figure out on intel platforms yet.
Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Added:
trunk/Documentation/
trunk/Documentation/mysteries_intel.txt (contents, props changed)
trunk/Documentation/serprog-protocol.txt (contents, props changed)
- copied, changed from r1464, trunk/serprog-protocol.txt
Deleted:
trunk/serprog-protocol.txt
Added: trunk/Documentation/mysteries_intel.txt
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ trunk/Documentation/mysteries_intel.txt Mon Nov 14 00:03:30 2011 (r1465)
@@ -0,0 +1,18 @@
+= BBAR on ICH8 =
+ There is no sign of BBAR (BIOS Base Address Configuration Register) in the
+ public datasheet (or specification update) of the ICH8. Also, the offset of
+ that register has changed between ICH7 (SPIBAR + 50h) and ICH9 (SPIBAR +
+ A0h), so we have no clue if or where it is on ICH8. Out current policy is to
+ not touch it at all and assume/hope it is 0.
+
+= Accesses beyond region bounds in descriptor mode =
+ Intel's flash image tool will always expand the last region so that it covers
+ the whole flash chip, but some boards ship with a different configuration.
+ It seems that in descriptor mode all addresses outside the used regions can not
+ be accessed whatsoever. This is not specified anywhere publicly as far as we
+ could tell. flashrom does not handle this explicitly yet. It will just fail
+ when trying to touch an address outside of any region.
+ See also http://www.flashrom.org/pipermail/flashrom/2011-August/007606.html
+
+= Unlocking the ME region =
+TODO
Copied and modified: trunk/Documentation/serprog-protocol.txt (from r1464, trunk/serprog-protocol.txt)
==============================================================================
Author: stefanct
Date: Sun Nov 13 16:17:10 2011
New Revision: 1464
URL: http://flashrom.org/trac/flashrom/changeset/1464
Log:
ichspi: fix ich_init_opcodes() calls in ich_init_spi()
By calling it early ichspi_lock was not set up correctly in accordance
with the corresponding register, hence ich_init_opcodes() was always
trying to programming the opcodes instead of reading them in from the
opmenu in case of a locked down configuration.
Thanks to Jonathan A. Kollasch for reporting this bug.
Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
Modified:
trunk/ichspi.c
Modified: trunk/ichspi.c
==============================================================================
--- trunk/ichspi.c Thu Nov 10 00:40:00 2011 (r1463)
+++ trunk/ichspi.c Sun Nov 13 16:17:10 2011 (r1464)
@@ -1562,8 +1562,6 @@
/* Assign Virtual Address */
ich_spibar = rcrb + spibar_offset;
- ich_init_opcodes();
-
switch (ich_generation) {
case CHIPSET_ICH7:
msg_pdbg("0x00: 0x%04x (SPIS)\n",
@@ -1601,6 +1599,7 @@
msg_pinfo("WARNING: SPI Configuration Lockdown activated.\n");
ichspi_lock = 1;
}
+ ich_init_opcodes();
ich_set_bbar(0);
register_spi_programmer(&spi_programmer_ich7);
break;
@@ -1643,6 +1642,7 @@
"by the FRAP and FREG registers are NOT in "
"effect. Please note that Protected\n"
"Range (PR) restrictions still apply.\n");
+ ich_init_opcodes();
if (desc_valid) {
tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFC);
Am 26.10.2011 15:35 schrieb Stefan Tauner:
> Based on the new opaque programmer framework this patch adds support
> for Intel Hardware Sequencing on ICH8 and its successors.
>
> By default (or when setting the ich_spi_mode option to auto)
> the module tries to use swseq and only activates hwseq if need be:
> - if important opcodes are inaccessible due to lockdown
> - if more than one flash chip is attached.
> The other options (swseq, hwseq) select the respective mode (if possible).
>
> A general description of Hardware Sequencing can be found in this blog entry:
> http://blogs.coreboot.org/blog/2011/06/11/gsoc-2011-flashrom-part-1/
>
> TODO: adding real documentation when we have a directory for it
>
> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>
with a few small comments.
> ---
> flashrom.8 | 20 +++++
> ichspi.c | 268 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 283 insertions(+), 5 deletions(-)
>
> diff --git a/flashrom.8 b/flashrom.8
> index a8f4660..66cde4f 100644
> --- a/flashrom.8
> +++ b/flashrom.8
> @@ -303,6 +303,26 @@ is the I/O port number (must be a multiple of 8). In the unlikely case
> flashrom doesn't detect an active IT87 LPC<->SPI bridge, please send a bug
> report so we can diagnose the problem.
> .sp
> +If you have an Intel chipset with an ICH8 or later southbridge with SPI flash
> +attached, and if a valid descriptor was written to it (e.g. by the vendor), the
> +chipset provides an alternative way to access the flash chip(s) named
> +.BR "Hardware Sequencing" .
> +It is much simpler than the normal access method (called
> +.BR "Software Sequencing" "),"
> +but does not allow the software to choose the SPI commands to be sent.
> +You can use the
> +.sp
> +.B " flashrom \-p internal:ich_spi_mode=value"
> +.sp
> +syntax where value can be
> +.BR auto ", " swseq " or " hwseq .
> +By default
> +.RB "(or when setting " ich_spi_mode=auto )
> +the module tries to use swseq and only activates hwseq if need be (e.g. if
> +important opcodes are inaccessible due to lockdown; or if more than one flash
> +chip is attached). The other options (swseq, hwseq) select the respective mode
> +(if possible).
> +.sp
> If you have an Intel chipset with an ICH6 or later southbridge and if you want
> to set specific IDSEL values for a non-default flash chip or an embedded
> controller (EC), you can use the
> diff --git a/ichspi.c b/ichspi.c
> index bc03554..1d5dd34 100644
> --- a/ichspi.c
> +++ b/ichspi.c
> @@ -575,6 +575,25 @@ static int program_opcodes(int ich_generation, OPCODES *op, int enable_undo)
> return 0;
> }
>
> +/* Returns true if the most important opcodes are accessible. */
You assume that some erase opcode will be available if BYTE_PROGRAM is
available. I think that assumption is reasonable, but it could be
documented in this comment above the function.
> +static int ich_check_opcodes()
> +{
> + uint8_t ops[] = {
> + JEDEC_READ,
> + JEDEC_BYTE_PROGRAM,
> + JEDEC_RDSR,
> + 0
> + };
> + int i = 0;
> + while (ops[i] != 0) {
> + msg_pspew("checking for opcode 0x%02x\n", ops[i]);
> + if (find_opcode(curopcodes, ops[i]) == -1)
> + return 0;
> + i++;
> + }
> + return 1;
> +}
> +
> /*
> * Try to set BBAR (BIOS Base Address Register), but read back the value in case
> * it didn't stick.
> @@ -1325,6 +1525,14 @@ static const struct spi_programmer spi_programmer_ich9 = {
> .write_256 = default_spi_write_256,
> };
>
> +static const struct opaque_programmer opaque_programmer_ich_hwseq = {
> + .max_data_read = 64,
> + .max_data_write = 64,
> + .probe = ich_hwseq_probe,
> + .read = ich_hwseq_read,
> + .write = ich_hwseq_write,
Please add ich_hwseq_block_erase here.
> +};
> +
> int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
> int ich_generation)
> {
> @@ -1332,7 +1540,14 @@ 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;
> + char *arg;
> int desc_valid = 0;
> + struct ich_descriptors desc = {{ 0 }};
> + enum ich_spi_mode {
> + ich_auto,
> + ich_hwseq,
> + ich_swseq
> + } ich_spi_mode = ich_auto;
>
> switch (ich_generation) {
> case 7:
> @@ -1399,6 +1614,22 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
> case 9:
> case 10:
> default: /* Future version might behave the same */
> + arg = extract_programmer_param("ich_spi_mode");
> + if (arg && !strcmp(arg, "hwseq")) {
> + ich_spi_mode = ich_hwseq;
> + msg_pspew("user selected hwseq\n");
> + } else if (arg && !strcmp(arg, "swseq")) {
> + ich_spi_mode = ich_swseq;
> + msg_pspew("user selected swseq\n");
> + } else if (arg && !strcmp(arg, "auto")) {
> + msg_pspew("user selected auto\n");
> + ich_spi_mode = ich_auto;
> + } else if (arg && !strlen(arg))
> + msg_pinfo("Missing argument for ich_spi_mode.\n");
> + else if (arg)
> + msg_pinfo("Unknown argument for ich_spi_mode: %s\n", arg);
We should return an error all the way up to programmer init for both
cases (and clean up everything). I know that this is a complicated code
path, so if you decide not to fail programmer init here, please add a
comment like the one below:
/* FIXME: Return an error to programmer_init. */
> + free(arg);
> +
> tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFS);
> msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2);
> prettyprint_ich9_reg_hsfs(tmp2);
> @@ -1484,14 +1715,41 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
>
> msg_pdbg("\n");
> if (desc_valid) {
> - struct ich_descriptors desc = {{ 0 }};
> if (read_ich_descriptors_via_fdo(ich_spibar, &desc) ==
> ICH_RET_OK)
> prettyprint_ich_descriptors(CHIPSET_ICH_UNKNOWN,
> &desc);
> + /* If the descriptor is valid and indicates multiple
> + * flash devices we need to use hwseq to be able to
> + * access the second flash device.
> + */
> + if (ich_spi_mode == ich_auto && desc.content.NC != 0) {
> + msg_pinfo("Enabling hardware sequencing due to "
> + "multiple flash chips detected.\n");
> + ich_spi_mode = ich_hwseq;
> + }
> + }
> +
> + if (ich_spi_mode == ich_auto && ichspi_lock &&
> + !ich_check_opcodes()) {
> + msg_pinfo("Enabling hardware sequencing because "
> + "some important opcodes are locked.\n");
> + ich_spi_mode = ich_hwseq;
> + }
> +
> + if (ich_spi_mode == ich_hwseq) {
> + if (!desc_valid) {
> + msg_perr("Hardware sequencing was requested "
> + "but the flash descriptor is not "
> + "valid. Aborting.\n");
> + return 1;
Can you check that this indeed causes flashrom to return an error from
programmer_init? Chipset init IIRC ignores most errors unless they are
somehow declared to be fatal.
> + }
> + hwseq_data.size_comp0 = getFCBA_component_density(&desc, 0);
> + hwseq_data.size_comp1 = getFCBA_component_density(&desc, 1);
> + register_opaque_programmer(&opaque_programmer_ich_hwseq);
> + } else {
> + register_spi_programmer(&spi_programmer_ich9);
> }
> - register_spi_programmer(&spi_programmer_ich9);
> - ich_init_opcodes();
> break;
> }
>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/