coreboot-gerrit June 2024

coreboot-gerrit@coreboot.org

1 participants
3527 discussions

[S] Change in coreboot[main]: commonlib/bsd/lz4_wrapper.c: Fix misaligned access
by Nico Huber (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Angel Pons, Arthur Heymans, Julius Werner, Maximilian Brune, Philipp Hug, Ron Minnich. Nico Huber has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/81910?usp=email ) Change subject: commonlib/bsd/lz4_wrapper.c: Fix misaligned access ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81910/comment/d4ea5dcd_70529912?us… : PS1, Line 18: The coreboot LZ4 decompression code does some misaligned access during : decompression > > NB. I'd expect that inlining LZ4_readLE16() has a bigger effect. > > No the actual lz4c.c.inc is included below and the compiler is sufficiently smart to do that already. Oh, right. à propos: ``` $ for cc in i386-elf x86_64-elf arm-eabi aarch64-elf riscv64-elf; do > echo $cc > echo 'unsigned short test(void *a) { return *(unsigned short *)a; }' | $cc-gcc -o test1.o -c -xc -Os - > echo 'unsigned short test(void *a) { return *(unsigned char *)a | *(unsigned char *)(a + 1) << 8; }' | $cc-gcc -o test2.o -c -xc -Os - > cmp test[12].o > done i386-elf x86_64-elf arm-eabi test1.o test2.o sind verschieden: Byte 33, Zeile 1 aarch64-elf riscv64-elf test1.o test2.o sind verschieden: Byte 41, Zeile 1 ``` `arm-eabi` passes too for any of `-march=armv7-[arm]`. Outing: I'm one of those weirdos who'd like to keep it as close as possible to defined C and let the compiler do its job. IOW, use read_le16() and friends. -- To view, visit https://review.coreboot.org/c/coreboot/+/81910?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: Id165829bfd35be2bce2bbb019c208a304f627add Gerrit-Change-Number: 81910 Gerrit-PatchSet: 5 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 11:27:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[M] Change in coreboot[main]: soc/intel: Adapt crashlog IP to also support 64-bit
by Sowmya Aralguppe (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Paul Menzel, Pratikkumar V Prajapati, Subrata Banik, Tarun. Sowmya Aralguppe has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83106?usp=email ) Change subject: soc/intel: Adapt crashlog IP to also support 64-bit ...................................................................... Patch Set 21: (9 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/83106/comment/016116e8_bfcb43ac?us… : PS19, Line 7: Enable crashlog IP for both 32-bit and 64-bit architectures > Adapt crashlog IP to also support 64-bit Done https://review.coreboot.org/c/coreboot/+/83106/comment/ce53f46a_98bd12b1?us… : PS19, Line 10: future generation SoCs > Aren’t current SoCs already 64-bit? From PTL onwards FSP will support only 64bit architecture .I think LNL had both 32 and 64 bit support https://review.coreboot.org/c/coreboot/+/83106/comment/f5d660e3_cd394946?us… : PS19, Line 12: > Please summarize the changes. […] Acknowledged https://review.coreboot.org/c/coreboot/+/83106/comment/41ea6fc8_25c8b91f?us… : PS19, Line 14: tested > Could you please elaborate on what was tested? As per my review comments, I doubt that reading Crash […] I am testing and comparing the boot logs - local copy for testing has src address incremented by 4 . File src/soc/intel/common/block/crashlog/crashlog.c: https://review.coreboot.org/c/coreboot/+/83106/comment/01c789f6_3819329e?us… : PS15, Line 321: sizeof(uintptr_t) > no sorry - in my local copy src_address is incremented by 4 since we are reading dw and it is uintp […] Done File src/soc/intel/common/block/crashlog/crashlog.c: https://review.coreboot.org/c/coreboot/+/83106/comment/299ef9cb_1a8090e8?us… : PS19, Line 318: /* DW by DW copy: byte access to PMC SRAM not allowed */ > Try adding this, then build for x86_64: […] Acknowledged https://review.coreboot.org/c/coreboot/+/83106/comment/b6e82152_d4b37e75?us… : PS19, Line 321: src_addr += sizeof(uintptr_t); > On 64-bit builds, this can't possibly work. `read32p()` always reads 4 bytes, but the address gets incremented by `sizeof(uintptr_t)`, which is 4 bytes on 32-bit builds but 8 bytes on 64-bit builds. > > Please change: > > ```suggestion > src_addr += sizeof(u32); > ``` I had uploaded the wrong patchset sorry - In my local copy i had incremented by 4 while testing File src/soc/intel/meteorlake/crashlog.c: https://review.coreboot.org/c/coreboot/+/83106/comment/d8c5960a_77b00f35?us… : PS19, Line 354: if (!(cl_buffer >> 32)) > it is cl buffer because it is same as cpu_cl_disc_tab.buffers[i]. […] Done https://review.coreboot.org/c/coreboot/+/83106/comment/5483d56f_b029875f?us… : PS19, Line 357: u32 dw0 = read32p(disc_tab_addr + cur_offset); > Crashlog_data_valid This is done for reading the header in line 342 […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?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: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 21 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(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: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 10:38:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[M] Change in coreboot[main]: soc/intel: Adapt crashlog IP to also support 64-bit
by Sowmya Aralguppe (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun. Hello Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83106?usp=email to look at the new patch set (#21). Change subject: soc/intel: Adapt crashlog IP to also support 64-bit ...................................................................... soc/intel: Adapt crashlog IP to also support 64-bit This patch extends the crashlog IP support beyond 32-bit mode to support Intel future generation SoCs, which may require crashlog support for 64-bit architectures. uintptr_t data type is used for Address pointers and void* for dereferencing BUG=b:346676856 TEST=Successfully built Meteor Lake (rex) and tested for google/rex0 and google/rex64 images. Change-Id: I552257d3770abb409e2dcd8a13392506b5e7feb7 Signed-off-by: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> --- M src/soc/intel/alderlake/crashlog.c M src/soc/intel/common/block/crashlog/crashlog.c M src/soc/intel/common/block/include/intelblocks/crashlog.h M src/soc/intel/meteorlake/crashlog.c M src/soc/intel/tigerlake/crashlog_lib.c 5 files changed, 76 insertions(+), 77 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/83106/21 -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?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: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 21 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/nivviks: Disable CNVi Bluetooth based on fw_config
by Poornima Tom (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Hello Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Poornima Tom, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83148?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: mb/google/nissa/var/nivviks: Disable CNVi Bluetooth based on fw_config ...................................................................... mb/google/nissa/var/nivviks: Disable CNVi Bluetooth based on fw_config When CNVi based Wifi6 is disabled, CNVi based Bluetooth must be turned off, based on fw_config. Otherwise, when device boots without the cbi settings for wifi6, boot may fail with assertion error for line 817 & 819 of file 'src/soc/intel/alderlake/fsp_params.c'. BUG=b:345596420 BRANCH=NONE TEST=Dut boots fine with both Wifi6 & Wifi7 based cbi settings, along with enumeration of corresponding BT device. Change-Id: I03fde02fa4b36f4e47d6f0e95675feddb3bee7cd Signed-off-by: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com> --- M src/mainboard/google/brya/variants/nivviks/fw_config.c 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/83148/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/83148?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: I03fde02fa4b36f4e47d6f0e95675feddb3bee7cd Gerrit-Change-Number: 83148 Gerrit-PatchSet: 2 Gerrit-Owner: Poornima Tom <poornima.tom(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>

1 0

[M] Change in coreboot[main]: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures
by Sowmya Aralguppe (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun. Sowmya Aralguppe has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83106?usp=email ) Change subject: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures ...................................................................... Patch Set 20: (3 comments) File src/soc/intel/meteorlake/crashlog.c: https://review.coreboot.org/c/coreboot/+/83106/comment/413816a3_e73315a2?us… : PS19, Line 352: cl_buffer > The lopping is done for number of child crashlog records and each child crahlog will hold buffer si […] I have changed the name as buffer size https://review.coreboot.org/c/coreboot/+/83106/comment/b05f4f45_c7f7a26f?us… : PS19, Line 353: */ > will correct it Done File src/soc/intel/tigerlake/crashlog_lib.c: https://review.coreboot.org/c/coreboot/+/83106/comment/d160236b_acda51bf?us… : PS19, Line 165: bar_addr > yes correct - will correct it - i had not changed anything in TGL other than pointer casting that ar […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?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: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 20 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 10:25:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[M] Change in coreboot[main]: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures
by Sowmya Aralguppe (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun. Hello Angel Pons, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/83106?usp=email to look at the new patch set (#20). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures ...................................................................... soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures This patch extends the crashlog IP support beyond 32-bit mode to support Intel future generation SoCs, which may require crashlog support for 64-bit architectures. BUG=b:346676856 TEST=Successfully built Meteor Lake (rex) and tested for google/rex0 and google/rex64 images. Change-Id: I552257d3770abb409e2dcd8a13392506b5e7feb7 Signed-off-by: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> --- M src/soc/intel/alderlake/crashlog.c M src/soc/intel/common/block/crashlog/crashlog.c M src/soc/intel/common/block/include/intelblocks/crashlog.h M src/soc/intel/meteorlake/crashlog.c M src/soc/intel/tigerlake/crashlog_lib.c 5 files changed, 76 insertions(+), 77 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/83106/20 -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?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: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 20 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/nivviks: Disable CNVi Bluetooth based on fw_config
by Poornima Tom (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Poornima Tom, Subrata Banik. Poornima Tom has posted comments on this change by Poornima Tom. ( https://review.coreboot.org/c/coreboot/+/83148?usp=email ) Change subject: mb/google/nissa/var/nivviks: Disable CNVi Bluetooth based on fw_config ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/brya/variants/nivviks/fw_config.c: https://review.coreboot.org/c/coreboot/+/83148/comment/8fdb5341_a2d51b87?us… : PS1, Line 120: } > shouldn't we also disable the cnvi wifi core too ? […] I think for cnvi wifi core , the function below already takes care - is_devfn_enabled(PCH_DEVFN_CNVI_WIFI) while filling cnvi params. -- To view, visit https://review.coreboot.org/c/coreboot/+/83148?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: I03fde02fa4b36f4e47d6f0e95675feddb3bee7cd Gerrit-Change-Number: 83148 Gerrit-PatchSet: 1 Gerrit-Owner: Poornima Tom <poornima.tom(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jun 2024 10:17:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[M] Change in coreboot[main]: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures
by Sowmya Aralguppe (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Subrata Banik, Tarun. Sowmya Aralguppe has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83106?usp=email ) Change subject: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures ...................................................................... Patch Set 19: (5 comments) File src/soc/intel/meteorlake/crashlog.c: https://review.coreboot.org/c/coreboot/+/83106/comment/b99d2e37_65886948?us… : PS19, Line 352: cl_buffer > this is now combined dw0 and dw1, if so, then why are we calling it as `cl_buffer`. […] The lopping is done for number of child crashlog records and each child crahlog will hold buffer size and offset where the CL is . It is named same as cpu_cl_disc_tab.buffers[i].data . https://review.coreboot.org/c/coreboot/+/83106/comment/6e1b176f_dc83118e?us… : PS19, Line 353: */ > space before ? will correct it https://review.coreboot.org/c/coreboot/+/83106/comment/041dad2a_05e95f9a?us… : PS19, Line 354: if (!(cl_buffer >> 32)) > this can check if MSB is zero or not. Not sure what is the purpose ? […] it is cl buffer because it is same as cpu_cl_disc_tab.buffers[i].data - the upper 32 bits reads buffer size and lower 32 bytes is offset .If the size is 0 then there is no point in going further so we continue https://review.coreboot.org/c/coreboot/+/83106/comment/3593abe7_ccb49524?us… : PS19, Line 357: u32 dw0 = read32p(disc_tab_addr + cur_offset); > this is same as https://review.coreboot.org/c/coreboot/+/83106/19/src/soc/intel/meteorlake/…. […] Crashlog_data_valid This is done for reading the header in line 342 The 31st bit will be set if the crashlog record has been already consumed File src/soc/intel/tigerlake/crashlog_lib.c: https://review.coreboot.org/c/coreboot/+/83106/comment/bd4739e4_f94f49b1?us… : PS19, Line 165: bar_addr > we should check if this value is non-zero ? yes correct - will correct it - i had not changed anything in TGL other than pointer casting that are in common code -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?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: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 19 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 09:32:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/nivviks: Disable CNVi Bluetooth based on fw_config
by Subrata Banik (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Poornima Tom, Poornima Tom. Subrata Banik has posted comments on this change by Poornima Tom. ( https://review.coreboot.org/c/coreboot/+/83148?usp=email ) Change subject: mb/google/nissa/var/nivviks: Disable CNVi Bluetooth based on fw_config ...................................................................... Patch Set 1: (1 comment) File src/mainboard/google/brya/variants/nivviks/fw_config.c: https://review.coreboot.org/c/coreboot/+/83148/comment/e76b31d0_336ccc9b?us… : PS1, Line 120: } shouldn't we also disable the cnvi wifi core too ? ideally I would say, if !wifi_6 then make sure to disable both cnvi wifi and bt -- To view, visit https://review.coreboot.org/c/coreboot/+/83148?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: I03fde02fa4b36f4e47d6f0e95675feddb3bee7cd Gerrit-Change-Number: 83148 Gerrit-PatchSet: 1 Gerrit-Owner: Poornima Tom <poornima.tom(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Poornima Tom <poornima.tom(a)intel.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jun 2024 09:19:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures
by Angel Pons (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Pratikkumar V Prajapati, Sowmya Aralguppe, Subrata Banik, Tarun. Angel Pons has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/83106?usp=email ) Change subject: soc/intel: Enable crashlog IP for both 32-bit and 64-bit architectures ...................................................................... Patch Set 19: (2 comments) File src/soc/intel/common/block/crashlog/crashlog.c: https://review.coreboot.org/c/coreboot/+/83106/comment/bfbac6a1_b7fa5ad4?us… : PS19, Line 318: /* DW by DW copy: byte access to PMC SRAM not allowed */ Try adding this, then build for x86_64: ```suggestion /* DW by DW copy: byte access to PMC SRAM not allowed */ _Static_assert(sizeof(u32) == sizeof(uintptr_t), "this is not a DW by DW copy"); ``` This will error out. So you have to apply the suggestion below (and remove the assertion). https://review.coreboot.org/c/coreboot/+/83106/comment/41544e65_38c3ab33?us… : PS19, Line 321: src_addr += sizeof(uintptr_t); On 64-bit builds, this can't possibly work. `read32p()` always reads 4 bytes, but the address gets incremented by `sizeof(uintptr_t)`, which is 4 bytes on 32-bit builds but 8 bytes on 64-bit builds. Please change: ```suggestion src_addr += sizeof(u32); ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/83106?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: I552257d3770abb409e2dcd8a13392506b5e7feb7 Gerrit-Change-Number: 83106 Gerrit-PatchSet: 19 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.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: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 09:07:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2024