coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: soc/intel/alderlake: Configure DDR5 Physical channel width to 64
by Tim Wawrzynczak (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Nico Huber, Furquan Shaikh, Paul Menzel, Meera Ravindranath, Subrata Banik, Nick Vaccaro, Ronak Kanabar, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52730 ) Change subject: soc/intel/alderlake: Configure DDR5 Physical channel width to 64 ...................................................................... Patch Set 8: (1 comment) File src/soc/intel/alderlake/meminit.c: https://review.coreboot.org/c/coreboot/+/52730/comment/b23e10ec_2075be85 PS8, Line 15: #define DDR5_CHANNELS CHANNEL_COUNT(DDR5_PHYSICAL_CH_WIDTH) > It seems this is the only place where DDR5_PHYSICAL_CH_WIDTH is used? […] I see your point, Nico. EDS vol. 1 does seem to indicates it is always effectively 128 bits wide 8 channels of x16 for lpddr4x and lpddr5x or you can also convert sub-channels into wider x32 and x64, but it's still 128 wide 2 channels of x64 for ddr4 4 channels of x32 for ddr5 are the configurations that I see -- To view, visit https://review.coreboot.org/c/coreboot/+/52730 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5e9c58f255bdf86a68ce90a4f853bf4e7c7ccfe Gerrit-Change-Number: 52730 Gerrit-PatchSet: 8 Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 17 Nov 2021 21:52:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/xeon_sp: Use BusLimit during PCI bus allocation
by Jonathan Zhang (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Arthur Heymans, Anjaneya "Reddy" Chagam, Johnny Lin, Christian Walter, Patrick Rudolph, Tim Chu. Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59396 ) Change subject: soc/intel/xeon_sp: Use BusLimit during PCI bus allocation ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/59396 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f698e3ddb73282d2351cde24091085cd02b9d97 Gerrit-Change-Number: 59396 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(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 <jonzhang(a)fb.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.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: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 21:38:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: ec/google/chromeec: Add PLD to EC conn in ACPI table
by Won Chung (Code Review) 17 Nov '21

17 Nov '21

Won Chung has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59401 ) Change subject: ec/google/chromeec: Add PLD to EC conn in ACPI table ...................................................................... ec/google/chromeec: Add PLD to EC conn in ACPI table BUG=b:202446737 TEST=emerge-brya coreboot Signed-off-by: Won Chung <wonchung(a)google.com> Change-Id: Ibc56ecd4e8954ffaace3acd9528a064b5fa2cf6f --- M src/acpi/acpigen_usb.c M src/ec/google/chromeec/ec_acpi.c M src/include/acpi/acpigen_usb.h 3 files changed, 35 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/59401/1 diff --git a/src/acpi/acpigen_usb.c b/src/acpi/acpigen_usb.c index 7448b3b..2868fee 100644 --- a/src/acpi/acpigen_usb.c +++ b/src/acpi/acpigen_usb.c @@ -2,6 +2,7 @@ #include <acpi/acpi.h> #include <acpi/acpi_device.h> +#include <acpi/acpi_pld.h> #include <acpi/acpigen.h> #include <acpi/acpigen_usb.h> @@ -132,5 +133,8 @@ add_custom_dsd_property(dsd, port_number); acpi_dp_write(dsd); + /* Add PLD */ + acpigen_write_pld(config->pld); + acpigen_pop_len(); /* Device */ } diff --git a/src/ec/google/chromeec/ec_acpi.c b/src/ec/google/chromeec/ec_acpi.c index 0598364..d1bc038 100644 --- a/src/ec/google/chromeec/ec_acpi.c +++ b/src/ec/google/chromeec/ec_acpi.c @@ -2,6 +2,7 @@ #include <acpi/acpi.h> #include <acpi/acpi_device.h> +#include <acpi/acpi_pld.h> #include <acpi/acpigen.h> #include <acpi/acpigen_ps2_keybd.h> #include <acpi/acpigen_usb.h> @@ -117,6 +118,28 @@ acpi_dp_add_string(dsd, "port-location", port_location_to_str(port_caps.port_location)); } +static void get_pld_from_usb_ports(struct acpi_pld *pld, + struct device *usb2_port, struct device *usb3_port, + struct device *usb4_port) +{ + struct drivers_usb_acpi_config *config; + + config = NULL; + if (usb4_port) + config = usb4_port->chip_info; + else if (usb3_port) + config = usb3_port->chip_info; + else if (usb2_port) + config = usb2_port->chip_info; + + if (config) { + if (config->use_custom_pld) + *pld = config->custom_pld; + else + acpi_pld_fill_usb(pld, config->type, &config->group); + } +} + static void fill_ssdt_typec_device(const struct device *dev) { struct ec_google_chromeec_config *config = dev->chip_info; @@ -126,6 +149,7 @@ struct device *usb2_port; struct device *usb3_port; struct device *usb4_port; + struct acpi_pld pld; if (google_chromeec_get_num_pd_ports(&num_ports)) return; @@ -146,6 +170,8 @@ usb4_port = NULL; get_usb_port_references(i, &usb2_port, &usb3_port, &usb4_port); + get_pld_from_usb_ports(&pld, usb2_port, usb3_port, usb4_port); + struct typec_connector_class_config typec_config = { .power_role = port_caps.power_role_cap, .try_power_role = port_caps.try_power_role_cap, @@ -156,6 +182,7 @@ .orientation_switch = config->mux_conn[i], .usb_role_switch = config->mux_conn[i], .mode_switch = config->mux_conn[i], + .pld = &pld, }; acpigen_write_typec_connector(&typec_config, i, add_port_location); diff --git a/src/include/acpi/acpigen_usb.h b/src/include/acpi/acpigen_usb.h index efc31f3..8042874 100644 --- a/src/include/acpi/acpigen_usb.h +++ b/src/include/acpi/acpigen_usb.h @@ -3,6 +3,8 @@ #ifndef ACPI_ACPIGEN_USB_H #define ACPI_ACPIGEN_USB_H +#include <acpi/acpi_pld.h> + enum usb_typec_power_role { TYPEC_POWER_ROLE_SOURCE, TYPEC_POWER_ROLE_SINK, @@ -39,6 +41,7 @@ * host or device, for the USB port * @mode_switch: Reference to the ACPI device that controls routing of data lines to * various endpoints (xHCI, DP, etc.) on the SoC. + * @pld: Reference to PLD information. */ struct typec_connector_class_config { enum usb_typec_power_role power_role; @@ -50,6 +53,7 @@ const struct device *orientation_switch; const struct device *usb_role_switch; const struct device *mode_switch; + const struct acpi_pld *pld; }; typedef void (*add_custom_dsd_property_cb)(struct acpi_dp *dsd, int port_number); -- To view, visit https://review.coreboot.org/c/coreboot/+/59401 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibc56ecd4e8954ffaace3acd9528a064b5fa2cf6f Gerrit-Change-Number: 59401 Gerrit-PatchSet: 1 Gerrit-Owner: Won Chung <wonchung(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: security/vboot: Add NVRAM counter for TPM 2.0
by Julius Werner (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Miriam Polzer, Andrey Pronin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59097 ) Change subject: security/vboot: Add NVRAM counter for TPM 2.0 ...................................................................... Patch Set 6: (4 comments) Patchset: PS6: > LGTM now but it would be good if Andrey also takes a look. Done File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59097/comment/b4ce648b_143044eb PS3, Line 150: .TPMA_NV_NO_DA = 1, > you can still fail auth, but trying to authenticate with an owner auth, or passing some random passw […] Ack File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59097/comment/ec905773_10519d34 PS6, Line 120: TPMA_NV_WRITE_STCLEAR > probably doesn't hurt, but needs a bit of analysis. […] Well if an attacker takes control in the middle of a rollback before the counter is incremented, we have bigger problems anyway. https://review.coreboot.org/c/coreboot/+/59097/comment/621b0bb4_a57f1f5f PS6, Line 385: enterprise_rollback_create_counter > ok, if this is per design, works for me. Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/59097 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I511dba3b3461713ce20fb2bda9fced0fee6517e1 Gerrit-Change-Number: 59097 Gerrit-PatchSet: 6 Gerrit-Owner: Miriam Polzer <mpolzer(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Miriam Polzer <mpolzer(a)google.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:49:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Miriam Polzer <mpolzer(a)google.com> Comment-In-Reply-To: Andrey Pronin <apronin(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/intel: Get cpu index based on LAPIC id
by Subrata Banik (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Furquan Shaikh, Sridhar Siricilla, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59357 ) Change subject: cpu/intel: Get cpu index based on LAPIC id ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: As requested earlier in the original CL Can you please help me to share the output of CPU index in multicore scenarios with this CL and with cpu_index() (src/arch/x86/cpu.c) in use ? -- To view, visit https://review.coreboot.org/c/coreboot/+/59357 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If18116472aaa78cfa88350f313c246a28f67303d Gerrit-Change-Number: 59357 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:46:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/intel: Get cpu index based on LAPIC id
by Tim Wawrzynczak (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Furquan Shaikh, Sridhar Siricilla, Subrata Banik, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59357 ) Change subject: cpu/intel: Get cpu index based on LAPIC id ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File src/cpu/x86/mp_init.c: https://review.coreboot.org/c/coreboot/+/59357/comment/1ce874ea_52610796 PS1, Line 216: nit: extra space -- To view, visit https://review.coreboot.org/c/coreboot/+/59357 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If18116472aaa78cfa88350f313c246a28f67303d Gerrit-Change-Number: 59357 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:38:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add CPU related APIs
by Tim Wawrzynczak (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Nico Huber, Maulik V Vaghela, Sugnan Prabhu S, Sridhar Siricilla, Subrata Banik, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59124 ) Change subject: soc/intel/common: Add CPU related APIs ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/common/block/cpu/cpulib.c: https://review.coreboot.org/c/coreboot/+/59124/comment/e9c54212_34000453 PS1, Line 192: res > may be ? […] How is this defined? Does it return supported (read: 1) on every core b/c the package supports hybrid cores? or does it only return supported on the efficiency cores or perofmance cores? -- To view, visit https://review.coreboot.org/c/coreboot/+/59124 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I680f43952ab4abce6e342206688ad32814970a91 Gerrit-Change-Number: 59124 Gerrit-PatchSet: 2 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:37:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Implements ACPI CPPCv3 package to support hybrid core
by Tim Wawrzynczak (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Nico Huber, Subrata Banik, Sridhar Siricilla, Michael Niewöhner, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59359 ) Change subject: soc/intel/common: Implements ACPI CPPCv3 package to support hybrid core ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/common/block/acpi/acpi/globalnvs.asl: https://review.coreboot.org/c/coreboot/+/59359/comment/f3e01930_e821c2f2 PS1, Line 28: SFBC, 16, // 0x48 - 0x49 Indicates Scaling factor for Big core : SFSC, 16, // 0x50 - 0x51 Indicates Scaling Factor for Small Core : NMFQ, 16, // 0x52 - 0x53 Indicates Nominal Frequency : CORE, 32, // 0x54 - 0x57 Each marked bit indicates Big Core corresponding to core index : INFS, 8, // 0x58 - Nominal Frequency is supported : > where are these used? do these values have to be in gnvs in contrast to generating and adding them t […] but with coreboot producing AML at runtime, it is rarely actually required to use GNVS, and we prefer to keep away from that when possible. in this case, I don't think I see a good reason that it has to be in NVS. -- To view, visit https://review.coreboot.org/c/coreboot/+/59359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icd5ea9e70bebd1e66d3cea2bcf8a6678e5cc95ca Gerrit-Change-Number: 59359 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:20:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Implements ACPI CPPCv3 package to support hybrid core
by Subrata Banik (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Nico Huber, Tim Wawrzynczak, Sridhar Siricilla, Michael Niewöhner, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59359 ) Change subject: soc/intel/common: Implements ACPI CPPCv3 package to support hybrid core ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/common/block/acpi/acpi/globalnvs.asl: https://review.coreboot.org/c/coreboot/+/59359/comment/37aefbc5_14b05484 PS1, Line 28: SFBC, 16, // 0x48 - 0x49 Indicates Scaling factor for Big core : SFSC, 16, // 0x50 - 0x51 Indicates Scaling Factor for Small Core : NMFQ, 16, // 0x52 - 0x53 Indicates Nominal Frequency : CORE, 32, // 0x54 - 0x57 Each marked bit indicates Big Core corresponding to core index : INFS, 8, // 0x58 - Nominal Frequency is supported : > where are these used? do these values have to be in gnvs in contrast to generating and adding them t […] Typically, as we all know the purpose of GNVS is to make those data collected from 'C' files and put into GNVS so that we can make those data available as part of ASL code to ACPI OS. Now your question is valid that, can this data exist in ASL itself if not borrowed these values from 'C' code as mentioned above. -- To view, visit https://review.coreboot.org/c/coreboot/+/59359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icd5ea9e70bebd1e66d3cea2bcf8a6678e5cc95ca Gerrit-Change-Number: 59359 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:19:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: security/vboot: Add NVRAM counter for TPM 2.0
by Andrey Pronin (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Miriam Polzer, Julius Werner, Yu-Ping Wu. Andrey Pronin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59097 ) Change subject: security/vboot: Add NVRAM counter for TPM 2.0 ...................................................................... Patch Set 6: (3 comments) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59097/comment/db197337_61717259 PS3, Line 150: .TPMA_NV_NO_DA = 1, > I don't know, maybe Andrey does. […] you can still fail auth, but trying to authenticate with an owner auth, or passing some random password as authValue. it allows access with PH or authValue=NULL specifically. yes, in practice, access to it with different auth is unlikely, but NO_DA doesn't cost us anything. File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59097/comment/313fbfa6_286b6552 PS6, Line 120: TPMA_NV_WRITE_STCLEAR > I was just thinking it doesn't hurt? Is there ever a reason not to set this flag (just in case the n […] probably doesn't hurt, but needs a bit of analysis. is there any attack where an attacker can gain something by preventing the counter from incrementing (until reboot)? with WriteLock they can do just that. https://review.coreboot.org/c/coreboot/+/59097/comment/25d96389_760178f0 PS6, Line 385: enterprise_rollback_create_counter > Yes, I was told that on ToT we should have firmware clean for new devices and if/when we merge this […] ok, if this is per design, works for me. -- To view, visit https://review.coreboot.org/c/coreboot/+/59097 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I511dba3b3461713ce20fb2bda9fced0fee6517e1 Gerrit-Change-Number: 59097 Gerrit-PatchSet: 6 Gerrit-Owner: Miriam Polzer <mpolzer(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Miriam Polzer <mpolzer(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 20:14:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Miriam Polzer <mpolzer(a)google.com> Comment-In-Reply-To: Andrey Pronin <apronin(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021