- flashrom-gerrit - mail.coreboot.org

Change in flashrom[stable]: ich_descriptors: Use MAX_NUM_FLREGS for entries[]
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21821 Change subject: ich_descriptors: Use MAX_NUM_FLREGS for entries[] ...................................................................... ich_descriptors: Use MAX_NUM_FLREGS for entries[] 5 regions made sense in 2013 when this bit of code was originally written. MAX_NUM_FLREGS is now used to keep track of the max number of flash regions and is >5 since Sunrise Point. Original-Change-Id: Idb559e618369fecf930724a7c1c84765247f3e38 Original-Reviewed-on: https://review.coreboot.org/21338 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Ic6b658a1625620009ffd23e526093609a6b0baf8 Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M ich_descriptors.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/21821/1 diff --git a/ich_descriptors.h b/ich_descriptors.h index 9a16031..db4d28f 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -568,7 +568,7 @@ struct ich_layout { struct flashrom_layout base; - struct romentry entries[5]; + struct romentry entries[MAX_NUM_FLREGS]; }; ssize_t ich_number_of_regions(enum ich_chipset cs, const struct ich_desc_content *content); -- To view, visit https://review.coreboot.org/21821 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Ic6b658a1625620009ffd23e526093609a6b0baf8 Gerrit-Change-Number: 21821 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: Move ich_layout from layout.h to ich_descriptors.h
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21820 Change subject: Move ich_layout from layout.h to ich_descriptors.h ...................................................................... Move ich_layout from layout.h to ich_descriptors.h This moves the ich_layout declaration from one header to another. This will avoid a circular dependency when we update the entries[] member in the follow-up patch to use MAX_NUM_FLREGS which is defined in ich_descriptors.h. Original-Change-Id: I08006f1f7c9ccdd17a9a6d74881ed2c8541d4de1 Original-Reviewed-on: https://review.coreboot.org/21337 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Ibcdf8ad694e88ffb14f70bb830ebd2a13d6cced8 Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M ich_descriptors.h M layout.h 2 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/21820/1 diff --git a/ich_descriptors.h b/ich_descriptors.h index 4ddc710..9a16031 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -566,6 +566,11 @@ struct ich_desc_upper_map upper; }; +struct ich_layout { + struct flashrom_layout base; + struct romentry entries[5]; +}; + ssize_t ich_number_of_regions(enum ich_chipset cs, const struct ich_desc_content *content); ssize_t ich_number_of_masters(enum ich_chipset cs, const struct ich_desc_content *content); diff --git a/layout.h b/layout.h index c93d754..f32dc82 100644 --- a/layout.h +++ b/layout.h @@ -57,11 +57,6 @@ struct romentry entry; }; -struct ich_layout { - struct flashrom_layout base; - struct romentry entries[5]; -}; - struct flashrom_layout *get_global_layout(void); int process_include_args(struct flashrom_layout *); -- To view, visit https://review.coreboot.org/21820 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Ibcdf8ad694e88ffb14f70bb830ebd2a13d6cced8 Gerrit-Change-Number: 21820 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: chipset_enable: Add support for C620-series Lewisburg PCH
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21819 Change subject: chipset_enable: Add support for C620-series Lewisburg PCH ...................................................................... chipset_enable: Add support for C620-series Lewisburg PCH This adds PCI IDs for C620-series PCHs and adds CHIPSET_C620_SERIES_LEWISBURG as a new entry in the ich_chipset enum. Lewisburg is very similar to Sunrise Point for Flashrom's purposes, however one important difference is the way the "number of masters" is interpreted from the flash descriptor (0-based vs. 1-based). There are also new flash regions defined. Original-Change-Id: I96c89bc28bdfcd953229c17679f2c28f8b874d0b Original-Reviewed-on: https://review.coreboot.org/20922 Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: Ia93f4616c9279173b5cd47be1999939aa2f02379 Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M chipset_enable.c M ich_descriptors.c M ichspi.c M programmer.h M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 119 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/21819/1 diff --git a/chipset_enable.c b/chipset_enable.c index 36e2838..a3b7f8d 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -601,6 +601,7 @@ top_swap = (gcs & 2) >> 1; break; case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: reg_name = "BIOS_SPI_BC"; gcs = pci_read_long(dev, 0xdc); bild = (gcs >> 7) & 1; @@ -658,6 +659,7 @@ case CHIPSET_8_SERIES_LYNX_POINT_LP: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: straps_names = straps_names_pch8_lp; break; case CHIPSET_8_SERIES_WELLSBURG: // FIXME: check datasheet @@ -681,6 +683,7 @@ bbs = (gcs >> 10) & 0x1; break; case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: bbs = (gcs >> 6) & 0x1; break; default: @@ -830,9 +833,8 @@ return 0; } -static int enable_flash_pch100(struct pci_dev *const dev, const char *const name) +static int enable_flash_pch100_or_c620(struct pci_dev *const dev, const char *const name, const enum ich_chipset pch_generation) { - const enum ich_chipset pch_generation = CHIPSET_100_SERIES_SUNRISE_POINT; int ret = ERROR_FATAL; /* @@ -886,6 +888,16 @@ pci_free_dev(spi_dev); pacc = saved_pacc; return ret; +} + +static int enable_flash_pch100(struct pci_dev *const dev, const char *const name) +{ + return enable_flash_pch100_or_c620(dev, name, CHIPSET_100_SERIES_SUNRISE_POINT); +} + +static int enable_flash_c620(struct pci_dev *const dev, const char *const name) +{ + return enable_flash_pch100_or_c620(dev, name, CHIPSET_C620_SERIES_LEWISBURG); } /* Silvermont architecture: Bay Trail(-T/-I), Avoton/Rangeley. @@ -1915,6 +1927,19 @@ {0x8086, 0xa153, NT, "Intel", "QM175", enable_flash_pch100}, {0x8086, 0xa154, NT, "Intel", "CM238", enable_flash_pch100}, {0x8086, 0xa155, NT, "Intel", "QMU185", enable_flash_pch100}, + {0x8086, 0xa1c1, NT, "Intel", "C621 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1c2, NT, "Intel", "C622 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1c3, NT, "Intel", "C624 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1c4, NT, "Intel", "C625 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1c5, NT, "Intel", "C626 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1c6, NT, "Intel", "C627 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1c7, NT, "Intel", "C628 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa242, NT, "Intel", "C624 Series Chipset Supersku", enable_flash_c620}, + {0x8086, 0xa243, NT, "Intel", "C627 Series Chipset Supersku", enable_flash_c620}, + {0x8086, 0xa244, NT, "Intel", "C621 Series Chipset Supersku", enable_flash_c620}, + {0x8086, 0xa245, NT, "Intel", "C627 Series Chipset Supersku", enable_flash_c620}, + {0x8086, 0xa246, NT, "Intel", "C628 Series Chipset Supersku", enable_flash_c620}, + {0x8086, 0xa247, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, #endif {0}, }; diff --git a/ich_descriptors.c b/ich_descriptors.c index 7916152..69fcd75 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -46,6 +46,8 @@ ssize_t ich_number_of_regions(const enum ich_chipset cs, const struct ich_desc_content *const cont) { switch (cs) { + case CHIPSET_C620_SERIES_LEWISBURG: + return 16; case CHIPSET_100_SERIES_SUNRISE_POINT: return 10; case CHIPSET_9_SERIES_WILDCAT_POINT_LP: @@ -67,10 +69,16 @@ ssize_t ich_number_of_masters(const enum ich_chipset cs, const struct ich_desc_content *const cont) { - if (cont->NM < MAX_NUM_MASTERS) - return cont->NM + 1; - else - return -1; + switch (cs) { + case CHIPSET_C620_SERIES_LEWISBURG: + if (cont->NM <= MAX_NUM_MASTERS) + return cont->NM; + default: + if (cont->NM < MAX_NUM_MASTERS) + return cont->NM + 1; + } + + return -1; } void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity, bool print_vcl) @@ -98,6 +106,7 @@ "5 series Ibex Peak", "6 series Cougar Point", "7 series Panther Point", "8 series Lynx Point", "Baytrail", "8 series Lynx Point LP", "8 series Wellsburg", "9 series Wildcat Point", "9 series Wildcat Point LP", "100 series Sunrise Point", + "C620 series Lewisburg", }; if (cs < CHIPSET_ICH8 || cs - CHIPSET_ICH8 + 1 >= ARRAY_SIZE(chipset_names)) cs = 0; @@ -187,7 +196,8 @@ case CHIPSET_8_SERIES_WELLSBURG: case CHIPSET_9_SERIES_WILDCAT_POINT: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: - case CHIPSET_100_SERIES_SUNRISE_POINT: { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: { uint8_t size_enc; if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; @@ -243,6 +253,7 @@ case CHIPSET_9_SERIES_WILDCAT_POINT_LP: return freq_str[0][value]; case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: return freq_str[1][value]; case CHIPSET_ICH_UNKNOWN: default: @@ -256,7 +267,7 @@ msg_pdbg2("=== Component Section ===\n"); msg_pdbg2("FLCOMP 0x%08x\n", desc->component.FLCOMP); msg_pdbg2("FLILL 0x%08x\n", desc->component.FLILL ); - if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG) msg_pdbg2("FLILL1 0x%08x\n", desc->component.FLILL1); msg_pdbg2("\n"); @@ -276,10 +287,10 @@ if (cs > CHIPSET_6_SERIES_COUGAR_POINT) msg_pdbg2("Dual Output Fast Read Support: %sabled\n", desc->component.modes.dual_output ? "dis" : "en"); - if (desc->component.FLILL == 0 && - (cs != CHIPSET_100_SERIES_SUNRISE_POINT || desc->component.FLILL1 == 0)) { - msg_pdbg2("No forbidden opcodes.\n"); - } else { + + int has_forbidden_opcode = 0; + if (desc->component.FLILL != 0) { + has_forbidden_opcode = 1; msg_pdbg2("Invalid instruction 0: 0x%02x\n", desc->component.invalid_instr0); msg_pdbg2("Invalid instruction 1: 0x%02x\n", @@ -288,7 +299,10 @@ desc->component.invalid_instr2); msg_pdbg2("Invalid instruction 3: 0x%02x\n", desc->component.invalid_instr3); - if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + } + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG) { + if (desc->component.FLILL1 != 0) { + has_forbidden_opcode = 1; msg_pdbg2("Invalid instruction 4: 0x%02x\n", desc->component.invalid_instr4); msg_pdbg2("Invalid instruction 5: 0x%02x\n", @@ -299,14 +313,17 @@ desc->component.invalid_instr7); } } + if (!has_forbidden_opcode) + msg_pdbg2("No forbidden opcodes.\n"); + msg_pdbg2("\n"); } static void pprint_freg(const struct ich_desc_region *reg, uint32_t i) { static const char *const region_names[] = { - "Descr.", "BIOS", "ME", "GbE", "Platf.", "unknown", "unknown", "unknown", - "EC", "unknown", + "Descr.", "BIOS", "ME", "GbE", "Platf.", "unknown", "BIOS2", "unknown", + "EC/BMC", "unknown", "IE", "10GbE", "unknown", "unknown", "unknown", "unknown" }; if (i >= ARRAY_SIZE(region_names)) { msg_pdbg2("%s: region index too high.\n", __func__); @@ -372,6 +389,32 @@ msg_pdbg2("%-4s", master_names[i]); for (j = 0; j < 10; j++) msg_pdbg2(" %c%c ", + desc->master.mstr[i].read & (1 << j) ? 'r' : ' ', + desc->master.mstr[i].write & (1 << j) ? 'w' : ' '); + msg_pdbg2("\n"); + } + } else if (cs == CHIPSET_C620_SERIES_LEWISBURG) { + const char *const master_names[] = { + "BIOS", "ME", "GbE", "DE", "BMC", "IE", + }; + /* NM starts at 1 instead of 0 for LBG */ + if (nm > ARRAY_SIZE(master_names)) { + msg_pdbg2("%s: number of masters too high (%d).\n", __func__, + desc->content.NM); + return; + } + + msg_pdbg2("%s %s %s %s %s %s %s %s %s %s %s %s %s %s %s %s %s\n", + " ", /* width of master name (4 chars minimum) */ + " FD ", " BIOS", " ME ", " GbE ", " Pltf", + " DE ", "BIOS2", " Reg7", " BMC ", " DE2 ", + " IE ", "10GbE", "OpROM", "Reg13", "Reg14", + "Reg15"); + for (i = 0; i < nm; i++) { + size_t j; + msg_pdbg2("%-4s", master_names[i]); + for (j = 0; j < 16; j++) + msg_pdbg2(" %c%c ", desc->master.mstr[i].read & (1 << j) ? 'r' : ' ', desc->master.mstr[i].write & (1 << j) ? 'w' : ' '); msg_pdbg2("\n"); @@ -829,6 +872,8 @@ return CHIPSET_8_SERIES_LYNX_POINT; msg_pwarn("Peculiar firmware descriptor, assuming Wildcat Point compatibility.\n"); return CHIPSET_9_SERIES_WILDCAT_POINT; + } else if (content->NM == 6) { + return CHIPSET_C620_SERIES_LEWISBURG; } else { return CHIPSET_100_SERIES_SUNRISE_POINT; } @@ -846,14 +891,16 @@ const enum ich_chipset guess = guess_ich_chipset_from_content(content); if (component->modes.freq_read == 6) { - if (guess != CHIPSET_100_SERIES_SUNRISE_POINT) + if ((guess != CHIPSET_100_SERIES_SUNRISE_POINT) && (guess != CHIPSET_C620_SERIES_LEWISBURG)) { msg_pwarn("\nThe firmware descriptor has the read frequency set to 17MHz. However,\n" "it doesn't look like a Skylake/Sunrise Point compatible descriptor.\n" "Please report this message, the output of `ich_descriptors_tool` for\n" "your descriptor and the output of `lspci -nn` to flashrom(a)flashrom.org\n\n"); - return CHIPSET_100_SERIES_SUNRISE_POINT; + return CHIPSET_100_SERIES_SUNRISE_POINT; + } + return guess; } else { - if (guess == CHIPSET_100_SERIES_SUNRISE_POINT) { + if (guess == CHIPSET_100_SERIES_SUNRISE_POINT || guess == CHIPSET_C620_SERIES_LEWISBURG) { msg_pwarn("\nThe firmware descriptor looks like a Skylake/Sunrise Point descriptor.\n" "However, the read frequency isn't set to 17MHz (the only valid value).\n" "Please report this message, the output of `ich_descriptors_tool` for\n" @@ -992,6 +1039,7 @@ case CHIPSET_9_SERIES_WILDCAT_POINT: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; } else { @@ -1022,14 +1070,13 @@ uint32_t control = 0; control |= (section << FDOC_FDSS_OFF) & FDOC_FDSS; control |= (offset << FDOC_FDSI_OFF) & FDOC_FDSI; - if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG) { mmio_le_writel(control, spibar + PCH100_REG_FDOC); return mmio_le_readl(spibar + PCH100_REG_FDOD); } else { mmio_le_writel(control, spibar + ICH9_REG_FDOC); return mmio_le_readl(spibar + ICH9_REG_FDOD); } - } int read_ich_descriptors_via_fdo(enum ich_chipset cs, void *spibar, struct ich_descriptors *desc) @@ -1113,7 +1160,8 @@ int layout_from_ich_descriptors(struct ich_layout *const layout, const void *const dump, const size_t len) { static const char *const regions[] = { - "fd", "bios", "me", "gbe", "pd", "reg5", "reg6", "reg7", "ec", "reg9" + "fd", "bios", "me", "gbe", "pd", "reg5", "bios2", "reg7", "ec", "reg9", "ie", + "10gbe", "reg12", "reg13", "reg14", "reg15" }; struct ich_descriptors desc; diff --git a/ichspi.c b/ichspi.c index 9ee8044..9d6b62b 100644 --- a/ichspi.c +++ b/ichspi.c @@ -389,11 +389,13 @@ pprint_reg(HSFS, FDONE, reg_val, ", "); pprint_reg(HSFS, FCERR, reg_val, ", "); pprint_reg(HSFS, AEL, reg_val, ", "); - if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { + if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT && + ich_generation != CHIPSET_C620_SERIES_LEWISBURG) { pprint_reg(HSFS, BERASE, reg_val, ", "); } pprint_reg(HSFS, SCIP, reg_val, ", "); - if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) { + if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT || + ich_generation == CHIPSET_C620_SERIES_LEWISBURG) { pprint_reg(HSFS, PRR34_LOCKDN, reg_val, ", "); pprint_reg(HSFS, WRSDIS, reg_val, ", "); } @@ -406,7 +408,8 @@ { msg_pdbg("HSFC: "); pprint_reg(HSFC, FGO, reg_val, ", "); - if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { + if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT && + ich_generation != CHIPSET_C620_SERIES_LEWISBURG) { pprint_reg(HSFC, FCYCLE, reg_val, ", "); } else { _pprint_reg(HSFC, PCH100_HSFC_FCYCLE, PCH100_HSFC_FCYCLE_OFF, reg_val, ", "); @@ -1703,7 +1706,18 @@ /* Moving registers / bits */ if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) { num_freg = 10; - num_pr = 6; + num_pr = 6; /* Includes GPR0 */ + reg_pr0 = PCH100_REG_FPR0; + swseq_data.reg_ssfsc = PCH100_REG_SSFSC; + swseq_data.reg_preop = PCH100_REG_PREOP; + swseq_data.reg_optype = PCH100_REG_OPTYPE; + swseq_data.reg_opmenu = PCH100_REG_OPMENU; + hwseq_data.addr_mask = PCH100_FADDR_FLA; + hwseq_data.only_4k = true; + hwseq_data.hsfc_fcycle = PCH100_HSFC_FCYCLE; + } if (ich_generation == CHIPSET_C620_SERIES_LEWISBURG) { + num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */ + num_pr = 6; /* Includes GPR0 */ reg_pr0 = PCH100_REG_FPR0; swseq_data.reg_ssfsc = PCH100_REG_SSFSC; swseq_data.reg_preop = PCH100_REG_PREOP; @@ -1824,7 +1838,7 @@ tmp = mmio_readl(ich_spibar + ICH9_REG_FADDR); msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp); - if (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT) { + if (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || ich_gen == CHIPSET_C620_SERIES_LEWISBURG) { const uint32_t dlock = mmio_readl(ich_spibar + PCH100_REG_DLOCK); msg_pdbg("0x0c: 0x%08x (DLOCK)\n", dlock); prettyprint_pch100_reg_dlock(dlock); @@ -1890,7 +1904,8 @@ msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp); msg_pdbg("VSCC: "); prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); - } else if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { + } else if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT && + ich_generation != CHIPSET_C620_SERIES_LEWISBURG) { if (ich_generation != CHIPSET_BAYTRAIL && desc_valid) { ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR); msg_pdbg("0xA0: 0x%08x (BBAR)\n", diff --git a/programmer.h b/programmer.h index e58fd32..a98b713 100644 --- a/programmer.h +++ b/programmer.h @@ -656,6 +656,7 @@ CHIPSET_9_SERIES_WILDCAT_POINT, CHIPSET_9_SERIES_WILDCAT_POINT_LP, CHIPSET_100_SERIES_SUNRISE_POINT, /* also 6th/7th gen Core i/o (LP) variants */ + CHIPSET_C620_SERIES_LEWISBURG, }; /* ichspi.c */ diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 82c74bd..5ab946f 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -42,7 +42,9 @@ static const char *const region_names[] = { "Descriptor", "BIOS", "ME", "GbE", "Platform", - "Region5", "Region6", "Region7", "EC", "Region9", + "Region5", "BIOS2", "Region7", "EC/BMC", "Region9", + "IE", "10GbE", "Region12", "Region13", "Region14", + "Region15" }; static void dump_file(const char *prefix, const uint32_t *dump, unsigned int len, -- To view, visit https://review.coreboot.org/21819 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Ia93f4616c9279173b5cd47be1999939aa2f02379 Gerrit-Change-Number: 21819 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: ichspi: "Fix" access permission reporting for regions > 7
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21818 Change subject: ichspi: "Fix" access permission reporting for regions > 7 ...................................................................... ichspi: "Fix" access permission reporting for regions > 7 Can't find bits that tell us the actual permissions in charge. So report them as unknown. Original-Change-Id: Ib73f95e0348f5c6d89988e3ea3529af0ec3b23a6 Original-Reviewed-on: https://review.coreboot.org/21106 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Change-Id: I59cce146c96e3c1f33b51265084c1352602ea5ac Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ichspi.c 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/18/21818/1 diff --git a/ichspi.c b/ichspi.c index 5f3e394..9ee8044 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1543,6 +1543,7 @@ static const char *const access_names[4] = { "locked", "read-only", "write-only", "read-write" }; + const int rwperms_unknown = ARRAY_SIZE(access_names); static const char *const region_names[5] = { "Flash Descriptor", "BIOS", "Management Engine", "Gigabit Ethernet", "Platform Data" @@ -1550,10 +1551,20 @@ const char *const region_name = i < ARRAY_SIZE(region_names) ? region_names[i] : "unknown"; uint32_t base, limit; - int rwperms = (((ICH_BRWA(frap) >> i) & 1) << 1) | - (((ICH_BRRA(frap) >> i) & 1) << 0); + int rwperms; int offset = ICH9_REG_FREG0 + i * 4; uint32_t freg = mmio_readl(ich_spibar + offset); + + if (i < 8) { + rwperms = (((ICH_BRWA(frap) >> i) & 1) << 1) | + (((ICH_BRRA(frap) >> i) & 1) << 0); + } else { + /* Datasheets don't define any access bits for regions > 7. We + can't rely on the actual descriptor settings either as there + are several overrides for them (those by other masters are + not even readable by us, *shrug*). */ + rwperms = rwperms_unknown; + } base = ICH_FREG_BASE(freg); limit = ICH_FREG_LIMIT(freg); @@ -1569,6 +1580,11 @@ region_name, base, limit, access_names[rwperms]); return 0; } + if (rwperms == rwperms_unknown) { + msg_pdbg("FREG%i: %s region (0x%08x-0x%08x) has unknown permissions.\n", + i, region_name, base, limit); + return 0; + } msg_pwarn("FREG%i: Warning: %s region (0x%08x-0x%08x) is %s.\n", i, region_name, base, limit, access_names[rwperms]); -- To view, visit https://review.coreboot.org/21818 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I59cce146c96e3c1f33b51265084c1352602ea5ac Gerrit-Change-Number: 21818 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: Add functions to read/erase/write/verify by layout
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21787 ) Change subject: Add functions to read/erase/write/verify by layout ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/582/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/527/ : SUCCESS -- To view, visit https://review.coreboot.org/21787 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: Id7d0397f29027ff16f2d5bf25811fd03d371a16f Gerrit-Change-Number: 21787 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:49:08 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: Add a convenient libflashrom interface
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21788 ) Change subject: Add a convenient libflashrom interface ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/583/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/528/ : SUCCESS -- To view, visit https://review.coreboot.org/21788 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: Ibe14c89c96e45ed7679332b0f5cf44b1c9f1d9ee Gerrit-Change-Number: 21788 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:49:06 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: chipset_enable: Mark Braswell as tested
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Hello David Hendricks, I'd like you to do a code review. Please visit https://review.coreboot.org/21817 to review the following change. Change subject: chipset_enable: Mark Braswell as tested ...................................................................... chipset_enable: Mark Braswell as tested Reported by Uwe Vieweg: https://mail.coreboot.org/pipermail/flashrom/2017-August/015059.html Original-Change-Id: Iaf7558af8737af36401f577ca7aba9fd7114a3df Original-Reviewed-on: https://review.coreboot.org/20923 Original-Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I77c49c89ca552e8483022e3cd2a1db10b52554b1 Signed-off-by: David Hendricks <david.hendricks(a)gmail.com> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/21817/1 diff --git a/chipset_enable.c b/chipset_enable.c index 6a93d0d..36e2838 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1740,7 +1740,7 @@ {0x8086, 0x1f39, NT, "Intel", "Avoton/Rangeley", enable_flash_silvermont}, {0x8086, 0x1f3a, NT, "Intel", "Avoton/Rangeley", enable_flash_silvermont}, {0x8086, 0x1f3b, NT, "Intel", "Avoton/Rangeley", enable_flash_silvermont}, - {0x8086, 0x229c, NT, "Intel", "Braswell", enable_flash_silvermont}, + {0x8086, 0x229c, OK, "Intel", "Braswell", enable_flash_silvermont}, {0x8086, 0x2310, NT, "Intel", "DH89xxCC (Cave Creek)", enable_flash_pch7}, {0x8086, 0x2390, NT, "Intel", "Coleto Creek", enable_flash_pch7}, {0x8086, 0x2410, OK, "Intel", "ICH", enable_flash_ich0}, -- To view, visit https://review.coreboot.org/21817 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I77c49c89ca552e8483022e3cd2a1db10b52554b1 Gerrit-Change-Number: 21817 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>

