July 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: ich_descriptors: Draw +0xfff into ICH_FREG_LIMIT()
by Nico Huber (Code Review) July 28, 2017

July 28, 2017

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/19046 ) Change subject: ich_descriptors: Draw +0xfff into ICH_FREG_LIMIT() ...................................................................... ich_descriptors: Draw +0xfff into ICH_FREG_LIMIT() The condition `base > limit` is still valid since `base` is always at least 4096 greater than `limit` in this case. Change-Id: I11ac0a50b3f32f47879e7cfb7a26068cd0572ede Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/19046 Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ich_descriptors.c M ich_descriptors.h M ichspi.c M util/ich_descriptors_tool/ich_descriptors_tool.c 4 files changed, 6 insertions(+), 9 deletions(-) Approvals: David Hendricks: Looks good to me, approved build bot (Jenkins): Verified diff --git a/ich_descriptors.c b/ich_descriptors.c index 1fc8835..3e53ec9 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -246,7 +246,7 @@ if (base > limit) msg_pdbg2("is unused.\n"); else - msg_pdbg2("0x%08x - 0x%08x\n", base, limit | 0x0fff); + msg_pdbg2("0x%08x - 0x%08x\n", base, limit); } void prettyprint_ich_descriptor_region(const struct ich_descriptors *desc) @@ -952,7 +952,7 @@ size_t i, j; for (i = 0, j = 0; i < min(desc.content.NR + 1, ARRAY_SIZE(regions)); ++i) { const chipoff_t base = ICH_FREG_BASE(desc.region.FLREGs[i]); - const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]) + 0xfff; + const chipoff_t limit = ICH_FREG_LIMIT(desc.region.FLREGs[i]); if (limit <= base) continue; layout->entries[j].start = base; diff --git a/ich_descriptors.h b/ich_descriptors.h index 831341a..ec85e0c 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -63,7 +63,7 @@ /* 24-31: reserved */ #define ICH_FREG_BASE(flreg) (((flreg) << 12) & 0x01fff000) -#define ICH_FREG_LIMIT(flreg) (((flreg) >> 4) & 0x01fff000) +#define ICH_FREG_LIMIT(flreg) ((((flreg) >> 4) & 0x01fff000) | 0x00000fff) void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity, bool print_vcl); diff --git a/ichspi.c b/ichspi.c index ecf0c42..5f3e394 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1566,14 +1566,12 @@ msg_pdbg("0x%02X: 0x%08x ", offset, freg); if (rwperms == 0x3) { msg_pdbg("FREG%i: %s region (0x%08x-0x%08x) is %s.\n", i, - region_name, base, (limit | 0x0fff), - access_names[rwperms]); + region_name, base, limit, access_names[rwperms]); return 0; } msg_pwarn("FREG%i: Warning: %s region (0x%08x-0x%08x) is %s.\n", i, - region_name, base, (limit | 0x0fff), - access_names[rwperms]); + region_name, base, limit, access_names[rwperms]); return 1; } @@ -1608,7 +1606,7 @@ msg_pdbg("0x%02X: 0x%08x ", off, pr); msg_pwarn("%sPR%u: Warning: 0x%08x-0x%08x is %s.\n", prefix, i, ICH_FREG_BASE(pr), - ICH_FREG_LIMIT(pr) | 0x0fff, access_names[rwperms]); + ICH_FREG_LIMIT(pr), access_names[rwperms]); return 1; } diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c b/util/ich_descriptors_tool/ich_descriptors_tool.c index 250abf0..7dfa5c0 100644 --- a/util/ich_descriptors_tool/ich_descriptors_tool.c +++ b/util/ich_descriptors_tool/ich_descriptors_tool.c @@ -58,7 +58,6 @@ return; } - limit = limit | 0x0fff; file_len = limit + 1 - base; if (base + file_len > len) { printf("The %s region is spanning 0x%08x-0x%08x, but it is " -- To view, visit https://review.coreboot.org/19046 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I11ac0a50b3f32f47879e7cfb7a26068cd0572ede Gerrit-Change-Number: 19046 Gerrit-PatchSet: 10 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: ichspi: Add support for Intel Skylake
by Nico Huber (Code Review) July 28, 2017

