Attention is currently required from: Arthur Heymans, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Hello Arthur Heymans, Kapil Porwal, Nick Vaccaro, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80403?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Code-Review+2 by Arthur Heymans, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: soc/intel/alderlake: Leverage IA common code for range calculations
......................................................................
soc/intel/alderlake: Leverage IA common code for range calculations
Improves code maintainability and potentially reduces redundancy by
using the IA common implementation.
Additionally, drop the unused macros from SoC local.
TEST=Build and boot successful on google/marasov.
Change-Id: I290fea99f04cfc9f18e5f1435ed07de42995869f
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/include/soc/systemagent.h
M src/soc/intel/alderlake/systemagent.c
2 files changed, 4 insertions(+), 126 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/80403/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/80403?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: I290fea99f04cfc9f18e5f1435ed07de42995869f
Gerrit-Change-Number: 80403
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Hello Arthur Heymans, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Tarun, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80404?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Code-Review+2 by Arthur Heymans, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: soc/intel/meteorlake: Leverage IA common code for range calculations
......................................................................
soc/intel/meteorlake: Leverage IA common code for range calculations
Improves code maintainability and potentially reduces redundancy by
using the IA common implementation.
Additionally, drop the unused macros from SoC local.
TEST=Build and boot successful on google/screebo.
Change-Id: Ie0baae1d3b0093389649dee3531902c5e86c02fe
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/meteorlake/include/soc/systemagent.h
M src/soc/intel/meteorlake/systemagent.c
2 files changed, 4 insertions(+), 126 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/80404/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/80404?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: Ie0baae1d3b0093389649dee3531902c5e86c02fe
Gerrit-Change-Number: 80404
Gerrit-PatchSet: 8
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: build bot (Jenkins) <no-reply(a)coreboot.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: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Kapil Porwal, Nick Vaccaro, sridhar siricilla.
Hello Arthur Heymans, Kapil Porwal, Nick Vaccaro, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80362?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/intel/cmn/sa: Add APIs into System Agent (SA) common code
......................................................................
soc/intel/cmn/sa: Add APIs into System Agent (SA) common code
This commit streamlines code and strengthens common code robustness
by moving the following SoC-layer functions to the common layer:
- sa_get_mmcfg_size: Retrieves the MMIO (Memory-Mapped I/O)
configuration space size by reading offset
0x60 of the PCI Host Bridge (D0:F0).
- sa_get_dsm_size: Calculates the size of the DSM (Device Stolen
Memory) by reading offset 0x50 of the PCI
Host Bridge (D0:F0) to determine pre-allocated
memory for the IGD (Integrated Graphics Device).
- sa_get_gsm_size: Calculates the size of the GSM (Graphics Stolen
Memory) by reading offset 0x52 of the PCI Host
Bridge (D0:F0).
- sa_get_dpr_size: Determines the size of the DMA Protection
Range (DPR) by reading offset 0x5C of the PCI
Host Bridge (D0:F0).
TEST= Build and boot successful on google/screebo.
Change-Id: Ic00e001563ec6f0d737a445964c716b45db43327
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/common/block/include/intelblocks/systemagent.h
M src/soc/intel/common/block/systemagent/systemagent.c
M src/soc/intel/common/block/systemagent/systemagent_def.h
3 files changed, 123 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/80362/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/80362?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: Ic00e001563ec6f0d737a445964c716b45db43327
Gerrit-Change-Number: 80362
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Ivy Jian, Nick Vaccaro, Paul Menzel, Shelley Chen.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80300?usp=email )
Change subject: mb/google/brox: Initialize TCHSCR_RST_L to 0
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
Patchset:
PS4:
> Marking resolved unless there is still something in question?
no I am fine with that.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80300?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: I5bf1901a3a40a38237b950abcb758f96aebcc1cf
Gerrit-Change-Number: 80300
Gerrit-PatchSet: 5
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Fri, 09 Feb 2024 10:36:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Shelley Chen <shchen(a)google.com>
Comment-In-Reply-To: Eric Lai <ericllai(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Nick Vaccaro, SH Kim, Subrata Banik.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80326?usp=email )
Change subject: mb/google/brya/var/xol: Update memory configuration
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80326?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: I59aabe0870317092f59701bdf88b53bf9731377a
Gerrit-Change-Number: 80326
Gerrit-PatchSet: 5
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(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: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Fri, 09 Feb 2024 10:35:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal, Nick Vaccaro, SH Kim.
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80410?usp=email )
Change subject: mb/google/brya/var/xol: Update DTT policy
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80410?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: I546b313a1e6af16029309174a5bed2d1e4aa4d11
Gerrit-Change-Number: 80410
Gerrit-PatchSet: 2
Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.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-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Fri, 09 Feb 2024 10:35:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Johnny Lin, Nico Huber, Shuo Liu, Tim Chu.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80093?usp=email )
Change subject: soc/intel/xeon_sp: Locate PCU by PCI device ID
......................................................................
Patch Set 11:
(1 comment)
File src/soc/intel/xeon_sp/skx/soc_util.c:
https://review.coreboot.org/c/coreboot/+/80093/comment/12c31587_38a6ef66 :
PS11, Line 115: #if ENV_RAMSTAGE
> i wonder why this #if is needed here while it's not needed for the other 2 socs that this patch chan […]
Since soc_util.c is compiled into romstage as well, while chip.c isn't. The pci_*config32 functions expect a pci_devfn_t instead of struct device * as first argument.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80093?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: I06694715cba76b101165f1cef66d161b0f896b26
Gerrit-Change-Number: 80093
Gerrit-PatchSet: 11
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Fri, 09 Feb 2024 10:31:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik, sridhar siricilla.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80362?usp=email )
Change subject: soc/intel/cmn/sa: Add APIs into System Agent (SA) common code
......................................................................
Patch Set 7:
(2 comments)
File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/80362/comment/0aa3ecba_b2db6240 :
PS6, Line 327: const struct device *dev
> SA is nothing but PCI_0_00_0 aka host bridge.
>
\_\_pci_0_00_0' is a real reference in the static.c code. You can do pci_read_config32(\_\_pci_0_00_0, PCIEXBAR);
> are you suggesting to use rather passing the `dev` argument to the function?
>
> ```
> struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT);
> ```
You can get a reference from the devicetree struct device directly, rather than using runtime functions (see build/mainboard/../../static.c) . Using the alias is indeed a better idea than the reference via pci numbers: _dev_system_agent_ptr would be it.
The advantage of this over the runtime, is that you get a direct reference (no searching for it in tree), and the if the device is not there (it's in chipset.cb so that won't ever happen) the linker will complain, which is a buildtime error, which are better than runtime ones.
https://review.coreboot.org/c/coreboot/+/80362/comment/6e520818_49d31f7c :
PS6, Line 432: sa_get_dpr_size
> > txt_get_chipset_dpr() in systemagent_early.c does the same.
>
> good candidate for the convergence at some point. At least it covers the raw read from the register (like what line#435 does below) and additionally, this function returning the size in MiB (which may not be needed for early code piece).
>
> I will pick that later for convergence. hoping u r good with that plan ?
sounds good.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80362?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: Ic00e001563ec6f0d737a445964c716b45db43327
Gerrit-Change-Number: 80362
Gerrit-PatchSet: 7
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Fri, 09 Feb 2024 09:44:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80253?usp=email )
Change subject: libpayload: Switch to commonlib ipchksum() algorithm
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> Yeah, I guess FILO was just always defining a function with the same name, and due to the weirdness […]
Nico does this in FILO change-set Ifedbe9241426198fb0cba7b9bfc43b91cd6370eb [1].
[1]: https://review.coreboot.org/c/filo/+/80390/
--
To view, visit https://review.coreboot.org/c/coreboot/+/80253?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: Ie8d323ce9f8d946758619761b4b22d54bce222b6
Gerrit-Change-Number: 80253
Gerrit-PatchSet: 4
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.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-Comment-Date: Fri, 09 Feb 2024 09:30:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment