coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3293 discussions

[S] Change in coreboot[main]: soc/intel/xeon_sp: Use fixed BDF for IBL
by Angel Pons (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Arthur Heymans, Christian Walter, David Hendricks, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, TangYiwei, Tim Chu. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81321?usp=email ) Change subject: soc/intel/xeon_sp: Use fixed BDF for IBL ...................................................................... Patch Set 20: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81321/comment/53199e1b_90bdd55a : PS20, Line 11: dyanmically typo: dynamically -- To view, visit https://review.coreboot.org/c/coreboot/+/81321?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: I3975cb00e215c4984c63bb8510e8aef7d4cc85a4 Gerrit-Change-Number: 81321 Gerrit-PatchSet: 20 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 26 Apr 2024 12:26:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City
by Angel Pons (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, David Hendricks, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81322?usp=email ) Change subject: mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City ...................................................................... Patch Set 28: (1 comment) File configs/builder/config.intel.crb.bnc: https://review.coreboot.org/c/coreboot/+/81322/comment/bbd3ba05_85e5cda9 : PS11, Line 27: # : CONFIG_IFD_BIN_PATH="site-local/beechnutcity/descriptor.bin" : CONFIG_CPU_UCODE_BINARIES="site-local/beechnutcity/ucode.mcb" : CONFIG_FSP_T_FILE="site-local/beechnutcity/Server_T.fd" : CONFIG_FSP_M_FILE="site-local/beechnutcity/Server_M.fd" : CONFIG_FSP_S_FILE="site-local/beechnutcity/Server_S.fd" : CONFIG_FSP_HEADER_PATH="src/vendorcode/intel/fsp/fsp2_0/graniterapids/sp/" > This is aligned with configs/builder/config.intel.crb. […] This describes the settings needed to build a bootable image for this board. As not all binaries are publicly available (yet), this config has to use site-local binaries. But when things eventually get published, the defaults in platform code should be updated. Ideally, the default config for a mainboard (just select board vendor and model, and maybe choose a compatible payload) should produce a bootable coreboot image. (Yes, it wouldn't include any IFD or ME regions, but you can flash the result to the BIOS region only and it should work). -- To view, visit https://review.coreboot.org/c/coreboot/+/81322?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: I3f6a0fb97b62baadb438fb9f11fdd78fccb3f89a Gerrit-Change-Number: 81322 Gerrit-PatchSet: 28 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Comment-Date: Fri, 26 Apr 2024 12:25:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/intel/avenuecity_crb: Add GNR/SRF-AP 2S server board Avenue City
by Angel Pons (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Arthur Heymans, David Hendricks, Jincheng Li, Jonathan Zhang, Lean Sheng Tan, Nicholas Chin, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei, Varshit Pandya. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81319?usp=email ) Change subject: mb/intel/avenuecity_crb: Add GNR/SRF-AP 2S server board Avenue City ...................................................................... Patch Set 50: (3 comments) File src/drivers/ocp/include/vpd.h: https://review.coreboot.org/c/coreboot/+/81319/comment/f9ec0d09_9f9b88ed : PS50, Line 6: #include <include/types.h> Huh? This header doesn't need `types.h` File src/mainboard/intel/avenuecity_crb/config/iio.c: https://review.coreboot.org/c/coreboot/+/81319/comment/cf189e74_46ff1c2d : PS36, Line 14: _IIO_PORT_CFG_STRUCT_X8(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x1), : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x2), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x3), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x4), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x5) > Yes, personally I agree. This is how FSP header defines and we have to follow ... […] If it's just the macros, we could define some macros to use in coreboot that use left-to-right order. Here's an example (`CB_` means coreboot), suggestions for better names are welcome: ``` #define CB_IIO_BIFURCATE_x8x2x2x2x2 IIO_BIFURCATE_x2x2x2x2x8 ``` File src/mainboard/intel/avenuecity_crb/config/iio.c: https://review.coreboot.org/c/coreboot/+/81319/comment/99b04f5c_579ed707 : PS50, Line 28: (sizeof(iio_config_table)/sizeof(struct iio_pe_config)) `ARRAY_SIZE(iio_config_table)` -- To view, visit https://review.coreboot.org/c/coreboot/+/81319?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: I64fdd5388aadf7732f6d3daa600c1455d3672a46 Gerrit-Change-Number: 81319 Gerrit-PatchSet: 50 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Fri, 26 Apr 2024 12:21:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp/gnr: Add IIO config utils
by Angel Pons (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Martin L Roth, Patrick Rudolph, Shuo Liu, TangYiwei, Tim Chu. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81318?usp=email ) Change subject: soc/intel/xeon_sp/gnr: Add IIO config utils ...................................................................... Patch Set 47: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/81318?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: If7146761db6f73a0c4b0d31b010c0d30a42bf690 Gerrit-Change-Number: 81318 Gerrit-PatchSet: 47 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 26 Apr 2024 12:15:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: MAINTAINERS: Add Granite Rapids FSP to Xeon-SP
by Angel Pons (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Paul Menzel, Shuo Liu. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82082?usp=email ) Change subject: MAINTAINERS: Add Granite Rapids FSP to Xeon-SP ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/82082?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: I5170a69d798d0e8198b89f6932a80e6051228ac2 Gerrit-Change-Number: 82082 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Comment-Date: Fri, 26 Apr 2024 12:14:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Support CHIPSET_LOCKDOWN_FSP
by Angel Pons (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, Tim Chu. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82081?usp=email ) Change subject: soc/intel/xeon_sp: Support CHIPSET_LOCKDOWN_FSP ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/82081?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: I8efcc1f27446be8e35f51e2568c4af6f8165486b Gerrit-Change-Number: 82081 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> 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: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 26 Apr 2024 12:14:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[main]: soc/intel/xeon_sp: Add Granite Rapids initial codes
by Angel Pons (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Jonathan Zhang, Nico Huber, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei, Tim Chu. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81316?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: soc/intel/xeon_sp: Add Granite Rapids initial codes ...................................................................... Patch Set 56: (8 comments) File src/soc/intel/xeon_sp/Makefile.mk: https://review.coreboot.org/c/coreboot/+/81316/comment/16bb83e9_dde16fb5 : PS56, Line 8: ## GNR IBL codes are initially reused from EBG, will update later. nit: Add a "TODO: " prefix? File src/soc/intel/xeon_sp/chip_gen6.c: https://review.coreboot.org/c/coreboot/+/81316/comment/5d678871_4ba894b3 : PS56, Line 35: static void iio_pci_domain_read_resources(struct device *dev) I'd recommend using the constructors in https://github.com/coreboot/coreboot/blob/90e835db2d2ef75ea7d0999090d1806cf… instead of manually specifying `IORESOURCE_` flags. Also, shouldn't all these resources be `IORESOURCE_FIXED` and `IORESOURCE_STORED`? https://github.com/coreboot/coreboot/blob/90e835db2d2ef75ea7d0999090d1806cf… https://review.coreboot.org/c/coreboot/+/81316/comment/76530acd_af9d0e8c : PS56, Line 56: res->flags = IORESOURCE_IO | IORESOURCE_SUBTRACTIVE | IORESOURCE_ASSIGNED; I asked on IRC, this resource should be `IORESOURCE_FIXED` so that the allocator doesn't try to allocate it. And `IORESOURCE_SUBTRACTIVE` shouldn't matter (the allocator doesn't allocate fixed resources). File src/soc/intel/xeon_sp/gnr/Kconfig: https://review.coreboot.org/c/coreboot/+/81316/comment/4e6ee804_04d19ce9 : PS56, Line 17: select FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND I don't remember if I mentioned this, but it would be great to fix FSP https://review.coreboot.org/c/coreboot/+/81316/comment/9d4f80cb_4fbf2d73 : PS56, Line 34: default 255 Given that previous gen platforms (e.g. ibm/sbp1) have more cores/threads than that, I imagine this is a placeholder. Probably embargo-related. File src/soc/intel/xeon_sp/gnr/romstage.c: https://review.coreboot.org/c/coreboot/+/81316/comment/c928080b_e75f3149 : PS56, Line 21: switch (base_addr) { OK, these are a bit tricky to handle because decimals, but how about: ``` static uint8_t get_mmcfg_base_upd_index(const uint64_t base_addr) { switch (base_addr) { case 1UL * GiB: // 1G return 0; case 1UL * GiB + 512UL * MiB: // 1.5G return 0x1; case 1UL * GiB + 768UL * MiB: // 1.75G return 0x2; case 2UL * GiB: // 2G return 0x3; case 2UL * GiB + 256UL * MiB: // 2.25G return 0x4; case 3UL * GiB: // 3G return 0x5; default: // Auto return 0x6; } } ``` I haven't tried, but this should compile fine. I know the comment alignment is wrong, Gerrit uses 4 spaces for tabs. File src/soc/intel/xeon_sp/spr/cpu.c: PS56: nit: Maybe move these changes to a separate commit? File src/soc/intel/xeon_sp/spr/soc_acpi.c: https://review.coreboot.org/c/coreboot/+/81316/comment/81418116_ce90b053 : PS56, Line 158: unsigned long xeonsp_acpi_create_madt_lapics(unsigned long current) Where did this go? -- To view, visit https://review.coreboot.org/c/coreboot/+/81316?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: I3084e1b5abf25d8d9504bebeaed2a15b916ed56b Gerrit-Change-Number: 81316 Gerrit-PatchSet: 56 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: TangYiwei Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 26 Apr 2024 12:14:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: drivers/crb,pc80/tpm: Add crb and pc80 prefixes to chip configs
by Felix Held (Code Review) 26 Apr '24

26 Apr '24

Attention is currently required from: Christian Walter, Jérémy Compostella, Michał Żygowski. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82057?usp=email ) Change subject: drivers/crb,pc80/tpm: Add crb and pc80 prefixes to chip configs ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: would probably be good to use the struct without hiding it behind a typedef and remove the typedef -- To view, visit https://review.coreboot.org/c/coreboot/+/82057?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: Id41717e265362303a17745303a907c9c8f4f4e12 Gerrit-Change-Number: 82057 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Comment-Date: Fri, 26 Apr 2024 12:00:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/pc80/tpm: Disable device if TPM not present
by Felix Held (Code Review) 26 Apr '24

