Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/34072 ) Change subject: ichspi: Add support for discrete Cannon Lake PCHs ...................................................................... ichspi: Add support for discrete Cannon Lake PCHs Only minor differences in the Firmware Descriptor, compared to their predecessors. We extend our check on the `ICCRIBA` field in the descriptor to dis- tinguish it from older generation. Alas, the `freq_read` field was repurposed, so we can't use it as sanity check any more. Change-Id: I1c2d1e8916cecd756e7ac1f0ba221d7cc361ba02 Signed-off-by: Nico Huber <nico.huber@secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/34072 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Matt DeVillier <matt.devillier@gmail.com> --- M chipset_enable.c M ich_descriptors.c M ich_descriptors.h M ichspi.c M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 87 insertions(+), 32 deletions(-) Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved diff --git a/chipset_enable.c b/chipset_enable.c index 877b8b6..427182c 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -2035,16 +2035,16 @@ {0x8086, 0xa2c9, B_S, NT, "Intel", "Z370", enable_flash_pch100}, {0x8086, 0xa2d2, B_S, NT, "Intel", "X299", enable_flash_pch100}, {0x8086, 0x5ae8, B_S, DEP, "Intel", "Apollo Lake", enable_flash_apl}, - {0x8086, 0xa303, B_S, BAD, "Intel", "H310", enable_flash_pch300}, - {0x8086, 0xa304, B_S, BAD, "Intel", "H370", enable_flash_pch300}, - {0x8086, 0xa305, B_S, BAD, "Intel", "Z390", enable_flash_pch300}, - {0x8086, 0xa306, B_S, BAD, "Intel", "Q370", enable_flash_pch300}, - {0x8086, 0xa308, B_S, BAD, "Intel", "B360", enable_flash_pch300}, - {0x8086, 0xa309, B_S, BAD, "Intel", "C246", enable_flash_pch300}, - {0x8086, 0xa30a, B_S, BAD, "Intel", "C242", enable_flash_pch300}, - {0x8086, 0xa30c, B_S, BAD, "Intel", "QM370", enable_flash_pch300}, - {0x8086, 0xa30d, B_S, BAD, "Intel", "HM370", enable_flash_pch300}, - {0x8086, 0xa30e, B_S, BAD, "Intel", "CM246", enable_flash_pch300}, + {0x8086, 0xa303, B_S, NT, "Intel", "H310", enable_flash_pch300}, + {0x8086, 0xa304, B_S, NT, "Intel", "H370", enable_flash_pch300}, + {0x8086, 0xa305, B_S, NT, "Intel", "Z390", enable_flash_pch300}, + {0x8086, 0xa306, B_S, NT, "Intel", "Q370", enable_flash_pch300}, + {0x8086, 0xa308, B_S, NT, "Intel", "B360", enable_flash_pch300}, + {0x8086, 0xa309, B_S, NT, "Intel", "C246", enable_flash_pch300}, + {0x8086, 0xa30a, B_S, NT, "Intel", "C242", enable_flash_pch300}, + {0x8086, 0xa30c, B_S, NT, "Intel", "QM370", enable_flash_pch300}, + {0x8086, 0xa30d, B_S, NT, "Intel", "HM370", enable_flash_pch300}, + {0x8086, 0xa30e, B_S, NT, "Intel", "CM246", enable_flash_pch300}, #endif {0}, }; diff --git a/ich_descriptors.c b/ich_descriptors.c index 684b845..6d7b020 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -41,6 +41,7 @@ case CHIPSET_APOLLO_LAKE: return 6; case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: return 16; case CHIPSET_100_SERIES_SUNRISE_POINT: return 10; @@ -102,7 +103,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", "Apollo Lake", + "C620 series Lewisburg", "300 series Cannon Point", "Apollo Lake", }; if (cs < CHIPSET_ICH8 || cs - CHIPSET_ICH8 + 1 >= ARRAY_SIZE(chipset_names)) cs = 0; @@ -194,6 +195,7 @@ case CHIPSET_9_SERIES_WILDCAT_POINT_LP: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: { uint8_t size_enc; if (idx == 0) { @@ -261,6 +263,7 @@ return freq_str[0][value]; case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: return freq_str[1][value]; case CHIPSET_APOLLO_LAKE: return freq_str[2][value]; @@ -277,6 +280,7 @@ switch (cs) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: has_flill1 = true; break; @@ -394,7 +398,8 @@ msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); - if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || + cs == CHIPSET_300_SERIES_CANNON_POINT) { const char *const master_names[] = { "BIOS", "ME", "GbE", "unknown", "EC", }; @@ -404,14 +409,26 @@ return; } - msg_pdbg2(" FD BIOS ME GbE Pltf Reg5 Reg6 Reg7 EC Reg9\n"); + size_t num_regions; + msg_pdbg2(" FD BIOS ME GbE Pltf Reg5 Reg6 Reg7 EC Reg9"); + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + num_regions = 10; + msg_pdbg2("\n"); + } else { + num_regions = 16; + msg_pdbg2(" RegA RegB RegC RegD RegE RegF\n"); + } for (i = 0; i < nm; i++) { size_t j; msg_pdbg2("%-4s", master_names[i]); - for (j = 0; j < 10; j++) + for (j = 0; j < min(num_regions, 12); j++) msg_pdbg2(" %c%c ", desc->master.mstr[i].read & (1 << j) ? 'r' : ' ', desc->master.mstr[i].write & (1 << j) ? 'w' : ' '); + for (; j < num_regions; j++) + msg_pdbg2(" %c%c ", + desc->master.mstr[i].ext_read & (1 << (j - 12)) ? 'r' : ' ', + desc->master.mstr[i].ext_write & (1 << (j - 12)) ? 'w' : ' '); msg_pdbg2("\n"); } } else if (cs == CHIPSET_C620_SERIES_LEWISBURG) { @@ -915,10 +932,15 @@ 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 if (content->ICCRIBA < 0x34) { + if (content->NM == 6) + return CHIPSET_C620_SERIES_LEWISBURG; + else + return CHIPSET_100_SERIES_SUNRISE_POINT; } else { - return CHIPSET_100_SERIES_SUNRISE_POINT; + if (content->ICCRIBA > 0x34) + msg_pwarn("Unknown firmware descriptor, assuming 300 series compatibility.\n"); + return CHIPSET_300_SERIES_CANNON_POINT; } } @@ -934,6 +956,9 @@ const enum ich_chipset guess = guess_ich_chipset_from_content(content); switch (guess) { + case CHIPSET_300_SERIES_CANNON_POINT: + /* `freq_read` was repurposed, so can't check on it any more. */ + return guess; case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: case CHIPSET_APOLLO_LAKE: @@ -1085,6 +1110,7 @@ case CHIPSET_9_SERIES_WILDCAT_POINT_LP: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; @@ -1119,6 +1145,7 @@ switch (cs) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: mmio_le_writel(control, spibar + PCH100_REG_FDOC); return mmio_le_readl(spibar + PCH100_REG_FDOD); diff --git a/ich_descriptors.h b/ich_descriptors.h index 4225286..c6bf5d8 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -170,8 +170,9 @@ * Chipset/Generation #FLREGs width (bits) * ICH8 .. Panther Point/7 5 13 * Lynx Point/8 .. Wildcat Point/9 7 15 - * Sunrise Point/100 .. 10 15 + * Sunrise Point/100 .. 200 Series 10 15 * Lewisburg/100 .. 16 15 + * Cannon Point/300 .. 16 15 */ union { uint32_t FLREGs[MAX_NUM_FLREGS]; /* Flash Descriptor Regions */ @@ -234,9 +235,10 @@ }; /* From Skylake on */ struct { - uint32_t :8, - read :12, - write :12; + uint32_t ext_read :4, + ext_write :4, + read :12, + write :12; } mstr[MAX_NUM_MASTERS]; }; }; diff --git a/ichspi.c b/ichspi.c index 8b8f0f6..5a86c96 100644 --- a/ichspi.c +++ b/ichspi.c @@ -395,15 +395,25 @@ 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 && - ich_generation != CHIPSET_C620_SERIES_LEWISBURG) { + switch (ich_generation) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: + break; + default: pprint_reg(HSFS, BERASE, reg_val, ", "); + break; } pprint_reg(HSFS, SCIP, reg_val, ", "); - if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT || - ich_generation == CHIPSET_C620_SERIES_LEWISBURG) { + switch (ich_generation) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: pprint_reg(HSFS, PRR34_LOCKDN, reg_val, ", "); pprint_reg(HSFS, WRSDIS, reg_val, ", "); + break; + default: + break; } pprint_reg(HSFS, FDOPSS, reg_val, ", "); pprint_reg(HSFS, FDV, reg_val, ", "); @@ -414,12 +424,16 @@ { msg_pdbg("HSFC: "); pprint_reg(HSFC, FGO, reg_val, ", "); - if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT && - ich_generation != CHIPSET_C620_SERIES_LEWISBURG) { - pprint_reg(HSFC, FCYCLE, reg_val, ", "); - } else { + switch (ich_generation) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: _pprint_reg(HSFC, PCH100_HSFC_FCYCLE, PCH100_HSFC_FCYCLE_OFF, reg_val, ", "); pprint_reg(HSFC, WET, reg_val, ", "); + break; + default: + pprint_reg(HSFC, FCYCLE, reg_val, ", "); + break; } pprint_reg(HSFC, FDBC, reg_val, ", "); pprint_reg(HSFC, SME, reg_val, "\n"); @@ -1567,9 +1581,10 @@ static enum ich_access_protection ich9_handle_frap(uint32_t frap, unsigned int i) { const int rwperms_unknown = ARRAY_SIZE(access_names); - static const char *const region_names[6] = { + static const char *const region_names[] = { "Flash Descriptor", "BIOS", "Management Engine", "Gigabit Ethernet", "Platform Data", "Device Expansion", + "BIOS2", "unknown", "EC/BMC", }; const char *const region_name = i < ARRAY_SIZE(region_names) ? region_names[i] : "unknown"; @@ -1724,6 +1739,7 @@ switch (ich_generation) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: num_pr = 6; /* Includes GPR0 */ reg_pr0 = PCH100_REG_FPR0; @@ -1754,6 +1770,7 @@ case CHIPSET_C620_SERIES_LEWISBURG: num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */ break; + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: num_freg = 16; break; @@ -1848,6 +1865,7 @@ switch (ich_gen) { case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: tmp = mmio_readl(ich_spibar + PCH100_REG_DLOCK); msg_pdbg("0x0c: 0x%08x (DLOCK)\n", tmp); @@ -1921,6 +1939,7 @@ case CHIPSET_ICH8: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: case CHIPSET_BAYTRAIL: break; @@ -1952,6 +1971,7 @@ case CHIPSET_ICH8: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_APOLLO_LAKE: break; default: @@ -1981,8 +2001,10 @@ ich_spi_mode = ich_hwseq; } - if (ich_spi_mode == ich_auto && ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT) { - msg_pdbg("Enabling hardware sequencing by default for 100 series PCH.\n"); + if (ich_spi_mode == ich_auto && + (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || + ich_gen == CHIPSET_300_SERIES_CANNON_POINT)) { + msg_pdbg("Enabling hardware sequencing by default for 100+ series PCH.\n"); ich_spi_mode = ich_hwseq; } diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 2c7966d..2db0f62 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -133,6 +133,7 @@ "\t- \"8\" or \"lynx\" for Intel's 8 series chipsets.\n" "\t- \"9\" or \"wildcat\" for Intel's 9 series chipsets.\n" "\t- \"100\" or \"sunrise\" for Intel's 100 series chipsets.\n" +"\t- \"300\" or \"cannon\" for Intel's 300 series chipsets.\n" "If '-d' is specified some regions such as the BIOS image as seen by the CPU or\n" "the GbE blob that is required to initialize the GbE are also dumped to files.\n", argv[0], argv[0]); @@ -221,6 +222,9 @@ else if ((strcmp(csn, "100") == 0) || (strcmp(csn, "sunrise") == 0)) cs = CHIPSET_100_SERIES_SUNRISE_POINT; + else if ((strcmp(csn, "300") == 0) || + (strcmp(csn, "cannon") == 0)) + cs = CHIPSET_300_SERIES_CANNON_POINT; else if (strcmp(csn, "apollo") == 0) cs = CHIPSET_APOLLO_LAKE; } -- To view, visit https://review.coreboot.org/c/flashrom/+/34072 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1c2d1e8916cecd756e7ac1f0ba221d7cc361ba02 Gerrit-Change-Number: 34072 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Jeremy Soller <jackpot51@gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged