From 4db921317fe1eec77e93b3c1628dac14fd812d39 Mon Sep 17 00:00:00 2001 From: Pratikkumar Prajapati Date: Thu, 5 Oct 2023 13:12:12 -0700 Subject: soc/intel/common,mtl: Refactor BERT generation flow for crashlog With earlier flow, a chunk of CBMEM region was allocated for each SRAM e.g., PUNIT SRAM, SOC PMC SRAM and IOE PMC SRAM. Then entire SRAM content was copied to dedicated CBMEM region. Later in acpi_bert.c, the BERT table was getting created for each chunk of CBMEM. This flow was not considering creating separate entries for each region of crashlog records. It resulted in only the first entry getting decoded from each SRAM. New flow aims to fix this issue. With new flow, a simple singly linked list is created to store each region of crashlog records from all SRAMs. The crashlog data is not copied to CBMEM. The nodes are allocated dynamically and then copied to ACPI BERT table and then freed. This flow also makes the overall crashlog code much simpler. BUG=b:298234592 TEST=With this change decoding crashlog show comprehensive details, tested on REX. Change-Id: I43bb61485b77d786647900ca284b7f492f412aee Signed-off-by: Pratikkumar Prajapati Reviewed-on: https://review.coreboot.org/c/coreboot/+/78257 Reviewed-by: Kapil Porwal Tested-by: build bot (Jenkins) --- src/soc/intel/common/block/acpi/acpi_bert.c | 83 +++----- src/soc/intel/common/block/crashlog/crashlog.c | 232 ++++++++++----------- .../common/block/include/intelblocks/crashlog.h | 18 +- 3 files changed, 143 insertions(+), 190 deletions(-) (limited to 'src/soc/intel/common/block') diff --git a/src/soc/intel/common/block/acpi/acpi_bert.c b/src/soc/intel/common/block/acpi/acpi_bert.c index 350d5d3c00..417dad3ac7 100644 --- a/src/soc/intel/common/block/acpi/acpi_bert.c +++ b/src/soc/intel/common/block/acpi/acpi_bert.c @@ -7,14 +7,14 @@ #include #include -static bool boot_error_src_present(void) +static bool boot_error_src_present(cl_node_t *head) { if (!discover_crashlog()) { printk(BIOS_SPEW, "Crashlog discovery result: crashlog not found\n"); return false; } - collect_pmc_and_cpu_crashlog_from_srams(); + collect_pmc_and_cpu_crashlog_from_srams(head); /* Discovery tables sizes can be larger than the actual valid collected data */ u32 crashlog_size = cl_get_total_data_size(); @@ -25,11 +25,11 @@ static bool boot_error_src_present(void) static enum cb_err record_crashlog_into_bert(void **region, size_t *length) { acpi_generic_error_status_t *status = NULL; - size_t cpu_record_size, pmc_record_size; size_t gesb_header_size; - void *cl_data = NULL; + void *cl_acpi_data = NULL; + cl_node_t cl_list_head = {.size = 0, .data = NULL, .next = NULL}; - if (!boot_error_src_present()) { + if (!boot_error_src_present(&cl_list_head)) { return CB_ERR; } @@ -47,67 +47,38 @@ static enum cb_err record_crashlog_into_bert(void **region, size_t *length) } if (cl_get_total_data_size() > bert_storage_remaining()) { - printk(BIOS_ERR, "Crashlog entry would exceed " - "available region\n"); + printk(BIOS_ERR, "Crashlog entry would exceed available region\n"); return CB_ERR; } - cpu_record_size = cl_get_cpu_record_size(); - if (cpu_record_size) { - cl_data = new_cper_fw_error_crashlog(status, cpu_record_size); - if (!cl_data) { - printk(BIOS_ERR, "Crashlog CPU entry(size 0x%zx) " - "would exceed available region\n", - cpu_record_size); - return CB_ERR; - } - printk(BIOS_DEBUG, "cl_data %p, cpu_record_size 0x%zx\n", - cl_data, cpu_record_size); - cl_fill_cpu_records(cl_data); - } + bool multi_entry = false; + cl_node_t *cl_node = cl_list_head.next; + while (cl_node) { - pmc_record_size = cl_get_pmc_record_size(); - if (pmc_record_size) { - /* Allocate new FW ERR structure in case PMC crashlog is present */ - if (pmc_record_size && !bert_append_fw_err(status)) { - printk(BIOS_ERR, "Crashlog PMC entry would " - "exceed available region\n"); - return CB_ERR; + if ((cl_node->size <= 0) || (!(cl_node->data))) { + cl_node = cl_node->next; + continue; } - cl_data = new_cper_fw_error_crashlog(status, pmc_record_size); - if (!cl_data) { - printk(BIOS_ERR, "Crashlog PMC entry(size 0x%zx) " - "would exceed available region\n", - pmc_record_size); - return CB_ERR; - } - printk(BIOS_DEBUG, "cl_data %p, pmc_record_size 0x%zx\n", - cl_data, pmc_record_size); - cl_fill_pmc_records(cl_data); - } - - if (CONFIG(SOC_INTEL_IOE_DIE_SUPPORT)) { - size_t ioe_record_size = cl_get_ioe_record_size(); - if (ioe_record_size) { - /* Allocate new FW ERR structure in case IOE crashlog is present */ - if (ioe_record_size && !bert_append_fw_err(status)) { - printk(BIOS_ERR, "Crashlog IOE entry would " - "exceed available region\n"); + if (multi_entry) { + if (!bert_append_fw_err(status)) { + printk(BIOS_ERR, "Crashlog entry would exceed available region\n"); return CB_ERR; } + } - cl_data = new_cper_fw_error_crashlog(status, ioe_record_size); - if (!cl_data) { - printk(BIOS_ERR, "Crashlog IOE entry(size 0x%zx) " - "would exceed available region\n", - ioe_record_size); - return CB_ERR; - } - printk(BIOS_DEBUG, "cl_data %p, ioe_record_size 0x%zx\n", - cl_data, ioe_record_size); - cl_fill_ioe_records(cl_data); + cl_acpi_data = new_cper_fw_error_crashlog(status, cl_node->size); + if (!cl_acpi_data) { + printk(BIOS_ERR, "Crashlog entry(size 0x%x) would exceed available region\n", + cl_node->size); + return CB_ERR; } + memcpy(cl_acpi_data, (void *) cl_node->data, cl_node->size); + + cl_node_t *temp = cl_node; + cl_node = cl_node->next; + free_cl_node(temp); + multi_entry = true; } *length = status->data_length + gesb_header_size; diff --git a/src/soc/intel/common/block/crashlog/crashlog.c b/src/soc/intel/common/block/crashlog/crashlog.c index e3d85cde22..6735d971ba 100644 --- a/src/soc/intel/common/block/crashlog/crashlog.c +++ b/src/soc/intel/common/block/crashlog/crashlog.c @@ -330,11 +330,36 @@ bool cl_copy_data_from_sram(u32 src_bar, return true; } -void __weak cl_get_pmc_sram_data(void) +cl_node_t *malloc_cl_node(size_t len) +{ + cl_node_t *node = malloc(sizeof(cl_node_t)); + if (!node) + return NULL; + + node->data = malloc(len * sizeof(u32)); + if (!(node->data)) + return NULL; + + node->size = len * sizeof(u32); + node->next = NULL; + + return node; +} + +void free_cl_node(cl_node_t *node) +{ + if (!node) + return; + if (node->data) + free(node->data); + free(node); +} + +void __weak cl_get_pmc_sram_data(cl_node_t *head) { - u32 *dest = NULL; u32 tmp_bar_addr = cl_get_cpu_tmp_bar(); u32 pmc_crashLog_size = cl_get_pmc_record_size(); + cl_node_t *cl_cur = head; if (!cl_pmc_sram_has_mmio_access() || !tmp_bar_addr) return; @@ -353,7 +378,7 @@ void __weak cl_get_pmc_sram_data(void) goto pmc_send_re_arm_after_reset; } printk(BIOS_DEBUG, "PMC crashLog size in discovery mode : 0x%X\n", - pmc_crashLog_size); + pmc_crashLog_size); } else { if (discovery_buf.bits.dis) { printk(BIOS_DEBUG, "PCH crashlog is disabled in legacy mode.\n"); @@ -362,51 +387,65 @@ void __weak cl_get_pmc_sram_data(void) pmc_crashLog_size = (discovery_buf.bits.size != 0) ? discovery_buf.bits.size : 0xC00; printk(BIOS_DEBUG, "PMC crashLog size in legacy mode : 0x%X\n", - pmc_crashLog_size); + pmc_crashLog_size); } - /* allocate mem for the record to be copied */ - unsigned long pmc_cl_cbmem_addr; + bool pmc_sram = true; + pmc_crashlog_desc_table_t descriptor_table = cl_get_pmc_descriptor_table(); - pmc_cl_cbmem_addr = (unsigned long)cbmem_add(CBMEM_ID_PMC_CRASHLOG, - pmc_crashLog_size); - if (!pmc_cl_cbmem_addr) { - printk(BIOS_ERR, "Unable to allocate CBMEM PMC crashLog entry.\n"); - return; + /* goto tail node */ + while (cl_cur && cl_cur->next) { + cl_cur = cl_cur->next; } - memset((void *)pmc_cl_cbmem_addr, 0, pmc_crashLog_size); - dest = (u32 *)(uintptr_t)pmc_cl_cbmem_addr; - bool pmc_sram = true; - pmc_crashlog_desc_table_t descriptor_table = cl_get_pmc_descriptor_table(); if (discovery_buf.bits.discov_mechanism == 1) { for (int i = 0; i < descriptor_table.numb_regions; i++) { + + cl_node_t *cl_node = malloc_cl_node(descriptor_table.regions[i].bits.size); + if (!cl_node) { + printk(BIOS_DEBUG, "failed to allocate cl_node [region = %d]\n", i); + goto pmc_send_re_arm_after_reset; + } + if (cl_copy_data_from_sram(tmp_bar_addr, - descriptor_table.regions[i].bits.offset, - descriptor_table.regions[i].bits.size, - dest, - i, - pmc_sram)) { - dest = (u32 *)((u32)dest + - (descriptor_table.regions[i].bits.size - * sizeof(u32))); + descriptor_table.regions[i].bits.offset, + descriptor_table.regions[i].bits.size, + cl_node->data, + i, + pmc_sram)) { + cl_cur->next = cl_node; + cl_cur = cl_cur->next; } else { pmc_crashLog_size -= descriptor_table.regions[i].bits.size * sizeof(u32); printk(BIOS_DEBUG, "discover mode PMC crashlog size adjusted" - " to: 0x%x\n", pmc_crashLog_size); + " to: 0x%x\n", pmc_crashLog_size); + /* free cl_node */ + free_cl_node(cl_node); } } } else { - if (!cl_copy_data_from_sram(tmp_bar_addr, - discovery_buf.bits.base_offset, - discovery_buf.bits.size, - dest, - 0, - pmc_sram)) { + + cl_node_t *cl_node = malloc_cl_node(discovery_buf.bits.size); + if (!cl_node) { + printk(BIOS_DEBUG, "failed to allocate cl_node\n"); + goto pmc_send_re_arm_after_reset; + } + + if (cl_copy_data_from_sram(tmp_bar_addr, + discovery_buf.bits.base_offset, + discovery_buf.bits.size, + cl_node->data, + 0, + pmc_sram)) { + cl_cur->next = cl_node; + cl_cur = cl_cur->next; + } else { pmc_crashLog_size -= discovery_buf.bits.size * sizeof(u32); printk(BIOS_DEBUG, "legacy mode PMC crashlog size adjusted to: 0x%x\n", - pmc_crashLog_size); + pmc_crashLog_size); + /* free cl_node */ + free_cl_node(cl_node); } } @@ -422,10 +461,9 @@ pmc_send_re_arm_after_reset: } -void cl_get_cpu_sram_data(void) +void cl_get_cpu_sram_data(cl_node_t *head) { - u32 tmp_bar_addr = 0; - u32 *dest = NULL; + cl_node_t *cl_cur = head; u32 m_cpu_crashLog_size = cl_get_cpu_record_size(); cpu_crashlog_discovery_table_t cpu_cl_disc_tab = cl_get_cpu_discovery_table(); @@ -435,35 +473,41 @@ void cl_get_cpu_sram_data(void) } printk(BIOS_DEBUG, "CPU crash data size: 0x%X bytes in 0x%X region(s).\n", - m_cpu_crashLog_size, cpu_cl_disc_tab.header.fields.count); - - /* allocate memory buffers for CPU crashog data to be copied */ - unsigned long cpu_crashlog_cbmem_addr; - cpu_crashlog_cbmem_addr = (unsigned long)cbmem_add(CBMEM_ID_CPU_CRASHLOG, - m_cpu_crashLog_size); - if (!cpu_crashlog_cbmem_addr) { - printk(BIOS_ERR, "Failed to add CPU main crashLog entries to CBMEM.\n"); - return; - } + m_cpu_crashLog_size, cpu_cl_disc_tab.header.fields.count); - memset((void *)cpu_crashlog_cbmem_addr, 0, m_cpu_crashLog_size); - tmp_bar_addr = cl_get_cpu_bar_addr(); - dest = (u32 *)(uintptr_t)cpu_crashlog_cbmem_addr; - bool pmc_sram = false; + /* goto tail node */ + while (cl_cur && cl_cur->next) { + cl_cur = cl_cur->next; + } for (int i = 0 ; i < cpu_cl_disc_tab.header.fields.count ; i++) { - if (cl_copy_data_from_sram(tmp_bar_addr, - cpu_cl_disc_tab.buffers[i].fields.offset, - cpu_cl_disc_tab.buffers[i].fields.size, - dest, - i, - pmc_sram)) { - dest = (u32 *)((u32)dest + - (cpu_cl_disc_tab.buffers[i].fields.size * sizeof(u32))); + + u32 cpu_bar_addr = cl_get_cpu_bar_addr(); + bool pmc_sram = false; + + if (!cpu_cl_disc_tab.buffers[i].fields.size) { + continue; + } + + cl_node_t *cl_node = malloc_cl_node(cpu_cl_disc_tab.buffers[i].fields.size); + if (!cl_node) { + printk(BIOS_DEBUG, "failed to allocate cl_node [buffer = %d]\n", i); + return; + } + + if (cl_copy_data_from_sram(cpu_bar_addr, + cpu_cl_disc_tab.buffers[i].fields.offset, + cpu_cl_disc_tab.buffers[i].fields.size, + cl_node->data, + i, + pmc_sram)) { + cl_cur->next = cl_node; + cl_cur = cl_cur->next; + } else { m_cpu_crashLog_size -= cpu_cl_disc_tab.buffers[i].fields.size - * sizeof(u32); - + * sizeof(u32); + free_cl_node(cl_node); /* for CPU skip all buffers if the 1st one is not valid */ if (i == 0) { m_cpu_crashLog_size = 0; @@ -482,7 +526,7 @@ void cl_get_cpu_sram_data(void) cpu_cl_rearm(); } -void collect_pmc_and_cpu_crashlog_from_srams(void) +void collect_pmc_and_cpu_crashlog_from_srams(cl_node_t *head) { if (pmc_crashlog_support() && cl_pmc_data_present() && (cl_get_pmc_record_size() > 0)) { @@ -490,7 +534,7 @@ void collect_pmc_and_cpu_crashlog_from_srams(void) cl_pmc_en_gen_on_all_reboot(); printk(BIOS_DEBUG, "Crashlog collection enabled on every reboot.\n"); } - cl_get_pmc_sram_data(); + cl_get_pmc_sram_data(head); } else { printk(BIOS_DEBUG, "Skipping PMC crashLog collection. Data not present.\n"); } @@ -500,74 +544,8 @@ void collect_pmc_and_cpu_crashlog_from_srams(void) if (cpu_crashlog_support() && cl_cpu_data_present() && (cl_get_cpu_record_size() > 0)) { printk(BIOS_DEBUG, "CPU crashLog present.\n"); - cl_get_cpu_sram_data(); + cl_get_cpu_sram_data(head); } else { printk(BIOS_DEBUG, "Skipping CPU crashLog collection. Data not present.\n"); } } - -bool cl_fill_cpu_records(void *cl_record) -{ - void *cl_src_addr = NULL; - - u32 m_cpu_crashLog_size = cl_get_cpu_record_size(); - - if (!cl_cpu_data_present() || m_cpu_crashLog_size == 0) { - printk(BIOS_DEBUG, "CPU crashLog not present, skipping.\n"); - return false; - } - - printk(BIOS_DEBUG, "CPU crash data collection.\n"); - cl_src_addr = cbmem_find(CBMEM_ID_CPU_CRASHLOG); - if (!cl_src_addr) { - printk(BIOS_DEBUG, "CPU crash data, CBMEM not found\n"); - return false; - } - memcpy(cl_record, cl_src_addr, m_cpu_crashLog_size); - - return true; -} - -bool cl_fill_pmc_records(void *cl_record) -{ - void *cl_src_addr = NULL; - - u32 m_pmc_crashLog_size = cl_get_pmc_record_size(); - - if (!cl_pmc_data_present() || m_pmc_crashLog_size == 0) { - printk(BIOS_DEBUG, "PMC crashLog not present, skipping.\n"); - return false; - } - - printk(BIOS_DEBUG, "PMC crash data collection.\n"); - cl_src_addr = cbmem_find(CBMEM_ID_PMC_CRASHLOG); - if (!cl_src_addr) { - printk(BIOS_DEBUG, "PMC crash data, CBMEM not found\n"); - return false; - } - memcpy(cl_record, cl_src_addr, m_pmc_crashLog_size); - - return true; -} - -bool cl_fill_ioe_records(void *cl_record) -{ - void *cl_src_addr = NULL; - - u32 m_ioe_crashLog_size = cl_get_ioe_record_size(); - - if (!cl_ioe_data_present() || m_ioe_crashLog_size == 0) { - printk(BIOS_DEBUG, "IOE crashLog not present, skipping.\n"); - return false; - } - - printk(BIOS_DEBUG, "IOE PMC crash data collection.\n"); - cl_src_addr = cbmem_find(CBMEM_ID_IOE_CRASHLOG); - if (!cl_src_addr) { - printk(BIOS_DEBUG, "IOE crash data, CBMEM not found\n"); - return false; - } - memcpy(cl_record, cl_src_addr, m_ioe_crashLog_size); - - return true; -} diff --git a/src/soc/intel/common/block/include/intelblocks/crashlog.h b/src/soc/intel/common/block/include/intelblocks/crashlog.h index 38d1a09de6..ff80b1c426 100644 --- a/src/soc/intel/common/block/include/intelblocks/crashlog.h +++ b/src/soc/intel/common/block/include/intelblocks/crashlog.h @@ -170,6 +170,14 @@ typedef struct { cpu_crashlog_buffer_info_t buffers[256]; } __packed cpu_crashlog_discovery_table_t; +typedef struct cl_node_t { + u32 size; + void *data; + struct cl_node_t *next; +} cl_node_t; + +cl_node_t *malloc_cl_node(size_t len); +void free_cl_node(cl_node_t *node); int cl_get_cpu_record_size(void); int cl_get_pmc_record_size(void); int cl_get_ioe_record_size(void); @@ -184,8 +192,8 @@ bool pmc_crashlog_support(void); bool cl_cpu_data_present(void); bool cl_pmc_data_present(void); bool cl_ioe_data_present(void); -void cl_get_cpu_sram_data(void); -void cl_get_pmc_sram_data(void); +void cl_get_cpu_sram_data(cl_node_t *head); +void cl_get_pmc_sram_data(cl_node_t *head); void reset_discovery_buffers(void); void update_new_pmc_crashlog_size(u32 *pmc_crash_size); void update_new_cpu_crashlog_size(u32 *cpu_crash_size); @@ -213,11 +221,7 @@ bool cl_copy_data_from_sram(u32 src_bar, u32 *dest_addr, u32 buffer_index, bool pmc_sram); -void collect_pmc_and_cpu_crashlog_from_srams(void); -bool cl_fill_cpu_records(void *cl_record); -bool cl_fill_pmc_records(void *cl_record); -bool cl_fill_ioe_records(void *cl_record); - +void collect_pmc_and_cpu_crashlog_from_srams(cl_node_t *head); static const EFI_GUID FW_ERR_SECTION_GUID = { 0x81212a96, 0x09ed, 0x4996, { 0x94, 0x71, 0x8d, 0x72, 0x9c, 0x8e, 0x69, 0xed } -- cgit v1.2.3