Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier.
Hello Fred Reitberger, Jason Glenesk, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83778?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/amd/common/psp_smi_flash: add buffer overflow checks
......................................................................
soc/amd/common/psp_smi_flash: add buffer overflow checks
Before 'handle_psp_command' calls any of the functions in this file, it
make sure that the 'size' field in the command buffer's header doesn't
indicate that the command buffer is larger than the SMM memory region
reserved for it.
The read/write command buffer has a 'num_bytes' field to indicate how
many bytes should be read from the SPI flash and put into the data
buffer within the command buffer or how many bytes from this buffer
should be written to the flash. While we should be able to assume that
the PSP won't send us malformed command buffer, we should still better
check this just to be sure.
Test=When selecting SOC_AMD_COMMON_BLOCK_PSP_SMI, Mandolin still builds
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ib4e8514eedc3ad154a705c8a1e85d367e452dbed
---
M src/soc/amd/common/block/psp/psp_smi_flash.c
1 file changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/83778/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/83778?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib4e8514eedc3ad154a705c8a1e85d367e452dbed
Gerrit-Change-Number: 83778
Gerrit-PatchSet: 3
Gerrit-Owner: 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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81659?usp=email )
Change subject: Kconfig: Reverse ARCH_SUPPORTS_CLANG
......................................................................
Kconfig: Reverse ARCH_SUPPORTS_CLANG
Since most targets support clang it's easier to reverse the semantics of
the Kconfig options.
Change-Id: Ib28e7a4cb286b9f8b05be94dae3947179f43c746
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/81659
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-by: Maximilian Brune <maximilian.brune(a)9elements.com>
---
M src/Kconfig
M src/arch/arm/Kconfig
M src/arch/arm64/Kconfig
M src/arch/ppc64/Kconfig
M src/arch/riscv/Kconfig
M src/arch/x86/Kconfig
6 files changed, 7 insertions(+), 8 deletions(-)
Approvals:
build bot (Jenkins): Verified
Maximilian Brune: Looks good to me, approved
Nico Huber: Looks good to me, but someone else must approve
diff --git a/src/Kconfig b/src/Kconfig
index ef3c72c..b6dc67f 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -74,7 +74,7 @@
config COMPILER_LLVM_CLANG
bool "LLVM/clang"
- depends on ALLOW_EXPERIMENTAL_CLANG || ARCH_SUPPORTS_CLANG
+ depends on ALLOW_EXPERIMENTAL_CLANG || !CLANG_UNSUPPORTED
help
Use LLVM/clang to build coreboot. To use this, you must build the
coreboot version of the clang compiler. Run the command
@@ -85,15 +85,15 @@
endchoice
-config ARCH_SUPPORTS_CLANG
+config CLANG_UNSUPPORTED
bool
help
- Opt-in flag for architectures that generally work well with CLANG.
- By default the option would be hidden.
+ Set this flag on platforms that do not support building with the
+ clang compiler.
config ALLOW_EXPERIMENTAL_CLANG
bool "Allow experimental LLVM/Clang"
- depends on !ARCH_SUPPORTS_CLANG
+ depends on CLANG_UNSUPPORTED
help
On some architectures CLANG does not work that well.
Use this only to try to get CLANG working.
diff --git a/src/arch/arm/Kconfig b/src/arch/arm/Kconfig
index 64fe915..0829dcb 100644
--- a/src/arch/arm/Kconfig
+++ b/src/arch/arm/Kconfig
@@ -2,6 +2,7 @@
config ARCH_ARM
bool
+ select CLANG_UNSUPPORTED
config ARCH_BOOTBLOCK_ARM
bool
diff --git a/src/arch/arm64/Kconfig b/src/arch/arm64/Kconfig
index af5050b..ea7b5aa 100644
--- a/src/arch/arm64/Kconfig
+++ b/src/arch/arm64/Kconfig
@@ -2,6 +2,7 @@
config ARCH_ARM64
bool
+ select CLANG_UNSUPPORTED
config ARCH_BOOTBLOCK_ARM64
bool
diff --git a/src/arch/ppc64/Kconfig b/src/arch/ppc64/Kconfig
index 93e4929..25a0f50 100644
--- a/src/arch/ppc64/Kconfig
+++ b/src/arch/ppc64/Kconfig
@@ -2,7 +2,6 @@
config ARCH_PPC64
bool
- select ARCH_SUPPORTS_CLANG
config ARCH_BOOTBLOCK_PPC64
bool
diff --git a/src/arch/riscv/Kconfig b/src/arch/riscv/Kconfig
index b570b01..66c64c2 100644
--- a/src/arch/riscv/Kconfig
+++ b/src/arch/riscv/Kconfig
@@ -10,7 +10,6 @@
config ARCH_RISCV
bool
- select ARCH_SUPPORTS_CLANG
if ARCH_RISCV
diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig
index c0fe6dc..16d8a70 100644
--- a/src/arch/x86/Kconfig
+++ b/src/arch/x86/Kconfig
@@ -5,7 +5,6 @@
select PCI
select RELOCATABLE_MODULES
select HAVE_ASAN_IN_RAMSTAGE
- select ARCH_SUPPORTS_CLANG
if ARCH_X86
--
To view, visit https://review.coreboot.org/c/coreboot/+/81659?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib28e7a4cb286b9f8b05be94dae3947179f43c746
Gerrit-Change-Number: 81659
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: coreboot org <coreboot.org(a)gmail.com>
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin Roth.
Matt DeVillier has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83773?usp=email )
Change subject: soc/amd/common/include/spi: add and use SPI_MISC_CNTRL define
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83773?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I48447dcfb3cee07619a9b42434731f0b21458021
Gerrit-Change-Number: 83773
Gerrit-PatchSet: 2
Gerrit-Owner: 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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:37:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Martin Roth has submitted this change. ( https://review.coreboot.org/c/blobs/+/83748?usp=email )
Change subject: soc/mediatek/mt8186: Update DRAM binary from 0.1.0 to 0.1.1
......................................................................
soc/mediatek/mt8186: Update DRAM binary from 0.1.0 to 0.1.1
For fast-k RX flow, Vref value is read from the MRC_CACHE, but the
preferred RX Vref value 0xE is set, with no re-calibration. But some
DRAM vendor may use higher RX Vref value, increase the default RX Vref
value (from full-k reference) to make different DRAM RX Vref compatible.
BUG=b:352632973
TEST=Check the Nanya DRAM fast-k RX Vref value is normal
Logs:
[3732][CH0][RK0][RX] Best Vref B0 = 22, Window Min 25 at DQ5 ...
[3732][CH0][RK0][RX] Best Vref B1 = 22, Window Min 28 at DQ10 ...
The "Best Vref" value is the same to full-k Vref.
Signed-off-by: Xi Chen <xixi.chen(a)mediatek.corp-partner.google.com>
Change-Id: Id7df502346d590d3cf3827f48d868da021f6ec9d
---
M soc/mediatek/mt8186/dram.elf
M soc/mediatek/mt8186/dram.elf.md5
M soc/mediatek/mt8186/dram_release_notes.txt
3 files changed, 9 insertions(+), 1 deletion(-)
Approvals:
Yidi Lin: Looks good to me, approved
Yu-Ping Wu: Verified; Looks good to me, approved
Paul Menzel: Looks good to me, but someone else must approve
diff --git a/soc/mediatek/mt8186/dram.elf b/soc/mediatek/mt8186/dram.elf
index 0f80e3d..60f3088 100644
--- a/soc/mediatek/mt8186/dram.elf
+++ b/soc/mediatek/mt8186/dram.elf
Binary files differ
diff --git a/soc/mediatek/mt8186/dram.elf.md5 b/soc/mediatek/mt8186/dram.elf.md5
index 97bbb16..48d7cd4 100644
--- a/soc/mediatek/mt8186/dram.elf.md5
+++ b/soc/mediatek/mt8186/dram.elf.md5
@@ -1 +1 @@
-5c7f96716ec681544803301184a33f4b *dram.elf
+2b51261de00b16312d933f24bf52ca9e *dram.elf
diff --git a/soc/mediatek/mt8186/dram_release_notes.txt b/soc/mediatek/mt8186/dram_release_notes.txt
index 16e773d..abfa421 100644
--- a/soc/mediatek/mt8186/dram_release_notes.txt
+++ b/soc/mediatek/mt8186/dram_release_notes.txt
@@ -1,3 +1,11 @@
+# 0.1.1
+
+1. ChromeOS build from 15979.0.0.
+
+2. Included changes:
+
+- CL:*7546745: mtk-dramk/mt8186: Fix fast-k RX Vref smaller issue
+
# 0.1.0
1. A local build.
--
To view, visit https://review.coreboot.org/c/blobs/+/83748?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: blobs
Gerrit-Branch: main
Gerrit-Change-Id: Id7df502346d590d3cf3827f48d868da021f6ec9d
Gerrit-Change-Number: 83748
Gerrit-PatchSet: 6
Gerrit-Owner: Xixi Chen <xixi.chen(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com>
Gerrit-CC: John Su <john_su(a)compal.corp-partner.google.com>
Attention is currently required from: Felix Singer, Julius Werner, Lean Sheng Tan, Maximilian Brune, Nico Huber.
Angel Pons has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83615?usp=email )
Change subject: util/cbfstool/cbfs-payload-linux: Do not compress bzImage
......................................................................
Patch Set 3: Code-Review+1
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83615/comment/c7d63b39_7dae75b1?us… :
PS3, Line 12: erros
typo: errors
File util/cbfstool/cbfs-payload-linux.c:
https://review.coreboot.org/c/coreboot/+/83615/comment/aefd8b00_e73f4296?us… :
PS3, Line 159: comp_func_ptr compress_func = compression_function(algo);
Looks like `algo` only needs to be different when `type == PAYLOAD_SEGMENT_CODE`. Would it make sense to keep `enum cbfs_compression algo` in `struct bzpayload` to avoid having to pass the algo as parameter?
```
/* There is no point in compressing the bzImage (it is already compressed) */
const cbfs_compression algo = type == PAYLOAD_SEGMENT_CODE ? CBFS_COMPRESS_NONE : bzp->algo;
comp_func_ptr compress_func = compression_function(algo);
compress_func(buffer_get(b), buffer_size(b), buffer_get(&out), &len);
```
https://review.coreboot.org/c/coreboot/+/83615/comment/be068c8b_41740b56?us… :
PS3, Line 294: // There is no point in compressing the bzImage (it is already compressed)
nit: consistent comment style
--
To view, visit https://review.coreboot.org/c/coreboot/+/83615?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I022982667515ce721d98af534414d9e336b5f35a
Gerrit-Change-Number: 83615
Gerrit-PatchSet: 3
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.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: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:35:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Elyes Haouas, Eran Mitrani, Felix Singer, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Ravishankar Sarawadi, Saurabh Mishra, Tarun.
Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83354?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock
......................................................................
Patch Set 53:
(3 comments)
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/fc8bc271_e719868f?us… :
PS51, Line 19: #define SAF_BASE_ADDRESS 0x3ffe000000
: #define SAF_BASE_SIZE 0x2000000
> We, , this is correct, SAF is moved below 4GB, but since there's a difficulty in sharing the intel restricted doc to be verified by you, i.e. FASv1.1, ideal way is to mark as "To-Do" and update with the previous value. Once the doc is ready to be sharing, we will update this to the newwer FAS aligned value. Let me know if this works?
In that case, can you please add a TODO here ?
File src/soc/intel/pantherlake/soc_info.c:
https://review.coreboot.org/c/coreboot/+/83354/comment/b216798f_ab59cee4?us… :
PS53, Line 31: CONFIG_MAX_ROOT_PORTS
where is this getting defined ? can you please check
you overlooked my review comment
https://review.coreboot.org/c/coreboot/+/83354/comment/6a9e0864_1f276911/
```
the same logic applies to others as well
```
https://review.coreboot.org/c/coreboot/+/83354/comment/e276ddd4_1f1585ce?us… :
PS53, Line 36: CONFIG_MAX_PCIE_CLOCK_SRC
where is this getting defined ? can you please check
--
To view, visit https://review.coreboot.org/c/coreboot/+/83354?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibcfe71eec27cebf04f10ec343a73dd92f1272aca
Gerrit-Change-Number: 83354
Gerrit-PatchSet: 53
Gerrit-Owner: Saurabh Mishra <mishra.saurabh(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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:33:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier.
Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83778?usp=email )
Change subject: soc/amd/common/psp_smi_flash: add buffer overflow checks
......................................................................
Patch Set 2:
(2 comments)
File src/soc/amd/common/block/psp/psp_smi_flash.c:
https://review.coreboot.org/c/coreboot/+/83778/comment/8a687d79_03bcca0d?us… :
PS1, Line 114: return num_bytes <= cmd_buf_size - payload_buffer_offset;
would be good if you can double-check in the review that i don't have an off-by-one here
https://review.coreboot.org/c/coreboot/+/83778/comment/16577b2b_efd2e35c?us… :
PS1, Line 251: if (!is_valid_rw_byte_count(cmd_buf, num_bytes))
: return MBOX_PSP_COMMAND_PROCESS_ERROR;
> Do we want to print a warning or error here and below? Maybe an assert so bad things happen if fatal […]
adding a warning sounds good to me; will do
--
To view, visit https://review.coreboot.org/c/coreboot/+/83778?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib4e8514eedc3ad154a705c8a1e85d367e452dbed
Gerrit-Change-Number: 83778
Gerrit-PatchSet: 2
Gerrit-Owner: 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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:33:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier, ritul guru.
Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83777?usp=email )
Change subject: soc/amd/common/psp_smi_flash: implement SPI read/wrire/erase command
......................................................................
Patch Set 2:
(1 comment)
File src/soc/amd/common/block/psp/psp_smi_flash.c:
https://review.coreboot.org/c/coreboot/+/83777/comment/4283afdc_34700c98?us… :
PS1, Line 237: }
> do we need to mark the spi controller as busy when we're using it? If not, what keeps something else […]
this code is in smm, so when that code gets interrupted, we have much larger issues, since the smm code isn't expected to ever be interrupted. i'll add something to the commit message
--
To view, visit https://review.coreboot.org/c/coreboot/+/83777?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4957a6d316015cc7037acf52facb6cc69188d446
Gerrit-Change-Number: 83777
Gerrit-PatchSet: 2
Gerrit-Owner: 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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:32:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83778?usp=email )
Change subject: soc/amd/common/psp_smi_flash: add buffer overflow checks
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/psp/psp_smi_flash.c:
https://review.coreboot.org/c/coreboot/+/83778/comment/38b76940_fe60f990?us… :
PS1, Line 251: if (!is_valid_rw_byte_count(cmd_buf, num_bytes))
: return MBOX_PSP_COMMAND_PROCESS_ERROR;
Do we want to print a warning or error here and below? Maybe an assert so bad things happen if fatal asserts are enabled (as they should be in debug builds)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83778?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib4e8514eedc3ad154a705c8a1e85d367e452dbed
Gerrit-Change-Number: 83778
Gerrit-PatchSet: 1
Gerrit-Owner: 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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:31:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier, ritul guru.
Felix Held has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83740?usp=email )
Change subject: soc/amd/common/psp_smi: implement P2C mailbox handling
......................................................................
Patch Set 5:
(3 comments)
Patchset:
PS4:
> it's software that talks to the other psp mailbox, so i'd say that it should be in amd/common/block/ […]
i'd like to keep this in this folder, since it belongs thematically to the other files in there and also uses a local header file shared by the files in that folder
File src/soc/amd/common/block/psp/psp_smi.c:
https://review.coreboot.org/c/coreboot/+/83740/comment/baee5751_311dcbf5?us… :
PS4, Line 33: tmp
> Nit: I know that the union is already named status, but why tmp and not something like 'status'? I m […]
good point; done
https://review.coreboot.org/c/coreboot/+/83740/comment/e49938f8_d2f380d2?us… :
PS4, Line 80: u32
> Nit: this seems to use a mix of u8/u32 and uint8_t/uint32_t. […]
the existing parts of the psp code in coreboot already use a mix of both; i can look into using only one type in some follow up in the future, but would prefer to not change this right now, since the scope is larger than just what gets added here. marking this as resolved for now
--
To view, visit https://review.coreboot.org/c/coreboot/+/83740?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I50479bed2332addae652026c6818460eeb6403af
Gerrit-Change-Number: 83740
Gerrit-PatchSet: 5
Gerrit-Owner: 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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:29:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>