Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36424 )
Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ......................................................................
sb600spi.c: Improve IMC runtime detection in handle_imc()
The IMC detection in sb600spi is very rudimentary and incomplete. This expands it so that it can actually detect not only whether an IMC is in the system, but whether it's active or not.
This is based off ChromiumOS's fork of flashrom where the solution looks to be originally authored by Martin Roth in b/62141938.
Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M sb600spi.c 1 file changed, 34 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/36424/1
diff --git a/sb600spi.c b/sb600spi.c index c89d5a9..3c83b7b 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -55,6 +55,15 @@ #define FIFO_SIZE_OLD 8 #define FIFO_SIZE_YANGTZE 71
+#define PM_INDEX 0xCD6 +#define PM_DATA 0xCD7 +#define ISA_CONTROL 0x04 +#define MMIO_EN 0x02 +#define ACPI_MMIO_BASE 0xFED80000 +#define MISC_BASE 0xE00 +#define STRAP_STATUS 0x80 +#define EC_ENABLE_STRAP 0x04 + static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); static int spi100_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, @@ -80,6 +89,12 @@ .write_aai = default_spi_write_aai, };
+static uint8_t pm_ioread(uint8_t addr) +{ + OUTB(addr, PM_INDEX); + return INB(PM_DATA); +} + static int find_smbus_dev(uint16_t vendor, uint16_t device) { struct pci_dev *smbus_dev = pci_dev_find(vendor, device); @@ -522,6 +537,25 @@ return 0; }
+ /* On Yangtze and newer chips, check the EcEnableStrap bit */ + if ((amd_gen >= CHIPSET_YANGTZE) && (pm_ioread(ISA_CONTROL) & MMIO_EN )) { + uint8_t *AcpiMMIO = rphysmap("AMD ACPI registers", ACPI_MMIO_BASE & 0xfffff000, 0x1000); + if (AcpiMMIO != ERROR_PTR) { + AcpiMMIO += MISC_BASE + STRAP_STATUS; + uint32_t straps = mmio_readl(AcpiMMIO); + if ((straps & EC_ENABLE_STRAP) == 0){ + msg_pinfo("IMC is not active.\n"); + return 0; + } else { + msg_pinfo("IMC IS active! Straps: %08x (%p)\n", straps, (void *)AcpiMMIO); + msg_pinfo("StrapOverride: %08x\n", mmio_readl(AcpiMMIO + 4)); + } + + } else { + msg_pinfo("Error reading AMD ACPI registers.\n"); + } + } + if (!amd_imc_force) programmer_may_write = 0; msg_pinfo("Writes have been disabled for safety reasons because the presence of the IMC\n"
Hello Daniel Kurtz, build bot (Jenkins), Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/36424
to look at the new patch set (#3).
Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ......................................................................
sb600spi.c: Improve IMC runtime detection in handle_imc()
The IMC detection in sb600spi is very rudimentary and incomplete. This expands it so that it can actually detect not only whether an IMC is in the system, but whether it's active or not.
This is based off ChromiumOS's fork of flashrom where the solution looks to be originally authored by Martin Roth in b/62141938.
Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M sb600spi.c 1 file changed, 35 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/36424/3
Hello Daniel Kurtz, build bot (Jenkins), Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/36424
to look at the new patch set (#4).
Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ......................................................................
sb600spi.c: Improve IMC runtime detection in handle_imc()
The IMC detection in sb600spi is very rudimentary and incomplete. This expands it so that it can actually detect not only whether an IMC is in the system, but whether it's active or not.
This is based off ChromiumOS's fork of flashrom where the solution looks to be originally authored by Martin Roth in b/62141938.
Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M sb600spi.c 1 file changed, 35 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/36424/4
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36424 )
Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c File sb600spi.c:
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@526 PS5, Line 526: * sb7xx, sp5100: PM_Reg: B0h-B1h) etc. */ Looks like this comment could use an update.
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@533 PS5, Line 533: /* On Yangtze and newer chips, check the EcEnableStrap bit */ I'm not familiar with the AMD chipsets... but comment above seems to suggest, that the strap existed before Yangtze, why the constraint?
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@534 PS5, Line 534: spurious space
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@535 PS5, Line 535: ACPI_MMIO_BASE & 0xfffff000 It's a constant, and this masking is a no-op?
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@535 PS5, Line 535: AcpiMMIO lower-case, please. e.g. `acpi_mmio`
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@548 PS5, Line 548: msg_pinfo msg_perr()?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/36424 )
Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c File sb600spi.c:
https://review.coreboot.org/c/flashrom/+/36424/5/sb600spi.c@535 PS5, Line 535: AcpiMMIO
lower-case, please. e.g. […]
or `acpimmio`
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Nico Huber (Code Review)