Hi All
I guess because coreboot does not provide any IMC firmware here.
Here I guess the bit which is telling that IMC is active survived reboot.
This confirms that IMC is "dead" here, there is no MBOX interface no bit which tells if IMC is active.
I admit this is strange. The only explanation is that the IMC tries to run non-existing stuff. So it in fact tries to execute something and there is no way it could tell us that it is active. Not sure what to do here. I know there is some soft strap override but I dont know if current strap can be read to detect if IMC is enabled but actually not doing anything.
Maybe this is getChipSysMode() in CIMX.
Power on Nothing
Yes because it erased most of the flash ;)
So... This is going to be complicated. Can you please send me in private both images? Or paste URL where i can get ami bios image.
I will check if IMC firmware is there or not (I guess none in coreboot). What we could possibly do is to re-add a firmware to coreboot image. But we would have to override the straps and activate the imc. There is also BuildParameters.ImcEnableOverWrite in SB800 CIMX.
What is your goal here anyway? To have smooth transition to coreboot? Or you just reported this issue?
I guess we need to implement some safety checks that if IMC strap is always enabled then IMC firmware must exist and ideally be same as in flash.
But first we need to know what CIMX is doing.
if 1) EC is disabled and it is re-enabled by soft strap
This seems not to be the case, because
if (validateImcFirmware (pConfig)) { softwareToggleImcStrapping (pConfig);
It will enable the IMC only if valid firmware is found. And for CIMX800 we are not doing this, only hudson has AMD firmware.
2) if it is enabled by some hardware strap
This could be detected most likely by getChipSysMode() in this case we need to disallow write of firmware which has no IMC code. Here are couple of problems, what to do if firmware is there, but different for example... Or with coreboot may happen it is on different location. Franly I don't know how to deal with that because for this I would need detailed knowledge of the firmware itself. I only disassembled that and I can see it just spins in memory when 0xb4 is invoked and waits for 0xb5 (and I have seen sometimes for 0xb6)
And this gets worse because Hudson has USB firmware and Gigabit ethernet firmware. Which are are same PITA.
Thanks Rudolf
Hi Rudolf,
The goal of these tests is to try to help you guys a bit. Eventually, our solution will use coreboot, or a derivative. The tool flashrom seems like a good option for us to upgrade BIOS in the field. I'll be happy to assist if I can, at least provide test feedback.
I will send to you directly the 2 roms I was using.
Cheers, Michel.
On Wed, 03 Apr 2013 00:35:09 +0200 Rudolf Marek r.marek@assembler.cz wrote:
I am not completely sure if I understood everything you wrote, but all of that is only relevant for "wrong" images, right? So the current patch* does improve the overall usability of flashrom on AMD boards and only users that try to move from an image with active IMC to an image w/o or improper IMC firmware will run into problems?
http://patchwork.coreboot.org/patch/3894/
Detect and temporarily disable the IMC while accessing the flash. Disable writes on default, but allow the user to enforce it.
Signed-off-by: Rudolf Marek r.marek@assembler.cz Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at ---
I changed a few bits in the execution path too, so please retest and ack if it works. Please test if writing is really disabled without the parameter too. Thanks Rudolf for the patch!
Makefile | 2 +- chipset_enable.c | 4 ++ flash.h | 1 + flashrom.8 | 17 ++++++ imc.c | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ programmer.h | 3 + sb600spi.c | 60 +++++++++++++------ 7 files changed, 238 insertions(+), 20 deletions(-) create mode 100644 imc.c
diff --git a/Makefile b/Makefile index 805290c..46d007f 100644 --- a/Makefile +++ b/Makefile @@ -424,7 +424,7 @@ 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 sb600spi.o wbsio_spi.o mcp6x_spi.o +PROGRAMMER_OBJS += it87spi.o it85spi.o sb600spi.o imc.o wbsio_spi.o mcp6x_spi.o PROGRAMMER_OBJS += ichspi.o ich_descriptors.o else endif diff --git a/chipset_enable.c b/chipset_enable.c index 3979347..c884c80 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -922,6 +922,10 @@ static int enable_flash_sb600(struct pci_dev *dev, const char *name) uint8_t reg; int ret;
+ if (imc_shutdown(dev) != 0) { + return ERROR_FATAL; + } + /* Clear ROM protect 0-3. */ for (reg = 0x50; reg < 0x60; reg += 4) { prot = pci_read_long(dev, reg); diff --git a/flash.h b/flash.h index 1857cc0..8f578bc 100644 --- a/flash.h +++ b/flash.h @@ -26,6 +26,7 @@
#include <inttypes.h> #include <stdint.h> +#include <stdbool.h> #include <stddef.h> #ifdef _WIN32 #include <windows.h> diff --git a/flashrom.8 b/flashrom.8 index 4e6ab55..885c9c0 100644 --- a/flashrom.8 +++ b/flashrom.8 @@ -329,6 +329,23 @@ flashrom doesn't detect an active IT87 LPC<->SPI bridge, please send a bug report so we can diagnose the problem. .sp .TP +.B AMD chipsets +.sp +Beginning with the SB700 chipset there is an integrated microcontroller (IMC) based on the 8051 embedded in +every AMD southbridge. Its firmware resides in the same flash chip as the host's which makes writing to the +flash risky if the IMC is active. Flashrom tries to temporarily disable the IMC but even then changing the +contents of the flash can have unwanted effects: when the IMC continues (at the latest after a reboot) it will +continue executing code from the flash. If the code was removed or changed in an unfortunate way it is +unpredictable what the IMC will do. Therefore, if flashrom detects an active IMC it will disable write support +unless the user forces it with the +.sp +.B " flashrom -p internal:amd_imc_force=yes" +.sp +syntax. The user is responsible for supplying a suitable image or leaving out the IMC region with the help of +a layout file. This limitation might be removed in the future when we understand the details better and have +received enough feedback from users. Please report the outcome if you had to use this option to write a chip. +.sp +.TP .B Intel chipsets .sp If you have an Intel chipset with an ICH8 or later southbridge with SPI flash diff --git a/imc.c b/imc.c new file mode 100644 index 0000000..9f12461 --- /dev/null +++ b/imc.c @@ -0,0 +1,171 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2013 Rudolf Marek r.marek@assembler.cz + * Copyright (C) 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 + * 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 "flash.h" +#include "programmer.h" +#include "hwaccess.h" +#include "spi.h" + +/* same as serverengines */ +static void enter_conf_mode_ec(uint16_t port) +{ + outb(0x5a, port); +} + +static void exit_conf_mode_ec(uint16_t port) +{ + outb(0xa5, port); +} + +static uint16_t get_sio_port(struct pci_dev *dev) +{ + uint16_t ec_port; + + if (!dev) { + return 0; + } + + if (!(pci_read_byte(dev, 0x40) & (1 << 7))) + return 0; + + ec_port = pci_read_word(dev, 0xa4); + + if (!(ec_port & 0x1)) + return 0; + + ec_port &= ~0x1; + + return ec_port; +} + + +static void write_data(uint16_t port, uint8_t off, uint8_t data) +{ + outb(off, port); + outb(data, port + 1); +} + +static uint8_t read_data(uint16_t port, uint8_t off) +{ + outb(off, port); + return inb(port + 1); +} + + +/* Wait for up to 10 ms for a response. */ +static int mbox_wait_ack(uint16_t mbox_port) +{ + int i = 10; + while (read_data(mbox_port, 0x82) != 0xfa) { + if (--i == 0) { + msg_pwarn("IMC MBOX: Timeout!\n"); + return 1; + } + programmer_delay(1000); + } + return 0; +} + +static uint16_t mbox_get_port(uint16_t sio_port) +{ + uint16_t mbox_port; + + enter_conf_mode_ec(sio_port); + + /* Go to LDN 9, mailbox */ + write_data(sio_port, 7, 9); + + /* MBOX inactive? */ + if ((read_data(sio_port, 0x30) & 1) == 0) { + exit_conf_mode_ec(sio_port); + return 0; + } + + mbox_port = read_data(sio_port, 0x60) << 8; + mbox_port |= read_data(sio_port, 0x61); + + exit_conf_mode_ec(sio_port); + return mbox_port; +} + +/* Returns negative values when IMC is inactive, positive values on errors */ +static int imc_send_cmd(struct pci_dev *dev, uint8_t cmd) +{ + uint16_t sio_port; + uint16_t mbox_port; + + sio_port = get_sio_port(dev); + if (!sio_port) + return -1; + msg_pdbg2("IMC SIO is at 0x%x.\n", sio_port); + + mbox_port = mbox_get_port(sio_port); + if (!mbox_port) + return -1; + msg_pdbg2("IMC MBOX is at 0x%x.\n", mbox_port); + + write_data(mbox_port, 0x82, 0x0); + write_data(mbox_port, 0x83, cmd); + write_data(mbox_port, 0x84, 0x0); + /* trigger transfer 0x96 with subcommand cmd */ + write_data(mbox_port, 0x80, 0x96); + + return mbox_wait_ack(mbox_port); +} + +static int imc_resume(void *data) +{ + struct pci_dev *dev = data; + int ret = imc_send_cmd(dev, 0xb5); + + if (ret != 0) + msg_pinfo("Resuming IMC failed)\n"); + else + msg_pdbg2("IMC resumed.\n"); + return ret; +} + +int imc_shutdown(struct pci_dev *dev) +{ + /* Try to put IMC to sleep */ + int ret = imc_send_cmd(dev, 0xb4); + + /* No IMC activity detectable, assume we are fine */ + if (ret < 0) { + msg_pdbg2("No IMC found.\n"); + return 0; + } + + if (ret != 0) { + msg_perr("Shutting down IMC failed.\n"); + return ret; + } + msg_pdbg2("Shutting down IMC successful.\n"); + + if (register_shutdown(imc_resume, dev)) + return 1; + + return ret; +} + +#endif diff --git a/programmer.h b/programmer.h index 2ea7b48..7de8f2c 100644 --- a/programmer.h +++ b/programmer.h @@ -572,6 +572,9 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, enum ich_chipset ich_generation); int via_init_spi(struct pci_dev *dev, uint32_t mmio_base);
+/* imc.c */ +int imc_shutdown(struct pci_dev *dev); + /* it85spi.c */ int it85xx_spi_init(struct superio s);
diff --git a/sb600spi.c b/sb600spi.c index a5c00d8..fba976b 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -23,6 +23,8 @@
#if defined(__i386__) || defined(__x86_64__)
+#include <string.h> +#include <stdlib.h> #include "flash.h" #include "programmer.h" #include "hwaccess.h" @@ -47,7 +49,7 @@ static void reset_internal_fifo_pointer(void) { mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2);
- /* FIXME: This loop makes no sense at all. */ + /* FIXME: This loop needs a timeout. */ while (mmio_readb(sb600_spibar + 0xD) & 0x7) msg_pspew("reset\n"); } @@ -59,8 +61,7 @@ static int compare_internal_fifo_pointer(uint8_t want) tmp = mmio_readb(sb600_spibar + 0xd) & 0x07; want &= 0x7; if (want != tmp) { - msg_perr("SB600 FIFO pointer corruption! Pointer is %d, wanted " - "%d\n", tmp, want); + msg_perr("FIFO pointer corruption! Pointer is %d, wanted %d\n", tmp, want); msg_perr("Something else is accessing the flash chip and " "causes random corruption.\nPlease stop all " "applications and drivers and IPMI which access the " @@ -210,10 +211,26 @@ int sb600_probe_spi(struct pci_dev *dev) struct pci_dev *smbus_dev; uint32_t tmp; uint8_t reg; + bool amd_imc_force = false; static const char *const speed_names[4] = { "66/reserved", "33", "22", "16.5" };
+ char *arg = extract_programmer_param("amd_imc_force"); + if (arg && !strcmp(arg, "yes")) { + amd_imc_force = true; + msg_pspew("amd_imc_force enabled.\n"); + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for amd_imc_force.\n"); + free(arg); + return ERROR_FATAL; + } else if (arg) { + msg_perr("Unknown argument for amd_imc_force: "%s" (not "yes").\n", arg); + free(arg); + return ERROR_FATAL; + } + free(arg); + /* Read SPI_BaseAddr */ tmp = pci_read_long(dev, 0xa0); tmp &= 0xffffffe0; /* remove bits 4-0 (reserved) */ @@ -300,23 +317,28 @@ int sb600_probe_spi(struct pci_dev *dev) return 0; }
+ /* TODO: we should not only look at IntegratedImcPresent (LPC Dev 20, Func 3, 40h) but also at + * IMCEnable(Strap) and Override EcEnable(Strap) (sb8xx, sb9xx?, a50: Misc_Reg: 80h-87h; + * sb7xx, sp5100: PM_Reg: B0h-B1h) etc. */ reg = pci_read_byte(dev, 0x40); - msg_pdbg("SB700 IMC is %sactive.\n", (reg & (1 << 7)) ? "" : "not "); - if (reg & (1 << 7)) { - /* If we touch any region used by the IMC, the IMC and the SPI - * interface will lock up, and the only way to recover is a - * hard reset, but that is a bad choice for a half-erased or - * half-written flash chip. - * There appears to be an undocumented register which can freeze - * or disable the IMC, but for now we want to play it safe. - */ - msg_perr("The SB700 IMC is active and may interfere with SPI " - "commands. Disabling write.\n"); - /* FIXME: Should we only disable SPI writes, or will the lockup - * affect LPC/FWH chips as well? - */ - programmer_may_write = 0; - } + if ((reg & (1 << 7)) != 0) { + if (!amd_imc_force) + programmer_may_write = 0; + msg_pinfo("Writes have been disabled for safety reasons because the IMC is active\n" + "and it could interfere with accessing flash memory. Flashrom will try\n" + "to disable it temporarily but even then this might not be safe:\n" + "when it is reenabled and after a reboot it expects to find working code\n" + "in the flash and it is unpredictable what happens if there is none.\n" + "\n" + "To be safe make sure that there is a working IMC firmware at the right\n" + "location in the image you intend to write and do not attempt to erase.\n" + "\n" + "You can enforce write support with the amd_imc_force programmer option.\n"); + if (amd_imc_force) + msg_pinfo("Continuing with write support because the user forced us to!\n"); + } else + msg_pdbg("IMC is not active.\n"); +
/* Bring the FIFO to a clean state. */ reset_internal_fifo_pointer();
-
Racine, Michel -
Rudolf Marek -
Stefan Tauner