July 28, 2017

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/18962 ) Change subject: ichspi: Add support for Intel Skylake ...................................................................... ichspi: Add support for Intel Skylake The Sunrise Point PCH, paired with Skylake, has some minor changes in the HW sequencing interface: * Support for more flash regions moved PR* registers * Only 4KiB erase blocks are supported by the primary erase command * A second erase command for 64KiB pages was added * More commands were added for status register access etc. * A "Dedicated Lock Bits" register was added No support for the new commands was added. The SW sequencing interface seems to have moved register location and is not supported any more officially. It's also untested. Changes are loosely based on the Skylake support commit in Chromium OS by Ramya Vijaykumar: commit a9a64f9e4d52c39fcd3c5f7d7b88065baed189b1 Author: Ramya Vijaykumar <ramya.vijaykumar(a)intel.com> flashrom: Add Skylake platform support Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/18962 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Reviewed-by: Youness Alaoui <snifikino(a)gmail.com> --- M ich_descriptors.c M ich_descriptors.h M ichspi.c 3 files changed, 233 insertions(+), 87 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Youness Alaoui: Looks good to me, but someone else must approve diff --git a/ich_descriptors.c b/ich_descriptors.c index a12022c..1fc8835 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -824,6 +824,7 @@ case CHIPSET_8_SERIES_WELLSBURG: case CHIPSET_9_SERIES_WILDCAT_POINT: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: + case CHIPSET_100_SERIES_SUNRISE_POINT: if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; } else { @@ -849,16 +850,22 @@ /* Only used by ichspi.c */ #if CONFIG_INTERNAL == 1 && (defined(__i386__) || defined(__x86_64__)) -static uint32_t read_descriptor_reg(uint8_t section, uint16_t offset, void *spibar) +static uint32_t read_descriptor_reg(enum ich_chipset cs, uint8_t section, uint16_t offset, void *spibar) { uint32_t control = 0; control |= (section << FDOC_FDSS_OFF) & FDOC_FDSS; control |= (offset << FDOC_FDSI_OFF) & FDOC_FDSI; - mmio_le_writel(control, spibar + ICH9_REG_FDOC); - return mmio_le_readl(spibar + ICH9_REG_FDOD); + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + 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(void *spibar, struct ich_descriptors *desc) +int read_ich_descriptors_via_fdo(enum ich_chipset cs, void *spibar, struct ich_descriptors *desc) { uint8_t i; uint8_t nr; @@ -888,15 +895,15 @@ msg_pdbg2("Reading flash descriptors mapped by the chipset via FDOC/FDOD..."); /* content section */ - desc->content.FLVALSIG = read_descriptor_reg(0, 0, spibar); - desc->content.FLMAP0 = read_descriptor_reg(0, 1, spibar); - desc->content.FLMAP1 = read_descriptor_reg(0, 2, spibar); - desc->content.FLMAP2 = read_descriptor_reg(0, 3, spibar); + desc->content.FLVALSIG = read_descriptor_reg(cs, 0, 0, spibar); + desc->content.FLMAP0 = read_descriptor_reg(cs, 0, 1, spibar); + desc->content.FLMAP1 = read_descriptor_reg(cs, 0, 2, spibar); + desc->content.FLMAP2 = read_descriptor_reg(cs, 0, 3, spibar); /* component section */ - desc->component.FLCOMP = read_descriptor_reg(1, 0, spibar); - desc->component.FLILL = read_descriptor_reg(1, 1, spibar); - desc->component.FLPB = read_descriptor_reg(1, 2, spibar); + desc->component.FLCOMP = read_descriptor_reg(cs, 1, 0, spibar); + desc->component.FLILL = read_descriptor_reg(cs, 1, 1, spibar); + desc->component.FLPB = read_descriptor_reg(cs, 1, 2, spibar); /* region section */ nr = desc->content.NR + 1; @@ -906,12 +913,12 @@ return ICH_RET_ERR; } for (i = 0; i < 5; i++) - desc->region.FLREGs[i] = read_descriptor_reg(2, i, spibar); + desc->region.FLREGs[i] = read_descriptor_reg(cs, 2, i, spibar); /* master section */ - desc->master.FLMSTR1 = read_descriptor_reg(3, 0, spibar); - desc->master.FLMSTR2 = read_descriptor_reg(3, 1, spibar); - desc->master.FLMSTR3 = read_descriptor_reg(3, 2, spibar); + desc->master.FLMSTR1 = read_descriptor_reg(cs, 3, 0, spibar); + desc->master.FLMSTR2 = read_descriptor_reg(cs, 3, 1, spibar); + desc->master.FLMSTR3 = read_descriptor_reg(cs, 3, 2, spibar); /* Accessing the strap section via FDOC/D is only possible on ICH8 and * reading the upper map is impossible on all chipsets, so don't bother. diff --git a/ich_descriptors.h b/ich_descriptors.h index ecf44bf..831341a 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -33,6 +33,7 @@ #define ICH_RET_OOB -4 #define ICH9_REG_FDOC 0xB0 /* 32 Bits Flash Descriptor Observability Control */ +#define PCH100_REG_FDOC 0xB4 /* New offset from Sunrise Point on */ /* 0-1: reserved */ #define FDOC_FDSI_OFF 2 /* 2-11: Flash Descriptor Section Index */ #define FDOC_FDSI (0x3f << FDOC_FDSI_OFF) @@ -41,6 +42,7 @@ /* 15-31: reserved */ #define ICH9_REG_FDOD 0xB4 /* 32 Bits Flash Descriptor Observability Data */ +#define PCH100_REG_FDOD 0xB8 /* New offset from Sunrise Point on */ /* Field locations and semantics for LVSCC, UVSCC and related words in the flash * descriptor are equal therefore they all share the same macros below. */ @@ -581,7 +583,7 @@ void prettyprint_ich_descriptor_straps(enum ich_chipset cs, const struct ich_descriptors *desc); int read_ich_descriptors_from_dump(const uint32_t *dump, unsigned int len, struct ich_descriptors *desc); -int read_ich_descriptors_via_fdo(void *spibar, struct ich_descriptors *desc); +int read_ich_descriptors_via_fdo(enum ich_chipset cs, void *spibar, struct ich_descriptors *desc); int getFCBA_component_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx); int layout_from_ich_descriptors(struct ich_layout *, const void *dump, size_t len); diff --git a/ichspi.c b/ichspi.c index be13f91..ecf0c42 100644 --- a/ichspi.c +++ b/ichspi.c @@ -33,6 +33,60 @@ #include "spi.h" #include "ich_descriptors.h" +/* Sunrise Point */ + +/* Added HSFS Status bits */ +#define HSFS_WRSDIS_OFF 11 /* 11: Flash Configuration Lock-Down */ +#define HSFS_WRSDIS (0x1 << HSFS_WRSDIS_OFF) +#define HSFS_PRR34_LOCKDN_OFF 12 /* 12: PRR3 PRR4 Lock-Down */ +#define HSFS_PRR34_LOCKDN (0x1 << HSFS_PRR34_LOCKDN_OFF) +/* HSFS_BERASE vanished */ + +/* + * HSFC and HSFS 16-bit registers are combined into the 32-bit + * BIOS_HSFSTS_CTL register in the Sunrise Point datasheet, + * however we still treat them separately in order to reuse code. + */ + +/* Changed HSFC Control bits */ +#define PCH100_HSFC_FCYCLE_OFF (17 - 16) /* 1-4: FLASH Cycle */ +#define PCH100_HSFC_FCYCLE (0xf << PCH100_HSFC_FCYCLE_OFF) +/* New HSFC Control bit */ +#define HSFC_WET_OFF (21 - 16) /* 5: Write Enable Type */ +#define HSFC_WET (0x1 << HSFC_WET_OFF) + +#define PCH100_FADDR_FLA 0x07ffffff + +#define PCH100_REG_DLOCK 0x0c /* 32 Bits Discrete Lock Bits */ +#define DLOCK_BMWAG_LOCKDN_OFF 0 +#define DLOCK_BMWAG_LOCKDN (0x1 << DLOCK_BMWAG_LOCKDN_OFF) +#define DLOCK_BMRAG_LOCKDN_OFF 1 +#define DLOCK_BMRAG_LOCKDN (0x1 << DLOCK_BMRAG_LOCKDN_OFF) +#define DLOCK_SBMWAG_LOCKDN_OFF 2 +#define DLOCK_SBMWAG_LOCKDN (0x1 << DLOCK_SBMWAG_LOCKDN_OFF) +#define DLOCK_SBMRAG_LOCKDN_OFF 3 +#define DLOCK_SBMRAG_LOCKDN (0x1 << DLOCK_SBMRAG_LOCKDN_OFF) +#define DLOCK_PR0_LOCKDN_OFF 8 +#define DLOCK_PR0_LOCKDN (0x1 << DLOCK_PR0_LOCKDN_OFF) +#define DLOCK_PR1_LOCKDN_OFF 9 +#define DLOCK_PR1_LOCKDN (0x1 << DLOCK_PR1_LOCKDN_OFF) +#define DLOCK_PR2_LOCKDN_OFF 10 +#define DLOCK_PR2_LOCKDN (0x1 << DLOCK_PR2_LOCKDN_OFF) +#define DLOCK_PR3_LOCKDN_OFF 11 +#define DLOCK_PR3_LOCKDN (0x1 << DLOCK_PR3_LOCKDN_OFF) +#define DLOCK_PR4_LOCKDN_OFF 12 +#define DLOCK_PR4_LOCKDN (0x1 << DLOCK_PR4_LOCKDN_OFF) +#define DLOCK_SSEQ_LOCKDN_OFF 16 +#define DLOCK_SSEQ_LOCKDN (0x1 << DLOCK_SSEQ_LOCKDN_OFF) + +#define PCH100_REG_FPR0 0x84 /* 32 Bits Protected Range 0 */ +#define PCH100_REG_GPR0 0x98 /* 32 Bits Global Protected Range 0 */ + +#define PCH100_REG_SSFSC 0xA0 /* 32 Bits Status (8) + Control (24) */ +#define PCH100_REG_PREOP 0xA4 /* 16 Bits */ +#define PCH100_REG_OPTYPE 0xA6 /* 16 Bits */ +#define PCH100_REG_OPMENU 0xA8 /* 64 Bits */ + /* ICH9 controller register definition */ #define ICH9_REG_HSFS 0x04 /* 16 Bits Hardware Sequencing Flash Status */ #define HSFS_FDONE_OFF 0 /* 0: Flash Cycle Done */ @@ -66,6 +120,7 @@ #define HSFC_SME (0x1 << HSFC_SME_OFF) #define ICH9_REG_FADDR 0x08 /* 32 Bits */ +#define ICH9_FADDR_FLA 0x01ffffff #define ICH9_REG_FDATA0 0x10 /* 64 Bytes */ #define ICH9_REG_FRAP 0x50 /* 32 Bytes Flash Region Access Permissions */ @@ -325,7 +380,8 @@ ops->preop[1]); } -#define pprint_reg(reg, bit, val, sep) msg_pdbg("%s=%d" sep, #bit, (val & reg##_##bit)>>reg##_##bit##_OFF) +#define _pprint_reg(bit, mask, off, val, sep) msg_pdbg("%s=%d" sep, #bit, (val & mask) >> off) +#define pprint_reg(reg, bit, val, sep) _pprint_reg(bit, reg##_##bit, reg##_##bit##_OFF, val, sep) static void prettyprint_ich9_reg_hsfs(uint16_t reg_val) { @@ -333,8 +389,14 @@ pprint_reg(HSFS, FDONE, reg_val, ", "); pprint_reg(HSFS, FCERR, reg_val, ", "); pprint_reg(HSFS, AEL, reg_val, ", "); - pprint_reg(HSFS, BERASE, reg_val, ", "); + if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { + pprint_reg(HSFS, BERASE, reg_val, ", "); + } pprint_reg(HSFS, SCIP, reg_val, ", "); + if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) { + pprint_reg(HSFS, PRR34_LOCKDN, reg_val, ", "); + pprint_reg(HSFS, WRSDIS, reg_val, ", "); + } pprint_reg(HSFS, FDOPSS, reg_val, ", "); pprint_reg(HSFS, FDV, reg_val, ", "); pprint_reg(HSFS, FLOCKDN, reg_val, "\n"); @@ -344,7 +406,12 @@ { msg_pdbg("HSFC: "); pprint_reg(HSFC, FGO, reg_val, ", "); - pprint_reg(HSFC, FCYCLE, reg_val, ", "); + if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { + pprint_reg(HSFC, FCYCLE, reg_val, ", "); + } else { + _pprint_reg(HSFC, PCH100_HSFC_FCYCLE, PCH100_HSFC_FCYCLE_OFF, reg_val, ", "); + pprint_reg(HSFC, WET, reg_val, ", "); + } pprint_reg(HSFC, FDBC, reg_val, ", "); pprint_reg(HSFC, SME, reg_val, "\n"); } @@ -369,6 +436,28 @@ pprint_reg(SSFC, SME, reg_val, ", "); pprint_reg(SSFC, SCF, reg_val, "\n"); } + +static void prettyprint_pch100_reg_dlock(const uint32_t reg_val) +{ + msg_pdbg("DLOCK: "); + pprint_reg(DLOCK, BMWAG_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, BMRAG_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, SBMWAG_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, SBMRAG_LOCKDN, reg_val, ",\n "); + pprint_reg(DLOCK, PR0_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR1_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR2_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR3_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR4_LOCKDN, reg_val, ",\n "); + pprint_reg(DLOCK, SSEQ_LOCKDN, reg_val, "\n"); +} + +static struct { + size_t reg_ssfsc; + size_t reg_preop; + size_t reg_optype; + size_t reg_opmenu; +} swseq_data; static uint8_t lookup_spi_type(uint8_t opcode) { @@ -475,10 +564,10 @@ break; case CHIPSET_ICH8: default: /* Future version might behave the same */ - preop = REGREAD16(ICH9_REG_PREOP); - optype = REGREAD16(ICH9_REG_OPTYPE); - opmenu[0] = REGREAD32(ICH9_REG_OPMENU); - opmenu[1] = REGREAD32(ICH9_REG_OPMENU + 4); + preop = REGREAD16(swseq_data.reg_preop); + optype = REGREAD16(swseq_data.reg_optype); + opmenu[0] = REGREAD32(swseq_data.reg_opmenu); + opmenu[1] = REGREAD32(swseq_data.reg_opmenu + 4); break; } @@ -558,15 +647,15 @@ default: /* Future version might behave the same */ /* Register undo only for enable_undo=1, i.e. first call. */ if (enable_undo) { - rmmio_valw(ich_spibar + ICH9_REG_PREOP); - rmmio_valw(ich_spibar + ICH9_REG_OPTYPE); - rmmio_vall(ich_spibar + ICH9_REG_OPMENU); - rmmio_vall(ich_spibar + ICH9_REG_OPMENU + 4); + rmmio_valw(ich_spibar + swseq_data.reg_preop); + rmmio_valw(ich_spibar + swseq_data.reg_optype); + rmmio_vall(ich_spibar + swseq_data.reg_opmenu); + rmmio_vall(ich_spibar + swseq_data.reg_opmenu + 4); } - mmio_writew(preop, ich_spibar + ICH9_REG_PREOP); - mmio_writew(optype, ich_spibar + ICH9_REG_OPTYPE); - mmio_writel(opmenu[0], ich_spibar + ICH9_REG_OPMENU); - mmio_writel(opmenu[1], ich_spibar + ICH9_REG_OPMENU + 4); + mmio_writew(preop, ich_spibar + swseq_data.reg_preop); + mmio_writew(optype, ich_spibar + swseq_data.reg_optype); + mmio_writel(opmenu[0], ich_spibar + swseq_data.reg_opmenu); + mmio_writel(opmenu[1], ich_spibar + swseq_data.reg_opmenu + 4); break; } @@ -853,7 +942,7 @@ } timeout = 100 * 60; /* 60 ms are 9.6 million cycles at 16 MHz. */ - while ((REGREAD8(ICH9_REG_SSFS) & SSFS_SCIP) && --timeout) { + while ((REGREAD8(swseq_data.reg_ssfsc) & SSFS_SCIP) && --timeout) { programmer_delay(10); } if (!timeout) { @@ -871,12 +960,12 @@ ich_fill_data(data, datalength, ICH9_REG_FDATA0); /* Assemble SSFS + SSFC */ - temp32 = REGREAD32(ICH9_REG_SSFS); + temp32 = REGREAD32(swseq_data.reg_ssfsc); /* Keep reserved bits only */ temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK; /* Clear cycle done and cycle error status registers */ temp32 |= (SSFS_FDONE | SSFS_FCERR); - REGWRITE32(ICH9_REG_SSFS, temp32); + REGWRITE32(swseq_data.reg_ssfsc, temp32); /* Use 20 MHz */ temp32 |= SSFC_SCF_20MHZ; @@ -930,21 +1019,21 @@ temp32 |= SSFC_SCGO; /* write it */ - REGWRITE32(ICH9_REG_SSFS, temp32); + REGWRITE32(swseq_data.reg_ssfsc, temp32); /* Wait for Cycle Done Status or Flash Cycle Error. */ - while (((REGREAD32(ICH9_REG_SSFS) & (SSFS_FDONE | SSFS_FCERR)) == 0) && + while (((REGREAD32(swseq_data.reg_ssfsc) & (SSFS_FDONE | SSFS_FCERR)) == 0) && --timeout) { programmer_delay(10); } if (!timeout) { - msg_perr("timeout, ICH9_REG_SSFS=0x%08x\n", - REGREAD32(ICH9_REG_SSFS)); + msg_perr("timeout, REG_SSFS=0x%08x\n", + REGREAD32(swseq_data.reg_ssfsc)); return 1; } /* FIXME make sure we do not needlessly cause transaction errors. */ - temp32 = REGREAD32(ICH9_REG_SSFS); + temp32 = REGREAD32(swseq_data.reg_ssfsc); if (temp32 & SSFS_FCERR) { msg_perr("Transaction error!\n"); prettyprint_ich9_reg_ssfs(temp32); @@ -952,7 +1041,7 @@ /* keep reserved bits */ temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK; /* Clear the transaction error. */ - REGWRITE32(ICH9_REG_SSFS, temp32 | SSFS_FCERR); + REGWRITE32(swseq_data.reg_ssfsc, temp32 | SSFS_FCERR); return 1; } @@ -1117,13 +1206,16 @@ static struct hwseq_data { uint32_t size_comp0; uint32_t size_comp1; + uint32_t addr_mask; + bool only_4k; + uint32_t hsfc_fcycle; } hwseq_data; -/* Sets FLA in FADDR to (addr & 0x01FFFFFF) without touching other bits. */ +/* Sets FLA in FADDR to (addr & hwseq_data.addr_mask) without touching other bits. */ static void ich_hwseq_set_addr(uint32_t addr) { - uint32_t addr_old = REGREAD32(ICH9_REG_FADDR) & ~0x01FFFFFF; - REGWRITE32(ICH9_REG_FADDR, (addr & 0x01FFFFFF) | addr_old); + uint32_t addr_old = REGREAD32(ICH9_REG_FADDR) & ~hwseq_data.addr_mask; + REGWRITE32(ICH9_REG_FADDR, (addr & hwseq_data.addr_mask) | addr_old); } /* Sets FADDR.FLA to 'addr' and returns the erase block size in bytes @@ -1135,18 +1227,21 @@ */ static uint32_t ich_hwseq_get_erase_block_size(unsigned int addr) { - uint8_t enc_berase; - static const uint32_t dec_berase[4] = { - 256, - 4 * 1024, - 8 * 1024, - 64 * 1024 - }; + if (hwseq_data.only_4k) { + return 4 * 1024; + } else { + uint8_t enc_berase; + static const uint32_t dec_berase[4] = { + 256, + 4 * 1024, + 8 * 1024, + 64 * 1024 + }; - ich_hwseq_set_addr(addr); - enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> - HSFS_BERASE_OFF; - return dec_berase[enc_berase]; + ich_hwseq_set_addr(addr); + enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> HSFS_BERASE_OFF; + return dec_berase[enc_berase]; + } } /* Polls for Cycle Done Status, Flash Cycle Error or timeout in 8 us intervals. @@ -1167,7 +1262,7 @@ } REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); if (!timeout) { - addr = REGREAD32(ICH9_REG_FADDR) & 0x01FFFFFF; + addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask; msg_perr("Timeout error between offset 0x%08x and " "0x%08x (= 0x%08x + %d)!\n", addr, addr + len - 1, addr, len - 1); @@ -1177,7 +1272,7 @@ } if (hsfs & HSFS_FCERR) { - addr = REGREAD32(ICH9_REG_FADDR) & 0x01FFFFFF; + addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask; msg_perr("Transaction error between offset 0x%08x and " "0x%08x (= 0x%08x + %d)!\n", addr, addr + len - 1, addr, len - 1); @@ -1205,7 +1300,10 @@ flash->chip->total_size = total_size / 1024; eraser = &(flash->chip->block_erasers[0]); - boundary = (REGREAD32(ICH9_REG_FPB) & FPB_FPBA) << 12; + if (!hwseq_data.only_4k) + boundary = (REGREAD32(ICH9_REG_FPB) & FPB_FPBA) << 12; + else + boundary = 0; size_high = total_size - boundary; erase_size_high = ich_hwseq_get_erase_block_size(boundary); @@ -1279,7 +1377,7 @@ REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~HSFC_FCYCLE; /* clear operation */ + hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */ hsfc |= (0x3 << HSFC_FCYCLE_OFF); /* set erase operation */ hsfc |= HSFC_FGO; /* start */ msg_pdbg("HSFC used for block erasing: "); @@ -1316,7 +1414,7 @@ ich_hwseq_set_addr(addr); hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~HSFC_FCYCLE; /* set read operation */ + hsfc &= ~hwseq_data.hsfc_fcycle; /* set read operation */ hsfc &= ~HSFC_FDBC; /* clear byte count */ /* set byte count */ hsfc |= (((block_len - 1) << HSFC_FDBC_OFF) & HSFC_FDBC); @@ -1357,7 +1455,7 @@ block_len = min(block_len, 256 - (addr & 0xFF)); ich_fill_data(buf, block_len, ICH9_REG_FDATA0); hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~HSFC_FCYCLE; /* clear operation */ + hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */ hsfc |= (0x2 << HSFC_FCYCLE_OFF); /* set write operation */ hsfc &= ~HSFC_FDBC; /* clear byte count */ /* set byte count */ @@ -1449,6 +1547,8 @@ "Flash Descriptor", "BIOS", "Management Engine", "Gigabit Ethernet", "Platform Data" }; + 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); @@ -1460,19 +1560,19 @@ if (base > limit || (freg == 0 && i > 0)) { /* this FREG is disabled */ msg_pdbg2("0x%02X: 0x%08x FREG%i: %s region is unused.\n", - offset, freg, i, region_names[i]); + offset, freg, i, region_name); return 0; } msg_pdbg("0x%02X: 0x%08x ", offset, freg); if (rwperms == 0x3) { msg_pdbg("FREG%i: %s region (0x%08x-0x%08x) is %s.\n", i, - region_names[i], base, (limit | 0x0fff), + region_name, base, (limit | 0x0fff), access_names[rwperms]); return 0; } msg_pwarn("FREG%i: Warning: %s region (0x%08x-0x%08x) is %s.\n", i, - region_names[i], base, (limit | 0x0fff), + region_name, base, (limit | 0x0fff), access_names[rwperms]); return 1; } @@ -1487,31 +1587,36 @@ ((~((pr) >> PR_WP_OFF) & 1) << 1)) /* returns 0 if range is unused (i.e. r/w) */ -static int ich9_handle_pr(int i) +static int ich9_handle_pr(const size_t reg_pr0, int i) { static const char *const access_names[3] = { "locked", "read-only", "write-only" }; - uint8_t off = ICH9_REG_PR0 + (i * 4); + uint8_t off = reg_pr0 + (i * 4); uint32_t pr = mmio_readl(ich_spibar + off); unsigned int rwperms = ICH_PR_PERMS(pr); + /* From 5 on we have GPR registers and start from 0 again. */ + const char *const prefix = i >= 5 ? "G" : ""; + if (i >= 5) + i -= 5; + if (rwperms == 0x3) { - msg_pdbg2("0x%02X: 0x%08x (PR%u is unused)\n", off, pr, i); + msg_pdbg2("0x%02X: 0x%08x (%sPR%u is unused)\n", off, pr, prefix, i); return 0; } msg_pdbg("0x%02X: 0x%08x ", off, pr); - msg_pwarn("PR%u: Warning: 0x%08x-0x%08x is %s.\n", i, ICH_FREG_BASE(pr), + msg_pwarn("%sPR%u: Warning: 0x%08x-0x%08x is %s.\n", prefix, i, ICH_FREG_BASE(pr), ICH_FREG_LIMIT(pr) | 0x0fff, access_names[rwperms]); return 1; } /* Set/Clear the read and write protection enable bits of PR register @i * according to @read_prot and @write_prot. */ -static void ich9_set_pr(int i, int read_prot, int write_prot) +static void ich9_set_pr(const size_t reg_pr0, int i, int read_prot, int write_prot) { - void *addr = ich_spibar + ICH9_REG_PR0 + (i * 4); + void *addr = ich_spibar + reg_pr0 + (i * 4); uint32_t old = mmio_readl(addr); uint32_t new; @@ -1576,9 +1681,35 @@ ich_hwseq, ich_swseq } ich_spi_mode = ich_auto; + size_t num_freg, num_pr, reg_pr0; ich_generation = ich_gen; ich_spibar = spibar; + + /* Moving registers / bits */ + if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) { + num_freg = 10; + num_pr = 6; + 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; + } else { + num_freg = 5; + num_pr = 5; + reg_pr0 = ICH9_REG_PR0; + swseq_data.reg_ssfsc = ICH9_REG_SSFS; + swseq_data.reg_preop = ICH9_REG_PREOP; + swseq_data.reg_optype = ICH9_REG_OPTYPE; + swseq_data.reg_opmenu = ICH9_REG_OPMENU; + hwseq_data.addr_mask = ICH9_FADDR_FLA; + hwseq_data.only_4k = false; + hwseq_data.hsfc_fcycle = HSFC_FCYCLE; + } switch (ich_generation) { case CHIPSET_ICH7: @@ -1679,6 +1810,12 @@ tmp = mmio_readl(ich_spibar + ICH9_REG_FADDR); msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp); + if (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT) { + const uint32_t dlock = mmio_readl(ich_spibar + PCH100_REG_DLOCK); + msg_pdbg("0x0c: 0x%08x (DLOCK)\n", dlock); + prettyprint_pch100_reg_dlock(dlock); + } + if (desc_valid) { tmp = mmio_readl(ich_spibar + ICH9_REG_FRAP); msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp); @@ -1688,7 +1825,7 @@ msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp)); /* Handle FREGx and FRAP registers */ - for (i = 0; i < 5; i++) + for (i = 0; i < num_freg; i++) ich_spi_rw_restricted |= ich9_handle_frap(tmp, i); if (ich_spi_rw_restricted) msg_pwarn("Not all flash regions are freely accessible by flashrom. This is " @@ -1697,11 +1834,11 @@ } /* Handle PR registers */ - for (i = 0; i < 5; i++) { + for (i = 0; i < num_pr; i++) { /* if not locked down try to disable PR locks first */ if (!ichspi_lock) - ich9_set_pr(i, 0, 0); - ich_spi_rw_restricted |= ich9_handle_pr(i); + ich9_set_pr(reg_pr0, i, 0, 0); + ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i); } if (ich_spi_rw_restricted) { @@ -1716,30 +1853,30 @@ msg_pinfo("Continuing with write support because the user forced us to!\n"); } - tmp = mmio_readl(ich_spibar + ICH9_REG_SSFS); - msg_pdbg("0x90: 0x%02x (SSFS)\n", tmp & 0xff); + tmp = mmio_readl(ich_spibar + swseq_data.reg_ssfsc); + msg_pdbg("0x%zx: 0x%02x (SSFS)\n", swseq_data.reg_ssfsc, tmp & 0xff); prettyprint_ich9_reg_ssfs(tmp); if (tmp & SSFS_FCERR) { msg_pdbg("Clearing SSFS.FCERR\n"); - mmio_writeb(SSFS_FCERR, ich_spibar + ICH9_REG_SSFS); + mmio_writeb(SSFS_FCERR, ich_spibar + swseq_data.reg_ssfsc); } - msg_pdbg("0x91: 0x%06x (SSFC)\n", tmp >> 8); + msg_pdbg("0x%zx: 0x%06x (SSFC)\n", swseq_data.reg_ssfsc + 1, tmp >> 8); prettyprint_ich9_reg_ssfc(tmp); - msg_pdbg("0x94: 0x%04x (PREOP)\n", - mmio_readw(ich_spibar + ICH9_REG_PREOP)); - msg_pdbg("0x96: 0x%04x (OPTYPE)\n", - mmio_readw(ich_spibar + ICH9_REG_OPTYPE)); - msg_pdbg("0x98: 0x%08x (OPMENU)\n", - mmio_readl(ich_spibar + ICH9_REG_OPMENU)); - msg_pdbg("0x9C: 0x%08x (OPMENU+4)\n", - mmio_readl(ich_spibar + ICH9_REG_OPMENU + 4)); + msg_pdbg("0x%zx: 0x%04x (PREOP)\n", + swseq_data.reg_preop, mmio_readw(ich_spibar + swseq_data.reg_preop)); + msg_pdbg("0x%zx: 0x%04x (OPTYPE)\n", + swseq_data.reg_optype, mmio_readw(ich_spibar + swseq_data.reg_optype)); + msg_pdbg("0x%zx: 0x%08x (OPMENU)\n", + swseq_data.reg_opmenu, mmio_readl(ich_spibar + swseq_data.reg_opmenu)); + msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n", + swseq_data.reg_opmenu + 4, mmio_readl(ich_spibar + swseq_data.reg_opmenu + 4)); if (ich_generation == CHIPSET_ICH8 && desc_valid) { tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC); msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp); msg_pdbg("VSCC: "); prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); - } else { + } else if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { if (ich_generation != CHIPSET_BAYTRAIL && desc_valid) { ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR); msg_pdbg("0xA0: 0x%08x (BBAR)\n", @@ -1764,7 +1901,7 @@ } if (desc_valid) { - if (read_ich_descriptors_via_fdo(ich_spibar, &desc) == ICH_RET_OK) + if (read_ich_descriptors_via_fdo(ich_gen, ich_spibar, &desc) == ICH_RET_OK) prettyprint_ich_descriptors(ich_gen, &desc); /* If the descriptor is valid and indicates multiple -- To view, visit https://review.coreboot.org/18962 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee Gerrit-Change-Number: 18962 Gerrit-PatchSet: 11 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: chipset_enable: Add support for Intel Skylake / Kabylake
by Nico Huber (Code Review) July 28, 2017

