coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: drivers: spi_flash: Add Fast Read Dual I/O support
by Julius Werner (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Shelley Chen, Furquan Shaikh. Hello Shelley Chen, Furquan Shaikh, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/56287 to review the following change. Change subject: drivers: spi_flash: Add Fast Read Dual I/O support ...................................................................... drivers: spi_flash: Add Fast Read Dual I/O support The Fast Read Dual Output and Fast Read Dual I/O commands are practically identical, the only difference being how the read address is transferred (saving a whooping 2 bytes which is totally irrelevant for the amounts of data coreboot tends to read). We originally implemented Fast Read Dual Output since it's the older command and some older Winbond chips only supported that one... but it seems that some older Macronix parts for whatever reason chose to only support Fast Read Dual I/O instead. So in order to make this work for as many parts as possible, I guess we'll have to implement both. (Also, the Macronix device ID situation is utter madness with different chips with different capabilities often having the same ID, so we basically have to make a best-effort guess to strike a trade-off between fast speeds and best chance at supporting all chips. If this turns out to be a problem later, we may have to add Kconfig overrides for this or resort to SFDP parsing, although that would defeat the whole point of trying to be fast.) BUG=b:193486682 TEST=Booted CoachZ (with Dual I/O) Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Ia1a20581f251615127f132eadea367b7b66c4709 --- M src/drivers/spi/gigadevice.c M src/drivers/spi/macronix.c M src/drivers/spi/spi_flash.c M src/drivers/spi/spi_flash_internal.h M src/drivers/spi/winbond.c M src/include/spi_flash.h 6 files changed, 100 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/56287/1 diff --git a/src/drivers/spi/gigadevice.c b/src/drivers/spi/gigadevice.c index 15ad907..fc2a405 100644 --- a/src/drivers/spi/gigadevice.c +++ b/src/drivers/spi/gigadevice.c @@ -31,72 +31,84 @@ .id[0] = 0x4014, .nr_sectors_shift = 8, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, /* also GD25Q80B */ { /* GD25Q16 */ .id[0] = 0x4015, .nr_sectors_shift = 9, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, /* also GD25Q16B */ { /* GD25Q32B */ .id[0] = 0x4016, .nr_sectors_shift = 10, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, /* also GD25Q32B */ { /* GD25Q64 */ .id[0] = 0x4017, .nr_sectors_shift = 11, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, /* also GD25Q64B, GD25B64C */ { /* GD25Q128 */ .id[0] = 0x4018, .nr_sectors_shift = 12, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, /* also GD25Q128B */ { /* GD25VQ80C */ .id[0] = 0x4214, .nr_sectors_shift = 8, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, { /* GD25VQ16C */ .id[0] = 0x4215, .nr_sectors_shift = 9, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, { /* GD25LQ80 */ .id[0] = 0x6014, .nr_sectors_shift = 8, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, { /* GD25LQ16 */ .id[0] = 0x6015, .nr_sectors_shift = 9, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, { /* GD25LQ32 */ .id[0] = 0x6016, .nr_sectors_shift = 10, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, { /* GD25LQ64C */ .id[0] = 0x6017, .nr_sectors_shift = 11, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, /* also GD25LB64C */ { /* GD25LQ128 */ .id[0] = 0x6018, .nr_sectors_shift = 12, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, }; diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index 69e7a9f..b6981e0 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -63,55 +63,75 @@ .id[0] = 0x2415, .nr_sectors_shift = 9, }, + /* + * NOTE: C225xx JEDEC IDs are basically useless because Macronix keeps + * reusing the same IDs for vastly different chips. 35E versions always + * seem to support Dual I/O but not Dual Output, while 35F versions seem + * to support both, so we only set Dual I/O here to improve our chances + * of compatibility. Since Macronix makes it impossible to search all + * different parts that it recklessly assigned the same IDs to, it's + * hard to know if there may be parts that don't even support Dual I/O + * with these IDs, though (or what we should do if there are). + */ { /* MX25L1635E */ .id[0] = 0x2515, .nr_sectors_shift = 9, + .fast_read_dual_io_support = 1, }, { /* MX25U8032E */ .id[0] = 0x2534, .nr_sectors_shift = 8, + .fast_read_dual_io_support = 1, }, { - /* MX25U1635E */ + /* MX25U1635E/MX25U1635F */ .id[0] = 0x2535, .nr_sectors_shift = 9, + .fast_read_dual_io_support = 1, }, { - /* MX25U3235E */ + /* MX25U3235E/MX25U3235F */ .id[0] = 0x2536, .nr_sectors_shift = 10, + .fast_read_dual_io_support = 1, }, { - /* MX25U6435F */ + /* MX25U6435E/MX25U6435F */ .id[0] = 0x2537, .nr_sectors_shift = 11, + .fast_read_dual_io_support = 1, }, { /* MX25U12835F */ .id[0] = 0x2538, .nr_sectors_shift = 12, + .fast_read_dual_io_support = 1, }, { /* MX25U25635F */ .id[0] = 0x2539, .nr_sectors_shift = 13, + .fast_read_dual_io_support = 1, }, { - /* MX25U51245G */ + /* MX25U51235F */ .id[0] = 0x253a, .nr_sectors_shift = 14, + .fast_read_dual_io_support = 1, }, { /* MX25L12855E */ .id[0] = 0x2618, .nr_sectors_shift = 12, + .fast_read_dual_io_support = 1, }, { /* MX25L3235D/MX25L3225D/MX25L3236D/MX25L3237D */ .id[0] = 0x5e16, .nr_sectors_shift = 10, + .fast_read_dual_io_support = 1, }, { /* MX25L6495F */ diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index b674749..02fe4de 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -20,7 +20,7 @@ cmd[3] = addr >> 0; } -static int do_spi_flash_cmd(const struct spi_slave *spi, const void *dout, +static int do_spi_flash_cmd(const struct spi_slave *spi, const u8 *dout, size_t bytes_out, void *din, size_t bytes_in) { int ret; @@ -51,8 +51,8 @@ return ret; } -static int do_dual_read_cmd(const struct spi_slave *spi, const void *dout, - size_t bytes_out, void *din, size_t bytes_in) +static int do_dual_output_cmd(const struct spi_slave *spi, const u8 *dout, + size_t bytes_out, void *din, size_t bytes_in) { int ret; @@ -79,6 +79,31 @@ return ret; } +static int do_dual_io_cmd(const struct spi_slave *spi, const u8 *dout, + size_t bytes_out, void *din, size_t bytes_in) +{ + int ret; + + /* Only the very first byte (opcode) is transfered in "single" mode. */ + struct spi_op vector = { .dout = dout, .bytesout = 1, + .din = NULL, .bytesin = 0 }; + + ret = spi_claim_bus(spi); + if (ret) + return ret; + + ret = spi_xfer_vector(spi, &vector, 1); + + if (!ret) + ret = spi->ctrlr->xfer_dual(spi, &dout[1], bytes_out - 1, NULL, 0); + + if (!ret) + ret = spi->ctrlr->xfer_dual(spi, NULL, 0, din, bytes_in); + + spi_release_bus(spi); + return ret; +} + int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, size_t len) { int ret = do_spi_flash_cmd(spi, &cmd, sizeof(cmd), response, len); @@ -119,18 +144,23 @@ { u8 cmd[5]; int ret, cmd_len; - int (*do_cmd)(const struct spi_slave *spi, const void *din, + int (*do_cmd)(const struct spi_slave *spi, const u8 *din, size_t in_bytes, void *out, size_t out_bytes); if (CONFIG(SPI_FLASH_NO_FAST_READ)) { cmd_len = 4; cmd[0] = CMD_READ_ARRAY_SLOW; do_cmd = do_spi_flash_cmd; - } else if (flash->flags.dual_spi && flash->spi.ctrlr->xfer_dual) { + } else if (flash->flags.dual_io && flash->spi.ctrlr->xfer_dual) { + cmd_len = 5; + cmd[0] = CMD_READ_FAST_DUAL_IO; + cmd[4] = 0; + do_cmd = do_dual_io_cmd; + } else if (flash->flags.dual_output && flash->spi.ctrlr->xfer_dual) { cmd_len = 5; cmd[0] = CMD_READ_FAST_DUAL_OUTPUT; cmd[4] = 0; - do_cmd = do_dual_read_cmd; + do_cmd = do_dual_output_cmd; } else { cmd_len = 5; cmd[0] = CMD_READ_ARRAY_FAST; @@ -352,7 +382,8 @@ flash->pp_cmd = vi->desc->pp_cmd; flash->wren_cmd = vi->desc->wren_cmd; - flash->flags.dual_spi = part->fast_read_dual_output_support; + flash->flags.dual_output = part->fast_read_dual_output_support; + flash->flags.dual_io = part->fast_read_dual_io_support; flash->ops = &vi->desc->ops; flash->prot_ops = vi->prot_ops; @@ -471,8 +502,10 @@ } const char *mode_string = ""; - if (flash->flags.dual_spi && spi.ctrlr->xfer_dual) - mode_string = " (Dual SPI mode)"; + if (flash->flags.dual_io && spi.ctrlr->xfer_dual) + mode_string = " (Dual I/O mode)"; + else if (flash->flags.dual_output && spi.ctrlr->xfer_dual) + mode_string = " (Dual Output mode)"; printk(BIOS_INFO, "SF: Detected %02x %04x with sector size 0x%x, total 0x%x%s\n", flash->vendor, flash->model, flash->sector_size, flash->size, mode_string); diff --git a/src/drivers/spi/spi_flash_internal.h b/src/drivers/spi/spi_flash_internal.h index b4d39b3..4a7beea 100644 --- a/src/drivers/spi/spi_flash_internal.h +++ b/src/drivers/spi/spi_flash_internal.h @@ -15,6 +15,7 @@ #define CMD_READ_ARRAY_LEGACY 0xe8 #define CMD_READ_FAST_DUAL_OUTPUT 0x3b +#define CMD_READ_FAST_DUAL_IO 0xbb #define CMD_READ_STATUS 0x05 #define CMD_WRITE_ENABLE 0x06 @@ -69,8 +70,9 @@ uint16_t id[2]; /* Log based 2 total number of sectors. */ uint16_t nr_sectors_shift: 4; - uint16_t fast_read_dual_output_support : 1; - uint16_t _reserved_for_flags: 3; + uint16_t fast_read_dual_output_support : 1; /* 1-1-2 read */ + uint16_t fast_read_dual_io_support : 1; /* 1-2-2 read */ + uint16_t _reserved_for_flags: 2; /* Block protection. Currently used by Winbond. */ uint16_t protection_granularity_shift : 5; uint16_t bp_bits : 3; diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 9fe0492..8f93e3b 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -101,12 +101,14 @@ .id[0] = 0x4014, .nr_sectors_shift = 8, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, }, { /* W25Q16_V */ .id[0] = 0x4015, .nr_sectors_shift = 9, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, }, @@ -115,6 +117,7 @@ .id[0] = 0x6015, .nr_sectors_shift = 9, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, }, @@ -123,6 +126,7 @@ .id[0] = 0x4016, .nr_sectors_shift = 10, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, }, @@ -131,6 +135,7 @@ .id[0] = 0x6016, .nr_sectors_shift = 10, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, }, @@ -139,6 +144,7 @@ .id[0] = 0x4017, .nr_sectors_shift = 11, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, }, @@ -147,6 +153,7 @@ .id[0] = 0x6017, .nr_sectors_shift = 11, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, }, @@ -155,6 +162,7 @@ .id[0] = 0x8017, .nr_sectors_shift = 11, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, }, @@ -163,6 +171,7 @@ .id[0] = 0x4018, .nr_sectors_shift = 12, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, }, @@ -171,6 +180,7 @@ .id[0] = 0x6018, .nr_sectors_shift = 12, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, }, @@ -179,6 +189,7 @@ .id[0] = 0x7018, .nr_sectors_shift = 12, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, }, @@ -187,6 +198,7 @@ .id[0] = 0x8018, .nr_sectors_shift = 12, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, }, @@ -195,6 +207,7 @@ .id[0] = 0x8020, .nr_sectors_shift = 11, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, }, @@ -203,6 +216,7 @@ .id[0] = 0x4019, .nr_sectors_shift = 13, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, }, @@ -211,6 +225,7 @@ .id[0] = 0x7019, .nr_sectors_shift = 13, .fast_read_dual_output_support = 1, + .fast_read_dual_io_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, }, diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h index a7f707e..fe77e3e 100644 --- a/src/include/spi_flash.h +++ b/src/include/spi_flash.h @@ -86,8 +86,9 @@ union { u8 raw; struct { - u8 dual_spi : 1; - u8 _reserved : 7; + u8 dual_output : 1; + u8 dual_io : 1; + u8 _reserved : 6; }; } flags; u16 model; -- To view, visit https://review.coreboot.org/c/coreboot/+/56287 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia1a20581f251615127f132eadea367b7b66c4709 Gerrit-Change-Number: 56287 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/stoneyridge: add and use mca_is_valid_bank & mca_get_bank_name
by build bot (Jenkins) (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56277 ) Change subject: soc/amd/stoneyridge: add and use mca_is_valid_bank & mca_get_bank_name ...................................................................... Patch Set 2: (1 comment) File src/soc/amd/stoneyridge/mca.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-124069): https://review.coreboot.org/c/coreboot/+/56277/comment/450d7f78_a8c1d46d PS2, Line 143: Accesing non-existing MSRs causes a general protection fault. */ 'Accesing' may be misspelled - perhaps 'Accessing'? -- To view, visit https://review.coreboot.org/c/coreboot/+/56277 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I40d6a6752504d804c45b445fce7e763e80161211 Gerrit-Change-Number: 56277 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 13 Jul 2021 23:03:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd: factor out check_mca to common code
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson. Hello Jason Glenesk, Raul Rangel, Marshall Dawson, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56281 to look at the new patch set (#2). Change subject: soc/amd: factor out check_mca to common code ...................................................................... soc/amd: factor out check_mca to common code Change-Id: I139d1fe41bad5213da8890c2867f275b6847e3e1 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/cezanne/Kconfig M src/soc/amd/cezanne/mca.c M src/soc/amd/common/block/cpu/Kconfig A src/soc/amd/common/block/cpu/mca/Makefile.inc A src/soc/amd/common/block/cpu/mca/mca_common.c M src/soc/amd/common/block/include/amdblocks/mca.h M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/mca.c M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/mca.c 10 files changed, 40 insertions(+), 28 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/56281/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56281 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I139d1fe41bad5213da8890c2867f275b6847e3e1 Gerrit-Change-Number: 56281 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/amd/common/blocks/cpu/mca: factor out common BERT helper functions
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56284 ) Change subject: soc/amd/common/blocks/cpu/mca: factor out common BERT helper functions ...................................................................... soc/amd/common/blocks/cpu/mca: factor out common BERT helper functions Change-Id: I03365c3820cbe7277f14adc5460e892fb8d9b7a5 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/block/cpu/mca/Makefile.inc M src/soc/amd/common/block/cpu/mca/mca_bert.c A src/soc/amd/common/block/cpu/mca/mca_common_bert.c A src/soc/amd/common/block/cpu/mca/mca_common_defs.h M src/soc/amd/common/block/cpu/mca/mcax_bert.c 5 files changed, 60 insertions(+), 71 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/56284/1 diff --git a/src/soc/amd/common/block/cpu/mca/Makefile.inc b/src/soc/amd/common/block/cpu/mca/Makefile.inc index 5c1d2c5..61d6974 100644 --- a/src/soc/amd/common/block/cpu/mca/Makefile.inc +++ b/src/soc/amd/common/block/cpu/mca/Makefile.inc @@ -1,3 +1,7 @@ -ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_MCA_COMMON) += mca_common.c +ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_MCA_COMMON),y) +ramstage-y += mca_common.c +ramstage-y += mca_common_bert.c +endif # CONFIG_SOC_AMD_COMMON_BLOCK_MCA_COMMON + ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_MCA) += mca_bert.c ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_MCAX) += mcax_bert.c diff --git a/src/soc/amd/common/block/cpu/mca/mca_bert.c b/src/soc/amd/common/block/cpu/mca/mca_bert.c index ffd8973..93d5749 100644 --- a/src/soc/amd/common/block/cpu/mca/mca_bert.c +++ b/src/soc/amd/common/block/cpu/mca/mca_bert.c @@ -8,6 +8,7 @@ #include <arch/bert_storage.h> #include <cper.h> #include <types.h> +#include "mca_common_defs.h" static inline size_t mca_report_size_reqd(void) { @@ -36,41 +37,6 @@ return size; } -static enum cper_x86_check_type error_to_chktype(struct mca_bank_status *mci) -{ - int error = mca_err_type(mci->sts); - - if (error == MCA_ERRTYPE_BUS) - return X86_PROCESSOR_BUS_CHK; - if (error == MCA_ERRTYPE_INT) - return X86_PROCESSOR_MS_CHK; - if (error == MCA_ERRTYPE_MEM) - return X86_PROCESSOR_CACHE_CHK; - if (error == MCA_ERRTYPE_TLB) - return X86_PROCESSOR_TLB_CHK; - - return X86_PROCESSOR_MS_CHK; /* unrecognized */ -} - -/* Fill additional information in the Generic Processor Error Section. */ -static void fill_generic_section(cper_proc_generic_error_section_t *sec, - struct mca_bank_status *mci) -{ - int type = mca_err_type(mci->sts); - - if (type == MCA_ERRTYPE_BUS) /* try to map MCA errors to CPER types */ - sec->error_type = GENPROC_ERRTYPE_BUS; - else if (type == MCA_ERRTYPE_INT) - sec->error_type = GENPROC_ERRTYPE_UARCH; - else if (type == MCA_ERRTYPE_MEM) - sec->error_type = GENPROC_ERRTYPE_CACHE; - else if (type == MCA_ERRTYPE_TLB) - sec->error_type = GENPROC_ERRTYPE_TLB; - else - sec->error_type = GENPROC_ERRTYPE_UNKNOWN; - sec->validation |= GENPROC_VALID_PROC_ERR_TYPE; -} - /* Convert an error reported by an MCA bank into BERT information to be reported * by the OS. The ACPI driver doesn't recognize/parse the IA32/X64 structure, * which is the best method to report MSR context. As a result, add two diff --git a/src/soc/amd/common/block/cpu/mca/mca_common_bert.c b/src/soc/amd/common/block/cpu/mca/mca_common_bert.c new file mode 100644 index 0000000..53a73d6 --- /dev/null +++ b/src/soc/amd/common/block/cpu/mca/mca_common_bert.c @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <amdblocks/mca.h> +#include <cper.h> +#include <types.h> +#include "mca_common_defs.h" + +enum cper_x86_check_type error_to_chktype(struct mca_bank_status *mci) +{ + int error = mca_err_type(mci->sts); + + if (error == MCA_ERRTYPE_BUS) + return X86_PROCESSOR_BUS_CHK; + if (error == MCA_ERRTYPE_INT) + return X86_PROCESSOR_MS_CHK; + if (error == MCA_ERRTYPE_MEM) + return X86_PROCESSOR_CACHE_CHK; + if (error == MCA_ERRTYPE_TLB) + return X86_PROCESSOR_TLB_CHK; + + return X86_PROCESSOR_MS_CHK; /* unrecognized */ +} + +/* Fill additional information in the Generic Processor Error Section. */ +void fill_generic_section(cper_proc_generic_error_section_t *sec, + struct mca_bank_status *mci) +{ + int type = mca_err_type(mci->sts); + + if (type == MCA_ERRTYPE_BUS) /* try to map MCA errors to CPER types */ + sec->error_type = GENPROC_ERRTYPE_BUS; + else if (type == MCA_ERRTYPE_INT) + sec->error_type = GENPROC_ERRTYPE_UARCH; + else if (type == MCA_ERRTYPE_MEM) + sec->error_type = GENPROC_ERRTYPE_CACHE; + else if (type == MCA_ERRTYPE_TLB) + sec->error_type = GENPROC_ERRTYPE_TLB; + else + sec->error_type = GENPROC_ERRTYPE_UNKNOWN; + sec->validation |= GENPROC_VALID_PROC_ERR_TYPE; +} diff --git a/src/soc/amd/common/block/cpu/mca/mca_common_defs.h b/src/soc/amd/common/block/cpu/mca/mca_common_defs.h new file mode 100644 index 0000000..41d73d8 --- /dev/null +++ b/src/soc/amd/common/block/cpu/mca/mca_common_defs.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef AMD_BLOCK_MCA_COMMON_DEF_H +#define AMD_BLOCK_MCA_COMMON_DEF_H + +#include <amdblocks/mca.h> +#include <cper.h> + +enum cper_x86_check_type error_to_chktype(struct mca_bank_status *mci); +void fill_generic_section(cper_proc_generic_error_section_t *sec, struct mca_bank_status *mci); + +#endif /* AMD_BLOCK_MCA_COMMON_DEF_H */ diff --git a/src/soc/amd/common/block/cpu/mca/mcax_bert.c b/src/soc/amd/common/block/cpu/mca/mcax_bert.c index f976158..8d864e8 100644 --- a/src/soc/amd/common/block/cpu/mca/mcax_bert.c +++ b/src/soc/amd/common/block/cpu/mca/mcax_bert.c @@ -8,6 +8,7 @@ #include <arch/bert_storage.h> #include <cper.h> #include <types.h> +#include "mca_common_defs.h" /* MISC4 is the last used register in the MCAX banks of Picasso */ #define MCAX_USED_REGISTERS_PER_BANK (MCAX_MISC4_OFFSET + 1) @@ -40,41 +41,6 @@ return size; } -static enum cper_x86_check_type error_to_chktype(struct mca_bank_status *mci) -{ - int error = mca_err_type(mci->sts); - - if (error == MCA_ERRTYPE_BUS) - return X86_PROCESSOR_BUS_CHK; - if (error == MCA_ERRTYPE_INT) - return X86_PROCESSOR_MS_CHK; - if (error == MCA_ERRTYPE_MEM) - return X86_PROCESSOR_CACHE_CHK; - if (error == MCA_ERRTYPE_TLB) - return X86_PROCESSOR_TLB_CHK; - - return X86_PROCESSOR_MS_CHK; /* unrecognized */ -} - -/* Fill additional information in the Generic Processor Error Section. */ -static void fill_generic_section(cper_proc_generic_error_section_t *sec, - struct mca_bank_status *mci) -{ - int type = mca_err_type(mci->sts); - - if (type == MCA_ERRTYPE_BUS) /* try to map MCA errors to CPER types */ - sec->error_type = GENPROC_ERRTYPE_BUS; - else if (type == MCA_ERRTYPE_INT) - sec->error_type = GENPROC_ERRTYPE_UARCH; - else if (type == MCA_ERRTYPE_MEM) - sec->error_type = GENPROC_ERRTYPE_CACHE; - else if (type == MCA_ERRTYPE_TLB) - sec->error_type = GENPROC_ERRTYPE_TLB; - else - sec->error_type = GENPROC_ERRTYPE_UNKNOWN; - sec->validation |= GENPROC_VALID_PROC_ERR_TYPE; -} - /* Convert an error reported by an MCA bank into BERT information to be reported * by the OS. The ACPI driver doesn't recognize/parse the IA32/X64 structure, * which is the best method to report MSR context. As a result, add two -- To view, visit https://review.coreboot.org/c/coreboot/+/56284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I03365c3820cbe7277f14adc5460e892fb8d9b7a5 Gerrit-Change-Number: 56284 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/intel/jasperlake_rvp: Rework Kconfig
by Felix Singer (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Subrata Banik. Hello build bot (Jenkins), Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56222 to look at the new patch set (#6). Change subject: mb/intel/jasperlake_rvp: Rework Kconfig ...................................................................... mb/intel/jasperlake_rvp: Rework Kconfig Rework Kconfig file that each variant has its own config option with their specific selects / configuration and move common selects to `BOARD_INTEL_JASPERLAKE_RVP_COMMON`, which is used as base for each variant. Also, move selects from Kconfig.name to Kconfig that the configuration is at one place and not distributed over two files. Built each variant with `BUILD_TIMELESS=1` and all generated coreboot.rom files remain identical. Change-Id: Ic7552195ed5a3ae6ab8e456d7d38d5539a052009 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/intel/jasperlake_rvp/Kconfig M src/mainboard/intel/jasperlake_rvp/Kconfig.name 2 files changed, 25 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/56222/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/56222 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic7552195ed5a3ae6ab8e456d7d38d5539a052009 Gerrit-Change-Number: 56222 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/intel/adlrvp: Rework Kconfig
by Felix Singer (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Subrata Banik. Hello build bot (Jenkins), Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56221 to look at the new patch set (#7). Change subject: mb/intel/adlrvp: Rework Kconfig ...................................................................... mb/intel/adlrvp: Rework Kconfig Rework Kconfig file that each variant has its own config option with their specific selects / configuration and move common selects to `BOARD_INTEL_ADLRVP_COMMON`, which is used as base for each variant. Also, move selects from Kconfig.name to Kconfig that the configuration is at one place and not distributed over two files. Built each variant with `BUILD_TIMELESS=1` and all generated coreboot.rom files remain identical. Change-Id: If68c118f22579cc0a3db570119798f0f535f9804 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/intel/adlrvp/Kconfig M src/mainboard/intel/adlrvp/Kconfig.name 2 files changed, 45 insertions(+), 29 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/56221/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/56221 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If68c118f22579cc0a3db570119798f0f535f9804 Gerrit-Change-Number: 56221 Gerrit-PatchSet: 7 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/intel/kblrvp: Rework Kconfig
by Felix Singer (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Subrata Banik. Hello build bot (Jenkins), Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56219 to look at the new patch set (#5). Change subject: mb/intel/kblrvp: Rework Kconfig ...................................................................... mb/intel/kblrvp: Rework Kconfig Rework Kconfig file that each variant has its own config option with their specific selects / configuration and move common selects to `BOARD_INTEL_KBLRVP_COMMON`, which is used as base for each variant. Built each variant with `BUILD_TIMELESS=1` and all generated coreboot.rom files remain identical. Change-Id: I2a9c12a15c098fcb64c006a707c94a1aed93d73a Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/intel/kblrvp/Kconfig M src/mainboard/intel/kblrvp/Kconfig.name 2 files changed, 36 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/56219/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/56219 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a9c12a15c098fcb64c006a707c94a1aed93d73a Gerrit-Change-Number: 56219 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/intel/coffeelake_rvp: Rework Kconfig
by Felix Singer (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Angel Pons, Subrata Banik, Michael Niewöhner. Hello build bot (Jenkins), Angel Pons, Subrata Banik, Michael Niewöhner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56217 to look at the new patch set (#3). Change subject: mb/intel/coffeelake_rvp: Rework Kconfig ...................................................................... mb/intel/coffeelake_rvp: Rework Kconfig Rework Kconfig file that each variant has its own config option with their specific selects / configuration and move common selects to `BOARD_INTEL_COFFEELAKE_COMMON`, which is used as base for each variant. Also, move selects from Kconfig.name to Kconfig that the configuration is at one place and not distributed over two files. Built each variant with `BUILD_TIMELESS=1` and all generated coreboot.rom files remain identical. Change-Id: I3b3d3cff5ea7a3f4d1c4ddd911240763e4891e06 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/mainboard/intel/coffeelake_rvp/Kconfig M src/mainboard/intel/coffeelake_rvp/Kconfig.name 2 files changed, 57 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/56217/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/56217 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3b3d3cff5ea7a3f4d1c4ddd911240763e4891e06 Gerrit-Change-Number: 56217 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: arch/x86: Helper functions to get deterministic cache parameters
by Tim Wawrzynczak (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Furquan Shaikh, Rizwan Qureshi, Subrata Banik, Angel Pons, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55965 ) Change subject: arch/x86: Helper functions to get deterministic cache parameters ...................................................................... Patch Set 7: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/55965/comment/3bf4da42_76fb2927 PS7, Line 20: cahce cache Patchset: PS7: Why bother with caching the results of this CPUID call? Why not something like: ``` bool get_cpu_cache_info(uint8_t level, struct cpu_cache_info *info) { if (!info) return false; uint32_t leaf = cpu_get_cache_info_leaf(); if (!leaf) return; struct cpuid_result result = cpuid_ext(leaf, level); info->type = cpu_get_cache_type(result); info->level = cpu_get_cache_level(result); info->....... } ``` then the other functions are more like: ``` size_t cache_size(const struct cpu_cache_info *info) { return info->num_ways * info->physical_partitions * info->line_size * info->num_sets; } ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/55965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0dd701fb47460092448b64c7fa2162f762bf3095 Gerrit-Change-Number: 55965 Gerrit-PatchSet: 7 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 13 Jul 2021 22:47:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/siemens/chili: Replace `def_bool` with `bool`
by Felix Singer (Code Review) 14 Jul '21

14 Jul '21

Felix Singer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/52807 ) Change subject: mb/siemens/chili: Replace `def_bool` with `bool` ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/52807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib7fdd04e3ea282afb67b4d7a326a36f15f0e0cac Gerrit-Change-Number: 52807 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: abandon

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021