Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
drivers/intel/fsp2_0: Move Debug options to "Debugging"
TODO: Is verify HOBS really 'Debugging' and should this really be optional?
Change-Id: I8e07c8186baf3d8e91b77c5afb731d26a1abfbaf Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/Kconfig M src/drivers/intel/fsp2_0/Kconfig 2 files changed, 40 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36165/1
diff --git a/src/Kconfig b/src/Kconfig index 4c71f28..80efb12 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -1078,6 +1078,46 @@ mainboard code supports this. On supported Intel platforms this works by changing the settings in the descriptor.bin file.
+config DISPLAY_FSP_CALLS_AND_STATUS + bool "Display the FSP calls and status" + depends on PLATFORM_USES_FSP2_0 + default n + help + Display the FSP call entry point and parameters prior to calling FSP + and display the status upon return from FSP. + +config DISPLAY_FSP_HEADER + bool "Display the FSP header" + depends on PLATFORM_USES_FSP2_0 + default n + help + Display the FSP header information when the FSP file is found. + +config DISPLAY_HOBS + bool "Display the hand-off-blocks" + depends on PLATFORM_USES_FSP2_0 + default n + help + Display the FSP HOBs which are provided for coreboot. + +config DISPLAY_UPD_DATA + bool "Display UPD data" + depends on PLATFORM_USES_FSP2_0 + default n + help + Display the user specified product data prior to memory + initialization. + +config VERIFY_HOBS + bool "Verify the FSP hand-off-blocks" + depends on PLATFORM_USES_FSP2_0 + default n + help + Verify that the HOBs required by coreboot are returned by FSP and + that the resource HOBs are in the correct order and position. + + + endmenu
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 1e84dab..fee5de4 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -39,32 +39,6 @@ Add the FSP-M and FSP-S binaries to CBFS. Currently coreboot does not use the FSP-T binary and it is not added.
-config DISPLAY_FSP_CALLS_AND_STATUS - bool "Display the FSP calls and status" - default n - help - Display the FSP call entry point and parameters prior to calling FSP - and display the status upon return from FSP. - -config DISPLAY_FSP_HEADER - bool "Display the FSP header" - default n - help - Display the FSP header information when the FSP file is found. - -config DISPLAY_HOBS - bool "Display the hand-off-blocks" - default n - help - Display the FSP HOBs which are provided for coreboot. - -config DISPLAY_UPD_DATA - bool "Display UPD data" - default n - help - Display the user specified product data prior to memory - initialization. - config CPU_MICROCODE_CBFS_LEN hex "Microcode update region length in bytes" depends on FSP_CAR @@ -161,13 +135,6 @@ stack with coreboot/bootloader. Sync this value with Platform FSP integration guide recommendation.
-config VERIFY_HOBS - bool "Verify the FSP hand-off-blocks" - default n - help - Verify that the HOBs required by coreboot are returned by FSP and - that the resource HOBs are in the correct order and position. - config RESET_ON_INVALID_RAMSTAGE_CACHE bool "Reset the system on S3 wake when ramstage cache invalid." default n
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 1:
(1 comment)
We started with a Kconfig.debug filename scheme so one can place things in the Debug menu but keep the defini- tions local.
https://review.coreboot.org/c/coreboot/+/36165/1/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/1/src/Kconfig@727 PS1, Line 727: source "src/cpu/*/Kconfig.debug" please make use of this scheme
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36165
to look at the new patch set (#2).
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
drivers/intel/fsp2_0: Move Debug options to "Debugging"
TODO: Is verify HOBS really 'Debugging' and should this really be optional?
Change-Id: I8e07c8186baf3d8e91b77c5afb731d26a1abfbaf Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/Kconfig M src/drivers/intel/fsp2_0/Kconfig A src/drivers/intel/fsp2_0/Kconfig.debug 3 files changed, 42 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36165/2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig@727 PS3, Line 727: source "src/cpu/*/Kconfig.debug" Maybe change these to Kconfig.cpu_debug or something so we can add them from anywhere?
source "src/*/Kconfig.cpu_debug" source "src/*/*/Kconfig.cpu_debug" source "src/*/*/*/Kconfig.cpu_debug" source "src/*/*/*/*/Kconfig.cpu_debug"
Then we could add corresponding kconfig.debug files for the general section below.
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig@728 PS3, Line 728: drivers we could be consistent about starting with "src/..." either have it everywhere or remove it everywhere.
https://review.coreboot.org/c/coreboot/+/36165/3/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig.debug:
https://review.coreboot.org/c/coreboot/+/36165/3/src/drivers/intel/fsp2_0/Kc... PS3, Line 2: Put this inside a menu?
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig@728 PS3, Line 728: drivers
we could be consistent about starting with "src/... […]
Ok, the 'src/' is actually needed here. I thought it was, but when I searched the code, there were some that didn't have it. Those are in libpayload or including things at the same level as src - util, payload...
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig@728 PS3, Line 728: source "drivers/intel/*/Kconfig.debug" Put another comment above this? I don't think it counts as CPU Debug Settings.
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36165
to look at the new patch set (#4).
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
drivers/intel/fsp2_0: Move Debug options to "Debugging"
Change-Id: I8e07c8186baf3d8e91b77c5afb731d26a1abfbaf Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/Kconfig M src/drivers/intel/fsp2_0/Kconfig A src/drivers/intel/fsp2_0/Kconfig.debug_blob 3 files changed, 47 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36165/4
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36165
to look at the new patch set (#5).
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
drivers/intel/fsp2_0: Move Debug options to "Debugging"
Change-Id: I8e07c8186baf3d8e91b77c5afb731d26a1abfbaf Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/Kconfig M src/drivers/intel/fsp2_0/Kconfig A src/drivers/intel/fsp2_0/Kconfig.debug_blob 3 files changed, 44 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/36165/5
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36165/5/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/5/src/Kconfig@744 PS5, Line 744: fsp*/ looks like the linter script only supports /*/. for some reason it's avoiding glob().
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36165/1/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/1/src/Kconfig@727 PS1, Line 727: source "src/cpu/*/Kconfig.debug"
please make use of this scheme
Done
https://review.coreboot.org/c/coreboot/+/36165/5/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/5/src/Kconfig@744 PS5, Line 744: fsp*/
looks like the linter script only supports /*/. for some reason it's avoiding glob().
Attempt to fix CB:36626
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 6: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/36165/5/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/5/src/Kconfig@744 PS5, Line 744: fsp*/
looks like the linter script only supports /*/. for some reason it's avoiding glob(). […]
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig@727 PS3, Line 727: source "src/cpu/*/Kconfig.debug"
Maybe change these to Kconfig.cpu_debug or something so we can add them from anywhere? […]
Done
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig@728 PS3, Line 728: drivers
Ok, the 'src/' is actually needed here. […]
Done
https://review.coreboot.org/c/coreboot/+/36165/3/src/Kconfig@728 PS3, Line 728: source "drivers/intel/*/Kconfig.debug"
Put another comment above this? I don't think it counts as CPU Debug Settings.
Done
https://review.coreboot.org/c/coreboot/+/36165/3/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig.debug:
https://review.coreboot.org/c/coreboot/+/36165/3/src/drivers/intel/fsp2_0/Kc... PS3, Line 2:
Put this inside a menu?
Done, kind of. It has a heading now.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36165 )
Change subject: drivers/intel/fsp2_0: Move Debug options to "Debugging" ......................................................................
drivers/intel/fsp2_0: Move Debug options to "Debugging"
Change-Id: I8e07c8186baf3d8e91b77c5afb731d26a1abfbaf Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/36165 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/Kconfig M src/drivers/intel/fsp2_0/Kconfig A src/drivers/intel/fsp2_0/Kconfig.debug_blob 3 files changed, 44 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/Kconfig b/src/Kconfig index 2b5f05e..4002d43 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -740,6 +740,9 @@ comment "CPU Debug Settings" source "src/cpu/*/Kconfig.debug_cpu"
+comment "BLOB Debug Settings" +source "src/drivers/intel/fsp*/Kconfig.debug_blob" + comment "General Debug Settings"
# TODO: Better help text and detailed instructions. diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index b434a51..3403df9 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -37,32 +37,6 @@ Add the FSP-M and FSP-S binaries to CBFS. Currently coreboot does not use the FSP-T binary and it is not added.
-config DISPLAY_FSP_CALLS_AND_STATUS - bool "Display the FSP calls and status" - default n - help - Display the FSP call entry point and parameters prior to calling FSP - and display the status upon return from FSP. - -config DISPLAY_FSP_HEADER - bool "Display the FSP header" - default n - help - Display the FSP header information when the FSP file is found. - -config DISPLAY_HOBS - bool "Display the hand-off-blocks" - default n - help - Display the FSP HOBs which are provided for coreboot. - -config DISPLAY_UPD_DATA - bool "Display UPD data" - default n - help - Display the user specified product data prior to memory - initialization. - config CPU_MICROCODE_CBFS_LEN hex "Microcode update region length in bytes" depends on FSP_CAR @@ -158,22 +132,10 @@ stack with coreboot/bootloader. Sync this value with Platform FSP integration guide recommendation.
-config VERIFY_HOBS - bool "Verify the FSP hand-off-blocks" - default n - help - Verify that the HOBs required by coreboot are returned by FSP and - that the resource HOBs are in the correct order and position. - config RESET_ON_INVALID_RAMSTAGE_CACHE bool "Reset the system on S3 wake when ramstage cache invalid." default n
-config DISPLAY_FSP_VERSION_INFO - bool "Display Firmware Ingredient Version Information" - help - Select this option to display Firmware version information. - config FSP2_0_USES_TPM_MRC_HASH bool depends on TPM1 || TPM2 diff --git a/src/drivers/intel/fsp2_0/Kconfig.debug_blob b/src/drivers/intel/fsp2_0/Kconfig.debug_blob new file mode 100644 index 0000000..2b10c2d --- /dev/null +++ b/src/drivers/intel/fsp2_0/Kconfig.debug_blob @@ -0,0 +1,41 @@ +if PLATFORM_USES_FSP2_0 + +config DISPLAY_FSP_CALLS_AND_STATUS + bool "Display the FSP calls and status" + default n + help + Display the FSP call entry point and parameters prior to calling FSP + and display the status upon return from FSP. + +config DISPLAY_FSP_HEADER + bool "Display the FSP header" + default n + help + Display the FSP header information when the FSP file is found. + +config DISPLAY_HOBS + bool "Display the hand-off-blocks" + default n + help + Display the FSP HOBs which are provided for coreboot. + +config DISPLAY_UPD_DATA + bool "Display UPD data" + default n + help + Display the user specified product data prior to memory + initialization. + +config VERIFY_HOBS + bool "Verify the FSP hand-off-blocks" + default n + help + Verify that the HOBs required by coreboot are returned by FSP and + that the resource HOBs are in the correct order and position. + +config DISPLAY_FSP_VERSION_INFO + bool "Display Firmware Ingredient Version Information" + help + Select this option to display Firmware version information. + +endif # PLATFORM_USES_FSP2_0
-
Arthur Heymans (Code Review) -
Martin Roth (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)