coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3043 discussions

[XS] Change in coreboot[main]: commonlib: cbmem_id: Add id for ACPI BDAT
by Gaggery (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Paul Menzel, Subrata Banik. Gaggery has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77254?usp=email ) Change subject: commonlib: cbmem_id: Add id for ACPI BDAT ...................................................................... Patch Set 11: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/77254/comment/a2074dfd_d3271a45 : PS8, Line 7: src/commonlib: Add ACPI BDAT in cbmem support > Please remove the leading `src/`. […] Done https://review.coreboot.org/c/coreboot/+/77254/comment/60d9cc06_7b0bb8d3 : PS8, Line 9: BDAT(BIOS Data) > Please add a space before the (. Done File src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h: https://review.coreboot.org/c/coreboot/+/77254/comment/7dc0d26e_05679e9a : PS8, Line 7: #define CBMEM_ID_ACPI_BERT 0x42455254 : #define CBMEM_ID_ACPI_BDAT 0x42444154 > Please sort it. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/77254?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic4925adfdaff36e28d47829322a8405ed92cad17 Gerrit-Change-Number: 77254 Gerrit-PatchSet: 11 Gerrit-Owner: Gaggery <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 31 Jan 2024 23:40:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Support FSP 2.4 64-bits
by Jérémy Compostella (Code Review) 01 Feb '24

01 Feb '24