July 28, 2017

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/18925 ) Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... chipset_enable: Add support for Intel Skylake / Kabylake All publicly known Skylake / Kabylake / Sunrise Point PCH variants share the same register interface [1..6]. Although all SPI configu- ration is now done through the SPI PCI device 1f.5, we can't probe for it directly since its PCI vendor and device IDs are usually hid- den. To work around the hidden IDs, we use another PCI accessor that doesn't rely on the OS seeing the PCI device. This handles SPI flashes only. While booting from LPC is still sup- ported, it seems nobody uses it any more. Some additional PCI IDs were gathered from driveridentifier.com. TEST=Compiled with B150 set to NT (instead of BAD) and checked for sane register readings. [1] 6th Generation Intel® Core(TM) Processor Families I/O Platform Datasheet - Volume 1 of 2 Revision 002EN Document Number 332995 [2] 6th Generation Intel® Processor I/O Datasheet for U/Y Platforms Volume 2 of 2 Revision 001EN Document Number 332996 [3] 7th Generation Intel® Processor Families I/O Platform Datasheet - Volume 1 of 2 Revision 002 Document Number 334658 [4] 7th Generation Intel® Processor Families I/O for U/Y Platforms Datasheet - Volume 2 of 2 Revision 002 Document Number 334659 [5] Intel® 100 Series and Intel® C230 Series Chipset Family Platform Controller Hub (PCH) Datasheet - Volume 1 of 2 Revision 004EN Document Number 332690 [6] Intel® 100 Series Chipset Family Platform Controller Hub (PCH) Datasheet - Volume 2 of 2 Revision 001EN Document Number 332691 Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Reviewed-on: https://review.coreboot.org/18925 Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Youness Alaoui <snifikino(a)gmail.com> --- M chipset_enable.c M programmer.h 2 files changed, 111 insertions(+), 7 deletions(-) Approvals: David Hendricks: Looks good to me, approved build bot (Jenkins): Verified Youness Alaoui: Looks good to me, approved diff --git a/chipset_enable.c b/chipset_enable.c index 83c470f..a85ae21 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -7,6 +7,8 @@ * Copyright (C) 2007,2008,2009 Carl-Daniel Hailfinger * Copyright (C) 2009 Kontron Modular Computers GmbH * Copyright (C) 2011, 2012 Stefan Tauner + * Copyright (C) 2017 secunet Security Networks AG + * (Written by Nico Huber <nico.huber(a)secunet.com> for secunet) * * 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 @@ -588,21 +590,32 @@ const uint8_t *const rcrb) { uint32_t gcs; - bool top_swap; + const char *reg_name; + bool bild, top_swap; switch (ich_generation) { case CHIPSET_BAYTRAIL: + reg_name = "GCS"; gcs = mmio_readl(rcrb + 0); + bild = gcs & 1; top_swap = (gcs & 2) >> 1; break; + case CHIPSET_100_SERIES_SUNRISE_POINT: + reg_name = "BIOS_SPI_BC"; + gcs = pci_read_long(dev, 0xdc); + bild = (gcs >> 7) & 1; + top_swap = (gcs >> 4) & 1; + break; default: + reg_name = "GCS"; gcs = mmio_readl(rcrb + 0x3410); + bild = gcs & 1; top_swap = mmio_readb(rcrb + 0x3414) & 1; break; } - msg_pdbg("GCS = 0x%x: ", gcs); - msg_pdbg("BIOS Interface Lock-Down: %sabled, ", (gcs & 0x1) ? "en" : "dis"); + msg_pdbg("%s = 0x%x: ", reg_name, gcs); + msg_pdbg("BIOS Interface Lock-Down: %sabled, ", bild ? "en" : "dis"); static const char *const straps_names_EP80579[] = { "SPI", "reserved", "reserved", "LPC" }; static const char *const straps_names_ich7_nm10[] = { "reserved", "SPI", "PCI", "LPC" }; @@ -644,6 +657,7 @@ break; case CHIPSET_8_SERIES_LYNX_POINT_LP: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: + case CHIPSET_100_SERIES_SUNRISE_POINT: straps_names = straps_names_pch8_lp; break; case CHIPSET_8_SERIES_WELLSBURG: // FIXME: check datasheet @@ -665,6 +679,9 @@ case CHIPSET_9_SERIES_WILDCAT_POINT_LP: /* LP PCHs use a single bit for BBS */ bbs = (gcs >> 10) & 0x1; + break; + case CHIPSET_100_SERIES_SUNRISE_POINT: + bbs = (gcs >> 6) & 0x1; break; default: /* Other chipsets use two bits for BBS */ @@ -804,6 +821,66 @@ static int enable_flash_pch9_lp(struct pci_dev *dev, const char *name) { return enable_flash_ich_spi(dev, CHIPSET_9_SERIES_WILDCAT_POINT_LP, 0xdc); +} + +/* Sunrise Point */ +static int enable_flash_pch100_shutdown(void *const pci_acc) +{ + pci_cleanup(pci_acc); + return 0; +} + +static int enable_flash_pch100(struct pci_dev *const dev, const char *const name) +{ + const enum ich_chipset pch_generation = CHIPSET_100_SERIES_SUNRISE_POINT; + int ret = ERROR_FATAL; + + /* + * The SPI PCI device is usually hidden (by hiding PCI vendor + * and device IDs). So we need a PCI access method that works + * even when the OS doesn't know the PCI device. We can't use + * this method globally since it would bring along other con- + * straints (e.g. on PCI domains, extended PCIe config space). + */ + struct pci_access *const pci_acc = pci_alloc(); + if (!pci_acc) { + msg_perr("Can't allocate PCI accessor.\n"); + return ret; + } + pci_acc->method = PCI_ACCESS_I386_TYPE1; + pci_init(pci_acc); + register_shutdown(enable_flash_pch100_shutdown, pci_acc); + + struct pci_dev *const spi_dev = pci_get_dev(pci_acc, dev->domain, dev->bus, 0x1f, 5); + if (!spi_dev) { + msg_perr("Can't allocate PCI device.\n"); + return ret; + } + + 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); + if (ret_bc == ERROR_FATAL) + goto _freepci_ret; + + const uint32_t phys_spibar = pci_read_long(spi_dev, PCI_BASE_ADDRESS_0) & 0xfffff000; + void *const spibar = rphysmap("SPIBAR", phys_spibar, 0x1000); + if (spibar == ERROR_PTR) + goto _freepci_ret; + msg_pdbg("SPIBAR = 0x%0*" PRIxPTR " (phys = 0x%08x)\n", PRIxPTR_WIDTH, (uintptr_t)spibar, phys_spibar); + + /* This adds BUS_SPI */ + const int ret_spi = ich_init_spi(spibar, pch_generation); + if (ret_spi != ERROR_FATAL) { + if (ret_bc || ret_spi) + ret = ERROR_NONFATAL; + else + ret = 0; + } + +_freepci_ret: + pci_free_dev(spi_dev); + return ret; } /* Silvermont architecture: Bay Trail(-T/-I), Avoton/Rangeley. @@ -1803,10 +1880,36 @@ {0x8086, 0x9cc7, NT, "Intel", "Broadwell Y Premium", enable_flash_pch9_lp}, {0x8086, 0x9cc9, NT, "Intel", "Broadwell Y Base", enable_flash_pch9_lp}, {0x8086, 0x9ccb, NT, "Intel", "Broadwell H", enable_flash_pch9}, - {0x8086, 0x9d41, BAD, "Intel", "Sunrise Point (Skylake LP Sample)", NULL}, - {0x8086, 0x9d43, BAD, "Intel", "Sunrise Point (Skylake-U Base)", NULL}, - {0x8086, 0x9d48, BAD, "Intel", "Sunrise Point (Skylake-U Premium)", NULL}, - {0x8086, 0x9d46, BAD, "Intel", "Sunrise Point (Skylake-Y Premium)", NULL}, + {0x8086, 0x9d41, BAD, "Intel", "Skylake / Kaby Lake Sample", enable_flash_pch100}, + {0x8086, 0x9d43, BAD, "Intel", "Skylake U Base", enable_flash_pch100}, + {0x8086, 0x9d46, BAD, "Intel", "Skylake Y Premium", enable_flash_pch100}, + {0x8086, 0x9d48, BAD, "Intel", "Skylake U Premium", enable_flash_pch100}, + {0x8086, 0x9d4b, BAD, "Intel", "Kaby Lake Y w/ iHDCP2.2 Prem.", enable_flash_pch100}, + {0x8086, 0x9d4e, BAD, "Intel", "Kaby Lake U w/ iHDCP2.2 Prem.", enable_flash_pch100}, + {0x8086, 0x9d50, BAD, "Intel", "Kaby Lake U w/ iHDCP2.2 Base", enable_flash_pch100}, + {0x8086, 0x9d51, BAD, "Intel", "Kabe Lake w/ iHDCP2.2 Sample", enable_flash_pch100}, + {0x8086, 0x9d53, BAD, "Intel", "Kaby Lake U Base", enable_flash_pch100}, + {0x8086, 0x9d56, BAD, "Intel", "Kaby Lake Y Premium", enable_flash_pch100}, + {0x8086, 0x9d58, BAD, "Intel", "Kaby Lake U Premium", enable_flash_pch100}, + {0x8086, 0xa141, BAD, "Intel", "Sunrise Point Desktop Sample", enable_flash_pch100}, + {0x8086, 0xa142, BAD, "Intel", "Sunrise Point Unknown Sample", enable_flash_pch100}, + {0x8086, 0xa143, BAD, "Intel", "H110", enable_flash_pch100}, + {0x8086, 0xa144, BAD, "Intel", "H170", enable_flash_pch100}, + {0x8086, 0xa145, BAD, "Intel", "Z170", enable_flash_pch100}, + {0x8086, 0xa146, BAD, "Intel", "Q170", enable_flash_pch100}, + {0x8086, 0xa147, BAD, "Intel", "Q150", enable_flash_pch100}, + {0x8086, 0xa148, BAD, "Intel", "B150", enable_flash_pch100}, + {0x8086, 0xa149, BAD, "Intel", "C236", enable_flash_pch100}, + {0x8086, 0xa14a, BAD, "Intel", "C232", enable_flash_pch100}, + {0x8086, 0xa14b, BAD, "Intel", "Sunrise Point Server Sample", enable_flash_pch100}, + {0x8086, 0xa14d, BAD, "Intel", "QM170", enable_flash_pch100}, + {0x8086, 0xa14e, BAD, "Intel", "HM170", enable_flash_pch100}, + {0x8086, 0xa150, BAD, "Intel", "CM236", enable_flash_pch100}, + {0x8086, 0xa151, BAD, "Intel", "QMS180", enable_flash_pch100}, + {0x8086, 0xa152, BAD, "Intel", "HM175", enable_flash_pch100}, + {0x8086, 0xa153, BAD, "Intel", "QM175", enable_flash_pch100}, + {0x8086, 0xa154, BAD, "Intel", "CM238", enable_flash_pch100}, + {0x8086, 0xa155, BAD, "Intel", "QMU185", enable_flash_pch100}, #endif {0}, }; diff --git a/programmer.h b/programmer.h index fd9da96..ec00bd9 100644 --- a/programmer.h +++ b/programmer.h @@ -650,6 +650,7 @@ CHIPSET_8_SERIES_WELLSBURG, 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 */ }; /* ichspi.c */ -- To view, visit https://review.coreboot.org/18925 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: merged Gerrit-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Gerrit-Change-Number: 18925 Gerrit-PatchSet: 9 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: ichspi: Add support for Intel Skylake
by Youness Alaoui (Code Review) July 27, 2017

July 27, 2017

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18962 ) Change subject: ichspi: Add support for Intel Skylake ...................................................................... Patch Set 10: Code-Review+1 I did a quick review, it all looks good, but I didn't verify with the datasheet if every offset is correct but I don't see any obvious errors in logic at least. -- To view, visit https://review.coreboot.org/18962 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee Gerrit-Change-Number: 18962 Gerrit-PatchSet: 10 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 20:09:02 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: chipset_enable: Add support for Intel Skylake / Kabylake
by Youness Alaoui (Code Review) July 27, 2017

July 27, 2017

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18925 ) Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... Patch Set 8: Code-Review+2 > > The RPCI fix is pushed here : https://review.coreboot.org/#/c/20784/ > > Now all we need is for this patch to set the 'pacc' variable and > > restore it at the end of the function. > > Um, I don't see why I have to wait for a seven years old bug to get > fixed before I add another chipset... also, if we fix it here and > not > in your patch, we'll lose the explicit information that the changes > are related (e.g. if anyone will ever fix the rpci interface he'll > more likely miss to remove the workaround here). Humm... you're right, the rpci bug is unrelated to skylake, I just see it only with skylake because that's the one that I build manually, I also associated it with this patch until I figured out the bug was from rpci. The patch looks good so I +2. I've half reviewed the ichspi one, but I'm not done yet. -- To view, visit https://review.coreboot.org/18925 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Gerrit-Change-Number: 18925 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 19:56:04 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by build bot (Jenkins) (Code Review) July 27, 2017

