coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: nb/intel/i440bx: Use PARALLEL_MP
by Arthur Heymans (Code Review) 26 Nov '21

26 Nov '21

Attention is currently required from: Patrick Rudolph. Hello Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59693 to look at the new patch set (#2). Change subject: nb/intel/i440bx: Use PARALLEL_MP ...................................................................... nb/intel/i440bx: Use PARALLEL_MP Now that PARALLEL_MP can also deal with !SMP it can be used for this northbridge, which always features one CPU. The ramstage size is decreased by roughly 5K, but the compressed size increase by ~1K. Change-Id: Ic8d2582b353069eecea8561cfe01b2dd8221779b Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/i440bx/Kconfig M src/northbridge/intel/i440bx/northbridge.c 2 files changed, 15 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/59693/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59693 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic8d2582b353069eecea8561cfe01b2dd8221779b Gerrit-Change-Number: 59693 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: cpu/x86/mp_init.c: Make it work for !CONFIG_SMP
by build bot (Jenkins) (Code Review) 26 Nov '21

26 Nov '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59692 ) Change subject: cpu/x86/mp_init.c: Make it work for !CONFIG_SMP ...................................................................... Patch Set 1: (1 comment) File src/cpu/x86/mp_init.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134495): https://review.coreboot.org/c/coreboot/+/59692/comment/8ae379ae_6382b2e4 PS1, Line 1084: state->smm_save_state_size = MAX(state->smm_real_save_state_size, smm_stub_size()); line over 96 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/59692 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia0e8fdf8ed7bc2e0e4ff01be8d3e3c3cb837e6c7 Gerrit-Change-Number: 59692 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 26 Nov 2021 13:58:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/i440bx: Use PARALLEL_MP
by Arthur Heymans (Code Review) 26 Nov '21

26 Nov '21

Attention is currently required from: Patrick Rudolph. Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59693 ) Change subject: nb/intel/i440bx: Use PARALLEL_MP ...................................................................... nb/intel/i440bx: Use PARALLEL_MP Now that PARALLEL_MP can also deal with !SMP it can be used for this northbridge, which always features one CPU. The ramstage size by roughly 5K, but the compressed size increase by ~1K. Change-Id: Ic8d2582b353069eecea8561cfe01b2dd8221779b Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/i440bx/Kconfig M src/northbridge/intel/i440bx/northbridge.c 2 files changed, 15 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/59693/1 diff --git a/src/northbridge/intel/i440bx/Kconfig b/src/northbridge/intel/i440bx/Kconfig index 4b1638c..d97c627 100644 --- a/src/northbridge/intel/i440bx/Kconfig +++ b/src/northbridge/intel/i440bx/Kconfig @@ -5,7 +5,7 @@ select NO_ECAM_MMCONF_SUPPORT select HAVE_DEBUG_RAM_SETUP select NO_CBFS_MCACHE - select LEGACY_SMP_INIT + select PARALLEL_MP config SDRAMPWR_4DIMM bool diff --git a/src/northbridge/intel/i440bx/northbridge.c b/src/northbridge/intel/i440bx/northbridge.c index 2659288..8225e1c 100644 --- a/src/northbridge/intel/i440bx/northbridge.c +++ b/src/northbridge/intel/i440bx/northbridge.c @@ -7,6 +7,7 @@ #include <device/pci.h> #include <device/pci_ids.h> #include <cpu/cpu.h> +#include <cpu/x86/mp.h> #include "i440bx.h" static void northbridge_init(struct device *dev) @@ -72,15 +73,25 @@ .scan_bus = pci_domain_scan_bus, }; -static void cpu_bus_init(struct device *dev) +static int get_cpu_count(void) { - initialize_cpus(dev->link_list); + return 1; +} + +static const struct mp_ops mp_ops = { + .get_cpu_count = get_cpu_count, +}; + +void mp_init_cpus(struct bus *cpu_bus) +{ + /* TODO: Handle mp_init_with_smm failure? */ + mp_init_with_smm(cpu_bus, &mp_ops); } static struct device_operations cpu_bus_ops = { .read_resources = noop_read_resources, .set_resources = noop_set_resources, - .init = cpu_bus_init, + .init = mp_cpu_bus_init, }; static void enable_dev(struct device *dev) -- To view, visit https://review.coreboot.org/c/coreboot/+/59693 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic8d2582b353069eecea8561cfe01b2dd8221779b Gerrit-Change-Number: 59693 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: cpu/x86/mp_init.c: Make it work for !CONFIG_SMP
by Arthur Heymans (Code Review) 26 Nov '21

26 Nov '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59692 ) Change subject: cpu/x86/mp_init.c: Make it work for !CONFIG_SMP ...................................................................... cpu/x86/mp_init.c: Make it work for !CONFIG_SMP With very little changes this code can be used to initialize systems without SMP. The linker will remove most of the code. Change-Id: Ia0e8fdf8ed7bc2e0e4ff01be8d3e3c3cb837e6c7 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/Kconfig M src/cpu/x86/mp_init.c 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/59692/1 diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 44c9cb6..2e7be02 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -1,12 +1,13 @@ config PARALLEL_MP def_bool y depends on !LEGACY_SMP_INIT - depends on SMP select CPU_INFO_V2 help This option uses common MP infrastructure for bringing up APs in parallel. It additionally provides a more flexible mechanism for sequencing the steps of bringing up the APs. + The code also works for just initialising the BSP in case there + are no APs. config PARALLEL_MP_AP_WORK def_bool n diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index c99732f..07bb787 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -593,6 +593,10 @@ return CB_ERR; } + /* We just need to run things on the BSP */ + if (!CONFIG(SMP)) + return bsp_do_flight_plan(p); + /* Default to currently running CPU. */ num_cpus = allocate_cpu_devices(cpu_bus, p); @@ -1076,7 +1080,8 @@ ops->get_smm_info(&state->perm_smbase, &state->perm_smsize, &state->smm_real_save_state_size); - state->smm_save_state_size = MAX(state->smm_real_save_state_size, smm_stub_size()); + if (CONFIG(HAVE_SMI_HANDLER)) + state->smm_save_state_size = MAX(state->smm_real_save_state_size, smm_stub_size()); /* * Make sure there is enough room for the SMM descriptor -- To view, visit https://review.coreboot.org/c/coreboot/+/59692 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia0e8fdf8ed7bc2e0e4ff01be8d3e3c3cb837e6c7 Gerrit-Change-Number: 59692 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: include/cpu/x86/mp.h: Remove indirect include
by Arthur Heymans (Code Review) 26 Nov '21

26 Nov '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59691 ) Change subject: include/cpu/x86/mp.h: Remove indirect include ...................................................................... include/cpu/x86/mp.h: Remove indirect include This one might conflict with '#include <smp/atomic.h>'. Change-Id: I7413406ca69e78e5a6e539a01e05033243107272 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/include/cpu/x86/mp.h 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/59691/1 diff --git a/src/include/cpu/x86/mp.h b/src/include/cpu/x86/mp.h index 7ed82dd..934d217 100644 --- a/src/include/cpu/x86/mp.h +++ b/src/include/cpu/x86/mp.h @@ -3,7 +3,6 @@ #ifndef _X86_MP_H_ #define _X86_MP_H_ -#include <arch/smp/atomic.h> #include <cpu/x86/smm.h> #include <types.h> -- To view, visit https://review.coreboot.org/c/coreboot/+/59691 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7413406ca69e78e5a6e539a01e05033243107272 Gerrit-Change-Number: 59691 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Introduce Intel PSE
by Lean Sheng Tan (Code Review) 26 Nov '21

26 Nov '21

Attention is currently required from: Arthur Heymans, Maulik V Vaghela, Paul Menzel, Angel Pons, Subrata Banik, Michael Niewöhner, Patrick Rudolph, Nico Huber, Martin Roth, Mario Scheithauer, Werner Zeh, Felix Held. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55367 ) Change subject: soc/intel/elkhartlake: Introduce Intel PSE ...................................................................... Patch Set 58: (1 comment) File src/soc/intel/elkhartlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/55367/comment/6e0a974f_defef8e4 PS57, Line 127: * > trailing whitespace Please fix. -- To view, visit https://review.coreboot.org/c/coreboot/+/55367 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifea08fb82fea18ef66bab04b3ce378e79a0afbf7 Gerrit-Change-Number: 55367 Gerrit-PatchSet: 58 Gerrit-Owner: Lean Sheng Tan Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 26 Nov 2021 13:55:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Introduce Intel PSE
by Lean Sheng Tan (Code Review) 26 Nov '21

26 Nov '21