Jérémy Compostella has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/80276?usp=email ) Change subject: drivers/intel/fsp2_0: Support FSP 2.4 64-bits ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/80276?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3fa012fa6f98e4bb3c2a033189554ad5a6ddfced Gerrit-Change-Number: 80276 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-MessageType: abandon

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Support FSP 2.4 64-bits
by Jérémy Compostella (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Andrey Petrov, Ronak Kanabar. Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80277?usp=email ) Change subject: drivers/intel/fsp2_0: Support FSP 2.4 64-bits ...................................................................... drivers/intel/fsp2_0: Support FSP 2.4 64-bits FSP 2.4 brings FSP 64-bits support which requires some adjustments in coreboot: - Stack alignment: 1. FSP functions must be called with the stack 16-bytes aligned. This is already setup properly with the default value of the `mpreferred-stack-boundary' compiler option (4). 2. The FSP stack buffer supplied by coreboot through the `StackBase' UPD must be 16-bytes aligned. - The EDK2 EFIAPI macro definition relies on compiler flags such as __GNUC__ which is not working well when included by coreboot. While it has no side-effect on i386 because the C calling convention used by coreboot and FSP are the same, it breaks on x86_64 because FSP/UEFI uses the Microsoft x64 calling convention while coreboot uses the System V AMD64 ABI. Fortunately, EDK2 header allows to override the EFIAPI definition. The __ms_abi__ attribute works for both i386 and x86_64. This attribute has to be set to all functions calling or called by the FSP. - The EFI_STATUS/efi_return_status_t size changes with the architecture (32-bits vs 64-bits). To print statuses independently of the architecture we leverage the size_t 'z' print format which is generally aligned with the architecture size too. In addition, this commit sets`PLATFORM_USES_FSP2_X86_32' to `n' by default if FSP 2.4 is enabled as 64-bits FSP should be norm moving forward. Change-Id: If0397f5cc8d0f4f1872bd37a001fe42e0c37ec99 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/debug.c M src/drivers/intel/fsp2_0/fsp_debug_event.c M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/soc_binding.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/ppi/mp_service1.c M src/drivers/intel/fsp2_0/ppi/mp_service2.c M src/drivers/intel/fsp2_0/silicon_init.c M src/drivers/intel/fsp2_0/util.c M src/include/efi/efi_datatype.h M src/soc/intel/common/fsp_reset.c 16 files changed, 88 insertions(+), 59 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/80277/1 diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 2c8bdc4..61d68f6 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -55,10 +55,11 @@ config PLATFORM_USES_FSP2_X86_32 bool + default n if PLATFORM_USES_FSP2_4 default y help - The FSP 2.0 runs in x86_32 protected mode. - Once there's a x86_64 FSP this needs to default to n. + Specify if the FSP binaries are 32-bits (yes) or 64-bits + (no). Starting with FSP specification 2.4 they can be 64-bits. config HAVE_INTEL_FSP_REPO bool diff --git a/src/drivers/intel/fsp2_0/debug.c b/src/drivers/intel/fsp2_0/debug.c index 8c0d67d..e74e745 100644 --- a/src/drivers/intel/fsp2_0/debug.c +++ b/src/drivers/intel/fsp2_0/debug.c @@ -91,10 +91,10 @@ printk(BIOS_SPEW, "\t%p: &hob_list_ptr\n", fsp_get_hob_list_ptr()); } -void fsp_debug_after_memory_init(uint32_t status) +void fsp_debug_after_memory_init(efi_return_status_t status) { if (CONFIG(DISPLAY_FSP_CALLS_AND_STATUS)) - printk(BIOS_SPEW, "FspMemoryInit returned 0x%08x\n", status); + printk(BIOS_SPEW, "FspMemoryInit returned 0x%zx\n", (size_t)status); if (status != FSP_SUCCESS) return; @@ -136,13 +136,13 @@ printk(BIOS_SPEW, "\t%p: upd\n", fsps_new_upd); } -void fsp_debug_after_silicon_init(uint32_t status) +void fsp_debug_after_silicon_init(efi_return_status_t status) { if (CONFIG(CHECK_GPIO_CONFIG_CHANGES)) fsp_gpio_config_check(AFTER_FSP_CALL, "FSP Silicon Init"); if (CONFIG(DISPLAY_FSP_CALLS_AND_STATUS)) - printk(BIOS_SPEW, "FspSiliconInit returned 0x%08x\n", status); + printk(BIOS_SPEW, "FspSiliconInit returned 0x%zx\n", (size_t)status); /* Display the HOBs */ if (CONFIG(DISPLAY_HOBS)) @@ -170,13 +170,13 @@ printk(BIOS_SPEW, "\t%p: notify_params\n", notify_params); } -void fsp_debug_after_notify(uint32_t status) +void fsp_debug_after_notify(efi_return_status_t status) { if (CONFIG(CHECK_GPIO_CONFIG_CHANGES)) fsp_gpio_config_check(AFTER_FSP_CALL, "FSP Notify"); if (CONFIG(DISPLAY_FSP_CALLS_AND_STATUS)) - printk(BIOS_SPEW, "FspNotify returned 0x%08x\n", status); + printk(BIOS_SPEW, "FspNotify returned 0x%zx\n", (size_t)status); /* Display the HOBs */ if (CONFIG(DISPLAY_HOBS)) diff --git a/src/drivers/intel/fsp2_0/fsp_debug_event.c b/src/drivers/intel/fsp2_0/fsp_debug_event.c index 2ebecc0..4b3e151a 100644 --- a/src/drivers/intel/fsp2_0/fsp_debug_event.c +++ b/src/drivers/intel/fsp2_0/fsp_debug_event.c @@ -17,7 +17,7 @@ return FSP_SUCCESS; } -efi_return_status_t fsp_debug_event_handler(efi_status_code_type_t ignored1, +__efiapi 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) { diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h index 971be0d..7d7494c 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/api.h +++ b/src/drivers/intel/fsp2_0/include/fsp/api.h @@ -8,6 +8,8 @@ #include <fsp/soc_binding.h> #include <soc/intel/common/mma.h> +#define __efiapi EFIAPI + #define FSP_SUCCESS EFI_SUCCESS #define FSP_INVALID_PARAMETER EFI_INVALID_PARAMETER #define FSP_DEVICE_ERROR EFI_DEVICE_ERROR diff --git a/src/drivers/intel/fsp2_0/include/fsp/debug.h b/src/drivers/intel/fsp2_0/include/fsp/debug.h index 044ce95..0e89fac 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/debug.h +++ b/src/drivers/intel/fsp2_0/include/fsp/debug.h @@ -19,14 +19,14 @@ void fsp_debug_before_memory_init(fsp_memory_init_fn memory_init, const FSPM_UPD *fspm_old_upd, const FSPM_UPD *fspm_new_upd); -void fsp_debug_after_memory_init(uint32_t status); +void fsp_debug_after_memory_init(efi_return_status_t status); void fsp_debug_before_silicon_init(fsp_silicon_init_fn silicon_init, const FSPS_UPD *fsps_old_upd, const FSPS_UPD *fsps_new_upd); -void fsp_debug_after_silicon_init(uint32_t status); +void fsp_debug_after_silicon_init(efi_return_status_t status); void fsp_before_debug_notify(fsp_notify_fn notify, const struct fsp_notify_params *notify_params); -void fsp_debug_after_notify(uint32_t status); +void fsp_debug_after_notify(efi_return_status_t status); void fspm_display_upd_values(const FSPM_UPD *old, const FSPM_UPD *new); void fsp_display_hobs(void); 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 index 7d50c1e..b6f7a51 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h +++ b/src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h @@ -8,11 +8,12 @@ * More details about this structure can be found here : * http://github.com/tianocore/edk2/blob/master/IntelFsp2Pkg/Include/FspEas/Fs… */ -#include <efi/efi_datatype.h> #include <fsp/soc_binding.h> +#include <fsp/api.h> +#include <efi/efi_datatype.h> /* fsp debug event handler */ -efi_return_status_t fsp_debug_event_handler(efi_status_code_type_t ignored1, +__efiapi 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); diff --git a/src/drivers/intel/fsp2_0/include/fsp/info_header.h b/src/drivers/intel/fsp2_0/include/fsp/info_header.h index 563a7e6..e8d853b 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/info_header.h +++ b/src/drivers/intel/fsp2_0/include/fsp/info_header.h @@ -12,7 +12,6 @@ #define FSP_HDR_ATTRIB_FSPS 3 #define FSP_IMAGE_ID_LENGTH 8 -#if CONFIG(PLATFORM_USES_FSP2_X86_32) struct fsp_header { uint32_t signature; //FSPH uint32_t header_length; @@ -42,9 +41,6 @@ uint32_t fsp_smm_init_entry_offset; #endif } __packed; -#else -#error You need to implement this struct for x86_64 FSP -#endif enum cb_err fsp_identify(struct fsp_header *hdr, const void *fsp_blob); diff --git a/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h b/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h index ba3e289..f6369b5 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h +++ b/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h @@ -5,6 +5,18 @@ #include <stddef.h> +/* + * EDK2 EFIAPI macro definition relies on compiler flags such as __GNUC__ which + * is not working well when included by coreboot. While it has no side-effect on + * i386 because the C calling convention used by coreboot and FSP are the same, + * it breaks on x86_64 because FSP/UEFI uses the Microsoft x64 calling + * convention while coreboot uses the System V AMD64 ABI. + * + * Fortunately, EDK2 header allows to override the EFIAPI. The __ms_abi__ + * attribute works for both i386 and x86_64. + */ +#define EFIAPI __attribute__((__ms_abi__)) + #pragma pack(push) /** diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h index becf91a..f64fdc3 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/util.h +++ b/src/drivers/intel/fsp2_0/include/fsp/util.h @@ -187,10 +187,10 @@ * SoC. If the requested status is not a reboot status or unhandled, this * function does nothing. */ -void fsp_handle_reset(uint32_t status); +void fsp_handle_reset(efi_return_status_t status); /* SoC/chipset must provide this to handle platform-specific reset codes */ -void chipset_handle_reset(uint32_t status); +void chipset_handle_reset(efi_return_status_t status); #if CONFIG(PLATFORM_USES_SECOND_FSP) /* The SoC must implement these to choose the appropriate FSP-M/FSP-S binary. */ @@ -207,12 +207,12 @@ } #endif -typedef asmlinkage uint32_t (*temp_ram_exit_fn)(void *param); -typedef asmlinkage uint32_t (*fsp_memory_init_fn) +typedef __efiapi efi_return_status_t (*temp_ram_exit_fn)(void *param); +typedef __efiapi efi_return_status_t (*fsp_memory_init_fn) (void *raminit_upd, void **hob_list); -typedef asmlinkage uint32_t (*fsp_silicon_init_fn)(void *silicon_upd); -typedef asmlinkage uint32_t (*fsp_multi_phase_init_fn)(struct fsp_multi_phase_params *); -typedef asmlinkage uint32_t (*fsp_notify_fn)(struct fsp_notify_params *); +typedef __efiapi efi_return_status_t (*fsp_silicon_init_fn)(void *silicon_upd); +typedef __efiapi efi_return_status_t (*fsp_multi_phase_init_fn)(struct fsp_multi_phase_params *); +typedef __efiapi efi_return_status_t (*fsp_notify_fn)(struct fsp_notify_params *); #include <fsp/debug.h> #endif /* _FSP2_0_UTIL_H_ */ diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index b48cc93..30b3f1d 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -34,7 +34,11 @@ /* Leave for the SoC/Mainboard to implement if necessary. */ } +#if CONFIG(PLATFORM_USES_FSP2_X86_32) static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(sizeof(uint64_t)); +#else +static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(16); +#endif /* * Helper function to store the MRC cache version into CBMEM @@ -275,15 +279,14 @@ return ver; } -static void error_handler(const char *context, uint32_t status, bool die_on_error) +static void error_handler(const char *context, efi_return_status_t status, bool die_on_error) { if (status == FSP_SUCCESS) return; fsp_handle_reset(status); if (die_on_error) - die_with_post_code(POSTCODE_RAM_FAILURE, - "%s returned with error 0x%zx!\n", + die_with_post_code(POSTCODE_RAM_FAILURE, "%s returned with error 0x%zx!\n", context, (size_t)status); printk(BIOS_SPEW, "%s returned 0x%zx\n", context, (size_t)status); @@ -292,7 +295,7 @@ #if CONFIG(PLATFORM_USES_FSP2_4) static void multi_phase_init(const struct fsp_header *hdr) { - uint32_t status; + efi_return_status_t status; fsp_multi_phase_init_fn fsp_multi_phase_init; struct fsp_multi_phase_params multi_phase_params; struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number; @@ -333,7 +336,7 @@ static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake) { - uint32_t status; + efi_return_status_t status; fsp_memory_init_fn fsp_raminit; FSPM_UPD fspm_upd, *upd; FSPM_ARCHx_UPD *arch_upd; diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service1.c b/src/drivers/intel/fsp2_0/ppi/mp_service1.c index 879a8e8..3c036c1 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service1.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service1.c @@ -6,14 +6,14 @@ typedef EFI_PEI_MP_SERVICES_PPI efi_pei_mp_services_ppi; -static efi_return_status_t mps1_get_number_of_processors(const +static __efiapi efi_return_status_t mps1_get_number_of_processors(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t *number_of_processors, efi_uintn_t *number_of_enabled_processors) { return mp_get_number_of_processors(number_of_processors, number_of_enabled_processors); } -static efi_return_status_t mps1_get_processor_info(const +static __efiapi efi_return_status_t mps1_get_processor_info(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t processor_number, efi_processor_information *processor_info_buffer) @@ -21,7 +21,7 @@ return mp_get_processor_info(processor_number, processor_info_buffer); } -static efi_return_status_t mps1_startup_all_aps(const +static __efiapi efi_return_status_t mps1_startup_all_aps(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_ap_procedure procedure, efi_boolean_t run_serial, efi_uintn_t timeout_usec, void *argument) @@ -29,7 +29,7 @@ return mp_startup_all_aps(procedure, run_serial, timeout_usec, argument); } -static efi_return_status_t mps1_startup_this_ap(const +static __efiapi efi_return_status_t mps1_startup_this_ap(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_ap_procedure procedure, efi_uintn_t processor_number, efi_uintn_t timeout_usec, void *argument) @@ -37,21 +37,21 @@ return mp_startup_this_ap(procedure, processor_number, timeout_usec, argument); } -static efi_return_status_t mps1_switch_bsp(const efi_pei_services **ignored1, +static __efiapi efi_return_status_t mps1_switch_bsp(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t ignored3, efi_boolean_t ignored4) { return mp_api_unsupported(); } -static efi_return_status_t mps1_enable_disable_ap(const +static __efiapi efi_return_status_t mps1_enable_disable_ap(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t ignored3, efi_boolean_t ignored4, efi_uint32_t *ignored5) { return mp_api_unsupported(); } -static efi_return_status_t mps1_identify_processor(const +static __efiapi efi_return_status_t mps1_identify_processor(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t *processor_number) { diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service2.c b/src/drivers/intel/fsp2_0/ppi/mp_service2.c index fd91b46..28bcadf 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service2.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service2.c @@ -6,7 +6,7 @@ typedef EDKII_PEI_MP_SERVICES2_PPI efi_pei_mp_services_ppi; -static efi_return_status_t mps2_get_number_of_processors( +static __efiapi efi_return_status_t mps2_get_number_of_processors( efi_pei_mp_services_ppi *ignored1, efi_uintn_t *number_of_processors, efi_uintn_t *number_of_enabled_processors) @@ -14,7 +14,7 @@ return mp_get_number_of_processors(number_of_processors, number_of_enabled_processors); } -static efi_return_status_t mps2_get_processor_info( +static __efiapi efi_return_status_t mps2_get_processor_info( efi_pei_mp_services_ppi *ignored1, efi_uintn_t processor_number, efi_processor_information *processor_info_buffer) @@ -22,7 +22,7 @@ return mp_get_processor_info(processor_number, processor_info_buffer); } -static efi_return_status_t mps2_startup_all_aps( +static __efiapi efi_return_status_t mps2_startup_all_aps( efi_pei_mp_services_ppi *ignored1, efi_ap_procedure procedure, efi_boolean_t run_serial, efi_uintn_t timeout_usec, void *argument) @@ -30,7 +30,7 @@ return mp_startup_all_aps(procedure, run_serial, timeout_usec, argument); } -static efi_return_status_t mps2_startup_all_cpus( +static __efiapi efi_return_status_t mps2_startup_all_cpus( efi_pei_mp_services_ppi *ignored1, efi_ap_procedure procedure, efi_uintn_t timeout_usec, void *argument) @@ -38,7 +38,7 @@ return mp_startup_all_cpus(procedure, timeout_usec, argument); } -static efi_return_status_t mps2_startup_this_ap( +static __efiapi efi_return_status_t mps2_startup_this_ap( efi_pei_mp_services_ppi *ignored1, efi_ap_procedure procedure, efi_uintn_t processor_number, efi_uintn_t timeout_usec, void *argument) @@ -46,21 +46,21 @@ return mp_startup_this_ap(procedure, processor_number, timeout_usec, argument); } -static efi_return_status_t mps2_switch_bsp( +static __efiapi efi_return_status_t mps2_switch_bsp( efi_pei_mp_services_ppi *ignored1, efi_uintn_t ignored2, efi_boolean_t ignored3) { return mp_api_unsupported(); } -static efi_return_status_t mps2_enable_disable_ap( +static __efiapi efi_return_status_t mps2_enable_disable_ap( efi_pei_mp_services_ppi *ignored1, efi_uintn_t ignored2, efi_boolean_t ignored3, efi_uint32_t *ignored4) { return mp_api_unsupported(); } -static efi_return_status_t mps2_identify_processor( +static __efiapi efi_return_status_t mps2_identify_processor( efi_pei_mp_services_ppi *ignored1, efi_uintn_t *processor_number) { diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 69b626b..4e48085 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -36,7 +36,8 @@ FSP_MULTI_PHASE_SI_INIT_EXECUTE_PHASE_API }; -static void fsps_return_value_handler(enum fsp_silicon_init_phases phases, uint32_t status) +static void fsps_return_value_handler(enum fsp_silicon_init_phases phases, + efi_return_status_t status) { uint8_t postcode; @@ -54,16 +55,17 @@ switch (phases) { case FSP_SILICON_INIT_API: - die_with_post_code(postcode, "FspSiliconInit returned with error 0x%08x\n", - status); + die_with_post_code(postcode, + "FspSiliconInit returned with error 0x%zx\n", + (size_t)status); break; case FSP_MULTI_PHASE_SI_INIT_GET_NUMBER_OF_PHASES_API: - printk(BIOS_SPEW, "FspMultiPhaseSiInit NumberOfPhases returned 0x%08x\n", - status); + printk(BIOS_SPEW, "FspMultiPhaseSiInit NumberOfPhases returned 0x%zx\n", + (size_t)status); break; case FSP_MULTI_PHASE_SI_INIT_EXECUTE_PHASE_API: - printk(BIOS_SPEW, "FspMultiPhaseSiInit ExecutePhase returned 0x%08x\n", - status); + printk(BIOS_SPEW, "FspMultiPhaseSiInit ExecutePhase returned 0x%zx\n", + (size_t)status); break; default: break; @@ -88,7 +90,7 @@ { FSPS_UPD *upd, *supd; fsp_silicon_init_fn silicon_init; - uint32_t status; + efi_return_status_t status; fsp_multi_phase_init_fn multi_phase_si_init; struct fsp_multi_phase_params multi_phase_params; struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number; @@ -139,7 +141,7 @@ status = silicon_init(upd); null_breakpoint_init(); - printk(BIOS_INFO, "FSPS returned %x\n", status); + printk(BIOS_INFO, "FSPS returned %zx\n", (size_t)status); timestamp_add_now(TS_FSP_SILICON_INIT_END); post_code(POSTCODE_FSP_SILICON_EXIT); diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index f912d9a..fab9958 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -83,18 +83,18 @@ return CB_SUCCESS; } -static bool fsp_reset_requested(uint32_t status) +static bool fsp_reset_requested(efi_return_status_t status) { return (status >= FSP_STATUS_RESET_REQUIRED_COLD && status <= FSP_STATUS_RESET_REQUIRED_8); } -void fsp_handle_reset(uint32_t status) +void fsp_handle_reset(efi_return_status_t status) { if (!fsp_reset_requested(status)) return; - printk(BIOS_SPEW, "FSP: handling reset type %x\n", status); + printk(BIOS_SPEW, "FSP: handling reset type %zx\n", (size_t)status); switch (status) { case FSP_STATUS_RESET_REQUIRED_COLD: diff --git a/src/include/efi/efi_datatype.h b/src/include/efi/efi_datatype.h index 917d0c8..0c33dcd 100644 --- a/src/include/efi/efi_datatype.h +++ b/src/include/efi/efi_datatype.h @@ -76,4 +76,16 @@ void (EFIAPI *efi_ap_procedure)(void *buffer); +/* + * EFI_STATUS/efi_return_status_t size changes with the architecture (32-bits vs + * 64-bits). To print statuses independently of the architecture we leverage the + * size_t 'z' print format which is generally aligned with the architecture size + * too. + * + * This assert is intended to detect unexpected mismatch which could lead to + * incorrect logs. + */ +_Static_assert(sizeof(size_t) == sizeof(efi_return_status_t), + "Unexpected EFI_STATUS size"); + #endif diff --git a/src/soc/intel/common/fsp_reset.c b/src/soc/intel/common/fsp_reset.c index 2bb39bd..0765710 100644 --- a/src/soc/intel/common/fsp_reset.c +++ b/src/soc/intel/common/fsp_reset.c @@ -30,14 +30,14 @@ struct pch_reset_data reset_data; }; -void chipset_handle_reset(uint32_t status) +void chipset_handle_reset(efi_return_status_t status) { if (status == CONFIG_FSP_STATUS_GLOBAL_RESET) { printk(BIOS_DEBUG, "GLOBAL RESET!\n"); global_reset(); } - printk(BIOS_ERR, "unhandled reset type %x\n", status); + printk(BIOS_ERR, "unhandled reset type %zx\n", (size_t)status); die("unknown reset type"); } -- To view, visit https://review.coreboot.org/c/coreboot/+/80277?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If0397f5cc8d0f4f1872bd37a001fe42e0c37ec99 Gerrit-Change-Number: 80277 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support
by Jérémy Compostella (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Andrey Petrov, Bora Guvendik, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Ronak Kanabar, Subrata Banik, Tarun, Wonkyu Kim. Hello Andrey Petrov, Bora Guvendik, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Ronak Kanabar, Tarun, Wonkyu Kim, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80275?usp=email to look at the new patch set (#6). Change subject: drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support ...................................................................... drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support Intel Firmware Support Package 2.4 specification brings some significant changes compared to version 2.3 (cf. documents 736809 and 644852 respectively): 1. It supports FSP-M multi-phase init. Some fields have been added to the FSP header data structure for this purpose. 2. The `FSPM_ARCH2_UPD' and `FSPS_ARCH2_UPD' data structures must be used in place of `FSPM_ARCH_UPD' and `FSPS_ARCH_UPD' respectively. 3. It support 64-bits FSP but 64-bits support will be provided by subsequent patch. Note that similarly to what is done for silicon initialization, timestamps and post-codes are used during the memory initialization multi-phase. However, since post-codes are in short supply, memory and silicon multi-phase init share the same post-codes. Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/commonlib/include/commonlib/console/post_codes.h M src/commonlib/include/commonlib/timestamp_serialized.h M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/drivers/intel/fsp2_0/upd_display.c M src/soc/intel/alderlake/fsp_params.c M src/soc/intel/meteorlake/fsp_params.c M src/soc/intel/tigerlake/fsp_params.c M util/cbfstool/eventlog.c 13 files changed, 128 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/80275/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/80275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Gerrit-Change-Number: 80275 Gerrit-PatchSet: 6 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Support FSP 2.4 64-bits
by Jérémy Compostella (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Andrey Petrov, Ronak Kanabar. Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80276?usp=email ) Change subject: drivers/intel/fsp2_0: Support FSP 2.4 64-bits ...................................................................... drivers/intel/fsp2_0: Support FSP 2.4 64-bits FSP 2.4 brings FSP 64-bits support which requires some adjustments in coreboot: - Stack alignment: 1. FSP functions must be called with the stack 16-bytes aligned. This is already setup properly with the default value of the `mpreferred-stack-boundary' compiler option (4). 2. The FSP stack buffer supplied by coreboot through the `StackBase' UPD must be 16-bytes aligned. - The EDK2 EFIAPI macro definition relies on compiler flags such as __GNUC__ which is not working well when included by coreboot. While it has no side-effect on i386 because the C calling convention used by coreboot and FSP are the same, it breaks on x86_64 because FSP/UEFI uses the Microsoft x64 calling convention while coreboot uses the System V AMD64 ABI. Fortunately, EDK2 header allows to override the EFIAPI definition. The __ms_abi__ attribute works for both i386 and x86_64. This attribute has to be set to all functions calling or called by the FSP. - The EFI_STATUS/efi_return_status_t size changes with the architecture (32-bits vs 64-bits). To print statuses independently of the architecture we leverage the size_t 'z' print format which is generally aligned with the architecture size too. In addition, this commit sets`PLATFORM_USES_FSP2_X86_32' to `n' by default if FSP 2.4 is enabled as 64-bits FSP should be norm moving forward. Change-Id: I3fa012fa6f98e4bb3c2a033189554ad5a6ddfced Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/debug.c M src/drivers/intel/fsp2_0/fsp_debug_event.c M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/soc_binding.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/ppi/mp_service1.c M src/drivers/intel/fsp2_0/ppi/mp_service2.c M src/drivers/intel/fsp2_0/silicon_init.c M src/drivers/intel/fsp2_0/util.c M src/include/efi/efi_datatype.h M src/soc/intel/common/fsp_reset.c 16 files changed, 90 insertions(+), 57 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/80276/1 diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 2c8bdc4..61d68f6 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -55,10 +55,11 @@ config PLATFORM_USES_FSP2_X86_32 bool + default n if PLATFORM_USES_FSP2_4 default y help - The FSP 2.0 runs in x86_32 protected mode. - Once there's a x86_64 FSP this needs to default to n. + Specify if the FSP binaries are 32-bits (yes) or 64-bits + (no). Starting with FSP specification 2.4 they can be 64-bits. config HAVE_INTEL_FSP_REPO bool diff --git a/src/drivers/intel/fsp2_0/debug.c b/src/drivers/intel/fsp2_0/debug.c index 8c0d67d..1152413 100644 --- a/src/drivers/intel/fsp2_0/debug.c +++ b/src/drivers/intel/fsp2_0/debug.c @@ -91,10 +91,10 @@ printk(BIOS_SPEW, "\t%p: &hob_list_ptr\n", fsp_get_hob_list_ptr()); } -void fsp_debug_after_memory_init(uint32_t status) +void fsp_debug_after_memory_init(efi_return_status_t status) { if (CONFIG(DISPLAY_FSP_CALLS_AND_STATUS)) - printk(BIOS_SPEW, "FspMemoryInit returned 0x%08x\n", status); + printk(BIOS_SPEW, "FspMemoryInit returned 0x%zx\n", (size_t)status); if (status != FSP_SUCCESS) return; @@ -136,13 +136,14 @@ printk(BIOS_SPEW, "\t%p: upd\n", fsps_new_upd); } -void fsp_debug_after_silicon_init(uint32_t status) +void fsp_debug_after_silicon_init(efi_return_status_t status) { if (CONFIG(CHECK_GPIO_CONFIG_CHANGES)) fsp_gpio_config_check(AFTER_FSP_CALL, "FSP Silicon Init"); if (CONFIG(DISPLAY_FSP_CALLS_AND_STATUS)) - printk(BIOS_SPEW, "FspSiliconInit returned 0x%08x\n", status); + printk(BIOS_SPEW, "FspSiliconInit returned 0x%zx\n", + (size_t)status); /* Display the HOBs */ if (CONFIG(DISPLAY_HOBS)) @@ -170,13 +171,14 @@ printk(BIOS_SPEW, "\t%p: notify_params\n", notify_params); } -void fsp_debug_after_notify(uint32_t status) +void fsp_debug_after_notify(efi_return_status_t status) { if (CONFIG(CHECK_GPIO_CONFIG_CHANGES)) fsp_gpio_config_check(AFTER_FSP_CALL, "FSP Notify"); if (CONFIG(DISPLAY_FSP_CALLS_AND_STATUS)) - printk(BIOS_SPEW, "FspNotify returned 0x%08x\n", status); + printk(BIOS_SPEW, "FspNotify returned 0x%zx\n", + (size_t)status); /* Display the HOBs */ if (CONFIG(DISPLAY_HOBS)) diff --git a/src/drivers/intel/fsp2_0/fsp_debug_event.c b/src/drivers/intel/fsp2_0/fsp_debug_event.c index 2ebecc0..4b3e151a 100644 --- a/src/drivers/intel/fsp2_0/fsp_debug_event.c +++ b/src/drivers/intel/fsp2_0/fsp_debug_event.c @@ -17,7 +17,7 @@ return FSP_SUCCESS; } -efi_return_status_t fsp_debug_event_handler(efi_status_code_type_t ignored1, +__efiapi 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) { diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h index 971be0d..7d7494c 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/api.h +++ b/src/drivers/intel/fsp2_0/include/fsp/api.h @@ -8,6 +8,8 @@ #include <fsp/soc_binding.h> #include <soc/intel/common/mma.h> +#define __efiapi EFIAPI + #define FSP_SUCCESS EFI_SUCCESS #define FSP_INVALID_PARAMETER EFI_INVALID_PARAMETER #define FSP_DEVICE_ERROR EFI_DEVICE_ERROR diff --git a/src/drivers/intel/fsp2_0/include/fsp/debug.h b/src/drivers/intel/fsp2_0/include/fsp/debug.h index 044ce95..0e89fac 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/debug.h +++ b/src/drivers/intel/fsp2_0/include/fsp/debug.h @@ -19,14 +19,14 @@ void fsp_debug_before_memory_init(fsp_memory_init_fn memory_init, const FSPM_UPD *fspm_old_upd, const FSPM_UPD *fspm_new_upd); -void fsp_debug_after_memory_init(uint32_t status); +void fsp_debug_after_memory_init(efi_return_status_t status); void fsp_debug_before_silicon_init(fsp_silicon_init_fn silicon_init, const FSPS_UPD *fsps_old_upd, const FSPS_UPD *fsps_new_upd); -void fsp_debug_after_silicon_init(uint32_t status); +void fsp_debug_after_silicon_init(efi_return_status_t status); void fsp_before_debug_notify(fsp_notify_fn notify, const struct fsp_notify_params *notify_params); -void fsp_debug_after_notify(uint32_t status); +void fsp_debug_after_notify(efi_return_status_t status); void fspm_display_upd_values(const FSPM_UPD *old, const FSPM_UPD *new); void fsp_display_hobs(void); 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 index 7d50c1e..b6f7a51 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h +++ b/src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h @@ -8,11 +8,12 @@ * More details about this structure can be found here : * http://github.com/tianocore/edk2/blob/master/IntelFsp2Pkg/Include/FspEas/Fs… */ -#include <efi/efi_datatype.h> #include <fsp/soc_binding.h> +#include <fsp/api.h> +#include <efi/efi_datatype.h> /* fsp debug event handler */ -efi_return_status_t fsp_debug_event_handler(efi_status_code_type_t ignored1, +__efiapi 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); diff --git a/src/drivers/intel/fsp2_0/include/fsp/info_header.h b/src/drivers/intel/fsp2_0/include/fsp/info_header.h index 563a7e6..e8d853b 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/info_header.h +++ b/src/drivers/intel/fsp2_0/include/fsp/info_header.h @@ -12,7 +12,6 @@ #define FSP_HDR_ATTRIB_FSPS 3 #define FSP_IMAGE_ID_LENGTH 8 -#if CONFIG(PLATFORM_USES_FSP2_X86_32) struct fsp_header { uint32_t signature; //FSPH uint32_t header_length; @@ -42,9 +41,6 @@ uint32_t fsp_smm_init_entry_offset; #endif } __packed; -#else -#error You need to implement this struct for x86_64 FSP -#endif enum cb_err fsp_identify(struct fsp_header *hdr, const void *fsp_blob); diff --git a/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h b/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h index ba3e289..f6369b5 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h +++ b/src/drivers/intel/fsp2_0/include/fsp/soc_binding.h @@ -5,6 +5,18 @@ #include <stddef.h> +/* + * EDK2 EFIAPI macro definition relies on compiler flags such as __GNUC__ which + * is not working well when included by coreboot. While it has no side-effect on + * i386 because the C calling convention used by coreboot and FSP are the same, + * it breaks on x86_64 because FSP/UEFI uses the Microsoft x64 calling + * convention while coreboot uses the System V AMD64 ABI. + * + * Fortunately, EDK2 header allows to override the EFIAPI. The __ms_abi__ + * attribute works for both i386 and x86_64. + */ +#define EFIAPI __attribute__((__ms_abi__)) + #pragma pack(push) /** diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h index becf91a..f64fdc3 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/util.h +++ b/src/drivers/intel/fsp2_0/include/fsp/util.h @@ -187,10 +187,10 @@ * SoC. If the requested status is not a reboot status or unhandled, this * function does nothing. */ -void fsp_handle_reset(uint32_t status); +void fsp_handle_reset(efi_return_status_t status); /* SoC/chipset must provide this to handle platform-specific reset codes */ -void chipset_handle_reset(uint32_t status); +void chipset_handle_reset(efi_return_status_t status); #if CONFIG(PLATFORM_USES_SECOND_FSP) /* The SoC must implement these to choose the appropriate FSP-M/FSP-S binary. */ @@ -207,12 +207,12 @@ } #endif -typedef asmlinkage uint32_t (*temp_ram_exit_fn)(void *param); -typedef asmlinkage uint32_t (*fsp_memory_init_fn) +typedef __efiapi efi_return_status_t (*temp_ram_exit_fn)(void *param); +typedef __efiapi efi_return_status_t (*fsp_memory_init_fn) (void *raminit_upd, void **hob_list); -typedef asmlinkage uint32_t (*fsp_silicon_init_fn)(void *silicon_upd); -typedef asmlinkage uint32_t (*fsp_multi_phase_init_fn)(struct fsp_multi_phase_params *); -typedef asmlinkage uint32_t (*fsp_notify_fn)(struct fsp_notify_params *); +typedef __efiapi efi_return_status_t (*fsp_silicon_init_fn)(void *silicon_upd); +typedef __efiapi efi_return_status_t (*fsp_multi_phase_init_fn)(struct fsp_multi_phase_params *); +typedef __efiapi efi_return_status_t (*fsp_notify_fn)(struct fsp_notify_params *); #include <fsp/debug.h> #endif /* _FSP2_0_UTIL_H_ */ diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index b48cc93..fb1a5eb 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -34,7 +34,11 @@ /* Leave for the SoC/Mainboard to implement if necessary. */ } +#if CONFIG(PLATFORM_USES_FSP2_X86_32) static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(sizeof(uint64_t)); +#else +static uint8_t temp_ram[CONFIG_FSP_TEMP_RAM_SIZE] __aligned(16); +#endif /* * Helper function to store the MRC cache version into CBMEM @@ -275,7 +279,8 @@ return ver; } -static void error_handler(const char *context, uint32_t status, bool die_on_error) +static void error_handler(const char *context, efi_return_status_t status, + bool die_on_error) { if (status == FSP_SUCCESS) return; @@ -292,7 +297,7 @@ #if CONFIG(PLATFORM_USES_FSP2_4) static void multi_phase_init(const struct fsp_header *hdr) { - uint32_t status; + efi_return_status_t status; fsp_multi_phase_init_fn fsp_multi_phase_init; struct fsp_multi_phase_params multi_phase_params; struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number; @@ -333,7 +338,7 @@ static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake) { - uint32_t status; + efi_return_status_t status; fsp_memory_init_fn fsp_raminit; FSPM_UPD fspm_upd, *upd; FSPM_ARCHx_UPD *arch_upd; diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service1.c b/src/drivers/intel/fsp2_0/ppi/mp_service1.c index 879a8e8..3c036c1 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service1.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service1.c @@ -6,14 +6,14 @@ typedef EFI_PEI_MP_SERVICES_PPI efi_pei_mp_services_ppi; -static efi_return_status_t mps1_get_number_of_processors(const +static __efiapi efi_return_status_t mps1_get_number_of_processors(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t *number_of_processors, efi_uintn_t *number_of_enabled_processors) { return mp_get_number_of_processors(number_of_processors, number_of_enabled_processors); } -static efi_return_status_t mps1_get_processor_info(const +static __efiapi efi_return_status_t mps1_get_processor_info(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t processor_number, efi_processor_information *processor_info_buffer) @@ -21,7 +21,7 @@ return mp_get_processor_info(processor_number, processor_info_buffer); } -static efi_return_status_t mps1_startup_all_aps(const +static __efiapi efi_return_status_t mps1_startup_all_aps(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_ap_procedure procedure, efi_boolean_t run_serial, efi_uintn_t timeout_usec, void *argument) @@ -29,7 +29,7 @@ return mp_startup_all_aps(procedure, run_serial, timeout_usec, argument); } -static efi_return_status_t mps1_startup_this_ap(const +static __efiapi efi_return_status_t mps1_startup_this_ap(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_ap_procedure procedure, efi_uintn_t processor_number, efi_uintn_t timeout_usec, void *argument) @@ -37,21 +37,21 @@ return mp_startup_this_ap(procedure, processor_number, timeout_usec, argument); } -static efi_return_status_t mps1_switch_bsp(const efi_pei_services **ignored1, +static __efiapi efi_return_status_t mps1_switch_bsp(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t ignored3, efi_boolean_t ignored4) { return mp_api_unsupported(); } -static efi_return_status_t mps1_enable_disable_ap(const +static __efiapi efi_return_status_t mps1_enable_disable_ap(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t ignored3, efi_boolean_t ignored4, efi_uint32_t *ignored5) { return mp_api_unsupported(); } -static efi_return_status_t mps1_identify_processor(const +static __efiapi efi_return_status_t mps1_identify_processor(const efi_pei_services **ignored1, efi_pei_mp_services_ppi *ignored2, efi_uintn_t *processor_number) { diff --git a/src/drivers/intel/fsp2_0/ppi/mp_service2.c b/src/drivers/intel/fsp2_0/ppi/mp_service2.c index fd91b46..28bcadf 100644 --- a/src/drivers/intel/fsp2_0/ppi/mp_service2.c +++ b/src/drivers/intel/fsp2_0/ppi/mp_service2.c @@ -6,7 +6,7 @@ typedef EDKII_PEI_MP_SERVICES2_PPI efi_pei_mp_services_ppi; -static efi_return_status_t mps2_get_number_of_processors( +static __efiapi efi_return_status_t mps2_get_number_of_processors( efi_pei_mp_services_ppi *ignored1, efi_uintn_t *number_of_processors, efi_uintn_t *number_of_enabled_processors) @@ -14,7 +14,7 @@ return mp_get_number_of_processors(number_of_processors, number_of_enabled_processors); } -static efi_return_status_t mps2_get_processor_info( +static __efiapi efi_return_status_t mps2_get_processor_info( efi_pei_mp_services_ppi *ignored1, efi_uintn_t processor_number, efi_processor_information *processor_info_buffer) @@ -22,7 +22,7 @@ return mp_get_processor_info(processor_number, processor_info_buffer); } -static efi_return_status_t mps2_startup_all_aps( +static __efiapi efi_return_status_t mps2_startup_all_aps( efi_pei_mp_services_ppi *ignored1, efi_ap_procedure procedure, efi_boolean_t run_serial, efi_uintn_t timeout_usec, void *argument) @@ -30,7 +30,7 @@ return mp_startup_all_aps(procedure, run_serial, timeout_usec, argument); } -static efi_return_status_t mps2_startup_all_cpus( +static __efiapi efi_return_status_t mps2_startup_all_cpus( efi_pei_mp_services_ppi *ignored1, efi_ap_procedure procedure, efi_uintn_t timeout_usec, void *argument) @@ -38,7 +38,7 @@ return mp_startup_all_cpus(procedure, timeout_usec, argument); } -static efi_return_status_t mps2_startup_this_ap( +static __efiapi efi_return_status_t mps2_startup_this_ap( efi_pei_mp_services_ppi *ignored1, efi_ap_procedure procedure, efi_uintn_t processor_number, efi_uintn_t timeout_usec, void *argument) @@ -46,21 +46,21 @@ return mp_startup_this_ap(procedure, processor_number, timeout_usec, argument); } -static efi_return_status_t mps2_switch_bsp( +static __efiapi efi_return_status_t mps2_switch_bsp( efi_pei_mp_services_ppi *ignored1, efi_uintn_t ignored2, efi_boolean_t ignored3) { return mp_api_unsupported(); } -static efi_return_status_t mps2_enable_disable_ap( +static __efiapi efi_return_status_t mps2_enable_disable_ap( efi_pei_mp_services_ppi *ignored1, efi_uintn_t ignored2, efi_boolean_t ignored3, efi_uint32_t *ignored4) { return mp_api_unsupported(); } -static efi_return_status_t mps2_identify_processor( +static __efiapi efi_return_status_t mps2_identify_processor( efi_pei_mp_services_ppi *ignored1, efi_uintn_t *processor_number) { diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index c1953b4..8a51d88 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -36,7 +36,8 @@ FSP_MULTI_PHASE_SI_INIT_EXECUTE_PHASE_API }; -static void fsps_return_value_handler(enum fsp_silicon_init_phases phases, uint32_t status) +static void fsps_return_value_handler(enum fsp_silicon_init_phases phases, + efi_return_status_t status) { uint8_t postcode; @@ -54,16 +55,17 @@ switch (phases) { case FSP_SILICON_INIT_API: - die_with_post_code(postcode, "FspSiliconInit returned with error 0x%08x\n", - status); + die_with_post_code(postcode, + "FspSiliconInit returned with error 0x%zx\n", + (size_t)status); break; case FSP_MULTI_PHASE_SI_INIT_GET_NUMBER_OF_PHASES_API: - printk(BIOS_SPEW, "FspMultiPhaseSiInit NumberOfPhases returned 0x%08x\n", - status); + printk(BIOS_SPEW, "FspMultiPhaseSiInit NumberOfPhases returned 0x%zx\n", + (size_t)status); break; case FSP_MULTI_PHASE_SI_INIT_EXECUTE_PHASE_API: - printk(BIOS_SPEW, "FspMultiPhaseSiInit ExecutePhase returned 0x%08x\n", - status); + printk(BIOS_SPEW, "FspMultiPhaseSiInit ExecutePhase returned 0x%zx\n", + (size_t)status); break; default: break; @@ -88,7 +90,7 @@ { FSPS_UPD *upd, *supd; fsp_silicon_init_fn silicon_init; - uint32_t status; + efi_return_status_t status; fsp_multi_phase_init_fn multi_phase_si_init; struct fsp_multi_phase_params multi_phase_params; struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number; @@ -139,7 +141,7 @@ status = silicon_init(upd); null_breakpoint_init(); - printk(BIOS_INFO, "FSPS returned %x\n", status); + printk(BIOS_INFO, "FSPS returned %zx\n", (size_t)status); timestamp_add_now(TS_FSP_SILICON_INIT_END); post_code(POSTCODE_FSP_SILICON_EXIT); diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index f912d9a..fab9958 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -83,18 +83,18 @@ return CB_SUCCESS; } -static bool fsp_reset_requested(uint32_t status) +static bool fsp_reset_requested(efi_return_status_t status) { return (status >= FSP_STATUS_RESET_REQUIRED_COLD && status <= FSP_STATUS_RESET_REQUIRED_8); } -void fsp_handle_reset(uint32_t status) +void fsp_handle_reset(efi_return_status_t status) { if (!fsp_reset_requested(status)) return; - printk(BIOS_SPEW, "FSP: handling reset type %x\n", status); + printk(BIOS_SPEW, "FSP: handling reset type %zx\n", (size_t)status); switch (status) { case FSP_STATUS_RESET_REQUIRED_COLD: diff --git a/src/include/efi/efi_datatype.h b/src/include/efi/efi_datatype.h index 917d0c8..0c33dcd 100644 --- a/src/include/efi/efi_datatype.h +++ b/src/include/efi/efi_datatype.h @@ -76,4 +76,16 @@ void (EFIAPI *efi_ap_procedure)(void *buffer); +/* + * EFI_STATUS/efi_return_status_t size changes with the architecture (32-bits vs + * 64-bits). To print statuses independently of the architecture we leverage the + * size_t 'z' print format which is generally aligned with the architecture size + * too. + * + * This assert is intended to detect unexpected mismatch which could lead to + * incorrect logs. + */ +_Static_assert(sizeof(size_t) == sizeof(efi_return_status_t), + "Unexpected EFI_STATUS size"); + #endif diff --git a/src/soc/intel/common/fsp_reset.c b/src/soc/intel/common/fsp_reset.c index 2bb39bd..0765710 100644 --- a/src/soc/intel/common/fsp_reset.c +++ b/src/soc/intel/common/fsp_reset.c @@ -30,14 +30,14 @@ struct pch_reset_data reset_data; }; -void chipset_handle_reset(uint32_t status) +void chipset_handle_reset(efi_return_status_t status) { if (status == CONFIG_FSP_STATUS_GLOBAL_RESET) { printk(BIOS_DEBUG, "GLOBAL RESET!\n"); global_reset(); } - printk(BIOS_ERR, "unhandled reset type %x\n", status); + printk(BIOS_ERR, "unhandled reset type %zx\n", (size_t)status); die("unknown reset type"); } -- To view, visit https://review.coreboot.org/c/coreboot/+/80276?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3fa012fa6f98e4bb3c2a033189554ad5a6ddfced Gerrit-Change-Number: 80276 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support
by Jérémy Compostella (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Andrey Petrov, Bora Guvendik, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Ronak Kanabar, Subrata Banik, Tarun, Wonkyu Kim. Hello Andrey Petrov, Bora Guvendik, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Ronak Kanabar, Tarun, Wonkyu Kim, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80275?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support ...................................................................... drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support Intel Firmware Support Package 2.4 specification brings some significant changes compared to version 2.3 (cf. documents 736809 and 644852 respectively): 1. It supports FSP-M multi-phase init. Some fields have been added to the FSP header data structure for this purpose. 2. The `FSPM_ARCH2_UPD' and `FSPS_ARCH2_UPD' data structures must be used in place of `FSPM_ARCH_UPD' and `FSPS_ARCH_UPD' respectively. 3. It support 64-bits FSP but 64-bits support will be provided by subsequent patch. Note that similarly to what is done for silicon initialization, timestamps and post-codes are used during the memory initialization multi-phase. However, since post-codes are in short supply, memory and silicon multi-phase init share the same post-codes. Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/commonlib/include/commonlib/console/post_codes.h M src/commonlib/include/commonlib/timestamp_serialized.h M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/include/fsp/api.h M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/drivers/intel/fsp2_0/upd_display.c M src/soc/intel/alderlake/fsp_params.c M src/soc/intel/meteorlake/fsp_params.c M src/soc/intel/tigerlake/fsp_params.c M util/cbfstool/eventlog.c 13 files changed, 128 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/80275/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/80275?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97 Gerrit-Change-Number: 80275 Gerrit-PatchSet: 5 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: [Squash]: Add ACPI BDAT support
by Gaggery (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Arthur Heymans, Cliff Huang, Gaggery, Karthik Ramasubramanian, Lance Zhao, Pratikkumar V Prajapati, Ravishankar Sarawadi, Subrata Banik, Tim Wawrzynczak. Hello Arthur Heymans, Cliff Huang, Karthik Ramasubramanian, Lance Zhao, Pratikkumar V Prajapati, Ravishankar Sarawadi, Subrata Banik, Tim Wawrzynczak, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77255?usp=email to look at the new patch set (#14). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: [Squash]: Add ACPI BDAT support ...................................................................... [Squash]: Add ACPI BDAT support src/acpi: Add BDAT support in ACPI This patch adds BDAT(BIOS Data) support for ACPI table generation. src/soc/intel/common: Add ACPI BDAT support This patch adds ACPI BDAT(BIOS Data) support. It parses the data from HOB and fills with a BDAT header. BUG=b:293441360 TEST=1. Add -DBDAT_SUPPORT=1 to build FSP packages. 2. Enable SOC_INTEL_RMT_PLUS flag with Brya. 3. Flash the image on Brya and ensure BDAT ACPI table is available under /sys/firmware/acpi/tables/. Change-Id: I5eb57f65ef5f24458f09587b7c7694156f2ed1ce Signed-off-by: Gaggery Tsai <gaggery.tsai(a)intel.com> --- M src/acpi/Kconfig M src/acpi/acpi.c M src/include/acpi/acpi.h M src/soc/intel/common/Kconfig.common M src/soc/intel/common/block/acpi/Makefile.mk A src/soc/intel/common/block/acpi/acpi_bdat.c A src/soc/intel/common/block/include/intelblocks/acpi_bdat.h 7 files changed, 307 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/77255/14 -- To view, visit https://review.coreboot.org/c/coreboot/+/77255?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5eb57f65ef5f24458f09587b7c7694156f2ed1ce Gerrit-Change-Number: 77255 Gerrit-PatchSet: 14 Gerrit-Owner: Gaggery <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Gaggery <gaggery.tsai(a)intel.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: commonlib: cbmem_id: Add id for ACPI BDAT
by Gaggery (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Gaggery, Subrata Banik. Hello Martin L Roth, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77254?usp=email to look at the new patch set (#11). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: commonlib: cbmem_id: Add id for ACPI BDAT ...................................................................... commonlib: cbmem_id: Add id for ACPI BDAT This patch adds ACPI BDAT (BIOS Data) in CBMEM support list. ACPI BDAT implementation will request memory allocation through CBMEM. BUG=b:293441360 TEST=1. Add -DBDAT_SUPPORT=1 to build FSP packages. 2. Enable SOC_INTEL_RMT_PLUS flag with Brya. 3. Flash the image on Brya and ensure BDAT ACPI table is available under /sys/firmware/acpi/tables/. 4. copy cbmem to the DUT and run 'cbmem --list' to ensure the existence of ACPI BDAT. Change-Id: Ic4925adfdaff36e28d47829322a8405ed92cad17 Signed-off-by: Gaggery Tsai <gaggery.tsai(a)intel.com> --- M src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/77254/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/77254?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic4925adfdaff36e28d47829322a8405ed92cad17 Gerrit-Change-Number: 77254 Gerrit-PatchSet: 11 Gerrit-Owner: Gaggery <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Gaggery <gaggery.tsai(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: smmstorev2: Load the communication buffer at SMM setup
by Arthur Heymans (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Jérémy Compostella, Patrick Rudolph. Hello Jérémy Compostella, Patrick Rudolph, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79738?usp=email to look at the new patch set (#5). Change subject: smmstorev2: Load the communication buffer at SMM setup ...................................................................... smmstorev2: Load the communication buffer at SMM setup This removes the runtime SMI call to set up the communication buffer for SMMSTORE in favor of setting this buffer up during the installation of the smihandler. The reason is that it's less code in the handler and a time costly SMI is also avoided in ramstage. Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: I94dce77711f37f87033530f5ae48cb850a39341b --- M Documentation/drivers/smmstorev2.md M src/cpu/x86/smm/smm_module_handler.c M src/cpu/x86/smm/smm_module_loader.c M src/drivers/smmstore/ramstage.c M src/drivers/smmstore/smi.c M src/drivers/smmstore/store.c M src/include/cpu/x86/smm.h M src/include/smmstore.h 8 files changed, 50 insertions(+), 77 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/79738/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/79738?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I94dce77711f37f87033530f5ae48cb850a39341b Gerrit-Change-Number: 79738 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: smmstorev2: Load the communication buffer at SMM setup
by Arthur Heymans (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Jérémy Compostella, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79738?usp=email ) Change subject: smmstorev2: Load the communication buffer at SMM setup ...................................................................... Patch Set 2: (3 comments) File src/cpu/x86/smm/smm_module_loader.c: https://review.coreboot.org/c/coreboot/+/79738/comment/ad1d9f77_91a32f35 : PS2, Line 13: #include <stdint.h> > not needed. Done File src/drivers/smmstore/smi.c: https://review.coreboot.org/c/coreboot/+/79738/comment/c04e60ed_c5c1896e : PS1, Line 88: if (base == 0 || size == 0) : return SMMSTORE_RET_FAILURE; > This check is not needed as smmstore_init does something similar. Done File src/include/cpu/x86/smm.h: https://review.coreboot.org/c/coreboot/+/79738/comment/9cf92675_3b0568b7 : PS2, Line 94: SMMSTORE_V2 > can we get rid of preprocessor guards by using a 0-sized variable when the condition isn't true? I don't think so. Only the last element can be such an array. -- To view, visit https://review.coreboot.org/c/coreboot/+/79738?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I94dce77711f37f87033530f5ae48cb850a39341b Gerrit-Change-Number: 79738 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Wed, 31 Jan 2024 21:37:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

← Newer
1
2
3
4
5
6
7
8
9
...
305
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024