26 Apr '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80455?usp=email ) Change subject: drivers/pc80/tpm: Disable device if TPM not present ...................................................................... drivers/pc80/tpm: Disable device if TPM not present If the TPM is not detected in the system it may mean it is inactive due to enabled ME with active PTT. In such case, the chipset will route the TPM traffic to PTT CRB TPM on Intel systems. If TPM is not probed, disable the PC80 TPM device driver, so that coreboot will not generate improper SSDT ACPI table. Change-Id: I05972ad74a36abaafa2f17a16f09710550a3a3f3 Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80455 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M src/drivers/pc80/tpm/tis.c 1 file changed, 9 insertions(+), 4 deletions(-) Approvals: Sergii Dmytruk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/drivers/pc80/tpm/tis.c b/src/drivers/pc80/tpm/tis.c index f922714..d811c52 100644 --- a/src/drivers/pc80/tpm/tis.c +++ b/src/drivers/pc80/tpm/tis.c @@ -905,11 +905,16 @@ static void enable_dev(struct device *dev) { - if (CONFIG(TPM)) - pnp_enable_devices(dev, &lpc_tpm_ops, - ARRAY_SIZE(pnp_dev_info), pnp_dev_info); - else + if (CONFIG(TPM)) { + if (pc80_tis_probe(NULL) == NULL) { + dev->enabled = 0; + return; + } + + pnp_enable_devices(dev, &lpc_tpm_ops, ARRAY_SIZE(pnp_dev_info), pnp_dev_info); + } else { pnp_enable_devices(dev, &noop_tpm_ops, ARRAY_SIZE(pnp_dev_info), pnp_dev_info); + } } struct chip_operations drivers_pc80_tpm_ops = { -- To view, visit https://review.coreboot.org/c/coreboot/+/80455?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: I05972ad74a36abaafa2f17a16f09710550a3a3f3 Gerrit-Change-Number: 80455 Gerrit-PatchSet: 5 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: drivers/crb: Disable device if CRB TPM not present
by Felix Held (Code Review) 26 Apr '24

26 Apr '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80454?usp=email ) Change subject: drivers/crb: Disable device if CRB TPM not present ...................................................................... drivers/crb: Disable device if CRB TPM not present If CRB TPM is not detected in the system it may mean it is inactive due to disabled or neutered ME. In such case, the chipset will route the TPM traffic to LPC/SPI on Intel systems. If CRB TPM is not probed, disable the CRB TPM device driver, so that coreboot will not generate improper SMBIOS/SSDT ACPI tables. Change-Id: Ie0928536d9042b1f680d585e1ca9ad2cadf0c8ef Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80454 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> Reviewed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> --- M src/drivers/crb/tis.c 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, but someone else must approve Sergii Dmytruk: Looks good to me, approved diff --git a/src/drivers/crb/tis.c b/src/drivers/crb/tis.c index 5e81935..df45125 100644 --- a/src/drivers/crb/tis.c +++ b/src/drivers/crb/tis.c @@ -207,6 +207,11 @@ static void enable_dev(struct device *dev) { + if (crb_tis_probe(NULL) == NULL) { + dev->enabled = 0; + return; + } + #if !DEVTREE_EARLY dev->ops = &crb_ops; #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/80454?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: Ie0928536d9042b1f680d585e1ca9ad2cadf0c8ef Gerrit-Change-Number: 80454 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024