July 27, 2017

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom_gerrit/449/ : SUCCESS https://qa.coreboot.org/job/flashrom-customrules/479/ : SUCCESS -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 3 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 19:49:09 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by Youness Alaoui (Code Review) July 27, 2017

July 27, 2017

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/20784/1/pcidev.c File pcidev.c: https://review.coreboot.org/#/c/20784/1/pcidev.c@297 PS1, Line 297: > Yes, the code was and is still checking the `pacc` pointer in It's not really nuts, it makes sense because the pci_access gets referenced in the pci_dev, and is used, so if the pacc is cleaned up and destroyed, then calling any pci_dev API will cause an error. The mistake here is that 'pacc' is not set to NULL after cleanup in its shutdown routine. Of course, this makes no more sense if the pacc variable is different like in the case of pch100, that check becomes wrong. https://review.coreboot.org/#/c/20784/2/programmer.h File programmer.h: https://review.coreboot.org/#/c/20784/2/programmer.h@200 PS2, Line 200: int rpci_write_word(struct pci_dev *dev, int reg, uint16_t data); > That's not accurate... how about: Sounds good to me. Thanks -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 1 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 19:48:44 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by Youness Alaoui (Code Review) July 27, 2017

July 27, 2017

Hello David Hendricks, Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/20784 to look at the new patch set (#3). 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/ Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Signed-off-by: Youness Alaoui <kakaroto(a)kakaroto.homelinux.net> --- M pcidev.c M programmer.h 2 files changed, 20 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/20784/3 -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 3 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by Nico Huber (Code Review) July 27, 2017

July 27, 2017

Nico Huber has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 2: (2 comments) Please revise the comment in the header file. Looks good, otherwise. https://review.coreboot.org/#/c/20784/1/pcidev.c File pcidev.c: https://review.coreboot.org/#/c/20784/1/pcidev.c@297 PS1, Line 297: } > Added in the header file. The code was already checking for validity of 'pa Yes, the code was and is still checking the `pacc` pointer in the shutdown handler, which is nuts IMHO. It has nothing to say about the sanity of the program as the pointer is not used anymore. `pacc` is only interesting for the pci_get_dev() call. https://review.coreboot.org/#/c/20784/2/programmer.h File programmer.h: https://review.coreboot.org/#/c/20784/2/programmer.h@200 PS2, Line 200: */ That's not accurate... how about: 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. -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 2 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 19:02:20 +0000 Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: rpci: Use pci_dev struct pointer to avoid API breaks
by Youness Alaoui (Code Review) July 27, 2017

July 27, 2017

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/20784 ) Change subject: rpci: Use pci_dev struct pointer to avoid API breaks ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/#/c/20784/1/pcidev.c File pcidev.c: https://review.coreboot.org/#/c/20784/1/pcidev.c@262 PS1, Line 262: struct pci_dev * dev; > No whitespace after the asterisk, please. oups, I noticed it but forgot to fix it. https://review.coreboot.org/#/c/20784/1/pcidev.c@297 PS1, Line 297: > Add a comment that `dev` passed to rpci_* has to be compatible Added in the header file. The code was already checking for validity of 'pacc', so it was already a requirement before my change, but I've changed things to make it more robust. https://review.coreboot.org/#/c/20784/1/pcidev.c@307 PS1, Line 307: a->domain, a->bus, a->dev, a->func); \ > Bail out if `undo_pci_write_data->dev` is NULL or check it Done -- To view, visit https://review.coreboot.org/20784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-MessageType: comment Gerrit-Change-Id: Icde2e587992ba964d4ff92c33aa659850ba06298 Gerrit-Change-Number: 20784 Gerrit-PatchSet: 1 Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 Jul 2017 18:42:39 +0000 Gerrit-HasComments: Yes

1 0