Attention is currently required from: Felix Singer, Jonathon Hall, Martin L Roth, Nico Huber.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80599?usp=email )
Change subject: mb/purism/librem_cnl: Drop devicetree entries identical to chipset.cb
......................................................................
Patch Set 2:
(1 comment)
File src/mainboard/purism/librem_cnl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80599/comment/5d7003de_42d30dc6 :
PS1, Line 47: device ref system_agent on end
> This one is identical as well.
good catch - done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80599?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: I6c656d227962548cebde61f1d82333837adbbf56
Gerrit-Change-Number: 80599
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Comment-Date: Sun, 18 Feb 2024 05:06:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Martin L Roth, Nico Huber, Patrick Rudolph, Paul Menzel, Ronak Kanabar.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email )
Change subject: drivers/intel/fsp2_0: Add FSP_ALLOCATES_TEMP_RAM
......................................................................
Patch Set 4:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80328/comment/e28318f4_dacf97a1 :
PS1, Line 9: FSP allocates temporary ram in its reserved memory ranges
: instead of requesting coreboot to do the allocation. This
: is supported by some FSP implementations for Xeon SP.
> I second this. If we used the defaults, we would still have to assert that […]
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/80328/comment/da3758cc_8d89e504 :
PS3, Line 8:
> Wouldn't it be easier to select `PLATFORM_USES_FSP2_0` instead of `PLATFORM_USES_FSP2_2`?
Not sure if I correctly get your points, Rudolph. I didn't use PLATFORM_USES_FSP2_0 or PLATFORM_USES_FSP2_2 as the selector due to 2 reasons.
1) PLATFORM_USES_FSP2_0/2 will be enabled by default by FSP2_3/4 forwardly, but we would like to keep this kconfig to be explicitly and optionally selected.
2) We cannot cover CPX sue to CPX's FSP doesn't update StackBase/Size's default value to fit for this usage. P.S. new logics is added in memory_init.c to check StackBase/Size's range validity before move ahead.
https://review.coreboot.org/c/coreboot/+/80328/comment/d52e3098_f20e0d4e :
PS3, Line 11: some
> Good catch, if CPX needs, maybe community could help to test CPX as well? Anyway ... […]
Done
Patchset:
PS3:
> I hope we are not stuck here. Shuo, if you could draw a small temp-ram memory […]
Good suggestion Nico. I'm tentatively adding a memory map into src/drivers/intel/fsp2_0/memory_init.c where there are more space to put long contents. Please review and let us discuss the next step and if it is more preferred to move the memory map into Kconfig.
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/80328/comment/8d0c4c00_559a7a84 :
PS3, Line 42: FSP_DOES_NOT_NEED_TEMP_RAM
> Maybe "FSP_ALLOCATES_TEMP_RAM"? […]
Done
https://review.coreboot.org/c/coreboot/+/80328/comment/8bbd3172_1d08d72c :
PS3, Line 46: FSP allocates temporary ram in its reserved memory ranges
: instead of requesting coreboot to do the allocation.
> The option says `NOT_NEED`? Isn’t that a contradiction?
Acknowledged
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/80328/comment/340d9d0b_c3d2e10b :
PS1, Line 156: goto temp_ram_assigned;
> > Yeah, for the RC sample fixes (thanks for providing the inputs :-)), we made careful evaluations. […]
Arthur, I'm trying to cover the needs above in the updated codes. Let us review and discuss.
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/80328/comment/59dd1bee_972c6cca :
PS3, Line 156: goto temp_ram_assigned;
> Please review together with https://review.coreboot. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80328?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: I17a73e4f627c91808ed17c712b72937662ae9293
Gerrit-Change-Number: 80328
Gerrit-PatchSet: 4
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
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: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 05:04:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
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: Shuo Liu <shuo.liu(a)intel.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Tim Chu.
Hello Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Tim Chu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80329?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/intel/xeon_sp/spr: Enable FSP_ALLOCATES_TEMP_RAM
......................................................................
soc/intel/xeon_sp/spr: Enable FSP_ALLOCATES_TEMP_RAM
TEST=intel/archercity CRB 1S/2S
Change-Id: Ic84c4332dd5b6980ca03f6ab601b7464e0b18001
Signed-off-by: Shuo Liu <shuo.liu(a)intel.com>
---
M src/soc/intel/xeon_sp/spr/Kconfig
1 file changed, 6 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/80329/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80329?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: Ic84c4332dd5b6980ca03f6ab601b7464e0b18001
Gerrit-Change-Number: 80329
Gerrit-PatchSet: 4
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Paul Menzel, Ronak Kanabar, Shuo Liu.
Hello Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80328?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review-1 by Arthur Heymans, Verified+1 by build bot (Jenkins)
Change subject: drivers/intel/fsp2_0: Add FSP_ALLOCATES_TEMP_RAM
......................................................................
drivers/intel/fsp2_0: Add FSP_ALLOCATES_TEMP_RAM
FSP allocates temporary ram in its reserved memory ranges
(covered in FSP_M_RC_HEAP_SIZE) instead of requesting coreboot to
do the allocation. This is required by some FSP implementations
of Xeon SP multi-socket platforms.
TEST=intel/archercity CRB 1S/2S
Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293
Signed-off-by: Shuo Liu <shuo.liu(a)intel.com>
---
M src/drivers/intel/fsp2_0/Kconfig
M src/drivers/intel/fsp2_0/memory_init.c
2 files changed, 57 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/80328/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80328?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: I17a73e4f627c91808ed17c712b72937662ae9293
Gerrit-Change-Number: 80328
Gerrit-PatchSet: 4
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Alper Nebi Yasak.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80370?usp=email )
Change subject: mainboard/qemu-aarch64: Enable QEMU fw_cfg driver
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80370/comment/8d9c3b9e_5e83da9c :
PS1, Line 12: [1] https://gitlab.com/qemu-project/qemu/-/blob/v8.2.1/hw/arm/virt.c#L150
Can you use an archive.org link instead of directly linking?
File src/mainboard/emulation/qemu-aarch64/include/mainboard/addressmap.h:
https://review.coreboot.org/c/coreboot/+/80370/comment/79867b22_e7e5de57 :
PS1, Line 25: #define VIRT_FW_CFG_BASE 0x09020000
Any reason this is needed in addition to the Kconfig option? It seems like you could use one or the other. By having it defined in two places, you run the risk of them not being the same.
At the very least, you could use CONFIG_QEMU_FW_CFG_BASE_ADDRESS here instead of a fixed value.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80370?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: I580ac207502ab1b441ab60e708f52883f1c69749
Gerrit-Change-Number: 80370
Gerrit-PatchSet: 1
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 03:19:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80567?usp=email )
Change subject: soc/intel/mtl: Skip RW CBFS ucode update if RO is locked
......................................................................
soc/intel/mtl: Skip RW CBFS ucode update if RO is locked
This patch eliminates coreboot from loading microcode from RW CBFS
(when the RO descriptor is locked, which indicates a fixed RO image)
because the kernel can already patch the microcode on BSPs and APs
while booting to OS.
This may be a chance to lower the burden on the AP FW side because
patching microcode on in-field devices is subject to firmware updates,
which are rarely published and, if required, must go through the
firmware qualification testing procedure (which is costly, unlike
kernel updates for ucode updates).
1. The FIT loads the necessary microcode from the RO during reset.
2. Reloading microcode from RW CBFS impacts boot time
(~60ms, core-dependent).
3. The kernel can still load microcode updates.
ChromeOS devices leverage RO+RW-A/RW-B booting. The RO's microcode is
sufficient for initial boot, and the kernel can apply updates later.
BUG=none
TEST=Verified boot optimization; in-field devices skip RW-CBFS microcode
loading when RO is locked.
Change-Id: Ia859809970406fca3fa14e6fa8e766ab16d94c8a
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80567
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: YH Lin <yueherngl(a)google.com>
---
M src/soc/intel/meteorlake/cpu.c
1 file changed, 22 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
YH Lin: Looks good to me, approved
diff --git a/src/soc/intel/meteorlake/cpu.c b/src/soc/intel/meteorlake/cpu.c
index 54217ea5..e990ae3 100644
--- a/src/soc/intel/meteorlake/cpu.c
+++ b/src/soc/intel/meteorlake/cpu.c
@@ -4,6 +4,7 @@
#include <console/console.h>
#include <cpu/cpu.h>
#include <cpu/intel/common/common.h>
+#include <cpu/intel/microcode.h>
#include <cpu/intel/smm_reloc.h>
#include <cpu/intel/turbo.h>
#include <cpu/x86/lapic.h>
@@ -214,3 +215,24 @@
/* Thermal throttle activation offset */
configure_tcc_thermal_target();
}
+
+int soc_skip_ucode_update(u32 current_patch_id, u32 new_patch_id)
+{
+ if (!CONFIG(CHROMEOS))
+ return 0;
+ /*
+ * Locked RO Descriptor Implications:
+ *
+ * - A locked descriptor signals the RO binary is fixed; the FIT will load the
+ * RO's microcode during system reset.
+ * - Attempts to load newer microcode from the RW CBFS will cause a boot-time
+ * delay (~60ms, core-dependent), as the microcode must be reloaded on BSP+APs.
+ * - The kernel can load microcode updates without impacting AP FW boot time.
+ * - Skipping RW CBFS microcode loading is low-risk when the RO is locked,
+ * prioritizing fast boot times.
+ */
+ if (CONFIG(LOCK_MANAGEMENT_ENGINE) && current_patch_id)
+ return 1;
+
+ return 0;
+}
--
To view, visit https://review.coreboot.org/c/coreboot/+/80567?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: Ia859809970406fca3fa14e6fa8e766ab16d94c8a
Gerrit-Change-Number: 80567
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Paul Menzel, Tarun.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80567?usp=email )
Change subject: soc/intel/mtl: Skip RW CBFS ucode update if RO is locked
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> @Arthur, i lost the CR+2 due to rebase, do you mind reviewing this CL once more.
thanks YH
--
To view, visit https://review.coreboot.org/c/coreboot/+/80567?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: Ia859809970406fca3fa14e6fa8e766ab16d94c8a
Gerrit-Change-Number: 80567
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
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: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 03:13:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Alper Nebi Yasak, Nico Huber.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80368?usp=email )
Change subject: mb/qemu/fw_cfg: Move fw_cfg driver to drivers/emulation/qemu
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
Instead of moving things to src/mb/emulation/qemu and src/include, maybe create src/mb/emulation/qemu_common and qemu_common/include and move the files there?
I don't think the qemu header files really belong in src/include.
Obviously you'd need to add qemu_common/include as an include directory in the appropriate makefiles.
File src/drivers/emulation/qemu/Kconfig:
https://review.coreboot.org/c/coreboot/+/80368/comment/919fa754_dfb16141 :
PS1, Line 29: bool "QEMU firmware configuration driver"
> This shouldn't have a prompt but only be selected by the mainboard ports that need it.
So just use `bool` instead of `bool "prompt"` - that way it won't show up as user configurable.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80368?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: I0348bb2fa3a4cc1ef6561206a5dd02feb16ae869
Gerrit-Change-Number: 80368
Gerrit-PatchSet: 1
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 03:03:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Alper Nebi Yasak.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80365?usp=email )
Change subject: mb/qemu/fw_cfg: Support using DMA to select fw_cfg file
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80365/comment/58e8cf92_91d7964c :
PS1, Line 11: "file"s
"file's" ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80365?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: I46f9915e6df04d371c7084815f16034c7e9879d4
Gerrit-Change-Number: 80365
Gerrit-PatchSet: 1
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 02:51:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Alper Nebi Yasak.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80367?usp=email )
Change subject: mb/qemu/fw_cfg: Use fw_cfg_read() to read SMBIOS data
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80367?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: I18e60b8e9de34f2b0ff67af4113beec1d7467329
Gerrit-Change-Number: 80367
Gerrit-PatchSet: 1
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Sun, 18 Feb 2024 02:51:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment