Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80251?usp=email )
Change subject: lib: Move IP checksum to commonlib
......................................................................
Patch Set 4:
(1 comment)
File src/commonlib/bsd/include/commonlib/bsd/ipchksum.h:
https://review.coreboot.org/c/coreboot/+/80251/comment/279a2c3c_0a69a7ac :
PS4, Line 9: uint16_t ipchksum(const void *data, size_t size);
: uint16_t ipchksum_add(size_t offset, uint16_t first, uint16_t second);
> Because the checksum is defined as a 16-bit number. […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/80251?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: Ic04c714c00439a17fc04a8a6e730cc2aa19b8e68
Gerrit-Change-Number: 80251
Gerrit-PatchSet: 4
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Fri, 09 Feb 2024 09:28:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Kapil Porwal, Nick Vaccaro, sridhar siricilla.
Subrata Banik 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/3cb5a7b4_21b6ad9b :
PS6, Line 327: const struct device *dev
> You know which device the sa. […]
SA is nothing but PCI_0_00_0 aka host bridge.
are you suggesting to use rather passing the `dev` argument to the function?
```
struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT);
```
https://review.coreboot.org/c/coreboot/+/80362/comment/60e6f259_e927845f :
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 ?
--
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: 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-Comment-Date: Fri, 09 Feb 2024 09:27:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80403?usp=email )
Change subject: soc/intel/alderlake: Leverage IA common code for range calculations
......................................................................
Patch Set 7: Code-Review+2
--
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: 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)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-Comment-Date: Fri, 09 Feb 2024 08:40:05 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80404?usp=email )
Change subject: soc/intel/meteorlake: Leverage IA common code for range calculations
......................................................................
Patch Set 7: Code-Review+2
--
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: 7
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: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Fri, 09 Feb 2024 08:39:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, 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 6: Code-Review+1
(6 comments)
Patchset:
PS6:
Looks good. I have a few suggestions.
File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/80362/comment/47cfb8d4_e251c4cc :
PS6, Line 327: const struct device *dev
You know which device the sa. Why not use that directly use _pci_0_00_0? Then you have compiletime checks vs runtime null check + your sure to use the right device.
Some for all the ones below.
https://review.coreboot.org/c/coreboot/+/80362/comment/4b546c18_8db06c03 :
PS6, Line 340: BIOS_DEBUG
Probably should be BIOS_ERR if CONFIG_ECAM_MMCONF_SUPPORT (that's always the case on modern HW)?
https://review.coreboot.org/c/coreboot/+/80362/comment/41ecc884_5b541956 :
PS6, Line 346: 4 * ((uint64_t)GiB);
'4ull * GiB' looks better IMO.
https://review.coreboot.org/c/coreboot/+/80362/comment/3ddd741e_36ef56a0 :
PS6, Line 367: BIOS_DEBUG
BIOS_ERR
https://review.coreboot.org/c/coreboot/+/80362/comment/26513c81_0503c092 :
PS6, Line 432: sa_get_dpr_size
txt_get_chipset_dpr() in systemagent_early.c does the same.
--
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: 6
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: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Fri, 09 Feb 2024 08:39:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik.
Hello 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 (#7).
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/7
--
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: 7
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
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-MessageType: newpatchset
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun.
Hello 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 (#7).
The following approvals got outdated and were removed:
Verified-1 by build bot (Jenkins)
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/7
--
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: 7
Gerrit-Owner: Subrata Banik <subratabanik(a)google.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: 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: Tarun <tstuli(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik, sridhar siricilla.
Hello 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 (#7).
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, 131 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/80362/7
--
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: 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-MessageType: newpatchset
Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik, sridhar siricilla.
Hello 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 (#6).
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, 131 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/80362/6
--
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: 6
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
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-MessageType: newpatchset