coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[L] Change in coreboot[main]: acpi: Add acpigen_write_OSC_pci_domain
by Paul Menzel (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Johnny Lin, Jonathan Zhang, Lance Zhao, Shuo Liu, Tim Chu, Tim Wawrzynczak. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81375?usp=email ) Change subject: acpi: Add acpigen_write_OSC_pci_domain ...................................................................... Patch Set 12: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81375/comment/40761d52_3e5e6d02 : PS12, Line 11: Tested how? File src/acpi/acpigen_pci.c: https://review.coreboot.org/c/coreboot/+/81375/comment/205da230_cf3044bc : PS12, Line 98: With Width https://review.coreboot.org/c/coreboot/+/81375/comment/54ef3f72_12947749 : PS12, Line 98: Name - With, Source, Offset, Description : * -------------------------------- : * QSUP - DWord, Local7, 0x00, Query support : * RETE - DWord, Arg3, 0x00, Returned errors : * SUPP - Dword, Arg3, 0x04, PCIe Features that OS supported : * CTRL - Dword, Arg3, 0x08, PCIe Features that firmware grant control to OS : * OTRL - Dword, Local7, 0x08, PCIe Features that OS requests for control : * SUPC - Dword, Arg3, 0x0C, CXL Features that OS supported : * CTRC - Dword, Arg3, 0x10, CXL Features that firmware grant control to OS : * OTRC - Dword, Local7, 0x10, CXL Features that OS requests for control If it’s a table, I wouldn’t separate the columns by a comma. -- To view, visit https://review.coreboot.org/c/coreboot/+/81375?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: I711ce5350d718e47feb2912555108801ad7f918d Gerrit-Change-Number: 81375 Gerrit-PatchSet: 12 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(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: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 26 Mar 2024 11:34:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: acpi: Add acpigen_write_OSC_pci_domain
by Nico Huber (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Johnny Lin, Jonathan Zhang, Lance Zhao, Shuo Liu, Tim Chu, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81375?usp=email ) Change subject: acpi: Add acpigen_write_OSC_pci_domain ...................................................................... Patch Set 12: (2 comments) File src/acpi/acpigen_pci.c: https://review.coreboot.org/c/coreboot/+/81375/comment/2c9bbe57_6b47ff85 : PS12, Line 272: |= > Why the `or`? Isn't `RETE` still the original input dword0? I see `RETE` is initially set to `0`. But the `|=` still seems odd (given how the code is guarded currently, see also other comment about PCIe bits in the CXL case). https://review.coreboot.org/c/coreboot/+/81375/comment/d16868fe_7e7e1acb : PS12, Line 317: acpigen_write_create_dword_field(LOCAL7_OP, 0x10, "OTRC"); What about the PCIe fields? On this path, it seems we would return CTRL unaltered. This seems to be an issue because the code generated by acpigen_OSC_handle_pcie_request() is always guarded by the PCI UUID, which is probably wrong. -- To view, visit https://review.coreboot.org/c/coreboot/+/81375?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: I711ce5350d718e47feb2912555108801ad7f918d Gerrit-Change-Number: 81375 Gerrit-PatchSet: 12 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(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: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 26 Mar 2024 11:29:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation
by Shuo Liu (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81377?usp=email ) Change subject: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation ...................................................................... Patch Set 11: (1 comment) File src/soc/intel/xeon_sp/chip_gen1.c: https://review.coreboot.org/c/coreboot/+/81377/comment/52397a47_540e5106 : PS11, Line 74: iio_pci_domain_fill_ssdt > that doesn't work on SKX/CPX since you keept the DSDT. Yes ... Should I use some macros to guard soc_pci_domain_fill_ssdt()? -- To view, visit https://review.coreboot.org/c/coreboot/+/81377?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: Icc5843feadc840d87c49b2aa4259716264520dba Gerrit-Change-Number: 81377 Gerrit-PatchSet: 11 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.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: 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: 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: Tue, 26 Mar 2024 11:24:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation
by Patrick Rudolph (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, Tim Chu. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81377?usp=email ) Change subject: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation ...................................................................... Patch Set 11: (1 comment) File src/soc/intel/xeon_sp/chip_gen1.c: https://review.coreboot.org/c/coreboot/+/81377/comment/b58de1b8_0bb6119b : PS11, Line 74: iio_pci_domain_fill_ssdt that doesn't work on SKX/CPX since you keept the DSDT. -- To view, visit https://review.coreboot.org/c/coreboot/+/81377?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: Icc5843feadc840d87c49b2aa4259716264520dba Gerrit-Change-Number: 81377 Gerrit-PatchSet: 11 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.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: Tue, 26 Mar 2024 11:19:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt
by Patrick Rudolph (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, Tim Chu. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81374?usp=email ) Change subject: soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81374?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: I28bfdf74d8044235f79f67d832860d8b4306670c Gerrit-Change-Number: 81374 Gerrit-PatchSet: 12 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.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: 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: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 26 Mar 2024 11:18:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: acpi: Add acpigen_write_OSC_pci_domain
by Nico Huber (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Johnny Lin, Jonathan Zhang, Lance Zhao, Shuo Liu, Tim Chu, Tim Wawrzynczak. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81375?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: acpi: Add acpigen_write_OSC_pci_domain ...................................................................... Patch Set 12: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81375/comment/9138bfcd_e7ef367d : PS12, Line 9: dynamic Why? What runtime information does it depend on? Patchset: PS12: This is not an easy to review/maintain form, and beside a lot of boilerplate the code seems to do a static thing. As we have the option to put it into the DSDT and call it from each device, maybe that would be better? (Ignore me if I missed that it relies on dynamic runtime information.) File src/acpi/acpigen_pci.c: https://review.coreboot.org/c/coreboot/+/81375/comment/54c965ba_571e7e9e : PS12, Line 127: : if (is_cxl_domain) { : /* : * If ((Arg0 != ToUUID (PCI_HOST_BRIDGE_OSC_UUID)) && : * (Arg0 != ToUUID (CXL_HOST_BRIDGE_OSC_UUID))) : */ : acpigen_write_if(); : acpigen_emit_byte(LAND_OP); : acpigen_emit_byte(LNOT_OP); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_byte(ARG0_OP); : acpigen_write_uuid(PCI_HOST_BRIDGE_OSC_UUID); : acpigen_emit_byte(LNOT_OP); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_byte(ARG0_OP); : acpigen_write_uuid(CXL_HOST_BRIDGE_OSC_UUID); : } else { : /* : * If (Arg0 != ToUUID (PCI_HOST_BRIDGE_OSC_UUID)) : */ : acpigen_write_if(); : acpigen_emit_byte(LNOT_OP); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_byte(ARG0_OP); : acpigen_write_uuid(PCI_HOST_BRIDGE_OSC_UUID); : } : : This seems unnecessary, you could also make it an `If ... ElseIf ... Else UNRECOGNIZED_UUID` below. https://review.coreboot.org/c/coreboot/+/81375/comment/161514ca_32b6727a : PS12, Line 272: |= Why the `or`? Isn't `RETE` still the original input dword0? https://review.coreboot.org/c/coreboot/+/81375/comment/c349817d_8e26f996 : PS12, Line 348: __weak unsigned long get_granted_pci_features(const struct device *domain) : { : return 0; : } : : __weak unsigned long get_granted_cxl_features(const struct device *domain) : { : return 0; : } The whole code above seems to do nothing without these. So why are they optional? -- To view, visit https://review.coreboot.org/c/coreboot/+/81375?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: I711ce5350d718e47feb2912555108801ad7f918d Gerrit-Change-Number: 81375 Gerrit-PatchSet: 12 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(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: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 26 Mar 2024 11:04:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: acpi: Add soc_pci_domain_fill_ssdt
by Shuo Liu (Code Review) 26 Mar '24

26 Mar '24

Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81373?usp=email ) Change subject: acpi: Add soc_pci_domain_fill_ssdt ...................................................................... Patch Set 8: (1 comment) File src/acpi/acpigen_pci_root_resource_producer.c: https://review.coreboot.org/c/coreboot/+/81373/comment/d77f4256_f27e72a6 : PS1, Line 49: /* SoC specific settings, device object creation could be placed here */ : soc_pci_domain_fill_ssdt(domain); > Good point, I will abandon this one and update others. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81373?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: I893eb64c776e78f46737072b475acde5e32a796a Gerrit-Change-Number: 81373 Gerrit-PatchSet: 8 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Mar 2024 10:32:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> 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]: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation
by Shuo Liu (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu. Hello Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81377?usp=email to look at the new patch set (#11). The following approvals got outdated and were removed: Code-Review+2 by Patrick Rudolph, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation ...................................................................... soc/intel/xeon_sp/spr: Support dynamic domain SSDT generation Domain SSDT is dynamically generated by soc_pci_domain_fill_ssdt. SPR has 2 SKUs, XCC and MCC. Dynamic domain SSDT generation could better fit both. One possible side-effect might be the extra performance cost for generating these tables, which should not bring big impact on high performance server CPUs. TEST=intel/archercity CRB Linux ACPI host bridge parsing logs are kept the same before and after, with some minor issue fixed. Change-Id: Icc5843feadc840d87c49b2aa4259716264520dba Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> --- M src/soc/intel/xeon_sp/chip_gen1.c D src/soc/intel/xeon_sp/spr/acpi/cxl_resource.asl D src/soc/intel/xeon_sp/spr/acpi/dino_resource.asl D src/soc/intel/xeon_sp/spr/acpi/iiostack.asl D src/soc/intel/xeon_sp/spr/acpi/pci_resource.asl D src/soc/intel/xeon_sp/spr/acpi/ubox_resource.asl M src/soc/intel/xeon_sp/spr/acpi/uncore.asl M src/soc/intel/xeon_sp/spr/ioat.c M src/soc/intel/xeon_sp/spr/soc_acpi.c 9 files changed, 95 insertions(+), 321 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/81377/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/81377?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: Icc5843feadc840d87c49b2aa4259716264520dba Gerrit-Change-Number: 81377 Gerrit-PatchSet: 11 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.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: 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: 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-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt
by Shuo Liu (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu. Hello Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81374?usp=email to look at the new patch set (#12). The following approvals got outdated and were removed: Code-Review+2 by Patrick Rudolph, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt ...................................................................... soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt Domain device objects are created with HID/CID/UID. Dynamic domain SSDT generation could benefit the support of SoCs with multiple SKUs, or the case where one set of codes supports multiple SoCs. One possible side-effect might be the extra performance cost for generating these tables, which should not bring big impact on high performance server CPUs. GNR codes run with dynamic domain SSDT generation to fit for both GraniteRapids and SierraForest SoCs. Change-Id: I28bfdf74d8044235f79f67d832860d8b4306670c Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> Signed-off-by: Jincheng Li <jincheng.li(a)intel.com> --- M src/soc/intel/xeon_sp/chip_gen6.c M src/soc/intel/xeon_sp/gnr/soc_acpi.c M src/soc/intel/xeon_sp/include/soc/chip_common.h 3 files changed, 42 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/81374/12 -- To view, visit https://review.coreboot.org/c/coreboot/+/81374?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: I28bfdf74d8044235f79f67d832860d8b4306670c Gerrit-Change-Number: 81374 Gerrit-PatchSet: 12 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.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: 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: 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: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp: Add device to proximity domain map utils
by Shuo Liu (Code Review) 26 Mar '24

26 Mar '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu, Ziang Wang. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81440?usp=email ) Change subject: soc/intel/xeon_sp: Add device to proximity domain map utils ...................................................................... Patch Set 4: (2 comments) File src/soc/intel/xeon_sp/numa.c: https://review.coreboot.org/c/coreboot/+/81440/comment/9fbeb105_7e769ad2 : PS1, Line 133: device_to_pd > doesn't account for SNC2 or SNC4. I didn't handle SNC in this patch and add comments in commit message as well. https://review.coreboot.org/c/coreboot/+/81440/comment/3dcd3577_c6a76db0 : PS1, Line 140: iio_pci_domain_socket_from_dev > doesn't work for DEVICE_PATH_DOMAIN when it's a CXL host bridge. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81440?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: Icd14a98823491ccfc38473e44a26dddfbbcaa7c0 Gerrit-Change-Number: 81440 Gerrit-PatchSet: 4 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: 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: Ziang Wang <ziang.wang(a)intel.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: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Ziang Wang <ziang.wang(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: Tue, 26 Mar 2024 10:24:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024