Attention is currently required from: Andrey Petrov. Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63007 )
Change subject: drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler ......................................................................
drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler
This patch implements coreboot native debug handler to manage the FSP event messages.
`FSP Event Handlers` feature introduced in FSP to generate event messages to aid in the debugging of firmware issues. This eliminates the need for FSP to directly write debug messages to the UART and FSP might not need to know the board related UART port configuration. Instead FSP signals the bootloader to inform it of a new debug message. This allows the coreboot to provide board specific methods of reporting debug messages, example: legacy UART or LPSS UART etc.
This implementation has several advantages as: 1. FSP relies on XIP `DebugLib` driver even while printing FSP-S debug messages, hence, without ROM being cached, post `romstage` would results into sluggish boot with FSP debug enabled.
This patch utilities coreboot native debug implementation which is XIP during FSP-M and relocatable to DRAM based resource for FSP-S.
2. This patch simplifies the FSP DebugLib implementation and remove the need to have serialport library. Instead coreboot `printk` can be used for display FSP serial messages. Additionally, unifies the debug library between coreboot and FSP.
3. This patch is also useful to get debug prints even with FSP non-serial image (refer to `Note` below) as FSP PEIMs are now leveraging coreboot debug library instead FSP `NULL` DebugLib reference for release build.
4. Can optimize the FSP binary size by removing the DebugLib dependency from most of FSP PEIMs, for example: on Alder Lake FSP-M debug binary size is reduced by ~100KB+ and FSP-S debug library size is also reduced by ~300KB+ (FSP-S debug and release binary size is exactly same with this code changes). The total savings is ~400KB for each FSP copy, and in case of Chrome AP firmware with 3 copies, the total savings would be 400KB * 3 = ~1.2MB.
Note: Need to modify FSP source code to remove `MDEPKG_NDEBUG` as compilation flag for release build and generate FSP binary with non-NULL FSP debug wrapper module injected (to allow FSP event handler to execute even with FSP non-serial image) in the final FSP.fd.
Additionally, this patch assigns FSP handler event for FSP-M with coreboot romstage debug handler when FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT Kconfig is enabled.
BUG=b:225544587 TEST=Able to build and boot brya. Also, verified the FSP-M debug log is exactly same before and with this code change.
Signed-off-by: Subrata Banik subratabanik@google.com Change-Id: I1018e67d70492b18c76531f9e78d3b58fa435cd4 --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/Makefile.inc A src/drivers/intel/fsp2_0/fsp_debug_event.c A src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h M src/drivers/intel/fsp2_0/memory_init.c 5 files changed, 60 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/63007/1
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index fdea4b8..9ba648d 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -352,4 +352,11 @@ to perform the required lock down and chipset register configuration prior boot to payload.
+config FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT + bool + default n + help + This option allows to create `Debug Event Handler` to print message to debug output + device using coreboot native implementation. + endif diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc index eaf99d1..0abf024 100644 --- a/src/drivers/intel/fsp2_0/Makefile.inc +++ b/src/drivers/intel/fsp2_0/Makefile.inc @@ -5,6 +5,7 @@ bootblock-$(CONFIG_FSP_CAR) += fspt_report.c
romstage-y += debug.c +romstage-$(CONFIG_FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT) += fsp_debug_event.c romstage-y += hand_off_block.c romstage-$(CONFIG_DISPLAY_FSP_HEADER) += header_display.c romstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c @@ -16,6 +17,7 @@ romstage-y += cbmem.c
ramstage-y += debug.c +ramstage-$(CONFIG_FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT) += fsp_debug_event.c ramstage-$(CONFIG_USE_INTEL_FSP_MP_INIT) += fsp_mpinit.c ramstage-$(CONFIG_RUN_FSP_GOP) += graphics.c ramstage-y += hand_off_block.c diff --git a/src/drivers/intel/fsp2_0/fsp_debug_event.c b/src/drivers/intel/fsp2_0/fsp_debug_event.c new file mode 100644 index 0000000..a347829 --- /dev/null +++ b/src/drivers/intel/fsp2_0/fsp_debug_event.c @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <fsp/api.h> +#include <fsp/fsp_debug_event.h> +#include <fsp/util.h> + +static const uint8_t fsp_string_type_guid[16] = { + 0x80, 0x10, 0xd1, 0x92, 0x6f, 0x49, 0x95, 0x4d, + 0xbe, 0x7e, 0x03, 0x74, 0x88, 0x38, 0x2b, 0x0a +}; + +static efi_return_status_t print_fsp_string_data(const efi_status_code_data_t *data) +{ + printk(BIOS_SPEW, "%s", ((efi_status_code_string_data *) data)->String.Ascii); + + return FSP_SUCCESS; +} + +efi_return_status_t fsp_debug_event_handler(efi_status_code_type_t ignored1, + efi_status_code_value_t ignored2, efi_uint32_t ignored3, efi_guid_t *ignored4, + efi_status_code_data_t *data) +{ + if (!fsp_guid_compare ((uint8_t *)&(data->Type), fsp_string_type_guid)) + return FSP_NOT_FOUND; + + return print_fsp_string_data(data); +} diff --git a/src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h b/src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h new file mode 100644 index 0000000..7d50c1e --- /dev/null +++ b/src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef FSP_DEBUG_EVENT_H +#define FSP_DEBUG_EVENT_H + +/* + * This file to implement FSP_EVENT_HANDLER for Intel FSP to use. + * More details about this structure can be found here : + * http://github.com/tianocore/edk2/blob/master/IntelFsp2Pkg/Include/FspEas/Fsp... + */ +#include <efi/efi_datatype.h> +#include <fsp/soc_binding.h> + +/* fsp debug event handler */ +efi_return_status_t fsp_debug_event_handler(efi_status_code_type_t ignored1, + efi_status_code_value_t ignored2, efi_uint32_t ignored3, efi_guid_t *ignored4, + efi_status_code_data_t *data); + +#endif /* FSP_DEBUG_EVENT_H */ diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 1612089..6569dc2 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -10,6 +10,7 @@ #include <console/console.h> #include <elog.h> #include <fsp/api.h> +#include <fsp/fsp_debug_event.h> #include <fsp/util.h> #include <memrange.h> #include <mrc_cache.h> @@ -180,6 +181,9 @@
printk(BIOS_SPEW, "bootmode is set to: %d\n", arch_upd->BootMode);
+ if (CONFIG(FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT)) + arch_upd->FspEventHandler = (UINT32)((FSP_EVENT_HANDLER *)fsp_debug_event_handler); + return CB_SUCCESS; }