coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

[S] Change in coreboot[main]: Docs/releases: Finalize 4.22/4.22.01 release notes
by Felix Singer (Code Review) 25 Nov '23

25 Nov '23

Attention is currently required from: Angel Pons, Jason Glenesk, Martin L Roth. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79147?usp=email ) Change subject: Docs/releases: Finalize 4.22/4.22.01 release notes ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79147?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: Iae9653a275fcc1d11efbb88e12676f332be0a5dc Gerrit-Change-Number: 79147 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 24 Nov 2023 23:08:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: acpi/acpi_gic: Add GIC ITS subtable
by Arthur Heymans (Code Review) 24 Nov '23

24 Nov '23

Attention is currently required from: Angel Pons, Cliff Huang, Lance Zhao, Maximilian Brune, Naresh Solanki, Nico Huber, Patrick Rudolph, Paul Menzel, Tim Wawrzynczak. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78115?usp=email ) Change subject: acpi/acpi_gic: Add GIC ITS subtable ...................................................................... Patch Set 4: (3 comments) File src/acpi/acpi_gic.c: https://review.coreboot.org/c/coreboot/+/78115/comment/f45204c6_7452fe18 : PS4, Line 85: } > Is this weak implementation necessary? The other platform_*() functions > above don't have one. Would it make sense to the ITS to the devicetree and just loop over them? https://review.coreboot.org/c/coreboot/+/78115/comment/1a871698_9313498d : PS4, Line 99: its_base + (i * its_size); This API is strange and possibly not sufficiently generic. It looks like the its_size is just there for some SOC specific alignment or spacing between ITS, but it still assumes that they are placed sequentially in memory. Why not pass an array of bases in platform_get_gic_its and their count? File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/78115/comment/7add6a4d_457d8851 : PS4, Line 1600: uintptr_t size_t -- To view, visit https://review.coreboot.org/c/coreboot/+/78115?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: I1bcb3ad24de64cbba8aeef7ba7254d3157e0dc43 Gerrit-Change-Number: 78115 Gerrit-PatchSet: 4 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Comment-Date: Fri, 24 Nov 2023 20:24:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Docs/releases: Finalize 4.22/4.22.01 release notes
by Martin L Roth (Code Review) 24 Nov '23

24 Nov '23

Attention is currently required from: Angel Pons, Felix Singer, Jason Glenesk. Hello Angel Pons, Felix Singer, Jason Glenesk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79147?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Docs/releases: Finalize 4.22/4.22.01 release notes ...................................................................... Docs/releases: Finalize 4.22/4.22.01 release notes Now that the 4.22 release tag has been added to git, update the release notes with the final statistics and wording. We also decided to add a fix submitted immediately after the 4.22 release was tagged into the release package and do a point release. This also adds an expected date for the next release Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: Iae9653a275fcc1d11efbb88e12676f332be0a5dc --- M Documentation/releases/coreboot-4.22-relnotes.md 1 file changed, 22 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/79147/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79147?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: Iae9653a275fcc1d11efbb88e12676f332be0a5dc Gerrit-Change-Number: 79147 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: treewide: Prepare mode switch code for dynamically generated paging
by Arthur Heymans (Code Review) 24 Nov '23

24 Nov '23

Attention is currently required from: Benjamin Doron, Felix Held, Fred Reitberger, Jason Glenesk, Jérémy Compostella, Matt DeVillier. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79226?usp=email ) Change subject: treewide: Prepare mode switch code for dynamically generated paging ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I tried https://review.coreboot.org/c/coreboot/+/56095 (rough idea, not finished) to turn it into a macro which allows both fixed offset as input as a registers. Let me know what you think. > > With that .inc file you have a problem if you want to jump to 64bit twice in same file, which you may just want for various reasons. Gerrit played some master/main tricks on me. I meant https://review.coreboot.org/c/coreboot/+/79261 -- To view, visit https://review.coreboot.org/c/coreboot/+/79226?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: I1ef7c8bee0c38395d5436f6479f11b3a4208f53e Gerrit-Change-Number: 79226 Gerrit-PatchSet: 1 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 24 Nov 2023 19:59:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: [RFC]cpu/x86/64bit: Turn jumping to long mode into a macro
by Arthur Heymans (Code Review) 24 Nov '23

24 Nov '23

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/56095?usp=email ) Change subject: [RFC]cpu/x86/64bit: Turn jumping to long mode into a macro ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/56095?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ida861338004187e4e714be41e17c8447fa4cf935 Gerrit-Change-Number: 56095 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: abandon

