Change in flashrom[master]: sb600spi.c: Improve IMC runtime detection in handle_imc()
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" -- To view, visit https://review.coreboot.org/c/flashrom/+/36424 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Gerrit-Change-Number: 36424 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: newchange
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 -- To view, visit https://review.coreboot.org/c/flashrom/+/36424 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Gerrit-Change-Number: 36424 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Daniel Kurtz <djkurtz@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
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 -- To view, visit https://review.coreboot.org/c/flashrom/+/36424 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Gerrit-Change-Number: 36424 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Daniel Kurtz <djkurtz@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
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()? -- To view, visit https://review.coreboot.org/c/flashrom/+/36424 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Gerrit-Change-Number: 36424 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Daniel Kurtz <djkurtz@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 14 Nov 2019 21:39:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
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`
-- To view, visit https://review.coreboot.org/c/flashrom/+/36424 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Gerrit-Change-Number: 36424 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Wed, 24 Jun 2020 16:13:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/36424?usp=email ) Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/flashrom/+/36424?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Gerrit-Change-Number: 36424 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz@chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org>
participants (4)
-
Anastasia Klimchuk (Code Review) -
Angel Pons (Code Review)
-
Edward O'Callaghan (Code Review)
-
Nico Huber (Code Review)