Attention is currently required from: Arthur Heymans, Maulik V Vaghela, Paul Menzel, Angel Pons, Subrata Banik, Michael Niewöhner, Patrick Rudolph, Nico Huber, Martin Roth, Mario Scheithauer, Werner Zeh, Felix Held. Hello build bot (Jenkins), Nico Huber, Maulik V Vaghela, Paul Menzel, Mario Scheithauer, Subrata Banik, Michael Niewöhner, Werner Zeh, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55367 to look at the new patch set (#58). Change subject: soc/intel/elkhartlake: Introduce Intel PSE ...................................................................... soc/intel/elkhartlake: Introduce Intel PSE The Intel® Programmable Services Engine (Intel® PSE) is a dedicated offload engine for IoT functions powered by an ARM Cortex-M7 microcontroller. It provides independent, low-DMIPS computing and low-speed I/Os for IoT applications, plus dedicated services for real-time computing and time-sensitive synchronization. The PSE hosts new functions, including remote out-of-band device management, network proxy, embedded controller lite and sensor hub. This CL enables the user to provide the base address of the PSE FW blob which will then be loaded by the FSP-S onto the ARM controller. PSE FW will do the initialization work of PSE controller and its peripherals. The loading of PSE FW should have negligible impact on boot time unless PSE controller could not locate the PSE FW and FSP will attempt to redo PSE FW loading and wait for PSE handshake until it times out. Once PSE controller locate the PSE FW, it will do initialization concurrently by itself with coreboot booting. It also adds PSE related FSP-S UPD settings which enable the setup of peripheral ownership (assigned to the PSE or x86 subsystem) and interrupts. These assignments need to take place at a given point in the boot process and cannot be changed later. To verify if PSE FW is loaded properly, the user could enable PchPseShellEnabled flag and the log will be printed at PSE UART 2. For further info please refer to doc #611825 (for HW overview) and #614110 (for PSE EDS). Signed-off-by: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Change-Id: Ifea08fb82fea18ef66bab04b3ce378e79a0afbf7 --- M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/elkhartlake/chip.h M src/soc/intel/elkhartlake/fsp_params.c M src/soc/intel/elkhartlake/romstage/fsp_params.c 5 files changed, 174 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/55367/58 -- To view, visit https://review.coreboot.org/c/coreboot/+/55367 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifea08fb82fea18ef66bab04b3ce378e79a0afbf7 Gerrit-Change-Number: 55367 Gerrit-PatchSet: 58 Gerrit-Owner: Lean Sheng Tan Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common: Rename compare_cse_version() function name
by Sridhar Siricilla (Code Review) 26 Nov '21

26 Nov '21

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Patrick Rudolph. Hello Maulik V Vaghela, Tim Wawrzynczak, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59689 to look at the new patch set (#3). Change subject: soc/intel/common: Rename compare_cse_version() function name ...................................................................... soc/intel/common: Rename compare_cse_version() function name The patch renames the compare_cse_version() function to the cse_compare_sub_part_version(). It makes the function generic so that it can be used to compare version of any CSE sub-partition like IOM, NPHY etc. TEST=Verified build for Brya Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Change-Id: I88a44a3c0ba2ad8a589602a35ea644dab535b287 --- M src/soc/intel/common/block/cse/cse_lite.c 1 file changed, 4 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/59689/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/59689 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I88a44a3c0ba2ad8a589602a35ea644dab535b287 Gerrit-Change-Number: 59689 Gerrit-PatchSet: 3 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: security/intel/txt: Remove unused region device
by Michał Żygowski (Code Review) 26 Nov '21

26 Nov '21

Attention is currently required from: Michał Żygowski. Hello build bot (Jenkins), Angel Pons, Arthur Heymans, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59515 to look at the new patch set (#8). Change subject: security/intel/txt: Remove unused region device ...................................................................... security/intel/txt: Remove unused region device Region device is no longer used to locate BIOS ACM. Use new CBFS API to map and unmap the file. Using rdev_munmap on the uninitialized region device variable causes the platform to jump to a random address. TEST=Dell OptiPlex 9010 does not raise #UD exception when Intel TXT is enabled, ACM SCHECK is successful Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Change-Id: I98afba35403d5d2cd9eeb7df6d1ca0171894e9d4 --- M src/security/intel/txt/common.c 1 file changed, 6 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/59515/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/59515 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I98afba35403d5d2cd9eeb7df6d1ca0171894e9d4 Gerrit-Change-Number: 59515 Gerrit-PatchSet: 8 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: src/arch/ppc64/*: pass FDT address to payload
by Krystian Hebel (Code Review) 26 Nov '21

26 Nov '21

Attention is currently required from: Sergii Dmytruk, Timothy Pearson, Ron Minnich. Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57084 ) Change subject: src/arch/ppc64/*: pass FDT address to payload ...................................................................... Patch Set 20: Code-Review-1 (3 comments) Patchset: PS20: All of the code in this commit is just for QEMU, yet it lands in architecture-specific files. This would make incoming commits for Talos2 unnecessarily complicated. File src/arch/ppc64/boot.c: https://review.coreboot.org/c/coreboot/+/57084/comment/8be41c6c_56dba2b3 PS20, Line 42: asm("ld 27,%0" :: "m"(fdt) : "27"); Only QEMU prepares FDT in advance, real hardware doesn't, so all of the new code should be moved to platform_prog_run() instead. Because this line loads to volatile register, it should be put in the same asm block as the final jump to the payload, otherwise compiler is free to move code around, potentially overwriting FDT pointer in register. File src/arch/ppc64/bootblock_crt0.S: https://review.coreboot.org/c/coreboot/+/57084/comment/6ea12989_3716f19c PS20, Line 40: /* Store FDT address that's available in %r3 to pass to payload */ Maybe use one of SPRs for holding this value so it won't have to be moved around in RAM? HSPRG0 or HSPRG1 seems good. No need for additional section in memlayout or CBMEM entry either. -- To view, visit https://review.coreboot.org/c/coreboot/+/57084 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0911f4b534c6f8cacfa057a5bad7576fec711637 Gerrit-Change-Number: 57084 Gerrit-PatchSet: 20 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Attention: Timothy Pearson <tpearson(a)raptorengineering.com> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Fri, 26 Nov 2021 13:28:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021