1 0

[M] Change in coreboot[main]: treewide: Prepare mode switch code for dynamically generated paging
by Arthur Heymans (Code Review) 24 Nov '23

24 Nov '23

Attention is currently required from: Benjamin Doron, Felix Held, Fred Reitberger, Jason Glenesk, Jérémy Compostella, Matt DeVillier. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79226?usp=email ) Change subject: treewide: Prepare mode switch code for dynamically generated paging ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I tried https://review.coreboot.org/c/coreboot/+/56095 (rough idea, not finished) to turn it into a macro which allows both fixed offset as input as a registers. Let me know what you think. With that .inc file you have a problem if you want to jump to 64bit twice in same file, which you may just want for various reasons. -- To view, visit https://review.coreboot.org/c/coreboot/+/79226?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: I1ef7c8bee0c38395d5436f6479f11b3a4208f53e Gerrit-Change-Number: 79226 Gerrit-PatchSet: 1 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 24 Nov 2023 19:46:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: [NOTREADY]cpu/x86/64bit: Turn jumping to long mode into a macro
by Arthur Heymans (Code Review) 24 Nov '23

24 Nov '23

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79261?usp=email ) Change subject: [NOTREADY]cpu/x86/64bit: Turn jumping to long mode into a macro ...................................................................... [NOTREADY]cpu/x86/64bit: Turn jumping to long mode into a macro This makes it easier to reuse, e.g. if you want to do it twice in one assembly file. Also having the .code64 explicit instead of inside the included file provides more clarity. Change-Id: Ida861338004187e4e714be41e17c8447fa4cf935 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/car/non-evict/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/qemu-x86/cache_as_ram_bootblock.S M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/sipi_vector.S M src/cpu/x86/smm/smm_stub.S M src/soc/intel/common/block/cpu/car/cache_as_ram.S 8 files changed, 34 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/79261/1 diff --git a/src/cpu/intel/car/non-evict/cache_as_ram.S b/src/cpu/intel/car/non-evict/cache_as_ram.S index 76986ff..bf96c1a 100644 --- a/src/cpu/intel/car/non-evict/cache_as_ram.S +++ b/src/cpu/intel/car/non-evict/cache_as_ram.S @@ -4,6 +4,7 @@ #include <cpu/x86/mtrr.h> #include <cpu/x86/cache.h> #include <cpu/x86/post_code.h> +#include <cpu/x86/64bit/entry64.inc> #define NoEvictMod_MSR 0x2e0 #define BBL_CR_CTL3_MSR 0x11e @@ -213,9 +214,9 @@ andl $0xfffffff0, %esp #if ENV_X86_64 - - #include <cpu/x86/64bit/entry64.inc> - + setup_longmode 1f +.code64 +1: movd %mm2, %rdi shlq $32, %rdi movd %mm1, %rsi diff --git a/src/cpu/intel/car/p4-netburst/cache_as_ram.S b/src/cpu/intel/car/p4-netburst/cache_as_ram.S index f7c023b..8f47b4d 100644 --- a/src/cpu/intel/car/p4-netburst/cache_as_ram.S +++ b/src/cpu/intel/car/p4-netburst/cache_as_ram.S @@ -13,6 +13,7 @@ .global bootblock_pre_c_entry #include <cpu/intel/car/cache_as_ram_symbols.inc> +#include <cpu/x86/64bit/entry64.inc> .code32 _cache_as_ram_setup: @@ -362,8 +363,9 @@ subl $4, %esp #if ENV_X86_64 - #include <cpu/x86/64bit/entry64.inc> - + setup_longmode 1f +.code64 +1: movd %mm2, %rdi shlq $32, %rdi /* BIST */ movd %mm1, %rsi diff --git a/src/cpu/qemu-x86/cache_as_ram_bootblock.S b/src/cpu/qemu-x86/cache_as_ram_bootblock.S index 0943e35..ad19a42 100644 --- a/src/cpu/qemu-x86/cache_as_ram_bootblock.S +++ b/src/cpu/qemu-x86/cache_as_ram_bootblock.S @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <cpu/x86/post_code.h> +#include <cpu/x86/64bit/entry64.inc> #define CBFS_FILE_MAGIC 0 #define CBFS_FILE_LEN (CBFS_FILE_MAGIC + 8) @@ -77,11 +78,12 @@ /* Align the stack and keep aligned for call to bootblock_c_entry() */ and $0xfffffff0, %esp - /* entry64.inc preserves ebx. */ -#include <cpu/x86/64bit/entry64.inc> - /* Restore the BIST result and timestamps. */ + movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %ebx #if ENV_X86_64 + setup_longmode 1f %ebx +.code64 +1: movd %mm2, %rdi shlq $32, %rdi movd %mm1, %rsi diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 7da68b4..6295bdd 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -22,10 +22,9 @@ #include <arch/rom_segs.h> #endif - -setup_longmode: +.macro setup_longmode jmp_addr page_table /* Get page table address */ - movl $(CONFIG_ARCH_X86_64_PGTBL_LOC), %eax + movl \page_table, %eax /* load identity mapped page tables */ movl %eax, %cr3 @@ -48,12 +47,11 @@ /* use long jump to switch to 64-bit code segment */ #if defined(__RAMSTAGE__) - ljmp $RAM_CODE_SEG64, $__longmode_start + ljmp $RAM_CODE_SEG64, $\jmp_addr #else - ljmp $ROM_CODE_SEG64, $__longmode_start + ljmp $ROM_CODE_SEG64, $\jmp_addr #endif -.code64 -__longmode_start: +.endm #endif diff --git a/src/cpu/x86/64bit/mode_switch.S b/src/cpu/x86/64bit/mode_switch.S index c27f540..45a62f0 100644 --- a/src/cpu/x86/64bit/mode_switch.S +++ b/src/cpu/x86/64bit/mode_switch.S @@ -1,4 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <cpu/x86/64bit/entry64.inc> .text .code64 @@ -55,7 +56,9 @@ movl %eax, %ebx /* Preserves ebx */ - #include <cpu/x86/64bit/entry64.inc> + setup_longmode continue $(CONFIG_ARCH_X86_64_PGTBL_LOC) +.code64 +continue: /* Place return value in rax */ movl %ebx, %eax diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index 39973db..f83b22b 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -6,6 +6,7 @@ #include <arch/ram_segs.h> #define __RAMSTAGE__ +#include <cpu/x86/64bit/entry64.inc> /* The SIPI vector is responsible for initializing the APs in the system. It * loads microcode, sets up MSRs, and enables caching before calling into @@ -223,7 +224,9 @@ #if ENV_X86_64 /* entry64.inc preserves ebx, esi, edi, ebp */ -#include <cpu/x86/64bit/entry64.inc> + setup_longmode 1f $(CONFIG_ARCH_X86_64_PGTBL_LOC) +.code64 +1: movabs c_handler, %eax call *%rax #else diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S index f97ab59..100f182 100644 --- a/src/cpu/x86/smm/smm_stub.S +++ b/src/cpu/x86/smm/smm_stub.S @@ -12,6 +12,7 @@ #include <cpu/x86/cr.h> #include <cpu/x86/msr.h> #include <cpu/x86/lapic_def.h> +#include <cpu/x86/64bit/entry64.inc> .code32 .section ".module_parameters", "aw", @progbits @@ -195,7 +196,9 @@ #if ENV_X86_64 mov %ecx, %edi /* entry64.inc preserves ebx, esi, edi, ebp */ -#include <cpu/x86/64bit/entry64.inc> + setup_longmode 1f +.code64 +1: mov %edi, %ecx diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S index 61cbe307..656306d 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S @@ -9,6 +9,7 @@ #include <cpu/x86/post_code.h> #include <intelblocks/msr.h> #include <intelblocks/post_codes.h> +#include <cpu/x86/64bit/entry64.inc> .section .init, "ax", @progbits @@ -279,7 +280,9 @@ andl $0xfffffff0, %esp #if ENV_X86_64 - #include <cpu/x86/64bit/entry64.inc> + setup_longmode 1f +.code64 +1: movd %mm2, %rdi shlq $32, %rdi movd %mm1, %rsi -- To view, visit https://review.coreboot.org/c/coreboot/+/79261?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: Ida861338004187e4e714be41e17c8447fa4cf935 Gerrit-Change-Number: 79261 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: [RFC]cpu/x86/64bit: Turn jumping to long mode into a macro
by Arthur Heymans (Code Review) 24 Nov '23

24 Nov '23

Arthur Heymans has restored this change. ( https://review.coreboot.org/c/coreboot/+/56095?usp=email ) Change subject: [RFC]cpu/x86/64bit: Turn jumping to long mode into a macro ...................................................................... Restored -- To view, visit https://review.coreboot.org/c/coreboot/+/56095?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ida861338004187e4e714be41e17c8447fa4cf935 Gerrit-Change-Number: 56095 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: restore

1 0

[XL] Change in coreboot[main]: mb/emulation: Add SIMICS QSP support
by Benjamin Doron (Code Review) 24 Nov '23

24 Nov '23

Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Lean Sheng Tan, Leon Groß, Nico Huber, Patrick Rudolph, Paul Menzel. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77905?usp=email ) Change subject: mb/emulation: Add SIMICS QSP support ...................................................................... Patch Set 15: (3 comments) File src/mainboard/emulation/simics-qsp/northbridge.c: https://review.coreboot.org/c/coreboot/+/77905/comment/c0becd16_04c76118 : PS7, Line 140: len += simics_get_smbios_data19(*handle + 2, *handle, current); > Any reason you're adding this here? All other boards use common code for type19 From memory, the common code needs some CBMEM mem info table to be able to generate the memory-related SMBIOS tables. Maybe generate that instead of writing SMBIOS here? File src/mainboard/emulation/simics-qsp/northbridge.c: https://review.coreboot.org/c/coreboot/+/77905/comment/15db2e91_05ec2ce0 : PS15, Line 153: if (!is_pci_dev_on_bus(dev, 0)) : return NULL; This looks unnecessary, if it falls-through it also returns NULL. Was this copied? https://review.coreboot.org/c/coreboot/+/77905/comment/f0a7fc21_9c7a27f4 : PS15, Line 172: static int fw_cfg_max_cpus(void) : { : return 1; : } This is (probably) incorrect, and misleads MP init. -- To view, visit https://review.coreboot.org/c/coreboot/+/77905?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: I175b20bb4746608e4d868aa96492fc06c149bd36 Gerrit-Change-Number: 77905 Gerrit-PatchSet: 15 Gerrit-Owner: Leon Groß <leon.gross(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Leon Groß <leon.gross(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Fri, 24 Nov 2023 18:55:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[main]: mb/emulation: Add SIMICS QSP support
by Benjamin Doron (Code Review) 24 Nov '23

24 Nov '23

Attention is currently required from: Angel Pons, Christian Walter, Lean Sheng Tan, Leon Groß, Nico Huber, Patrick Rudolph, Paul Menzel. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77905?usp=email ) Change subject: mb/emulation: Add SIMICS QSP support ...................................................................... Patch Set 15: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/77905/comment/cb3f1f40_bf738173 : PS9, Line 20: > Sorry for the confusion. […] Intel provides some documentation as part of the Slim Bootloader project: https://slimbootloader.github.io/supported-hardware/qsp.html The short of it is: download and install the public release, open the GUI and start a .simics script file. Note: I do not recommend running "firststeps.simics". As part of 'improving the user experience,' or something like that, they suppress Simics messages. These sometimes tell you which registers are unimplemented. I can potentially provide the script I use, but I don't have Simics installed at this time, and I haven't memorised the paths. Patchset: PS4: Sheng, I think it's important to distinguish between Simics Base, the backend (provides general device modelling, an instruction interpreter and JIT compiler), only needing some architecture-specific code to enable a fast simulator, and Simics QSP, a specific board model. I agree that Simics Base is - as far as I know - an industry standard simulator used both internally and externally, but here we have to think of Simics QSP. Simics QSP is not as capable/accurate a model as others. (For instance, to my knowledge, SeaBIOS is unsupported because the model doesn't emulate/simulate the PAM registers. If they would be added, it would become more capable.) That said, this board port is fairly self-contained, and Simics is a good development environment, in my option. To those familiar with it, it would be nice to work on coreboot in it. I agree it would be nice to get this into the official public release. That would also help the case for implementing more chipset support to the model, if anyone there is open to that. > This sounds really nice. Why not start with that? If the binary is accepted, let's maintain it upstream? Can we work on this? -- To view, visit https://review.coreboot.org/c/coreboot/+/77905?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: I175b20bb4746608e4d868aa96492fc06c149bd36 Gerrit-Change-Number: 77905 Gerrit-PatchSet: 15 Gerrit-Owner: Leon Groß <leon.gross(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Leon Groß <leon.gross(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Fri, 24 Nov 2023 18:14:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Leon Groß <leon.gross(a)9elements.com> Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023