coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: mb/google/guybrush: Define ACPI Power Resources for FPMCU
by Karthik Ramasubramanian (Code Review) 11 Nov '21

11 Nov '21

Karthik Ramasubramanian has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58705 ) Change subject: mb/google/guybrush: Define ACPI Power Resources for FPMCU ...................................................................... mb/google/guybrush: Define ACPI Power Resources for FPMCU Currently all the power sequencing for FPMCU is done explicitly in different stages of coreboot. This can all be done by adding ACPI power resources for FPMCU and clean up the unused code. Here is the expected power sequence: PowerUp : Assert EN_PWR_FP -> 3 ms delay -> De-assert FPMCU_RST_ODL Shutdown : De-assert EN_PWR_FP -> Assert FPMCU_RST_ODL Reboot : Shutdown -> 200 ms delay -> PowerUp BUG=None TEST=Build and boot to OS in Guybrush. Ensure that the FP is able to unlock the system after the first login attempt. Ensure that the FP is able to wakeup the system. Observed that the power resource is added correctly in the FPMCU ACPI object Name (_PR0, Package (0x01) // _PR0: Power Resources for D0 { PR01 }) Name (_PR3, Package (0x01) // _PR3: Power Resources for D3hot { PR01 }) PowerResource (PR01, 0x00, 0x0000) { Method (_STA, 0, NotSerialized) // _STA: Status { Return (0x01) } Method (_ON, 0, Serialized) // _ON_: Power On { \_SB.CTXS (0x0B) \_SB.STXS (0x20) \_SB.STXS (0x0B) } Method (_OFF, 0, Serialized) // _OFF: Power Off { \_SB.CTXS (0x0B) \_SB.CTXS (0x20) } } Change-Id: I52322eaecf6961ff9a196ca9ab2d58b7d4599d4f Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58705 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Reviewed-by: Rob Barnes <robbarnes(a)google.com> --- M src/mainboard/google/guybrush/bootblock.c M src/mainboard/google/guybrush/mainboard.c M src/mainboard/google/guybrush/variants/baseboard/gpio.c M src/mainboard/google/guybrush/variants/baseboard/helpers.c M src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/guybrush/variants/guybrush/gpio.c M src/mainboard/google/guybrush/variants/guybrush/overridetree.cb M src/mainboard/google/guybrush/variants/nipperkin/gpio.c M src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb M src/mainboard/google/guybrush/variants/nipperkin/ramstage.c 10 files changed, 35 insertions(+), 180 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved Rob Barnes: Looks good to me, approved diff --git a/src/mainboard/google/guybrush/bootblock.c b/src/mainboard/google/guybrush/bootblock.c index 187b2ef..bffcb0c 100644 --- a/src/mainboard/google/guybrush/bootblock.c +++ b/src/mainboard/google/guybrush/bootblock.c @@ -99,8 +99,4 @@ gpio_configure_pads_with_override(base_gpios, base_num_gpios, override_gpios, override_num_gpios); - - /* FPMCU check needs to happen after EC initialization for FW_CONFIG bits */ - if (variant_has_fpmcu()) - variant_fpmcu_reset(); } diff --git a/src/mainboard/google/guybrush/mainboard.c b/src/mainboard/google/guybrush/mainboard.c index b2d669c..c50304d 100644 --- a/src/mainboard/google/guybrush/mainboard.c +++ b/src/mainboard/google/guybrush/mainboard.c @@ -209,13 +209,7 @@ pm_write32(PM_ESPI_INTR_CTRL, PM_ESPI_DEV_INTR_MASK & ~(BIT(1))); } -static void mainboard_final(void *chip_info) -{ - variant_finalize_gpios(); -} - struct chip_operations mainboard_ops = { .init = mainboard_init, .enable_dev = mainboard_enable, - .final = mainboard_final, }; diff --git a/src/mainboard/google/guybrush/variants/baseboard/gpio.c b/src/mainboard/google/guybrush/variants/baseboard/gpio.c index 66b40be..fb01789 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/gpio.c +++ b/src/mainboard/google/guybrush/variants/baseboard/gpio.c @@ -18,7 +18,7 @@ /* WAKE_L */ PAD_NF_SCI(GPIO_2, WAKE_L, PULL_NONE, EDGE_LOW), /* EN_PWR_FP */ - PAD_GPO(GPIO_3, HIGH), + PAD_GPO(GPIO_3, LOW), /* SOC_PEN_DETECT_ODL */ PAD_WAKE(GPIO_4, PULL_NONE, EDGE_HIGH, S0i3), /* SD_AUX_RESET_L */ @@ -33,7 +33,7 @@ PAD_SCI(GPIO_9, PULL_NONE, EDGE_HIGH), /* S0A3 */ PAD_NF(GPIO_10, S0A3, PULL_NONE), - /* SOC_FP_RST_L - Brought high in finalize */ + /* SOC_FP_RST_L */ PAD_GPO(GPIO_11, LOW), /* SLP_S3_GATED */ PAD_GPO(GPIO_12, LOW), @@ -291,20 +291,6 @@ PAD_NFO(GPIO_26, PCIE_RST_L, HIGH), }; -static const struct soc_amd_gpio fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_3, LOW), -}; - -static const struct soc_amd_gpio fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_3), -}; - const struct soc_amd_gpio *__weak variant_pcie_gpio_table(size_t *size) { *size = ARRAY_SIZE(pcie_gpio_table); @@ -355,49 +341,6 @@ const __weak struct soc_amd_gpio *variant_sleep_gpio_table(size_t *size) { - if (acpi_get_sleep_type() == ACPI_S5) - return variant_fpmcu_shutdown_gpio_table(size); - *size = ARRAY_SIZE(sleep_gpio_table); return sleep_gpio_table; } - -const __weak struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_shutdown_gpio_table); - return fpmcu_shutdown_gpio_table; -} - -const __weak struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_disable_gpio_table); - return fpmcu_disable_gpio_table; -} - -__weak void variant_fpmcu_reset(void) -{ - size_t size; - const struct soc_amd_gpio *gpio_table; - - if (acpi_get_sleep_type() == ACPI_S3) - return; - /* If the system is not resuming from S3, power off the FPMCU */ - gpio_table = variant_fpmcu_shutdown_gpio_table(&size); - gpio_configure_pads(gpio_table, size); -} - -__weak void variant_finalize_gpios(void) -{ - size_t size; - const struct soc_amd_gpio *gpio_table; - - if (variant_has_fpmcu()) { - if (acpi_get_sleep_type() == ACPI_S3) - return; - /* Deassert the FPMCU reset to enable the FPMCU */ - gpio_set(GPIO_11, 1); /* FPMCU_RST_L */ - } else { - gpio_table = variant_fpmcu_disable_gpio_table(&size); - gpio_configure_pads(gpio_table, size); - } -} diff --git a/src/mainboard/google/guybrush/variants/baseboard/helpers.c b/src/mainboard/google/guybrush/variants/baseboard/helpers.c index 04c05bb..36f7b05 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/helpers.c +++ b/src/mainboard/google/guybrush/variants/baseboard/helpers.c @@ -4,13 +4,6 @@ #include <device/device.h> #include <soc/gpio.h> -WEAK_DEV_PTR(fpmcu); - -bool variant_has_fpmcu(void) -{ - return is_dev_enabled(DEV_PTR(fpmcu)); -} - bool __weak variant_has_pcie_wwan(void) { return false; diff --git a/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h index 6cc96f8..955d170 100644 --- a/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/guybrush/variants/baseboard/include/baseboard/variants.h @@ -40,19 +40,6 @@ /* This function provides GPIO settings before entering sleep. */ const struct soc_amd_gpio *variant_sleep_gpio_table(size_t *size); -/* This function provides GPIO settings for fpmcu shutdown. */ -const struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size); - -/* This function provides GPIO settings for fpmcu disable. */ -const struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size); - -/* Finalize GPIOs, such as FPMCU power */ -void variant_finalize_gpios(void); - -void variant_fpmcu_reset(void); - -bool variant_has_fpmcu(void); - bool variant_has_pcie_wwan(void); void variant_update_dxio_descriptors(fsp_dxio_descriptor *dxio_descriptors); diff --git a/src/mainboard/google/guybrush/variants/guybrush/gpio.c b/src/mainboard/google/guybrush/variants/guybrush/gpio.c index 4634c5a..bf8e999 100644 --- a/src/mainboard/google/guybrush/variants/guybrush/gpio.c +++ b/src/mainboard/google/guybrush/variants/guybrush/gpio.c @@ -24,7 +24,7 @@ /* Unused */ PAD_NC(GPIO_85), /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), + PAD_GPO(GPIO_32, LOW), }; /* This table is used by guybrush variant with board version >= 2. */ @@ -38,7 +38,7 @@ /* Unused */ PAD_NC(GPIO_85), /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), + PAD_GPO(GPIO_32, LOW), }; static const struct soc_amd_gpio override_early_gpio_table[] = { @@ -67,20 +67,6 @@ PAD_GPO(GPIO_69, HIGH), }; -static const struct soc_amd_gpio fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_32, LOW), -}; - -static const struct soc_amd_gpio fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_32), -}; - const struct soc_amd_gpio *variant_override_gpio_table(size_t *size) { uint32_t board_version = board_id(); @@ -118,15 +104,3 @@ *size = ARRAY_SIZE(bid2_pcie_gpio_table); return bid2_pcie_gpio_table; } - -const struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_shutdown_gpio_table); - return fpmcu_shutdown_gpio_table; -} - -const struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size) -{ - *size = ARRAY_SIZE(fpmcu_disable_gpio_table); - return fpmcu_disable_gpio_table; -} diff --git a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb index 92b1980..01876c8 100644 --- a/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb +++ b/src/mainboard/google/guybrush/variants/guybrush/overridetree.cb @@ -172,6 +172,10 @@ register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW(GPIO_21)" register "wake" = "GEVENT_5" register "uart" = "ACPI_UART_RAW_DEVICE(3000000, 64)" + register "has_power_resource" = "1" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPIO_11)" + register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_32)" + register "enable_delay_ms" = "3" device generic 0 alias fpmcu on probe FP FP_PRESENT end diff --git a/src/mainboard/google/guybrush/variants/nipperkin/gpio.c b/src/mainboard/google/guybrush/variants/nipperkin/gpio.c index 6a12ad3..4d6e045 100644 --- a/src/mainboard/google/guybrush/variants/nipperkin/gpio.c +++ b/src/mainboard/google/guybrush/variants/nipperkin/gpio.c @@ -23,7 +23,7 @@ /* Unused */ PAD_NC(GPIO_85), /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), + PAD_GPO(GPIO_32, LOW), }; /* This table is used by nipperkin variant with board version >= 2. */ @@ -60,38 +60,6 @@ PAD_NC(GPIO_69), }; -/* This table is used by nipperkin variant with board version < 2. */ -static const struct soc_amd_gpio bid1_fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_32, LOW), -}; - -/* This table is used by nipperkin variant with board version >= 2. */ -static const struct soc_amd_gpio bid2_fpmcu_shutdown_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, LOW), - /* EN_PWR_FP */ - PAD_GPO(GPIO_3, LOW), -}; - -/* This table is used by nipperkin variant with board version < 2. */ -static const struct soc_amd_gpio bid1_fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_32), -}; - -/* This table is used by nipperkin variant with board version >= 2. */ -static const struct soc_amd_gpio bid2_fpmcu_disable_gpio_table[] = { - /* FPMCU_RST_L */ - PAD_NC(GPIO_11), - /* EN_PWR_FP */ - PAD_NC(GPIO_3), -}; - const struct soc_amd_gpio *variant_override_gpio_table(size_t *size) { uint32_t board_version = board_id(); @@ -123,29 +91,3 @@ *size = ARRAY_SIZE(bid2_override_pcie_gpio_table); return bid2_override_pcie_gpio_table; } - -const struct soc_amd_gpio *variant_fpmcu_shutdown_gpio_table(size_t *size) -{ - uint32_t board_version = board_id(); - - if (board_version < 2) { - *size = ARRAY_SIZE(bid1_fpmcu_shutdown_gpio_table); - return bid1_fpmcu_shutdown_gpio_table; - } - - *size = ARRAY_SIZE(bid2_fpmcu_shutdown_gpio_table); - return bid2_fpmcu_shutdown_gpio_table; -} - -const struct soc_amd_gpio *variant_fpmcu_disable_gpio_table(size_t *size) -{ - uint32_t board_version = board_id(); - - if (board_version < 2) { - *size = ARRAY_SIZE(bid1_fpmcu_disable_gpio_table); - return bid1_fpmcu_disable_gpio_table; - } - - *size = ARRAY_SIZE(bid2_fpmcu_disable_gpio_table); - return bid2_fpmcu_disable_gpio_table; -} diff --git a/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb b/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb index 48b04fb..3599b28 100644 --- a/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb +++ b/src/mainboard/google/guybrush/variants/nipperkin/overridetree.cb @@ -154,6 +154,10 @@ register "irq_gpio" = "ACPI_GPIO_IRQ_LEVEL_LOW(GPIO_21)" register "wake" = "GEVENT_5" register "uart" = "ACPI_UART_RAW_DEVICE(3000000, 64)" + register "has_power_resource" = "1" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPIO_11)" + register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_3)" + register "enable_delay_ms" = "3" device generic 0 alias fpmcu on probe FP FP_PRESENT end diff --git a/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c b/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c index 1042805..c2f4fbf 100644 --- a/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c +++ b/src/mainboard/google/guybrush/variants/nipperkin/ramstage.c @@ -4,18 +4,36 @@ #include <boardid.h> #include <device/device.h> #include <drivers/i2c/tpm/chip.h> +#include <drivers/uart/acpi/chip.h> #include <soc/gpio.h> -void variant_devtree_update(void) +static void cr50_devtree_update(void) { - uint32_t board_ver = board_id(); const struct device *cr50_dev = DEV_PTR(cr50); struct drivers_i2c_tpm_config *cfg; struct acpi_gpio cr50_irq_gpio = ACPI_GPIO_IRQ_EDGE_LOW(GPIO_3); + cfg = config_of(cr50_dev); + cfg->irq_gpio = cr50_irq_gpio; +} + +static void fpmcu_devtree_update(void) +{ + const struct device *fpmcu_dev = DEV_PTR(fpmcu); + struct drivers_uart_acpi_config *cfg; + struct acpi_gpio fpmcu_enable_gpio = ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_32); + + cfg = config_of(fpmcu_dev); + cfg->enable_gpio = fpmcu_enable_gpio; +} + +void variant_devtree_update(void) +{ + uint32_t board_ver = board_id(); + if (board_ver > 1) return; - cfg = config_of(cr50_dev); - cfg->irq_gpio = cr50_irq_gpio; + cr50_devtree_update(); + fpmcu_devtree_update(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/58705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I52322eaecf6961ff9a196ca9ab2d58b7d4599d4f Gerrit-Change-Number: 58705 Gerrit-PatchSet: 6 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bhanu Prakash Maiya <bhanumaiya(a)google.com> Gerrit-CC: Craig Hesling <hesling(a)chromium.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: amdfwtool: Add support for AMD's BIOS A/B recovery feature
by Felix Held (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Bao Zheng, Jason Glenesk, Raul Rangel, Martin Roth, Marshall Dawson, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56773 ) Change subject: amdfwtool: Add support for AMD's BIOS A/B recovery feature ...................................................................... Patch Set 32: (2 comments) File util/amdfwtool/amdfwtool.c: https://review.coreboot.org/c/coreboot/+/56773/comment/d794d73c_0e245d4e PS32, Line 1751: 0x1000 TABLE_ALIGNMENT https://review.coreboot.org/c/coreboot/+/56773/comment/aa1d2361_54a32351 PS32, Line 1754: 0x1000 TABLE_ALIGNMENT -- To view, visit https://review.coreboot.org/c/coreboot/+/56773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27f5d3476f648fcecafb8d258ccb6cfad4f50036 Gerrit-Change-Number: 56773 Gerrit-PatchSet: 32 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bao Zheng <zheng.bao(a)amd.corp-partner.google.com> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Comment-Date: Thu, 11 Nov 2021 17:56:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: amdfwtool: Add ISH header support for A/B recovery layout
by Felix Held (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Bao Zheng, Jason Glenesk, Martin Roth, Marshall Dawson, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57747 ) Change subject: amdfwtool: Add ISH header support for A/B recovery layout ...................................................................... Patch Set 24: (1 comment) File util/amdfwtool/amdfwtool.c: https://review.coreboot.org/c/coreboot/+/57747/comment/b7287721_4b8279b8 PS24, Line 751: 4096 not directly related to this patch, but it would also be good to use ERASE_ALIGNMENT or TABLE_ALIGNMENT here instead of the constant. there is at least one more instance 4 lines below; haven't checked the rest of the file -- To view, visit https://review.coreboot.org/c/coreboot/+/57747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib0690cde1dce949514c7aacebe13096b7814ceff Gerrit-Change-Number: 57747 Gerrit-PatchSet: 24 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bao Zheng <zheng.bao(a)amd.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Comment-Date: Thu, 11 Nov 2021 17:49:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: Allow enable/disable ME via CMOS
by Felix Singer (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Sean Rhodes, SRIDHAR SIRICILLA, Furquan Shaikh, Paul Menzel, Rizwan Qureshi, Subrata Banik, Sridhar Siricilla, Arthur Heymans, Evgeny Zinoviev, Patrick Rudolph, Felix Held. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52800 ) Change subject: soc/intel: Allow enable/disable ME via CMOS ...................................................................... Patch Set 85: (4 comments) File src/soc/intel/common/block/cse/cse.c: https://review.coreboot.org/c/coreboot/+/52800/comment/c3431575_90569219 PS85, Line 1057: int send; : int result; Declare variables at the beginning of that function. https://review.coreboot.org/c/coreboot/+/52800/comment/cd07e278_bce1d529 PS85, Line 1060: struct state_reply { : struct mkhi_hdr hdr; : uint32_t rule_id; : } __packed; Not sure about this. I would put this at the beginning too. https://review.coreboot.org/c/coreboot/+/52800/comment/47f1b952_9fdfcbed PS85, Line 1065: struct state_reply reply; : : size_t reply_size; Declare variables at the beginning of that function. https://review.coreboot.org/c/coreboot/+/52800/comment/e440d269_743b651f PS85, Line 1109: Remove empty line -- To view, visit https://review.coreboot.org/c/coreboot/+/52800 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I374db3b7c0ded71cdc18f27970252fec7220cc20 Gerrit-Change-Number: 52800 Gerrit-PatchSet: 85 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: SRIDHAR SIRICILLA Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(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: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: SRIDHAR SIRICILLA Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 11 Nov 2021 17:49:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: Allow enable/disable ME via CMOS
by Tim Crawford (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Sean Rhodes, SRIDHAR SIRICILLA, Furquan Shaikh, Paul Menzel, Rizwan Qureshi, Subrata Banik, Sridhar Siricilla, Arthur Heymans, Evgeny Zinoviev, Patrick Rudolph, Felix Held. Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52800 ) Change subject: soc/intel: Allow enable/disable ME via CMOS ...................................................................... Patch Set 85: (1 comment) File src/soc/intel/common/block/cse/cse.c: https://review.coreboot.org/c/coreboot/+/52800/comment/47931640_ceba9a2c PS85, Line 1062: uint32_t rule_id; When setting to Normal mode, the reply does not have a rule_id. -- To view, visit https://review.coreboot.org/c/coreboot/+/52800 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I374db3b7c0ded71cdc18f27970252fec7220cc20 Gerrit-Change-Number: 52800 Gerrit-PatchSet: 85 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: SRIDHAR SIRICILLA Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(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: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: SRIDHAR SIRICILLA Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 11 Nov 2021 17:48:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: Allow enable/disable ME via CMOS
by Tim Crawford (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Sean Rhodes, SRIDHAR SIRICILLA, Furquan Shaikh, Paul Menzel, Rizwan Qureshi, Subrata Banik, Sridhar Siricilla, Arthur Heymans, Evgeny Zinoviev, Patrick Rudolph, Felix Held. Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52800 ) Change subject: soc/intel: Allow enable/disable ME via CMOS ...................................................................... Patch Set 85: (1 comment) Patchset: PS81: > Which Tim did you mean? (Hint hint :D ) On system76/oryp8, it fails to enable the IME: CMOS: me_state = 0 ME needs to be enabled. HECI: ME state change send success! HECI: ME state change result failure! CMOS: me_state_counter = 3 Error: Failed to change ME state in 3 attempts! -- To view, visit https://review.coreboot.org/c/coreboot/+/52800 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I374db3b7c0ded71cdc18f27970252fec7220cc20 Gerrit-Change-Number: 52800 Gerrit-PatchSet: 85 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: SRIDHAR SIRICILLA Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(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: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: SRIDHAR SIRICILLA Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 11 Nov 2021 17:42:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <admin(a)starlabs.systems> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: amdfwtool: Add ISH header support for A/B recovery layout
by Felix Held (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Bao Zheng, Jason Glenesk, Martin Roth, Marshall Dawson, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57747 ) Change subject: amdfwtool: Add ISH header support for A/B recovery layout ...................................................................... Patch Set 24: (1 comment) File util/amdfwtool/amdfwtool.c: https://review.coreboot.org/c/coreboot/+/57747/comment/470b7537_d00d2072 PS24, Line 396: 0x1000 > would be good to have a define for this from the documentation i'd say that this value is correct; I think all modern spi flash chips have an erase block size of 4kByte. not sure if ERASE_ALIGNMENT or TABLE_ALIGNMENT would be the definition to use here, but would be good to use one of those two defines -- To view, visit https://review.coreboot.org/c/coreboot/+/57747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib0690cde1dce949514c7aacebe13096b7814ceff Gerrit-Change-Number: 57747 Gerrit-PatchSet: 24 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bao Zheng <zheng.bao(a)amd.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Comment-Date: Thu, 11 Nov 2021 17:39:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: move SGX ACPI code to block/acpi
by Michael Niewöhner (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Nico Huber, Michael Niewöhner, Patrick Rudolph. Hello build bot (Jenkins), Nico Huber, Tim Wawrzynczak, Angel Pons, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/58925 to look at the new patch set (#6). Change subject: soc/intel: move SGX ACPI code to block/acpi ...................................................................... soc/intel: move SGX ACPI code to block/acpi Move SGX ACPI code to block/acpi. Also move the register definitions there, since they are misplaced in intelblocks/msr.h and are used only once anyways. Change-Id: I089d0ee97c37df2be060b5996183201bfa9b49ca Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/soc/intel/common/block/acpi/Makefile.inc A src/soc/intel/common/block/acpi/sgx.c M src/soc/intel/common/block/include/intelblocks/acpi.h M src/soc/intel/common/block/include/intelblocks/msr.h M src/soc/intel/common/block/include/intelblocks/sgx.h M src/soc/intel/common/block/sgx/sgx.c 6 files changed, 68 insertions(+), 59 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/58925/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/58925 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I089d0ee97c37df2be060b5996183201bfa9b49ca Gerrit-Change-Number: 58925 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/xeon_sp: Fix size_t type mismatch in print statement
by Paul Menzel (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Paul Menzel, Christian Walter, Arthur Heymans, Patrick Rudolph, Tim Chu. Hello Lance Zhao, build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Christian Walter, Arthur Heymans, Patrick Rudolph, Felix Held, Tim Chu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59054 to look at the new patch set (#2). Change subject: soc/intel/xeon_sp: Fix size_t type mismatch in print statement ...................................................................... soc/intel/xeon_sp: Fix size_t type mismatch in print statement The 64-bit compiler x86_64-linux-gnu-gcc-10 aborts the build with the format warning below: CC romstage/soc/intel/xeon_sp/memmap.o src/soc/intel/xeon_sp/memmap.c: In function 'fill_postcar_frame': src/soc/intel/xeon_sp/memmap.c:39:62: error: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Werror=format=] 39 | printk(BIOS_DEBUG, "cbmem base_ptr: 0x%lx, size: 0x%lx\n", cbmem_base, cbmem_size); | ~~^ ~~~~~~~~~~ | | | | long unsigned int size_t {aka unsigned int} | %x As `cbmem_size` is of type `size_t` use the appropriate length modifier `z`. Change-Id: I1ca77de1ce33ce1e97d7c8895c6e75424f0769f5 Found-by: gcc (Debian 11.2.0-10) 11.2.0 Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M src/soc/intel/xeon_sp/memmap.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/59054/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59054 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1ca77de1ce33ce1e97d7c8895c6e75424f0769f5 Gerrit-Change-Number: 59054 Gerrit-PatchSet: 2 Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org> 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: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Lance Zhao 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: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> 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-MessageType: newpatchset

1 0

Change in coreboot[master]: WIP soc: Added dram information to cbmem WIP
by Paul Menzel (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Ravi kumar, Shelley Chen, mturney mturney, Julius Werner. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59195 ) Change subject: WIP soc: Added dram information to cbmem WIP ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/59195/comment/7802ac48_7634f0b5 PS1, Line 7: Added Add -- To view, visit https://review.coreboot.org/c/coreboot/+/59195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f1dd05ee224bf8284661c0afaa01d0a9d71daa7 Gerrit-Change-Number: 59195 Gerrit-PatchSet: 1 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org> Gerrit-Reviewer: mturney mturney <quic_mturney(a)quicinc.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: mturney mturney <mturney(a)codeaurora.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: mturney mturney <quic_mturney(a)quicinc.com> Gerrit-Comment-Date: Thu, 11 Nov 2021 16:50:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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