1 0

Change in flashrom[stable]: Include <sys/types.h> wherever ssize_t is used
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21816 Change subject: Include <sys/types.h> wherever ssize_t is used ...................................................................... Include <sys/types.h> wherever ssize_t is used `ssize_t` is a POSIX type (cf. IEEE Std 1003.1). Original-Change-Id: I5f6f114523f541b3a8d845c6faee2c0b9f753bae Original-Reviewed-on: https://review.coreboot.org/21015 Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Reviewed-by: Urja Rannikko <urjaman(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: Iffabf97a2e3f3540e5969e214f9c49ac06fa2597 Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reported-by: Urja Rannikko <urjaman(a)gmail.com> --- M dediprog.c M ich_descriptors.c M ich_descriptors.h 3 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/16/21816/1 diff --git a/dediprog.c b/dediprog.c index 6f82772..2f5b441 100644 --- a/dediprog.c +++ b/dediprog.c @@ -21,6 +21,7 @@ #include "platform.h" +#include <sys/types.h> #include <stdlib.h> #include <stdio.h> #include <string.h> diff --git a/ich_descriptors.c b/ich_descriptors.c index a2f8edf..7916152 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -34,6 +34,7 @@ #define UPPER_MAP_OFFSET (4096 - 256 - 4) #define getVTBA(flumap) (((flumap)->FLUMAP1 << 4) & 0x00000ff0) +#include <sys/types.h> #include <string.h> #include "flash.h" /* for msg_* */ #include "programmer.h" diff --git a/ich_descriptors.h b/ich_descriptors.h index 574bc5f..4ddc710 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -22,6 +22,7 @@ #ifndef __ICH_DESCRIPTORS_H__ #define __ICH_DESCRIPTORS_H__ 1 +#include <sys/types.h> #include <stdint.h> #include "programmer.h" /* for enum ich_chipset */ -- To view, visit https://review.coreboot.org/21816 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Iffabf97a2e3f3540e5969e214f9c49ac06fa2597 Gerrit-Change-Number: 21816 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: ich_descriptors: Modify limits for C620/Lewisburg PCH
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21815 Change subject: ich_descriptors: Modify limits for C620/Lewisburg PCH ...................................................................... ich_descriptors: Modify limits for C620/Lewisburg PCH Original-Change-Id: Ic8adc4b87993e65096166fa6d665432697070b4c Original-Reviewed-on: https://review.coreboot.org/20936 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Id755183cd2d86d050b722f150450eb67cd90b1d4 Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M ich_descriptors.h 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/21815/1 diff --git a/ich_descriptors.h b/ich_descriptors.h index 8c313d1..574bc5f 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -165,7 +165,7 @@ }; }; -#define MAX_NUM_FLREGS 10 /* 16 on unsupported Lewisburg PCH */ +#define MAX_NUM_FLREGS 16 struct ich_desc_region { /* * Number of entries and width differ on various generations: @@ -174,6 +174,7 @@ * ICH8 .. Panther Point/7 5 13 * Lynx Point/8 .. Wildcat Point/9 7 15 * Sunrise Point/100 .. 10 15 + * Lewisburg/100 .. 16 15 */ union { uint32_t FLREGs[MAX_NUM_FLREGS]; /* Flash Descriptor Regions */ @@ -188,7 +189,7 @@ }; }; -#define MAX_NUM_MASTERS 5 /* 6 on unsupported Lewisburg PCH */ +#define MAX_NUM_MASTERS 6 /* 5 prior to C620/Lewisburg PCH */ struct ich_desc_master { union { uint32_t FLMSTRs[MAX_NUM_MASTERS]; /* Flash Masters */ -- To view, visit https://review.coreboot.org/21815 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Id755183cd2d86d050b722f150450eb67cd90b1d4 Gerrit-Change-Number: 21815 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: rpci: Use pci_dev struct pointer to avoid API breaks
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21814 Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... rpci: Use pci_dev struct pointer to avoid API breaks The pci_dev structure is never meant to be used as is, but always as a pointer. By using the struct itself in undo_pci_write_data, we are risking data corruption, or buffer overflows if the structure size changes. This is especially apparent on my system where flashrom segfaults because I compile it with pciutils 3.3.0 and I run it on a system with pciutils 3.5.2. The struture size is different and causes a struct with the wrong size to be sent to the library, with invalid internal field values. This has been discovered and discussed in Change ID 18925 [1] [1] https://review.coreboot.org/#/c/18925/ Original-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Original-Reviewed-on: https://review.coreboot.org/20784 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> Change-Id: I58dc4f37c5ea29cbad30777ebc0e6c2e3c6ad446 Signed-off-by: Youness Alaoui <kakaroto(a)kakaroto.homelinux.net> --- M chipset_enable.c M pcidev.c M programmer.h 3 files changed, 25 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/14/21814/1 diff --git a/chipset_enable.c b/chipset_enable.c index 20d2662..6a93d0d 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -843,6 +843,7 @@ * straints (e.g. on PCI domains, extended PCIe config space). */ struct pci_access *const pci_acc = pci_alloc(); + struct pci_access *const saved_pacc = pacc; if (!pci_acc) { msg_perr("Can't allocate PCI accessor.\n"); return ret; @@ -857,6 +858,9 @@ return ret; } + /* Modify pacc so the rpci_write can register the undo callback with a + * device using the correct pci_access */ + pacc = pci_acc; enable_flash_ich_report_gcs(spi_dev, pch_generation, NULL); const int ret_bc = enable_flash_ich_bios_cntl_config_space(spi_dev, pch_generation, 0xdc); @@ -880,6 +884,7 @@ _freepci_ret: pci_free_dev(spi_dev); + pacc = saved_pacc; return ret; } diff --git a/pcidev.c b/pcidev.c index 2c78063..f4e5542 100644 --- a/pcidev.c +++ b/pcidev.c @@ -160,6 +160,7 @@ return 1; } pci_cleanup(pacc); + pacc = NULL; return 0; } @@ -259,7 +260,7 @@ }; struct undo_pci_write_data { - struct pci_dev dev; + struct pci_dev *dev; int reg; enum pci_write_type type; union { @@ -272,22 +273,23 @@ int undo_pci_write(void *p) { struct undo_pci_write_data *data = p; - if (pacc == NULL) { - msg_perr("%s: Tried to undo PCI writes without a valid PCI context!\n" - "Please report a bug at flashrom(a)flashrom.org\n", __func__); + if (pacc == NULL || data->dev == NULL) { + msg_perr("%s: Tried to undo PCI writes without a valid PCI %s!\n" + "Please report a bug at flashrom(a)flashrom.org\n", + __func__, data->dev == NULL ? "device" : "context"); return 1; } msg_pdbg("Restoring PCI config space for %02x:%02x:%01x reg 0x%02x\n", - data->dev.bus, data->dev.dev, data->dev.func, data->reg); + data->dev->bus, data->dev->dev, data->dev->func, data->reg); switch (data->type) { case pci_write_type_byte: - pci_write_byte(&data->dev, data->reg, data->bytedata); + pci_write_byte(data->dev, data->reg, data->bytedata); break; case pci_write_type_word: - pci_write_word(&data->dev, data->reg, data->worddata); + pci_write_word(data->dev, data->reg, data->worddata); break; case pci_write_type_long: - pci_write_long(&data->dev, data->reg, data->longdata); + pci_write_long(data->dev, data->reg, data->longdata); break; } /* p was allocated in register_undo_pci_write. */ @@ -303,7 +305,11 @@ msg_gerr("Out of memory!\n"); \ exit(1); \ } \ - undo_pci_write_data->dev = *a; \ + if (pacc) \ + undo_pci_write_data->dev = pci_get_dev(pacc, \ + a->domain, a->bus, a->dev, a->func); \ + else \ + undo_pci_write_data->dev = NULL; \ undo_pci_write_data->reg = b; \ undo_pci_write_data->type = pci_write_type_##c; \ undo_pci_write_data->c##data = pci_read_##c(dev, reg); \ diff --git a/programmer.h b/programmer.h index ec00bd9..e58fd32 100644 --- a/programmer.h +++ b/programmer.h @@ -195,6 +195,11 @@ struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar); /* rpci_write_* are reversible writes. The original PCI config space register * contents will be restored on shutdown. + * To clone the pci_dev instances internally, the `pacc` global + * variable has to reference a pci_access method that is compatible + * with the given pci_dev handle. The referenced pci_access (not + * the variable) has to stay valid until the shutdown handlers are + * finished. */ int rpci_write_byte(struct pci_dev *dev, int reg, uint8_t data); int rpci_write_word(struct pci_dev *dev, int reg, uint16_t data); -- To view, visit https://review.coreboot.org/21814 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I58dc4f37c5ea29cbad30777ebc0e6c2e3c6ad446 Gerrit-Change-Number: 21814 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit