coreboot-gerrit October 2020

coreboot-gerrit@coreboot.org

1 participants
3474 discussions

Change in coreboot[master]: soc/mediatek/mt8192: add rtc MT6359P driver
by Roger Lu (Code Review) 04 Dec '20

04 Dec '20

Hello Roger Lu, Yuchen Huang, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/46877 to review the following change. Change subject: soc/mediatek/mt8192: add rtc MT6359P driver ...................................................................... soc/mediatek/mt8192: add rtc MT6359P driver fix rtc for boot reason check TEST=boot asurada Signed-off-by: Yuchen Huang <yuchen.huang(a)mediatek.corp-partner.google.com> Change-Id: I86b5c1b8d8ccedbb51bbc6e4c9d490c893fb4506 --- M src/soc/mediatek/common/rtc.c M src/soc/mediatek/mt8192/include/soc/rtc.h M src/soc/mediatek/mt8192/rtc.c 3 files changed, 71 insertions(+), 34 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/46877/1 diff --git a/src/soc/mediatek/common/rtc.c b/src/soc/mediatek/common/rtc.c index d0b90f7..5ac19eb 100755 --- a/src/soc/mediatek/common/rtc.c +++ b/src/soc/mediatek/common/rtc.c @@ -141,9 +141,6 @@ rtc_info("con=%x, pwrkey1=%x, pwrkey2=%x\n", con, pwrky1, pwrky2); - if (con & RTC_CON_LPSTA_RAW) - return RTC_STATE_INIT; - if (!rtc_busy_wait()) return RTC_STATE_RECOVER; @@ -168,9 +165,11 @@ rtc_write(RTC_BBPU, bbpu | RTC_BBPU_KEY | RTC_BBPU_RELOAD); rtc_write_trigger(); rtc_osc_init(); + rtc_info("RTC_STATE_REBOOT\n"); break; case RTC_STATE_RECOVER: rtc_init(1); + rtc_info("RTC_STATE_RECOVER\n"); break; case RTC_STATE_INIT: default: diff --git a/src/soc/mediatek/mt8192/include/soc/rtc.h b/src/soc/mediatek/mt8192/include/soc/rtc.h index b1be85c..bad7c68 100755 --- a/src/soc/mediatek/mt8192/include/soc/rtc.h +++ b/src/soc/mediatek/mt8192/include/soc/rtc.h @@ -108,10 +108,18 @@ RTC_EMBCK_SRC_SEL = 1U << 8, RTC_EMBCK_SEL_OPTION = 1U << 9, RTC_GPS_CKOUT_EN = 1U << 10, + RTC_EOSC32_VCT_EN = 1U << 11, + RTC_EOSC32_CHOP_EN = 1U << 12, + RTC_GP_OSC32_CON = 2U << 13, /* Keep RG_EOSC_RSV[0] to low for lower leakage current hw design change */ RTC_REG_XOSC32_ENB = 1U << 15 }; enum { + OSC32CON_ANALOG_SETTING = RTC_GP_OSC32_CON | RTC_EOSC32_CHOP_EN | RTC_EOSC32_VCT_EN |\ + RTC_GPS_CKOUT_EN | RTC_EMBCK_SEL_OPTION | RTC_EMB_K_EOSC32_MODE +}; + +enum { RTC_LPD_OPT_XOSC_AND_EOSC_LPD = 0U << 13, RTC_LPD_OPT_EOSC_LPD = 1U << 13, RTC_LPD_OPT_XOSC_LPD = 2U << 13, @@ -142,6 +150,15 @@ PMIC_RG_FQMTR_CK_PDN_SHIFT = 11 }; +enum { + PMIC_RG_BANK_FQMTR_RST = 0x522 +}; + +enum { + PMIC_RG_FQMTR_DCXO26M_EN_SHIFT = 4, + PMIC_RG_BANK_FQMTR_RST_SHIFT = 6 +}; + /* PMIC DCXO Register Definition */ enum { PMIC_RG_DCXO_CW00 = 0x0788, @@ -208,6 +225,12 @@ RTC_TC_MTH_MASK = 0xf }; +enum { + RTC_K_EOSC_RSV_0 = 1 << 8, + RTC_K_EOSC_RSV_1 = 1 << 9, + RTC_K_EOSC_RSV_2 = 1 << 10 +}; + int rtc_read(u16 addr, u16 *rdata); int rtc_write(u16 addr, u16 wdata); void rtc_write_field(u16 reg, u16 val, u16 mask, u16 shift); diff --git a/src/soc/mediatek/mt8192/rtc.c b/src/soc/mediatek/mt8192/rtc.c index a8e28b5..f02c633 100755 --- a/src/soc/mediatek/mt8192/rtc.c +++ b/src/soc/mediatek/mt8192/rtc.c @@ -57,15 +57,12 @@ } u16 bbpu, con, osc32con, sec; - rtc_read(RTC_BBPU, &bbpu); rtc_write(RTC_BBPU, bbpu | RTC_BBPU_KEY | RTC_BBPU_RELOAD); rtc_write_trigger(); rtc_read(RTC_OSC32CON, &osc32con); - osc32con &= ~(RTC_EMBCK_SRC_SEL | RTC_EMBCK_SEL_MODE_MASK - | RTC_GPS_CKOUT_EN); - osc32con |= RTC_XOSC32_ENB | RTC_REG_XOSC32_ENB - | RTC_EMB_K_EOSC32_MODE | RTC_EMBCK_SEL_OPTION; + osc32con &= ~(RTC_EMBCK_SRC_SEL | RTC_EMBCK_SEL_MODE_MASK); + osc32con |= (OSC32CON_ANALOG_SETTING | RTC_REG_XOSC32_ENB); if (!rtc_xosc_write(osc32con)) { rtc_info("rtc_xosc_write() failed\n"); @@ -99,7 +96,7 @@ static u16 rtc_get_frequency_meter(u16 val, u16 measure_src, u16 window_size) { u16 bbpu, osc32con; - u16 fqmtr_busy, fqmtr_data, fqmtr_rst, fqmtr_tcksel; + u16 fqmtr_busy, fqmtr_data, fqmtr_tcksel; struct stopwatch sw; if (val) { @@ -111,32 +108,34 @@ (val & RTC_XOSCCALI_MASK)); } + /* RG_BANK_FQMTR_RST=1, reset FQMTR*/ + rtc_write_field(PMIC_RG_BANK_FQMTR_RST, 1, 1, + PMIC_RG_BANK_FQMTR_RST_SHIFT); + udelay(20); + /* RG_BANK_FQMTR_RST=0, release FQMTR*/ + rtc_write_field(PMIC_RG_BANK_FQMTR_RST, 0, 1, + PMIC_RG_BANK_FQMTR_RST_SHIFT); + /* enable FQMTR clock */ rtc_write_field(PMIC_RG_TOP_CKPDN_CON0_CLR, 1, 1, - PMIC_RG_FQMTR_32K_CK_PDN_SHIFT); - rtc_write_field(PMIC_RG_TOP_CKPDN_CON0_CLR, 1, 1, PMIC_RG_FQMTR_CK_PDN_SHIFT); - /* FQMTR reset */ - rtc_write_field(PMIC_RG_FQMTR_RST, 1, 1, PMIC_FQMTR_RST_SHIFT); + rtc_write_field(PMIC_RG_TOP_CKPDN_CON0_CLR, 1, 1, + PMIC_RG_FQMTR_32K_CK_PDN_SHIFT); - do { - rtc_read(PMIC_RG_FQMTR_DATA, &fqmtr_data); - rtc_read(PMIC_RG_FQMTR_CON0, &fqmtr_busy); - } while (fqmtr_data && (fqmtr_busy & PMIC_FQMTR_CON0_BUSY)); - rtc_read(PMIC_RG_FQMTR_RST, &fqmtr_rst); - /* FQMTR normal */ - rtc_write_field(PMIC_RG_FQMTR_RST, 0, 1, PMIC_FQMTR_RST_SHIFT); + rtc_write_field(PMIC_RG_FQMTR_CON0, 1, 1, + PMIC_RG_FQMTR_DCXO26M_EN_SHIFT); + /* set frequency meter window value (0=1X32K(fixed clock)) */ rtc_write(PMIC_RG_FQMTR_WINSET, window_size); /* enable 26M and set test clock source */ rtc_write(PMIC_RG_FQMTR_CON0, PMIC_FQMTR_CON0_DCXO26M_EN | measure_src); /* enable 26M -> delay 100us -> enable FQMTR */ - udelay(100); + mdelay(1); rtc_read(PMIC_RG_FQMTR_CON0, &fqmtr_tcksel); /* enable FQMTR */ rtc_write(PMIC_RG_FQMTR_CON0, fqmtr_tcksel | PMIC_FQMTR_CON0_FQMTR_EN); - udelay(100); + mdelay(1); stopwatch_init_usecs_expire(&sw, FQMTR_TIMEOUT_US); /* FQMTR read until ready */ do { @@ -149,30 +148,35 @@ /* read data should be closed to 26M/32k = 794 */ rtc_read(PMIC_RG_FQMTR_DATA, &fqmtr_data); - rtc_read(PMIC_RG_FQMTR_CON0, &fqmtr_tcksel); + /* disable FQMTR */ + rtc_read(PMIC_RG_FQMTR_CON0, &fqmtr_tcksel); rtc_write(PMIC_RG_FQMTR_CON0, fqmtr_tcksel & ~PMIC_FQMTR_CON0_FQMTR_EN); /* disable FQMTR -> delay 100us -> disable 26M */ - udelay(100); + mdelay(1); /* disable 26M */ rtc_read(PMIC_RG_FQMTR_CON0, &fqmtr_tcksel); rtc_write(PMIC_RG_FQMTR_CON0, fqmtr_tcksel & ~PMIC_FQMTR_CON0_DCXO26M_EN); - rtc_info("input=0x%x, output=%d\n", val, fqmtr_data); + rtc_info("input=%d, output=%d\n", val, fqmtr_data); + /* disable FQMTR clock */ rtc_write_field(PMIC_RG_TOP_CKPDN_CON0_SET, 1, 1, - PMIC_RG_FQMTR_32K_CK_PDN_SHIFT); - rtc_write_field(PMIC_RG_TOP_CKPDN_CON0_SET, 1, 1, PMIC_RG_FQMTR_CK_PDN_SHIFT); + rtc_write_field(PMIC_RG_TOP_CKPDN_CON0_SET, 1, 1, + PMIC_RG_FQMTR_32K_CK_PDN_SHIFT); + return fqmtr_data; } /* 32k clock calibration */ static u16 rtc_eosc_cali(void) { - u16 middle, diff1, diff2, cksel; + u16 diff1, diff2, cksel; u16 val = 0; - u16 left = RTC_XOSCCALI_START, right = RTC_XOSCCALI_END; + static u16 middle = 0; + static u16 left = RTC_XOSCCALI_START; + static u16 right = RTC_XOSCCALI_END; rtc_read(PMIC_RG_FQMTR_CKSEL, &cksel); cksel &= ~PMIC_FQMTR_CKSEL_MASK; @@ -183,16 +187,20 @@ while (left <= right) { middle = (right + left) / 2; - if (middle == left) + if (middle == left) { break; + } /* select 26M as target clock */ val = rtc_get_frequency_meter(middle, PMIC_FQMTR_CON0_FQM26M_CK, 0); - if (val >= RTC_FQMTR_LOW_BASE && val <= RTC_FQMTR_HIGH_BASE) + if (val >= RTC_FQMTR_LOW_BASE && val <= RTC_FQMTR_HIGH_BASE) { break; - if (val > RTC_FQMTR_HIGH_BASE) + } + if (val > RTC_FQMTR_HIGH_BASE) { right = middle; - else + } + else { left = middle; + } } if (val >= RTC_FQMTR_LOW_BASE && val <= RTC_FQMTR_HIGH_BASE) @@ -212,6 +220,7 @@ else diff2 = RTC_FQMTR_LOW_BASE - val; + rtc_info("left:%d, middle:%d, right: %d\n", left, middle, right); if (diff1 < diff2) return left; else @@ -332,6 +341,7 @@ int rtc_init(u8 recover) { int ret; + u16 year; rtc_info("recovery: %d\n", recover); @@ -347,7 +357,7 @@ goto err; } - //rtc_osc_init(); + rtc_osc_init(); if (recover) mdelay(20); @@ -367,6 +377,11 @@ goto err; } + /* solution1 for EOSC cali*/ + rtc_read(RTC_AL_YEA, &year); + rtc_write(RTC_AL_YEA, (year | RTC_K_EOSC_RSV_0) & (~RTC_K_EOSC_RSV_1) & (~RTC_K_EOSC_RSV_2)); + rtc_write_trigger(); + if (!rtc_lpd_init()) { ret = -RTC_STATUS_LPD_INIT_FAIL; goto err; -- To view, visit https://review.coreboot.org/c/coreboot/+/46877 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86b5c1b8d8ccedbb51bbc6e4c9d490c893fb4506 Gerrit-Change-Number: 46877 Gerrit-PatchSet: 1 Gerrit-Owner: Roger Lu <roger.lu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Roger Lu <roger.lu(a)mediatek.com> Gerrit-Reviewer: Yuchen Huang <yuchen.huang(a)mediatek.corp-partner.google.com> Gerrit-MessageType: newchange

3 10

Change in coreboot[master]: Retrieve CBI about Retimer and notify TBT firmware
by John Zhao (Code Review) 04 Dec '20

04 Dec '20

John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46571 ) Change subject: Retrieve CBI about Retimer and notify TBT firmware ...................................................................... Retrieve CBI about Retimer and notify TBT firmware Boards might have 20 Gbps or 40 Gbps retimer respectively. It is required to inform Thunderbolt firmware about the Type-C port speed capability. This change retrieves CBI FW_CONFIG tag which has on board retimer setting and notifies LC by programming the dedicated PCIe configuration register. LC delays link establishment until it gets the port speed notification. BUG=b:163110905 TEST=booted to kernel and validated TCSS functions on Volteer boards which have 20 Gbps and 40 Gbps retimer respectively. Signed-off-by: John Zhao <john.zhao(a)intel.com> Change-Id: I0bbad465dcab5b44b1ddbd6431391b5a3a18fa12 --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 42 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/46571/1 diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c index 03a78fd..9a0f5c9 100644 --- a/src/mainboard/google/volteer/mainboard.c +++ b/src/mainboard/google/volteer/mainboard.c @@ -4,6 +4,7 @@ #include <acpi/acpi.h> #include <baseboard/variants.h> #include <device/device.h> +#include <device/pci.h> #include <drivers/spi/tpm/tpm.h> #include <ec/ec.h> #include <fw_config.h> @@ -79,19 +80,57 @@ override_pads, override_num); } +static void mainboard_retimer_notify(struct device *dev, bool usb4_gen) +{ + uint32_t reg32; + + /* + * Program host interface PCIe configuration register TBT_DMA_CFG_VS_CAP_17 for + * 20G and 40G retimer respectively. + * TBT_DMA_CFG_VS_CAP_17[0]: "speed indication valid" - 0->invalid, 1->valid + * TBT_DMA_CFG_VS_CAP_17[1]: "speed indication value" - 0->20 Gbps, 1->40 Gbps + */ + #define TBT_DMA_CFG_VS_CAP_17 0xe8 + + reg32 = pci_read_config32(dev, TBT_DMA_CFG_VS_CAP_17); + /* Skip programming if it had previously been configured */ + if (reg32 & 0x1) + return; + + if (usb4_gen) + reg32 |= 0x2; + else + reg32 &= 0x2; + + pci_write_config32(dev, TBT_DMA_CFG_VS_CAP_17, (reg32 | 0x1)); +} + void mainboard_silicon_init_params(FSP_S_CONFIG *params) { + struct device *dev; bool has_usb4; + bool usb4_gen2, usb4_gen3; /* If device doesn't have USB4 hardware, disable tbt */ - has_usb4 = (fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN2)) || - fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN3))); + usb4_gen2 = fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN2)); + usb4_gen3 = fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN3)); + has_usb4 = (usb4_gen2 || usb4_gen3); - if (!has_usb4) + if (!has_usb4) { memset(params->ITbtPcieRootPortEn, 0, ARRAY_SIZE(params->ITbtPcieRootPortEn) * sizeof(*params->ITbtPcieRootPortEn)); + return; + } + + dev = pcidev_path_on_root(SA_DEVFN_TCSS_DMA0); + if (!dev) + return; + + /* usb4_gen2 for 20G and usb4_gen3 for 40G retimer */ + dev = pcidev_path_on_root(SA_DEVFN_TCSS_DMA0); + mainboard_retimer_notify(dev, ((!usb4_gen2 && usb4_gen3) ? 1 : 0)); } struct chip_operations mainboard_ops = { -- To view, visit https://review.coreboot.org/c/coreboot/+/46571 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0bbad465dcab5b44b1ddbd6431391b5a3a18fa12 Gerrit-Change-Number: 46571 Gerrit-PatchSet: 1 Gerrit-Owner: John Zhao <john.zhao(a)intel.com> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: [TESTONLY] build with TRACE=y
by Kyösti Mälkki (Code Review) 03 Dec '20

03 Dec '20

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43870 ) Change subject: [TESTONLY] build with TRACE=y ...................................................................... [TESTONLY] build with TRACE=y Change-Id: Iee20e3c1fc31aba704d813f909c39986df3e2c29 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/43870/1 diff --git a/src/Kconfig b/src/Kconfig index a4c2fa6..fb7cd2d 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -1055,7 +1055,7 @@ config TRACE bool "Trace function calls" - default n + default y help If enabled, every function will print information to console once the function is entered. The syntax is ~0xaaaabbbb(0xccccdddd) -- To view, visit https://review.coreboot.org/c/coreboot/+/43870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iee20e3c1fc31aba704d813f909c39986df3e2c29 Gerrit-Change-Number: 43870 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

1 3

Change in coreboot[master]: src/soc/tigerlake: Define and use a Kconfig for no of USB ports.
by Meera Ravindranath (Code Review) 03 Dec '20

03 Dec '20

Meera Ravindranath has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38682 ) Change subject: src/soc/tigerlake: Define and use a Kconfig for no of USB ports. ...................................................................... src/soc/tigerlake: Define and use a Kconfig for no of USB ports. BUG=None BRANCH=None TEST=Compilation for jasper lake board is working Change-Id: Ia8e88e92989fe40d7bd1c28942e005cb0d862fcb Signed-off-by: Meera Ravindranath <meera.ravindranath(a)intel.com> --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/fsp_params_jsl.c M src/soc/intel/tigerlake/fsp_params_tgl.c 3 files changed, 14 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/38682/1 diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index cef1fd0..f50e40e 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -156,6 +156,16 @@ default 3 if SOC_INTEL_JASPERLAKE default 4 if SOC_INTEL_TIGERLAKE +config SOC_INTEL_USB2_DEV_MAX + int + default 8 if SOC_INTEL_JASPERLAKE + default 10 if SOC_INTEL_TIGERLAKE + +config SOC_INTEL_USB3_DEV_MAX + int + default 6 if SOC_INTEL_JASPERLAKE + default 4 if SOC_INTEL_TIGERLAKE + config SOC_INTEL_I2C_DEV_MAX int default 6 diff --git a/src/soc/intel/tigerlake/fsp_params_jsl.c b/src/soc/intel/tigerlake/fsp_params_jsl.c index 8046b2e..d5d623b 100644 --- a/src/soc/intel/tigerlake/fsp_params_jsl.c +++ b/src/soc/intel/tigerlake/fsp_params_jsl.c @@ -106,7 +106,7 @@ memset(params->PcieRpPmSci, 0, sizeof(params->PcieRpPmSci)); /* USB configuration */ - for (i = 0; i < ARRAY_SIZE(config->usb2_ports); i++) { + for (i = 0; i < CONFIG_SOC_INTEL_USB2_DEV_MAX; i++) { params->PortUsb20Enable[i] = config->usb2_ports[i].enable; params->Usb2OverCurrentPin[i] = config->usb2_ports[i].ocpin; @@ -116,7 +116,7 @@ params->Usb2PhyPehalfbit[i] = config->usb2_ports[i].pre_emp_bit; } - for (i = 0; i < ARRAY_SIZE(config->usb3_ports); i++) { + for (i = 0; i < CONFIG_SOC_INTEL_USB3_DEV_MAX; i++) { params->PortUsb30Enable[i] = config->usb3_ports[i].enable; params->Usb3OverCurrentPin[i] = config->usb3_ports[i].ocpin; diff --git a/src/soc/intel/tigerlake/fsp_params_tgl.c b/src/soc/intel/tigerlake/fsp_params_tgl.c index 305748e..f027ee0 100644 --- a/src/soc/intel/tigerlake/fsp_params_tgl.c +++ b/src/soc/intel/tigerlake/fsp_params_tgl.c @@ -89,7 +89,7 @@ params->IomTypeCPortPadCfg[i] = 0x09000000; /* USB */ - for (i = 0; i < ARRAY_SIZE(config->usb2_ports); i++) { + for (i = 0; i < CONFIG_SOC_INTEL_USB2_DEV_MAX; i++) { params->PortUsb20Enable[i] = config->usb2_ports[i].enable; params->Usb2OverCurrentPin[i] = config->usb2_ports[i].ocpin; params->Usb2PhyPetxiset[i] = config->usb2_ports[i].pre_emp_bias; @@ -98,7 +98,7 @@ params->Usb2PhyPehalfbit[i] = config->usb2_ports[i].pre_emp_bit; } - for (i = 0; i < ARRAY_SIZE(config->usb3_ports); i++) { + for (i = 0; i < CONFIG_SOC_INTEL_USB3_DEV_MAX; i++) { params->PortUsb30Enable[i] = config->usb3_ports[i].enable; params->Usb3OverCurrentPin[i] = config->usb3_ports[i].ocpin; if (config->usb3_ports[i].tx_de_emp) { -- To view, visit https://review.coreboot.org/c/coreboot/+/38682 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia8e88e92989fe40d7bd1c28942e005cb0d862fcb Gerrit-Change-Number: 38682 Gerrit-PatchSet: 1 Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-MessageType: newchange

7 27

Change in coreboot[master]: x86: Put bootblock startup code into .text._start section
by Julius Werner (Code Review) 03 Dec '20

03 Dec '20

Hello Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/46834 to review the following change. Change subject: x86: Put bootblock startup code into .text._start section ...................................................................... x86: Put bootblock startup code into .text._start section The initial bootblock assembly code on x86 is just put into the .text section, which just happens to come before all the individual .text.* function sections in the program.ld script. So it tends to be at the start of the image, bit if you inserted another linker script section with contents before .text, it would cause a problem. (I'm not sure if it's an architectural requirement for _start16bit to come at the start of the image, but at least its 4K alignment requirement would waste a lot of space if it didn't.) This patch moves the section to .text._start which is the name other architectures use for the code they want in the very front of the image and which is listed first in program.ld. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Ia84e6e33ec29584d356e226e8fdcb8c9334d49af --- M src/arch/x86/bootblock_crt0.S 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46834/1 diff --git a/src/arch/x86/bootblock_crt0.S b/src/arch/x86/bootblock_crt0.S index 3f41464..82ae97f 100644 --- a/src/arch/x86/bootblock_crt0.S +++ b/src/arch/x86/bootblock_crt0.S @@ -10,7 +10,7 @@ #include <cpu/x86/cr.h> -.section .text +.section .text._start /* * Include the old code for reset vector and protected mode entry. That code has -- To view, visit https://review.coreboot.org/c/coreboot/+/46834 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia84e6e33ec29584d356e226e8fdcb8c9334d49af Gerrit-Change-Number: 46834 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: newchange

6 11

Change in coreboot[master]: cbfstool: Hide hash printing behind -v and add to parseable output
by Julius Werner (Code Review) 03 Dec '20

03 Dec '20

Hello Patrick Georgi, Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/41119 to review the following change. Change subject: cbfstool: Hide hash printing behind -v and add to parseable output ...................................................................... cbfstool: Hide hash printing behind -v and add to parseable output With the upcoming introduction of CBFS verification, a lot more CBFS files will have hashes. The current cbfstool default of always printing hash attributes when they exist will make cbfstool print very messy. Therefore, hide hash attribute output unless the user passed -v. It would also be useful to be able to get file attributes like hashes in machine parseable output. Unfortunately, our machine parseable format (-k) doesn't really seem designed to be extensible. To avoid breaking older parsers, this patch adds new attribute output behind -v (which hopefully no current users pass since it doesn't change anything for -k at the moment). With this patch cbfstool print -k -v may print an arbitrary amount of extra tokens behind the predefined ones on a file line. Tokens always begin with an identifying string (e.g. 'hash'), followed by extra fields that should be separated by colons. Multiple tokens are separated by the normal separator character (tab). cbfstool print -k -v may also print additional information that applies to the whole CBFS on separate lines. These lines will always begin with a '[' (which hopefully nobody would use as a CBFS filename character although we technically have no restrictions at the moment). Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I9e16cda393fa0bc1d8734d4b699e30e2ae99a36d --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h M util/cbfstool/cbfstool.c 3 files changed, 41 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/41119/1 diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 7942b27..3a83694 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -1503,6 +1503,9 @@ decompressed_size ); + if (!verbose) + return 0; + struct cbfs_file_attr_hash *attr = NULL; while ((attr = cbfs_file_get_next_hash(entry, attr)) != NULL) { size_t hash_len = vb2_digest_size(attr->hash.algo); @@ -1522,9 +1525,6 @@ free(hash_str); } - if (!verbose) - return 0; - DEBUG(" cbfs_file=0x%x, offset=0x%x, content_address=0x%x+0x%x\n", cbfs_get_entry_addr(image, entry), ntohl(entry->offset), cbfs_get_entry_addr(image, entry) + ntohl(entry->offset), @@ -1587,21 +1587,45 @@ fprintf(fp, "%s%s", type, sep); fprintf(fp, "0x%zx%s", metadata_size, sep); fprintf(fp, "0x%zx%s", data_size, sep); - fprintf(fp, "0x%zx\n", metadata_size + data_size); + fprintf(fp, "0x%zx", metadata_size + data_size); + + if (verbose) { + unsigned int decompressed_size = 0; + unsigned int compression = cbfs_file_get_compression_info(entry, + &decompressed_size); + if (compression != CBFS_COMPRESS_NONE) + fprintf(fp, "%scomp:%s:0x%x", sep, lookup_name_by_type( + types_cbfs_compression, compression, "????"), + decompressed_size); + + struct cbfs_file_attr_hash *attr = NULL; + while ((attr = cbfs_file_get_next_hash(entry, attr)) != NULL) { + size_t hash_len = vb2_digest_size(attr->hash.algo); + if (!hash_len) + continue; + char *hash_str = bintohex(attr->hash.raw, hash_len); + int valid = vb2_hash_verify(CBFS_SUBHEADER(entry), + ntohl(entry->len), &attr->hash) == VB2_SUCCESS; + fprintf(fp, "%shash:%s:%s:%s", sep, + vb2_get_hash_algorithm_name(attr->hash.algo), + hash_str, valid ? "valid" : "invalid"); + free(hash_str); + } + } + fprintf(fp, "\n"); return 0; } -int cbfs_print_directory(struct cbfs_image *image) +void cbfs_print_directory(struct cbfs_image *image) { if (cbfs_is_legacy_cbfs(image)) cbfs_print_header_info(image); printf("%-30s %-10s %-12s Size Comp\n", "Name", "Offset", "Type"); cbfs_legacy_walk(image, cbfs_print_entry_info, NULL); - return 0; } -int cbfs_print_parseable_directory(struct cbfs_image *image) +void cbfs_print_parseable_directory(struct cbfs_image *image) { size_t i; const char *header[] = { @@ -1618,7 +1642,6 @@ fprintf(stdout, "%s%s", header[i], sep); fprintf(stdout, "%s\n", header[i]); cbfs_legacy_walk(image, cbfs_print_parseable_entry_info, stdout); - return 0; } int cbfs_merge_empty_entry(struct cbfs_image *image, struct cbfs_file *entry, diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index a3a6fb9..d633fee 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -174,8 +174,8 @@ int cbfs_is_valid_entry(struct cbfs_image *image, struct cbfs_file *entry); /* Print CBFS component information. */ -int cbfs_print_directory(struct cbfs_image *image); -int cbfs_print_parseable_directory(struct cbfs_image *image); +void cbfs_print_directory(struct cbfs_image *image); +void cbfs_print_parseable_directory(struct cbfs_image *image); int cbfs_print_header_info(struct cbfs_image *image); int cbfs_print_entry_info(struct cbfs_image *image, struct cbfs_file *entry, void *arg); diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index f97745e..6015603 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -1087,12 +1087,16 @@ if (cbfs_image_from_buffer(&image, param.image_region, param.headeroffset)) return 1; - if (param.machine_parseable) - return cbfs_print_parseable_directory(&image); - else { + if (param.machine_parseable) { + if (verbose) + printf("[FMAP REGION]\t%s\n", param.region_name); + cbfs_print_parseable_directory(&image); + } else { printf("FMAP REGION: %s\n", param.region_name); - return cbfs_print_directory(&image); + cbfs_print_directory(&image); } + + return 0; } static int cbfs_extract(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/41119 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9e16cda393fa0bc1d8734d4b699e30e2ae99a36d Gerrit-Change-Number: 41119 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: cbfstool: Rename cbfs_walk() to cbfs_legacy_walk()
by Julius Werner (Code Review) 03 Dec '20

03 Dec '20

Hello Patrick Georgi, Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/41118 to review the following change. Change subject: cbfstool: Rename cbfs_walk() to cbfs_legacy_walk() ...................................................................... cbfstool: Rename cbfs_walk() to cbfs_legacy_walk() This function name clashes with cbfs_walk() in the new commonlib CBFS stack, so rename it to cbfs_legacy_walk(). While we could replace it with the new commonlib implementation, it still has support for certain features in the deprecated pre-FMAP CBFSes (such as non-standard header alignment), which are needed to handle old files but probably not something we'd want to burden the commonlib implementation with. So until we decide to deprecate support for those files from cbfstool as well, it seems easier to just keep the existing implementation here. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I37c7e7aa9a206372817d8d0b8f66d72bafb4f346 --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h 2 files changed, 13 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/41118/1 diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 035b196..7942b27 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -469,7 +469,7 @@ cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, last_entry_size, ""); /* If the last entry was an empty file, merge them. */ - cbfs_walk(&image, cbfs_merge_empty_entry, NULL); + cbfs_legacy_walk(&image, cbfs_merge_empty_entry, NULL); } return 0; @@ -621,7 +621,7 @@ prev_size, ""); /* Merge any potential empty entries together. */ - cbfs_walk(image, cbfs_merge_empty_entry, NULL); + cbfs_legacy_walk(image, cbfs_merge_empty_entry, NULL); /* * Since current switched to an empty file keep track of it. @@ -760,7 +760,7 @@ // Merge empty entries. DEBUG("(trying to merge empty entries...)\n"); - cbfs_walk(image, cbfs_merge_empty_entry, NULL); + cbfs_legacy_walk(image, cbfs_merge_empty_entry, NULL); for (entry = cbfs_find_first_entry(image); entry && cbfs_is_valid_entry(image, entry); @@ -1380,7 +1380,7 @@ DEBUG("cbfs_remove_entry: Removed %s @ 0x%x\n", entry->filename, cbfs_get_entry_addr(image, entry)); entry->type = htonl(CBFS_TYPE_DELETED); - cbfs_walk(image, cbfs_merge_empty_entry, NULL); + cbfs_legacy_walk(image, cbfs_merge_empty_entry, NULL); return 0; } @@ -1597,7 +1597,7 @@ if (cbfs_is_legacy_cbfs(image)) cbfs_print_header_info(image); printf("%-30s %-10s %-12s Size Comp\n", "Name", "Offset", "Type"); - cbfs_walk(image, cbfs_print_entry_info, NULL); + cbfs_legacy_walk(image, cbfs_print_entry_info, NULL); return 0; } @@ -1617,7 +1617,7 @@ for (i = 0; i < ARRAY_SIZE(header) - 1; i++) fprintf(stdout, "%s%s", header[i], sep); fprintf(stdout, "%s\n", header[i]); - cbfs_walk(image, cbfs_print_parseable_entry_info, stdout); + cbfs_legacy_walk(image, cbfs_print_parseable_entry_info, stdout); return 0; } @@ -1660,7 +1660,7 @@ return 0; } -int cbfs_walk(struct cbfs_image *image, cbfs_entry_callback callback, +int cbfs_legacy_walk(struct cbfs_image *image, cbfs_entry_callback callback, void *arg) { int count = 0; @@ -1970,7 +1970,7 @@ need_len = metadata_size + size; // Merge empty entries to build get max available space. - cbfs_walk(image, cbfs_merge_empty_entry, NULL); + cbfs_legacy_walk(image, cbfs_merge_empty_entry, NULL); /* Three cases of content location on memory page: * case 1. diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index 6ff0746..a3a6fb9 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -129,16 +129,18 @@ int32_t cbfs_locate_entry(struct cbfs_image *image, size_t size, size_t page_size, size_t align, size_t metadata_size); -/* Callback function used by cbfs_walk. +/* Callback function used by cbfs_legacy_walk. * Returns 0 on success, or non-zero to stop further iteration. */ typedef int (*cbfs_entry_callback)(struct cbfs_image *image, struct cbfs_file *file, void *arg); /* Iterates through all entries in CBFS image, and invoke with callback. - * Stops if callback returns non-zero values. + * Stops if callback returns non-zero values. Unlike the commonlib cbfs_walk(), + * this can deal with different alignments in legacy CBFS (with master header). * Returns number of entries invoked. */ -int cbfs_walk(struct cbfs_image *image, cbfs_entry_callback callback, void *arg); +int cbfs_legacy_walk(struct cbfs_image *image, cbfs_entry_callback callback, + void *arg); /* Primitive CBFS utilities */ -- To view, visit https://review.coreboot.org/c/coreboot/+/41118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I37c7e7aa9a206372817d8d0b8f66d72bafb4f346 Gerrit-Change-Number: 41118 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

2 2

Change in coreboot[master]: src: Remove redundant use of ACPI offset(0)
by HAOUAS Elyes (Code Review) 03 Dec '20

03 Dec '20

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43283 ) Change subject: src: Remove redundant use of ACPI offset(0) ...................................................................... src: Remove redundant use of ACPI offset(0) Change-Id: I260a79ef77025b4befbccc21f5999f89d90c1154 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/ec/compal/ene932/acpi/ec.asl M src/ec/google/chromeec/acpi/ec.asl M src/ec/google/chromeec/acpi/emem.asl M src/ec/quanta/it8518/acpi/ec.asl M src/ec/roda/it8518/acpi/ec.asl M src/mainboard/facebook/monolith/acpi/ec.asl M src/mainboard/lenovo/s230u/acpi/ec.asl M src/mainboard/lippert/frontrunner-af/dsdt.asl M src/mainboard/lippert/toucan-af/dsdt.asl M src/soc/amd/picasso/acpi/globalnvs.asl M src/soc/amd/stoneyridge/acpi/globalnvs.asl M src/soc/intel/apollolake/acpi/globalnvs.asl M src/soc/intel/baytrail/acpi/globalnvs.asl M src/soc/intel/broadwell/acpi/globalnvs.asl M src/soc/intel/broadwell/acpi/xhci.asl M src/soc/intel/cannonlake/acpi/scs.asl M src/soc/intel/denverton_ns/acpi/globalnvs.asl M src/soc/intel/skylake/acpi/globalnvs.asl M src/soc/intel/skylake/acpi/xhci.asl M src/southbridge/amd/cimx/sb800/acpi/fch.asl M src/southbridge/intel/bd82x6x/acpi/globalnvs.asl M src/southbridge/intel/bd82x6x/acpi/pch.asl M src/southbridge/intel/i82801ix/acpi/globalnvs.asl M src/southbridge/intel/i82801ix/acpi/ich9.asl M src/southbridge/intel/i82801jx/acpi/globalnvs.asl M src/southbridge/intel/i82801jx/acpi/ich10.asl M src/southbridge/intel/lynxpoint/acpi/globalnvs.asl M src/southbridge/intel/lynxpoint/acpi/usb.asl M src/superio/fintek/f81803a/acpi/superio.asl 29 files changed, 0 insertions(+), 29 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/43283/1 diff --git a/src/ec/compal/ene932/acpi/ec.asl b/src/ec/compal/ene932/acpi/ec.asl index 6b0997e..db53737 100644 --- a/src/ec/compal/ene932/acpi/ec.asl +++ b/src/ec/compal/ene932/acpi/ec.asl @@ -16,7 +16,6 @@ Field (ERAM, ByteAcc, Lock, Preserve) { // EC Name Space Configuration - Offset(0x00), , 1, // Reserved ; 00h.0 LCDS, 1, // 1= BACKLIGHT ON , 0= BACKLIGHT OFF ; 00h.1 , 6, // Reserved ; 00h.2 ~ 00h.6 diff --git a/src/ec/google/chromeec/acpi/ec.asl b/src/ec/google/chromeec/acpi/ec.asl index f8d4bdf..837bbd1 100644 --- a/src/ec/google/chromeec/acpi/ec.asl +++ b/src/ec/google/chromeec/acpi/ec.asl @@ -28,7 +28,6 @@ OperationRegion (ERAM, EmbeddedControl, 0x00, EC_ACPI_MEM_MAPPED_BEGIN) Field (ERAM, ByteAcc, Lock, Preserve) { - Offset (0x00), RAMV, 8, // EC RAM Version TSTB, 8, // Test Byte TSTC, 8, // Complement of Test Byte diff --git a/src/ec/google/chromeec/acpi/emem.asl b/src/ec/google/chromeec/acpi/emem.asl index 77a6f71..2a556ed 100644 --- a/src/ec/google/chromeec/acpi/emem.asl +++ b/src/ec/google/chromeec/acpi/emem.asl @@ -4,7 +4,6 @@ * EMEM data may be accessed through port 62/66 or through LPC at 900h. */ -Offset (0x00), TIN0, 8, // Temperature 0 TIN1, 8, // Temperature 1 TIN2, 8, // Temperature 2 diff --git a/src/ec/quanta/it8518/acpi/ec.asl b/src/ec/quanta/it8518/acpi/ec.asl index a3598be..5f2300a 100644 --- a/src/ec/quanta/it8518/acpi/ec.asl +++ b/src/ec/quanta/it8518/acpi/ec.asl @@ -16,7 +16,6 @@ OperationRegion(ERAM, EmbeddedControl, 0, 0xFF) Field (ERAM, ByteAcc, NoLock, Preserve) { - Offset(0x00), // [Configuration Space 0] , 1, // Reserved bit[0] ENGA, 1, // Enable Global attention ENHY, 1, // Enable Hotkey function diff --git a/src/ec/roda/it8518/acpi/ec.asl b/src/ec/roda/it8518/acpi/ec.asl index d500084..de121c6 100644 --- a/src/ec/roda/it8518/acpi/ec.asl +++ b/src/ec/roda/it8518/acpi/ec.asl @@ -14,7 +14,6 @@ OperationRegion(ERAM, EmbeddedControl, 0, 0xFF) Field (ERAM, ByteAcc, NoLock, Preserve) { - Offset(0x00), // [Configuration Space 0x0] Offset(0x02), // [Configuration Space 0x2] B0ST, 16, // Battery 0 State B0FC, 16, // Battery 0 Last Full Charge Capacity diff --git a/src/mainboard/facebook/monolith/acpi/ec.asl b/src/mainboard/facebook/monolith/acpi/ec.asl index 8aff541..5e72456 100644 --- a/src/mainboard/facebook/monolith/acpi/ec.asl +++ b/src/mainboard/facebook/monolith/acpi/ec.asl @@ -29,7 +29,6 @@ OperationRegion (ERAM, EmbeddedControl, 0x00, 0xFF) Field (ERAM, ByteAcc, NoLock, Preserve) { - Offset (0x00), CPUT, 8, } diff --git a/src/mainboard/lenovo/s230u/acpi/ec.asl b/src/mainboard/lenovo/s230u/acpi/ec.asl index 888feac..20a9096 100644 --- a/src/mainboard/lenovo/s230u/acpi/ec.asl +++ b/src/mainboard/lenovo/s230u/acpi/ec.asl @@ -12,7 +12,6 @@ OperationRegion (ERAM, SystemMemory, (CONFIG_EC_BASE_ADDRESS + 0x100), 0x100) Field (ERAM, ByteAcc, Lock, Preserve) { - Offset(0x00), , 1, , 1, HKFA, 1, // FN lock (Hotkey / FN row toggle) diff --git a/src/mainboard/lippert/frontrunner-af/dsdt.asl b/src/mainboard/lippert/frontrunner-af/dsdt.asl index a6a7ea5..52a7cbc 100644 --- a/src/mainboard/lippert/frontrunner-af/dsdt.asl +++ b/src/mainboard/lippert/frontrunner-af/dsdt.asl @@ -135,7 +135,6 @@ PIOD, 0x00000008, } IndexField (PIOI, PIOD, ByteAcc, NoLock, Preserve) { - Offset(0x00), /* MiscControl */ , 1, T1EE, 1, T2EE, 1, diff --git a/src/mainboard/lippert/toucan-af/dsdt.asl b/src/mainboard/lippert/toucan-af/dsdt.asl index a6562f5..eb40daf 100644 --- a/src/mainboard/lippert/toucan-af/dsdt.asl +++ b/src/mainboard/lippert/toucan-af/dsdt.asl @@ -135,7 +135,6 @@ PIOD, 0x00000008, } IndexField (PIOI, PIOD, ByteAcc, NoLock, Preserve) { - Offset(0x00), /* MiscControl */ , 1, T1EE, 1, T2EE, 1, diff --git a/src/soc/amd/picasso/acpi/globalnvs.asl b/src/soc/amd/picasso/acpi/globalnvs.asl index 3ab35df..4a541e6 100644 --- a/src/soc/amd/picasso/acpi/globalnvs.asl +++ b/src/soc/amd/picasso/acpi/globalnvs.asl @@ -12,7 +12,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), PCNT, 8, // 0x00 - Processor Count PPCM, 8, // 0x01 - Max PPC State LIDS, 8, // 0x02 - LID State diff --git a/src/soc/amd/stoneyridge/acpi/globalnvs.asl b/src/soc/amd/stoneyridge/acpi/globalnvs.asl index 2865352..9189df0 100644 --- a/src/soc/amd/stoneyridge/acpi/globalnvs.asl +++ b/src/soc/amd/stoneyridge/acpi/globalnvs.asl @@ -12,7 +12,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), PCNT, 8, // 0x00 - Processor Count PPCM, 8, // 0x01 - Max PPC State LIDS, 8, // 0x02 - LID State diff --git a/src/soc/intel/apollolake/acpi/globalnvs.asl b/src/soc/intel/apollolake/acpi/globalnvs.asl index 82f41b3..8b03713 100644 --- a/src/soc/intel/apollolake/acpi/globalnvs.asl +++ b/src/soc/intel/apollolake/acpi/globalnvs.asl @@ -12,7 +12,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), PCNT, 8, // 0x00 - Processor Count PPCM, 8, // 0x01 - Max PPC State LIDS, 8, // 0x02 - LID State diff --git a/src/soc/intel/baytrail/acpi/globalnvs.asl b/src/soc/intel/baytrail/acpi/globalnvs.asl index 6cb68ba..7084ff2 100644 --- a/src/soc/intel/baytrail/acpi/globalnvs.asl +++ b/src/soc/intel/baytrail/acpi/globalnvs.asl @@ -16,7 +16,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/soc/intel/broadwell/acpi/globalnvs.asl b/src/soc/intel/broadwell/acpi/globalnvs.asl index 3c6c5f5..9a4a0b5 100644 --- a/src/soc/intel/broadwell/acpi/globalnvs.asl +++ b/src/soc/intel/broadwell/acpi/globalnvs.asl @@ -16,7 +16,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/soc/intel/broadwell/acpi/xhci.asl b/src/soc/intel/broadwell/acpi/xhci.asl index 22e3cbc..3d87751 100644 --- a/src/soc/intel/broadwell/acpi/xhci.asl +++ b/src/soc/intel/broadwell/acpi/xhci.asl @@ -12,7 +12,6 @@ OperationRegion (XPRT, PCI_Config, 0x00, 0x100) Field (XPRT, AnyAcc, NoLock, Preserve) { - Offset (0x0), DVID, 16, Offset (0x10), , 16, diff --git a/src/soc/intel/cannonlake/acpi/scs.asl b/src/soc/intel/cannonlake/acpi/scs.asl index 8e1de1e..8e2ea3e 100644 --- a/src/soc/intel/cannonlake/acpi/scs.asl +++ b/src/soc/intel/cannonlake/acpi/scs.asl @@ -23,7 +23,6 @@ OperationRegion(SCSR, PCI_Config, 0x00, 0x100) Field(SCSR, WordAcc, NoLock, Preserve) { - Offset (0x0), /* PCI VID DID */ VDID, 32, Offset (0x84), /* PMECTRLSTATUS */ PMCR, 16, diff --git a/src/soc/intel/denverton_ns/acpi/globalnvs.asl b/src/soc/intel/denverton_ns/acpi/globalnvs.asl index f858cdf..36bb724 100644 --- a/src/soc/intel/denverton_ns/acpi/globalnvs.asl +++ b/src/soc/intel/denverton_ns/acpi/globalnvs.asl @@ -16,7 +16,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/soc/intel/skylake/acpi/globalnvs.asl b/src/soc/intel/skylake/acpi/globalnvs.asl index abcde94..bc99698 100644 --- a/src/soc/intel/skylake/acpi/globalnvs.asl +++ b/src/soc/intel/skylake/acpi/globalnvs.asl @@ -17,7 +17,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/soc/intel/skylake/acpi/xhci.asl b/src/soc/intel/skylake/acpi/xhci.asl index 2f5d8b2..978c7e3 100644 --- a/src/soc/intel/skylake/acpi/xhci.asl +++ b/src/soc/intel/skylake/acpi/xhci.asl @@ -83,7 +83,6 @@ OperationRegion (XPRT, PCI_Config, 0x00, 0x100) Field (XPRT, AnyAcc, NoLock, Preserve) { - Offset (0x0), DVID, 16, /* VENDORID */ Offset (0x10), , 16, diff --git a/src/southbridge/amd/cimx/sb800/acpi/fch.asl b/src/southbridge/amd/cimx/sb800/acpi/fch.asl index 0c0150d..6091593 100644 --- a/src/southbridge/amd/cimx/sb800/acpi/fch.asl +++ b/src/southbridge/amd/cimx/sb800/acpi/fch.asl @@ -213,7 +213,6 @@ PIOD, 0x00000008, } IndexField (PIOI, PIOD, ByteAcc, NoLock, Preserve) { - Offset(0x00), /* MiscControl */ , 1, T1EE, 1, T2EE, 1, diff --git a/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl b/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl index f994472..46096cc 100644 --- a/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl +++ b/src/southbridge/intel/bd82x6x/acpi/globalnvs.asl @@ -15,7 +15,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/southbridge/intel/bd82x6x/acpi/pch.asl b/src/southbridge/intel/bd82x6x/acpi/pch.asl index 4f08a64..b3d12ff 100644 --- a/src/southbridge/intel/bd82x6x/acpi/pch.asl +++ b/src/southbridge/intel/bd82x6x/acpi/pch.asl @@ -63,7 +63,6 @@ OperationRegion(GPIO, SystemIO, DEFAULT_GPIOBASE, 0x6c) Field(GPIO, ByteAcc, NoLock, Preserve) { - Offset(0x00), // GPIO Use Select GU00, 8, GU01, 8, GU02, 8, diff --git a/src/southbridge/intel/i82801ix/acpi/globalnvs.asl b/src/southbridge/intel/i82801ix/acpi/globalnvs.asl index 3b6115f..d428f4a 100644 --- a/src/southbridge/intel/i82801ix/acpi/globalnvs.asl +++ b/src/southbridge/intel/i82801ix/acpi/globalnvs.asl @@ -16,7 +16,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/southbridge/intel/i82801ix/acpi/ich9.asl b/src/southbridge/intel/i82801ix/acpi/ich9.asl index 115c254..dcb3dd2 100644 --- a/src/southbridge/intel/i82801ix/acpi/ich9.asl +++ b/src/southbridge/intel/i82801ix/acpi/ich9.asl @@ -32,7 +32,6 @@ OperationRegion(GPIO, SystemIO, DEFAULT_GPIOBASE, 0x3c) Field(GPIO, ByteAcc, NoLock, Preserve) { - Offset(0x00), // GPIO Use Select GU00, 8, GU01, 8, GU02, 8, diff --git a/src/southbridge/intel/i82801jx/acpi/globalnvs.asl b/src/southbridge/intel/i82801jx/acpi/globalnvs.asl index 3b6115f..d428f4a 100644 --- a/src/southbridge/intel/i82801jx/acpi/globalnvs.asl +++ b/src/southbridge/intel/i82801jx/acpi/globalnvs.asl @@ -16,7 +16,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/southbridge/intel/i82801jx/acpi/ich10.asl b/src/southbridge/intel/i82801jx/acpi/ich10.asl index a79b8a3..66ea0e2 100644 --- a/src/southbridge/intel/i82801jx/acpi/ich10.asl +++ b/src/southbridge/intel/i82801jx/acpi/ich10.asl @@ -33,7 +33,6 @@ OperationRegion(GPIO, SystemIO, DEFAULT_GPIOBASE, 0x3c) Field(GPIO, ByteAcc, NoLock, Preserve) { - Offset(0x00), // GPIO Use Select GU00, 8, GU01, 8, GU02, 8, diff --git a/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl b/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl index 16c4b75..104574f 100644 --- a/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl +++ b/src/southbridge/intel/lynxpoint/acpi/globalnvs.asl @@ -15,7 +15,6 @@ Field (GNVS, ByteAcc, NoLock, Preserve) { /* Miscellaneous */ - Offset (0x00), OSYS, 16, // 0x00 - Operating System SMIF, 8, // 0x02 - SMI function PRM0, 8, // 0x03 - SMI function parameter diff --git a/src/southbridge/intel/lynxpoint/acpi/usb.asl b/src/southbridge/intel/lynxpoint/acpi/usb.asl index 4fd4a8e..bf38c24 100644 --- a/src/southbridge/intel/lynxpoint/acpi/usb.asl +++ b/src/southbridge/intel/lynxpoint/acpi/usb.asl @@ -56,7 +56,6 @@ OperationRegion (XPRT, PCI_Config, 0x00, 0x100) Field (XPRT, AnyAcc, NoLock, Preserve) { - Offset (0x0), DVID, 16, Offset (0x10), , 16, diff --git a/src/superio/fintek/f81803a/acpi/superio.asl b/src/superio/fintek/f81803a/acpi/superio.asl index 302f2f4..e20f009 100644 --- a/src/superio/fintek/f81803a/acpi/superio.asl +++ b/src/superio/fintek/f81803a/acpi/superio.asl @@ -234,7 +234,6 @@ OperationRegion(APCx, SystemIO, APC5, 0x01) Field(APCx, ByteAcc, Nolock, Preserve) /* bits in PME ACPI CONTROL Reg 5*/ { - Offset(0x00), /*Control Reg 5 */ , 7, PSIN, 1 /* PSIN_FLAG */ } -- To view, visit https://review.coreboot.org/c/coreboot/+/43283 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I260a79ef77025b4befbccc21f5999f89d90c1154 Gerrit-Change-Number: 43283 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

6 26

Change in coreboot[master]: cbfstool: Use cbfs_serialized.h and standard vboot helpers
by Julius Werner (Code Review) 03 Dec '20

03 Dec '20

Hello Patrick Georgi, Aaron Durbin, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/41117 to review the following change. Change subject: cbfstool: Use cbfs_serialized.h and standard vboot helpers ...................................................................... cbfstool: Use cbfs_serialized.h and standard vboot helpers This patch reduces some code duplication in cbfstool by switching it to use the CBFS data structure definitions in commonlib rather than its own private copy. In addition, replace a few custom helpers related to hash algorithms with the official vboot APIs of the same purpose. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I22eae1bcd76d85fff17749617cfe4f1de55603f4 --- M src/commonlib/bsd/cbfs_mcache.c M src/commonlib/bsd/cbfs_private.c M src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h M src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h M src/commonlib/cbfs.c M src/mainboard/facebook/fbg1701/board_mboot.h M util/cbfstool/Makefile.inc M util/cbfstool/cbfs-mkpayload.c M util/cbfstool/cbfs-mkstage.c M util/cbfstool/cbfs-payload-linux.c M util/cbfstool/cbfs.h M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h M util/cbfstool/cbfscomptool.c M util/cbfstool/cbfstool.c M util/cbfstool/common.h M util/cbfstool/compress.c 17 files changed, 201 insertions(+), 409 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/41117/1 diff --git a/src/commonlib/bsd/cbfs_mcache.c b/src/commonlib/bsd/cbfs_mcache.c index 5a967dd..45255c5 100644 --- a/src/commonlib/bsd/cbfs_mcache.c +++ b/src/commonlib/bsd/cbfs_mcache.c @@ -108,8 +108,8 @@ const uint32_t data_offset = be32toh(entry->file.h.offset); const uint32_t data_length = be32toh(entry->file.h.len); - if (namesize <= data_offset - offsetof(union cbfs_mdata, filename) && - memcmp(name, entry->file.filename, namesize) == 0) { + if (namesize <= data_offset - offsetof(union cbfs_mdata, h.filename) && + memcmp(name, entry->file.h.filename, namesize) == 0) { LOG("Found '%s' @%#x size %#x in mcache @%p\n", name, entry->offset, data_length, current); *data_offset_out = entry->offset + data_offset; diff --git a/src/commonlib/bsd/cbfs_private.c b/src/commonlib/bsd/cbfs_private.c index 0b6b888..e575cd3 100644 --- a/src/commonlib/bsd/cbfs_private.c +++ b/src/commonlib/bsd/cbfs_private.c @@ -46,7 +46,7 @@ const uint32_t data_offset = be32toh(mdata.h.offset); const uint32_t data_length = be32toh(mdata.h.len); const uint32_t type = be32toh(mdata.h.type); - const bool empty = (type == CBFS_TYPE_DELETED || type == CBFS_TYPE_DELETED2); + const bool empty = (type == CBFS_TYPE_DELETED || type == CBFS_TYPE_NULL); DEBUG("Found CBFS header @%#zx (type %d, attr +%#x, data +%#x, length %#x)\n", offset, be32toh(mdata.h.type), attr_offset, data_offset, data_length); @@ -74,7 +74,7 @@ if (cbfs_dev_read(dev, mdata.raw + sizeof(mdata.h), offset + sizeof(mdata.h), todo) != todo) return CB_CBFS_IO; - DEBUG("File name: '%s'\n", mdata.filename); + DEBUG("File name: '%s'\n", mdata.h.filename); if (do_hash && !empty && vb2_digest_extend(&dc, mdata.raw, data_offset)) return CB_ERR; @@ -132,8 +132,8 @@ size_t already_read, void *arg) { struct cbfs_lookup_args *args = arg; - if (args->namesize > already_read - offsetof(union cbfs_mdata, filename) || - memcmp(args->name, mdata->filename, args->namesize) != 0) + if (args->namesize > already_read - offsetof(union cbfs_mdata, h.filename) || + memcmp(args->name, mdata->h.filename, args->namesize) != 0) return CB_CBFS_NOT_FOUND; LOG("Found '%s' @%#zx size %#x\n", args->name, offset, be32toh(mdata->h.len)); diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h index ac7b1b1..39e4e10 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h @@ -31,12 +31,8 @@ #include <cbfs_glue.h> /* Helper structure to allocate space for a blob of metadata on the stack. */ -#define CBFS_METADATA_MAX_SIZE 256 union cbfs_mdata { - struct { - struct cbfs_file h; - char filename[]; - }; + struct cbfs_file h; uint8_t raw[CBFS_METADATA_MAX_SIZE]; }; diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h index 556c8e4..334c9f6 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h @@ -6,44 +6,40 @@ #include <stdint.h> #include <vb2_sha.h> -/** These are standard values for the known compression - algorithms that coreboot knows about for stages and - payloads. Of course, other CBFS users can use whatever - values they want, as long as they understand them. */ +enum cbfs_compression { + CBFS_COMPRESS_NONE = 0, + CBFS_COMPRESS_LZMA = 1, + CBFS_COMPRESS_LZ4 = 2, +}; -#define CBFS_COMPRESS_NONE 0 -#define CBFS_COMPRESS_LZMA 1 -#define CBFS_COMPRESS_LZ4 2 +enum cbfs_type { + CBFS_TYPE_DELETED = 0x00000000, + CBFS_TYPE_NULL = 0xffffffff, + CBFS_TYPE_BOOTBLOCK = 0x01, + CBFS_TYPE_CBFSHEADER = 0x02, + CBFS_TYPE_STAGE = 0x10, + CBFS_TYPE_SELF = 0x20, + CBFS_TYPE_FIT = 0x21, + CBFS_TYPE_OPTIONROM = 0x30, + CBFS_TYPE_BOOTSPLASH = 0x40, + CBFS_TYPE_RAW = 0x50, + CBFS_TYPE_VSA = 0x51, + CBFS_TYPE_MBI = 0x52, + CBFS_TYPE_MICROCODE = 0x53, + CBFS_TYPE_FSP = 0x60, + CBFS_TYPE_MRC = 0x61, + CBFS_TYPE_MMA = 0x62, + CBFS_TYPE_EFI = 0x63, + CBFS_TYPE_STRUCT = 0x70, + CBFS_TYPE_CMOS_DEFAULT = 0xaa, + CBFS_TYPE_SPD = 0xab, + CBFS_TYPE_MRC_CACHE = 0xac, + CBFS_TYPE_CMOS_LAYOUT = 0x01aa, +}; -/** These are standard component types for well known - components (i.e - those that coreboot needs to consume. - Users are welcome to use any other value for their - components */ - -#define CBFS_TYPE_DELETED 0x00000000 -#define CBFS_TYPE_DELETED2 0xffffffff -#define CBFS_TYPE_STAGE 0x10 -#define CBFS_TYPE_SELF 0x20 -#define CBFS_TYPE_FIT 0x21 -#define CBFS_TYPE_OPTIONROM 0x30 -#define CBFS_TYPE_BOOTSPLASH 0x40 -#define CBFS_TYPE_RAW 0x50 -#define CBFS_TYPE_VSA 0x51 -#define CBFS_TYPE_MBI 0x52 -#define CBFS_TYPE_MICROCODE 0x53 -#define CBFS_TYPE_FSP 0x60 -#define CBFS_TYPE_MRC 0x61 -#define CBFS_TYPE_MMA 0x62 -#define CBFS_TYPE_EFI 0x63 -#define CBFS_TYPE_STRUCT 0x70 -#define CBFS_COMPONENT_CMOS_DEFAULT 0xaa -#define CBFS_TYPE_SPD 0xab -#define CBFS_TYPE_MRC_CACHE 0xac -#define CBFS_COMPONENT_CMOS_LAYOUT 0x01aa - -#define CBFS_HEADER_MAGIC 0x4F524243 -#define CBFS_HEADER_VERSION1 0x31313131 -#define CBFS_HEADER_VERSION2 0x31313132 +#define CBFS_HEADER_MAGIC 0x4F524243 /* BE: 'ORBC' */ +#define CBFS_HEADER_VERSION1 0x31313131 /* BE: '1111' */ +#define CBFS_HEADER_VERSION2 0x31313132 /* BE: '1112' */ #define CBFS_HEADER_VERSION CBFS_HEADER_VERSION2 /* this is the master cbfs header - it must be located somewhere available @@ -67,9 +63,15 @@ /* "Unknown" refers to CBFS headers version 1, * before the architecture was defined (i.e., x86 only). */ -#define CBFS_ARCHITECTURE_UNKNOWN 0xFFFFFFFF -#define CBFS_ARCHITECTURE_X86 0x00000001 -#define CBFS_ARCHITECTURE_ARM 0x00000010 +enum cbfs_architecture { + CBFS_ARCHITECTURE_UNKNOWN = 0xFFFFFFFF, + CBFS_ARCHITECTURE_X86 = 0x00000001, + CBFS_ARCHITECTURE_ARM = 0x00000010, + CBFS_ARCHITECTURE_AARCH64 = 0x0000aa64, + CBFS_ARCHITECTURE_MIPS = 0x00000100, /* deprecated */ + CBFS_ARCHITECTURE_RISCV = 0xc001d0de, + CBFS_ARCHITECTURE_PPC64 = 0x407570ff, +}; /** This is a component header - every entry in the CBFS will have this header. @@ -87,6 +89,7 @@ */ #define CBFS_FILE_MAGIC "LARCHIVE" +#define CBFS_METADATA_MAX_SIZE 256 struct cbfs_file { char magic[8]; @@ -94,8 +97,13 @@ uint32_t type; uint32_t attributes_offset; uint32_t offset; + char filename[0]; } __packed; +#if defined __GNUC__ && (__GNUC__ * 100 + __GNUC_MINOR__) >= 406 +_Static_assert(sizeof(struct cbfs_file) == 24, "cbfs_file size mismatch"); +#endif + /* The common fields of extended cbfs file attributes. Attributes are expected to start with tag/len, then append their specific fields. */ @@ -108,13 +116,16 @@ /* Depending on how the header was initialized, it may be backed with 0x00 or * 0xff. Support both. */ -#define CBFS_FILE_ATTR_TAG_UNUSED 0 -#define CBFS_FILE_ATTR_TAG_UNUSED2 0xffffffff -#define CBFS_FILE_ATTR_TAG_COMPRESSION 0x42435a4c -#define CBFS_FILE_ATTR_TAG_HASH 0x68736148 -#define CBFS_FILE_ATTR_TAG_POSITION 0x42435350 /* PSCB */ -#define CBFS_FILE_ATTR_TAG_ALIGNMENT 0x42434c41 /* ALCB */ -#define CBFS_FILE_ATTR_TAG_IBB 0x32494242 /* Initial BootBlock */ +enum cbfs_file_attr_tag { + CBFS_FILE_ATTR_TAG_UNUSED = 0, + CBFS_FILE_ATTR_TAG_UNUSED2 = 0xffffffff, + CBFS_FILE_ATTR_TAG_COMPRESSION = 0x42435a4c, /* BE: 'BCZL' */ + CBFS_FILE_ATTR_TAG_HASH = 0x68736148, /* BE: 'hsaH' */ + CBFS_FILE_ATTR_TAG_POSITION = 0x42435350, /* BE: 'BCSP' */ + CBFS_FILE_ATTR_TAG_ALIGNMENT = 0x42434c41, /* BE: 'BCLA' */ + CBFS_FILE_ATTR_TAG_IBB = 0x32494242, /* BE: '2IBB' */ + CBFS_FILE_ATTR_TAG_PADDING = 0x47444150, /* BE: 'GNDP' */ +}; struct cbfs_file_attr_compression { uint32_t tag; @@ -176,11 +187,13 @@ struct cbfs_payload_segment segments; }; -#define PAYLOAD_SEGMENT_CODE 0x434F4445 -#define PAYLOAD_SEGMENT_DATA 0x44415441 -#define PAYLOAD_SEGMENT_BSS 0x42535320 -#define PAYLOAD_SEGMENT_PARAMS 0x50415241 -#define PAYLOAD_SEGMENT_ENTRY 0x454E5452 +enum cbfs_payload_segment_type { + PAYLOAD_SEGMENT_CODE = 0x434F4445, /* BE: 'CODE' */ + PAYLOAD_SEGMENT_DATA = 0x44415441, /* BE: 'DATA' */ + PAYLOAD_SEGMENT_BSS = 0x42535320, /* BE: 'BSS ' */ + PAYLOAD_SEGMENT_PARAMS = 0x50415241, /* BE: 'PARA' */ + PAYLOAD_SEGMENT_ENTRY = 0x454E5452, /* BE: 'ENTR' */ +}; struct cbfs_optionrom { uint32_t compression; diff --git a/src/commonlib/cbfs.c b/src/commonlib/cbfs.c index caa81ea..a4cdc60 100644 --- a/src/commonlib/cbfs.c +++ b/src/commonlib/cbfs.c @@ -351,7 +351,7 @@ if (cbfsf_file_type(fh, &ftype)) return VB2_ERROR_UNKNOWN; - if (ftype == CBFS_TYPE_DELETED || ftype == CBFS_TYPE_DELETED2) + if (ftype == CBFS_TYPE_DELETED || ftype == CBFS_TYPE_NULL) continue; rv = cbfs_extend_hash_with_offset(&ctx, cbfs, &fh->data); diff --git a/src/mainboard/facebook/fbg1701/board_mboot.h b/src/mainboard/facebook/fbg1701/board_mboot.h index 5864d4b..1b93162 100644 --- a/src/mainboard/facebook/fbg1701/board_mboot.h +++ b/src/mainboard/facebook/fbg1701/board_mboot.h @@ -7,7 +7,7 @@ const mboot_measure_item_t mb_log_list[] = { { "config", CBFS_TYPE_RAW, MBOOT_PCR_INDEX_0, EV_NO_ACTION, NULL }, { "revision", CBFS_TYPE_RAW, MBOOT_PCR_INDEX_0, EV_NO_ACTION, NULL }, - { "cmos_layout.bin", CBFS_COMPONENT_CMOS_LAYOUT, MBOOT_PCR_INDEX_0, + { "cmos_layout.bin", CBFS_TYPE_CMOS_LAYOUT, MBOOT_PCR_INDEX_0, EV_NO_ACTION, NULL }, #if CONFIG(VENDORCODE_ELTAN_VBOOT) { "oemmanifest.bin", CBFS_TYPE_RAW, MBOOT_PCR_INDEX_7, EV_NO_ACTION, diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc index f38c825..000ceb2 100644 --- a/util/cbfstool/Makefile.inc +++ b/util/cbfstool/Makefile.inc @@ -108,6 +108,7 @@ TOOLCPPFLAGS += -include $(top)/src/commonlib/bsd/include/commonlib/bsd/compiler.h TOOLCPPFLAGS += -I$(VBOOT_SOURCE)/firmware/include TOOLCPPFLAGS += -I$(VBOOT_SOURCE)/firmware/2lib/include +TOOLCPPFLAGS += -I$(VBOOT_SOURCE)/host/include # UEFI header file support. It's not pretty, but that's what we currently # have right now. TOOLCPPFLAGS += -I$(top)/src diff --git a/util/cbfstool/cbfs-mkpayload.c b/util/cbfstool/cbfs-mkpayload.c index 830fe16..da8fdee 100644 --- a/util/cbfstool/cbfs-mkpayload.c +++ b/util/cbfstool/cbfs-mkpayload.c @@ -66,7 +66,7 @@ } int parse_elf_to_payload(const struct buffer *input, struct buffer *output, - enum comp_algo algo) + enum cbfs_compression algo) { Elf64_Phdr *phdr; Elf64_Ehdr ehdr; @@ -247,7 +247,7 @@ struct buffer *output, uint32_t loadaddress, uint32_t entrypoint, - enum comp_algo algo) + enum cbfs_compression algo) { comp_func_ptr compress; struct cbfs_payload_segment segs[2] = { {0} }; @@ -292,7 +292,7 @@ } int parse_fv_to_payload(const struct buffer *input, struct buffer *output, - enum comp_algo algo) + enum cbfs_compression algo) { comp_func_ptr compress; struct cbfs_payload_segment segs[2] = { {0} }; @@ -420,7 +420,7 @@ } int parse_fit_to_payload(const struct buffer *input, struct buffer *output, - enum comp_algo algo) + enum cbfs_compression algo) { struct fdt_header *fdt_h; diff --git a/util/cbfstool/cbfs-mkstage.c b/util/cbfstool/cbfs-mkstage.c index be920b4..cfa820b 100644 --- a/util/cbfstool/cbfs-mkstage.c +++ b/util/cbfstool/cbfs-mkstage.c @@ -89,7 +89,7 @@ return NULL; } -static void fill_cbfs_stage(struct buffer *outheader, enum comp_algo algo, +static void fill_cbfs_stage(struct buffer *outheader, enum cbfs_compression algo, uint64_t entry, uint64_t loadaddr, uint32_t filesize, uint32_t memsize) { @@ -109,7 +109,7 @@ * works for all elf files, not just the restricted set. */ int parse_elf_to_stage(const struct buffer *input, struct buffer *output, - enum comp_algo algo, uint32_t *location, + enum cbfs_compression algo, uint32_t *location, const char *ignore_section) { struct parsed_elf pelf; diff --git a/util/cbfstool/cbfs-payload-linux.c b/util/cbfstool/cbfs-payload-linux.c index 6b4bf27..b9e56ea 100644 --- a/util/cbfstool/cbfs-payload-linux.c +++ b/util/cbfstool/cbfs-payload-linux.c @@ -47,14 +47,14 @@ struct buffer cmdline; struct buffer initrd; /* Output variables. */ - enum comp_algo algo; + enum cbfs_compression algo; comp_func_ptr compress; struct buffer output; size_t offset; struct cbfs_payload_segment *out_seg; }; -static int bzp_init(struct bzpayload *bzp, enum comp_algo algo) +static int bzp_init(struct bzpayload *bzp, enum cbfs_compression algo) { memset(bzp, 0, sizeof(*bzp)); @@ -199,7 +199,7 @@ */ int parse_bzImage_to_payload(const struct buffer *input, struct buffer *output, const char *initrd_name, - char *cmdline, enum comp_algo algo) + char *cmdline, enum cbfs_compression algo) { struct bzpayload bzp; unsigned int initrd_base = 64*1024*1024; diff --git a/util/cbfstool/cbfs.h b/util/cbfstool/cbfs.h index f0c215d..10b478c 100644 --- a/util/cbfstool/cbfs.h +++ b/util/cbfstool/cbfs.h @@ -17,234 +17,57 @@ #define __CBFS_H #include "common.h" -#include <stdint.h> - -#include <vb2_api.h> - -/* cbfstool will fail when trying to build a cbfs_file header that's larger - * than MAX_CBFS_FILE_HEADER_BUFFER. 1K should give plenty of room. */ -#define MAX_CBFS_FILE_HEADER_BUFFER 1024 - -/* create a magic number in host-byte order. - * b3 is the high order byte. - * in the coreboot tools, we go with the 32-bit - * magic number convention. - * This was an inline func but that breaks anything - * that uses it in a case statement. - */ - -#define makemagic(b3, b2, b1, b0)\ - (((b3)<<24) | ((b2) << 16) | ((b1) << 8) | (b0)) +#include <commonlib/bsd/cbfs_serialized.h> /* To make CBFS more friendly to ROM, fill -1 (0xFF) instead of zero. */ #define CBFS_CONTENT_DEFAULT_VALUE (-1) -// Alignment (in bytes) to be used when no master header is present -#define CBFS_ENTRY_ALIGNMENT 64 - -#define CBFS_HEADER_MAGIC 0x4F524243 #define CBFS_HEADPTR_ADDR_X86 0xFFFFFFFC -#define CBFS_HEADER_VERSION1 0x31313131 -#define CBFS_HEADER_VERSION2 0x31313132 -#define CBFS_HEADER_VERSION CBFS_HEADER_VERSION2 -#define CBFS_ALIGNMENT 64 - -struct cbfs_header { - uint32_t magic; - uint32_t version; - uint32_t romsize; - uint32_t bootblocksize; - uint32_t align; /* hard coded to 64 byte */ - uint32_t offset; - uint32_t architecture; /* Version 2 */ - uint32_t pad[1]; -} __packed; - -#define CBFS_ARCHITECTURE_UNKNOWN 0xFFFFFFFF -#define CBFS_ARCHITECTURE_X86 0x00000001 -#define CBFS_ARCHITECTURE_ARM 0x00000010 -#define CBFS_ARCHITECTURE_AARCH64 0x0000aa64 -#define CBFS_ARCHITECTURE_MIPS 0x00000100 /* deprecated */ -#define CBFS_ARCHITECTURE_RISCV 0xc001d0de -#define CBFS_ARCHITECTURE_PPC64 0x407570ff - -#define CBFS_FILE_MAGIC "LARCHIVE" - -struct cbfs_file { - uint8_t magic[8]; - /* length of file data */ - uint32_t len; +struct typedesc_t { uint32_t type; - /* offset to struct cbfs_file_attribute or 0 */ - uint32_t attributes_offset; - /* length of header incl. variable data */ - uint32_t offset; - char filename[]; -} __packed; + const char *name; +}; -#if defined __GNUC__ && (__GNUC__ * 100 + __GNUC_MINOR__) >= 406 -_Static_assert(sizeof(struct cbfs_file) == 24, "cbfs_file size mismatch"); -#endif - -/* The common fields of extended cbfs file attributes. - Attributes are expected to start with tag/len, then append their - specific fields. */ -struct cbfs_file_attribute { - uint32_t tag; - /* len covers the whole structure, incl. tag and len */ - uint32_t len; - uint8_t data[0]; -} __packed; - -/* Depending on how the header was initialized, it may be backed with 0x00 or - * 0xff. Support both. */ -#define CBFS_FILE_ATTR_TAG_UNUSED 0 -#define CBFS_FILE_ATTR_TAG_UNUSED2 0xffffffff -#define CBFS_FILE_ATTR_TAG_COMPRESSION 0x42435a4c -#define CBFS_FILE_ATTR_TAG_HASH 0x68736148 -#define CBFS_FILE_ATTR_TAG_POSITION 0x42435350 /* PSCB */ -#define CBFS_FILE_ATTR_TAG_ALIGNMENT 0x42434c41 /* ALCB */ -#define CBFS_FILE_ATTR_TAG_PADDING 0x47444150 /* PDNG */ -#define CBFS_FILE_ATTR_TAG_IBB 0x32494242 /* Initial BootBlock */ - -struct cbfs_file_attr_compression { - uint32_t tag; - uint32_t len; - /* whole file compression format. 0 if no compression. */ - uint32_t compression; - uint32_t decompressed_size; -} __packed; - -struct cbfs_file_attr_hash { - uint32_t tag; - uint32_t len; - uint32_t hash_type; - /* hash_data is len - sizeof(struct) bytes */ - uint8_t hash_data[]; -} __packed; - -struct cbfs_file_attr_position { - uint32_t tag; - uint32_t len; - uint32_t position; -} __packed; - -struct cbfs_file_attr_align { - uint32_t tag; - uint32_t len; - uint32_t alignment; -} __packed; - -struct cbfs_stage { - uint32_t compression; - uint64_t entry; - uint64_t load; - uint32_t len; - uint32_t memlen; -} __packed; - -#define PAYLOAD_SEGMENT_CODE makemagic('C', 'O', 'D', 'E') -#define PAYLOAD_SEGMENT_DATA makemagic('D', 'A', 'T', 'A') -#define PAYLOAD_SEGMENT_BSS makemagic('B', 'S', 'S', ' ') -#define PAYLOAD_SEGMENT_PARAMS makemagic('P', 'A', 'R', 'A') -#define PAYLOAD_SEGMENT_ENTRY makemagic('E', 'N', 'T', 'R') - -struct cbfs_payload_segment { - uint32_t type; - uint32_t compression; - uint32_t offset; - uint64_t load_addr; - uint32_t len; - uint32_t mem_len; -} __packed; - -struct cbfs_payload { - struct cbfs_payload_segment segments; -} __packed; - -/** These are standard component types for well known - components (i.e - those that coreboot needs to consume. - Users are welcome to use any other value for their - components */ - -#define CBFS_COMPONENT_BOOTBLOCK 0x01 -#define CBFS_COMPONENT_CBFSHEADER 0x02 -#define CBFS_COMPONENT_STAGE 0x10 -#define CBFS_COMPONENT_SELF 0x20 -#define CBFS_COMPONENT_FIT 0x21 -#define CBFS_COMPONENT_OPTIONROM 0x30 -#define CBFS_COMPONENT_BOOTSPLASH 0x40 -#define CBFS_COMPONENT_RAW 0x50 -#define CBFS_COMPONENT_VSA 0x51 -#define CBFS_COMPONENT_MBI 0x52 -#define CBFS_COMPONENT_MICROCODE 0x53 -#define CBFS_COMPONENT_FSP 0x60 -#define CBFS_COMPONENT_MRC 0x61 -#define CBFS_COMPONENT_MMA 0x62 -#define CBFS_COMPONENT_EFI 0x63 -#define CBFS_COMPONENT_STRUCT 0x70 -#define CBFS_COMPONENT_CMOS_DEFAULT 0xaa -#define CBFS_COMPONENT_SPD 0xab -#define CBFS_COMPONENT_MRC_CACHE 0xac -#define CBFS_COMPONENT_CMOS_LAYOUT 0x01aa - -/* The deleted type is chosen to be a value - * that can be written in a FLASH from all other - * values. - */ -#define CBFS_COMPONENT_DELETED 0 - -/* for all known FLASH, this value can be changed - * to all other values. This allows NULL files to be - * changed without a block erase - */ -#define CBFS_COMPONENT_NULL 0xFFFFFFFF +static const struct typedesc_t types_cbfs_compression[] = { + {CBFS_COMPRESS_NONE, "none"}, + {CBFS_COMPRESS_LZMA, "LZMA"}, + {CBFS_COMPRESS_LZ4, "LZ4"}, + {0, NULL}, +}; static struct typedesc_t filetypes[] unused = { - {CBFS_COMPONENT_BOOTBLOCK, "bootblock"}, - {CBFS_COMPONENT_CBFSHEADER, "cbfs header"}, - {CBFS_COMPONENT_STAGE, "stage"}, - {CBFS_COMPONENT_SELF, "simple elf"}, - {CBFS_COMPONENT_FIT, "fit"}, - {CBFS_COMPONENT_OPTIONROM, "optionrom"}, - {CBFS_COMPONENT_BOOTSPLASH, "bootsplash"}, - {CBFS_COMPONENT_RAW, "raw"}, - {CBFS_COMPONENT_VSA, "vsa"}, - {CBFS_COMPONENT_MBI, "mbi"}, - {CBFS_COMPONENT_MICROCODE, "microcode"}, - {CBFS_COMPONENT_FSP, "fsp"}, - {CBFS_COMPONENT_MRC, "mrc"}, - {CBFS_COMPONENT_CMOS_DEFAULT, "cmos_default"}, - {CBFS_COMPONENT_CMOS_LAYOUT, "cmos_layout"}, - {CBFS_COMPONENT_SPD, "spd"}, - {CBFS_COMPONENT_MRC_CACHE, "mrc_cache"}, - {CBFS_COMPONENT_MMA, "mma"}, - {CBFS_COMPONENT_EFI, "efi"}, - {CBFS_COMPONENT_STRUCT, "struct"}, - {CBFS_COMPONENT_DELETED, "deleted"}, - {CBFS_COMPONENT_NULL, "null"} + {CBFS_TYPE_BOOTBLOCK, "bootblock"}, + {CBFS_TYPE_CBFSHEADER, "cbfs header"}, + {CBFS_TYPE_STAGE, "stage"}, + {CBFS_TYPE_SELF, "simple elf"}, + {CBFS_TYPE_FIT, "fit"}, + {CBFS_TYPE_OPTIONROM, "optionrom"}, + {CBFS_TYPE_BOOTSPLASH, "bootsplash"}, + {CBFS_TYPE_RAW, "raw"}, + {CBFS_TYPE_VSA, "vsa"}, + {CBFS_TYPE_MBI, "mbi"}, + {CBFS_TYPE_MICROCODE, "microcode"}, + {CBFS_TYPE_FSP, "fsp"}, + {CBFS_TYPE_MRC, "mrc"}, + {CBFS_TYPE_CMOS_DEFAULT, "cmos_default"}, + {CBFS_TYPE_CMOS_LAYOUT, "cmos_layout"}, + {CBFS_TYPE_SPD, "spd"}, + {CBFS_TYPE_MRC_CACHE, "mrc_cache"}, + {CBFS_TYPE_MMA, "mma"}, + {CBFS_TYPE_EFI, "efi"}, + {CBFS_TYPE_STRUCT, "struct"}, + {CBFS_TYPE_DELETED, "deleted"}, + {CBFS_TYPE_NULL, "null"} }; -static const struct typedesc_t types_cbfs_hash[] unused = { - {VB2_HASH_INVALID, "none"}, - {VB2_HASH_SHA1, "sha1"}, - {VB2_HASH_SHA256, "sha256"}, - {VB2_HASH_SHA512, "sha512"}, - {0, NULL} -}; - -static size_t widths_cbfs_hash[] unused = { - [VB2_HASH_INVALID] = 0, - [VB2_HASH_SHA1] = 20, - [VB2_HASH_SHA256] = 32, - [VB2_HASH_SHA512] = 64, -}; - -#define CBFS_NUM_SUPPORTED_HASHES ARRAY_SIZE(widths_cbfs_hash) - #define CBFS_SUBHEADER(_p) ( (void *) ((((uint8_t *) (_p)) + ntohl((_p)->offset))) ) +static inline size_t cbfs_file_attr_hash_size(enum vb2_hash_algorithm algo) { + return offsetof(struct cbfs_file_attr_hash, hash.raw) + + vb2_digest_size(algo); +} + /* cbfs_image.c */ uint32_t get_cbfs_entry_type(const char *name, uint32_t default_value); uint32_t get_cbfs_compression(const char *name, uint32_t unknown); diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 9bf3688..035b196 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -80,16 +80,6 @@ return lookup_type_by_name(types_cbfs_compression, name); } -static const char *get_hash_attr_name(uint16_t hash_type) -{ - return lookup_name_by_type(types_cbfs_hash, hash_type, "(invalid)"); -} - -int cbfs_parse_hash_algo(const char *name) -{ - return lookup_type_by_name(types_cbfs_hash, name); -} - /* CBFS image */ size_t cbfs_calculate_file_header_size(const char *name) @@ -227,7 +217,7 @@ size_t empty_header_len = cbfs_calculate_file_header_size(""); uint32_t entries_offset = 0; - uint32_t align = CBFS_ENTRY_ALIGNMENT; + uint32_t align = CBFS_ALIGNMENT; if (image->has_header) { entries_offset = image->header.offset; @@ -266,7 +256,7 @@ size_t capacity = entries_size - empty_header_len; LOG("Created CBFS (capacity = %zu bytes)\n", capacity); - return cbfs_create_empty_entry(entry_header, CBFS_COMPONENT_NULL, + return cbfs_create_empty_entry(entry_header, CBFS_TYPE_NULL, capacity, ""); } @@ -402,7 +392,7 @@ size_t copy_end = buffer_size(dst); - align = CBFS_ENTRY_ALIGNMENT; + align = CBFS_ALIGNMENT; dst_entry = (struct cbfs_file *)buffer_get(dst); @@ -412,9 +402,9 @@ src_entry = cbfs_find_next_entry(image, src_entry)) { size_t entry_size; - if ((src_entry->type == htonl(CBFS_COMPONENT_NULL)) || - (src_entry->type == htonl(CBFS_COMPONENT_CBFSHEADER)) || - (src_entry->type == htonl(CBFS_COMPONENT_DELETED))) + if ((src_entry->type == htonl(CBFS_TYPE_NULL)) || + (src_entry->type == htonl(CBFS_TYPE_CBFSHEADER)) || + (src_entry->type == htonl(CBFS_TYPE_DELETED))) continue; entry_size = htonl(src_entry->len) + htonl(src_entry->offset); @@ -440,7 +430,7 @@ if (last_entry_size < 0) WARN("No room to create the last entry!\n") else - cbfs_create_empty_entry(dst_entry, CBFS_COMPONENT_NULL, + cbfs_create_empty_entry(dst_entry, CBFS_TYPE_NULL, last_entry_size, ""); return 0; @@ -476,7 +466,7 @@ cbfs_calculate_file_header_size("") - sizeof(int32_t); if (last_entry_size > 0) { - cbfs_create_empty_entry(entry, CBFS_COMPONENT_NULL, + cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, last_entry_size, ""); /* If the last entry was an empty file, merge them. */ cbfs_walk(&image, cbfs_merge_empty_entry, NULL); @@ -510,8 +500,8 @@ * maximum size. */ if ((strlen(trailer->filename) != 0) && - (trailer->type != htonl(CBFS_COMPONENT_NULL)) && - (trailer->type != htonl(CBFS_COMPONENT_DELETED))) { + (trailer->type != htonl(CBFS_TYPE_NULL)) && + (trailer->type != htonl(CBFS_TYPE_DELETED))) { /* nothing to truncate. Return de-facto CBFS size in case it * was already truncated. */ *size = (uint8_t *)entry - (uint8_t *)buffer_get(region); @@ -565,8 +555,8 @@ uint32_t type = htonl(cur->type); /* Current entry is empty. Kepp track of it. */ - if ((type == htonl(CBFS_COMPONENT_NULL)) || - (type == htonl(CBFS_COMPONENT_DELETED))) { + if ((type == htonl(CBFS_TYPE_NULL)) || + (type == htonl(CBFS_TYPE_DELETED))) { prev = cur; continue; } @@ -627,7 +617,7 @@ prev_size -= spill_size + empty_metadata_size; /* Create new empty file. */ - cbfs_create_empty_entry(cur, CBFS_COMPONENT_NULL, + cbfs_create_empty_entry(cur, CBFS_TYPE_NULL, prev_size, ""); /* Merge any potential empty entries together. */ @@ -667,7 +657,7 @@ uint32_t min_entry_size = cbfs_calculate_file_header_size(""); uint32_t len, header_offset; uint32_t align = image->has_header ? image->header.align : - CBFS_ENTRY_ALIGNMENT; + CBFS_ALIGNMENT; uint32_t header_size = ntohl(header->offset); header_offset = content_offset - header_size; @@ -682,7 +672,7 @@ if (header_offset - addr > min_entry_size) { DEBUG("|min|...|header|content|... <create new entry>\n"); len = header_offset - addr - min_entry_size; - cbfs_create_empty_entry(entry, CBFS_COMPONENT_NULL, len, ""); + cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, ""); if (verbose > 1) cbfs_print_entry_info(image, entry, stderr); entry = cbfs_find_next_entry(image, entry); addr = cbfs_get_entry_addr(image, entry); @@ -741,7 +731,7 @@ buffer_size(&image->buffer) - sizeof(int32_t)) { len -= sizeof(int32_t); } - cbfs_create_empty_entry(entry, CBFS_COMPONENT_NULL, len, ""); + cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, ""); if (verbose > 1) cbfs_print_entry_info(image, entry, stderr); return 0; } @@ -777,7 +767,7 @@ entry = cbfs_find_next_entry(image, entry)) { entry_type = ntohl(entry->type); - if (entry_type != CBFS_COMPONENT_NULL) + if (entry_type != CBFS_TYPE_NULL) continue; addr = cbfs_get_entry_addr(image, entry); @@ -1352,10 +1342,10 @@ if (do_processing) { int (*make_elf)(struct buffer *, uint32_t) = NULL; switch (ntohl(entry->type)) { - case CBFS_COMPONENT_STAGE: + case CBFS_TYPE_STAGE: make_elf = cbfs_stage_make_elf; break; - case CBFS_COMPONENT_SELF: + case CBFS_TYPE_SELF: make_elf = cbfs_payload_make_elf; break; } @@ -1389,7 +1379,7 @@ } DEBUG("cbfs_remove_entry: Removed %s @ 0x%x\n", entry->filename, cbfs_get_entry_addr(image, entry)); - entry->type = htonl(CBFS_COMPONENT_DELETED); + entry->type = htonl(CBFS_TYPE_DELETED); cbfs_walk(image, cbfs_merge_empty_entry, NULL); return 0; } @@ -1513,28 +1503,21 @@ decompressed_size ); - struct cbfs_file_attr_hash *hash = NULL; - while ((hash = cbfs_file_get_next_hash(entry, hash)) != NULL) { - unsigned int hash_type = ntohl(hash->hash_type); - if (hash_type >= CBFS_NUM_SUPPORTED_HASHES) { - fprintf(fp, "invalid hash type %d\n", hash_type); + struct cbfs_file_attr_hash *attr = NULL; + while ((attr = cbfs_file_get_next_hash(entry, attr)) != NULL) { + size_t hash_len = vb2_digest_size(attr->hash.algo); + if (!hash_len) { + fprintf(fp, "invalid/unsupported hash algorithm: %d\n", + attr->hash.algo); break; } - size_t hash_len = widths_cbfs_hash[hash_type]; - char *hash_str = bintohex(hash->hash_data, hash_len); - uint8_t local_hash[hash_len]; - if (vb2_digest_buffer(CBFS_SUBHEADER(entry), - ntohl(entry->len), hash_type, local_hash, - hash_len) != VB2_SUCCESS) { - fprintf(fp, "failed to hash '%s'\n", name); - free(hash_str); - break; - } - int valid = memcmp(local_hash, hash->hash_data, hash_len) == 0; + char *hash_str = bintohex(attr->hash.raw, hash_len); + int valid = vb2_hash_verify(CBFS_SUBHEADER(entry), + ntohl(entry->len), &attr->hash) == VB2_SUCCESS; const char *valid_str = valid ? "valid" : "invalid"; fprintf(fp, " hash %s:%s %s\n", - get_hash_attr_name(hash_type), + vb2_get_hash_algorithm_name(attr->hash.algo), hash_str, valid_str); free(hash_str); } @@ -1549,12 +1532,12 @@ /* note the components of the subheader may be in host order ... */ switch (ntohl(entry->type)) { - case CBFS_COMPONENT_STAGE: + case CBFS_TYPE_STAGE: cbfs_print_stage_info((struct cbfs_stage *) CBFS_SUBHEADER(entry), fp); break; - case CBFS_COMPONENT_SELF: + case CBFS_TYPE_SELF: payload = (struct cbfs_payload_segment *) CBFS_SUBHEADER(entry); while (payload) { @@ -1650,8 +1633,8 @@ /* Loop until non-empty entry is found, starting from the current entry. After the loop, next_addr points to the next non-empty entry. */ next = entry; - while (ntohl(next->type) == CBFS_COMPONENT_DELETED || - ntohl(next->type) == CBFS_COMPONENT_NULL) { + while (ntohl(next->type) == CBFS_TYPE_DELETED || + ntohl(next->type) == CBFS_TYPE_NULL) { next = cbfs_find_next_entry(image, next); if (!next) break; @@ -1672,7 +1655,7 @@ uint32_t addr = cbfs_get_entry_addr(image, entry); size_t len = next_addr - addr - cbfs_calculate_file_header_size(""); DEBUG("join_empty_entry: [0x%x, 0x%x) len=%zu\n", addr, next_addr, len); - cbfs_create_empty_entry(entry, CBFS_COMPONENT_NULL, len, ""); + cbfs_create_empty_entry(entry, CBFS_TYPE_NULL, len, ""); return 0; } @@ -1766,8 +1749,7 @@ struct cbfs_file *entry) { uint32_t addr = cbfs_get_entry_addr(image, entry); - int align = image->has_header ? image->header.align : - CBFS_ENTRY_ALIGNMENT; + int align = image->has_header ? image->header.align : CBFS_ALIGNMENT; assert(entry && cbfs_is_valid_entry(image, entry)); addr += ntohl(entry->offset) + ntohl(entry->len); addr = align_up(addr, align); @@ -1808,8 +1790,8 @@ struct cbfs_file *cbfs_create_file_header(int type, size_t len, const char *name) { - struct cbfs_file *entry = malloc(MAX_CBFS_FILE_HEADER_BUFFER); - memset(entry, CBFS_CONTENT_DEFAULT_VALUE, MAX_CBFS_FILE_HEADER_BUFFER); + struct cbfs_file *entry = malloc(CBFS_METADATA_MAX_SIZE); + memset(entry, CBFS_CONTENT_DEFAULT_VALUE, CBFS_METADATA_MAX_SIZE); memcpy(entry->magic, CBFS_FILE_MAGIC, sizeof(entry->magic)); entry->type = htonl(type); entry->len = htonl(len); @@ -1882,7 +1864,7 @@ next = cbfs_file_next_attr(header, attr); } while (next != NULL); uint32_t header_size = ntohl(header->offset) + size; - if (header_size > MAX_CBFS_FILE_HEADER_BUFFER) { + if (header_size > CBFS_METADATA_MAX_SIZE) { DEBUG("exceeding allocated space for cbfs_file headers"); return NULL; } @@ -1904,35 +1886,31 @@ ntohl(attr->len)); } header->offset = htonl(header_size); - memset(attr, CBFS_CONTENT_DEFAULT_VALUE, size); + /* Attributes are expected to be small (much smaller than a flash page) + and not really meant to be overwritten in-place. To avoid surprising + values in reserved fields of attribute structures, initialize them to + 0, not 0xff. */ + memset(attr, 0, size); attr->tag = htonl(tag); attr->len = htonl(size); return attr; } int cbfs_add_file_hash(struct cbfs_file *header, struct buffer *buffer, - enum vb2_hash_algorithm hash_type) + enum vb2_hash_algorithm alg) { - uint32_t hash_index = hash_type; - - if (hash_index >= CBFS_NUM_SUPPORTED_HASHES) + if (!vb2_digest_size(alg)) return -1; - unsigned hash_size = widths_cbfs_hash[hash_type]; - if (hash_size == 0) - return -1; - - struct cbfs_file_attr_hash *attrs = + struct cbfs_file_attr_hash *attr = (struct cbfs_file_attr_hash *)cbfs_add_file_attr(header, - CBFS_FILE_ATTR_TAG_HASH, - sizeof(struct cbfs_file_attr_hash) + hash_size); + CBFS_FILE_ATTR_TAG_HASH, cbfs_file_attr_hash_size(alg)); - if (attrs == NULL) + if (attr == NULL) return -1; - attrs->hash_type = htonl(hash_type); - if (vb2_digest_buffer(buffer_get(buffer), buffer_size(buffer), - hash_type, attrs->hash_data, hash_size) != VB2_SUCCESS) + if (vb2_hash_calculate(buffer_get(buffer), buffer_size(buffer), + alg, &attr->hash) != VB2_SUCCESS) return -1; return 0; @@ -1984,7 +1962,7 @@ size, page_size); size_t image_align = image->has_header ? image->header.align : - CBFS_ENTRY_ALIGNMENT; + CBFS_ALIGNMENT; if (page_size % image_align) WARN("%s: Page size (%#zx) not aligned with CBFS image (%#zx).\n", __func__, page_size, image_align); @@ -2020,7 +1998,7 @@ entry = cbfs_find_next_entry(image, entry)) { uint32_t type = ntohl(entry->type); - if (type != CBFS_COMPONENT_NULL) + if (type != CBFS_TYPE_NULL) continue; addr = cbfs_get_entry_addr(image, entry); diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index 1bb3c68..6ff0746 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -54,7 +54,7 @@ /* Creates an empty CBFS image by given size, and description to its content * (bootblock, align, header location, starting offset of CBFS entries). * The output image will contain a valid cbfs_header, with one cbfs_file - * entry with type CBFS_COMPONENT_NULL, with max available size. + * entry with type CBFS_TYPE_NULL, with max available size. * Only call this if you want a legacy CBFS with a master header. * Returns 0 on success, otherwise nonzero. */ int cbfs_legacy_image_create(struct cbfs_image *image, diff --git a/util/cbfstool/cbfscomptool.c b/util/cbfstool/cbfscomptool.c index 028893c..a1245b5 100644 --- a/util/cbfstool/cbfscomptool.c +++ b/util/cbfstool/cbfscomptool.c @@ -19,6 +19,7 @@ #include <strings.h> #include <time.h> +#include "cbfs.h" #include "common.h" const char *usage_text = "cbfs-compression-tool benchmark\n" diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index f15c65b..f97745e 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -34,6 +34,7 @@ #include <commonlib/fsp.h> #include <commonlib/endian.h> #include <commonlib/helpers.h> +#include <vboot_host.h> #define SECTION_WITH_FIT_TABLE "BOOTBLOCK" @@ -86,7 +87,7 @@ bool machine_parseable; bool unprocessed; bool ibb; - enum comp_algo compression; + enum cbfs_compression compression; int precompression; enum vb2_hash_algorithm hash; /* For linux payloads */ @@ -198,7 +199,7 @@ /* Take care of the hash attribute if it is used */ if (param.hash != VB2_HASH_INVALID) - metadata_size += sizeof(struct cbfs_file_attr_hash); + metadata_size += cbfs_file_attr_hash_size(param.hash); int32_t address = cbfs_locate_entry(&image, data_size, param.pagesize, param.alignment, metadata_size); @@ -250,7 +251,7 @@ offset = convert_to_from_top_aligned(param.image_region, -offset); - header = cbfs_create_file_header(CBFS_COMPONENT_RAW, + header = cbfs_create_file_header(CBFS_TYPE_RAW, buffer.size, name); if (cbfs_add_entry(&image, &buffer, offset, header, 0) != 0) { ERROR("Failed to add %llu into ROM image as '%s'.\n", @@ -351,7 +352,7 @@ * image is at 4GB == 0. */ h->bootblocksize = htonl(4); - h->align = htonl(CBFS_ENTRY_ALIGNMENT); + h->align = htonl(CBFS_ALIGNMENT); /* The offset and romsize fields within the master header are absolute * values within the boot media. As such, romsize needs to relfect * the end 'offset' for a CBFS. To achieve that the current buffer @@ -365,7 +366,7 @@ h->offset = htonl(offset); h->architecture = htonl(CBFS_ARCHITECTURE_UNKNOWN); - header = cbfs_create_file_header(CBFS_COMPONENT_CBFSHEADER, + header = cbfs_create_file_header(CBFS_TYPE_CBFSHEADER, buffer_size(&buffer), name); if (cbfs_add_entry(&image, &buffer, 0, header, 0) != 0) { ERROR("Failed to add cbfs master header into ROM image.\n"); @@ -487,7 +488,7 @@ * Check if Intel CPU topswap is specified this will require a * second bootblock to be added. */ - if (type == CBFS_COMPONENT_BOOTBLOCK && param.topswap_size) + if (type == CBFS_TYPE_BOOTBLOCK && param.topswap_size) if (add_topswap_bootblock(&buffer, &offset)) return 1; @@ -522,10 +523,10 @@ /* care about the additional metadata that is added */ /* to the cbfs file and therefore set the position */ /* the real beginning of the data. */ - if (type == CBFS_COMPONENT_STAGE) + if (type == CBFS_TYPE_STAGE) attrs->position = htonl(offset + sizeof(struct cbfs_stage)); - else if (type == CBFS_COMPONENT_SELF) + else if (type == CBFS_TYPE_SELF) attrs->position = htonl(offset + sizeof(struct cbfs_payload)); else @@ -759,7 +760,7 @@ if (ret != 0) { ret = parse_fit_to_payload(buffer, &output, param.compression); if (ret == 0) - header->type = htonl(CBFS_COMPONENT_FIT); + header->type = htonl(CBFS_TYPE_FIT); } /* If it's not an FIT, see if it's a UEFI FV */ @@ -817,7 +818,7 @@ /* Set the alignment to 4KiB minimum for FSP blobs when no base address * is provided so that relocation can occur. */ - if (param.type == CBFS_COMPONENT_FSP) { + if (param.type == CBFS_TYPE_FSP) { if (!param.baseaddress_assigned) param.alignment = 4*1024; convert = cbfstool_convert_fsp; @@ -858,7 +859,7 @@ return cbfs_add_component(param.filename, param.name, - CBFS_COMPONENT_STAGE, + CBFS_TYPE_STAGE, param.baseaddress, param.headeroffset, cbfstool_convert_mkstage); @@ -868,7 +869,7 @@ { return cbfs_add_component(param.filename, param.name, - CBFS_COMPONENT_SELF, + CBFS_TYPE_SELF, param.baseaddress, param.headeroffset, cbfstool_convert_mkpayload); @@ -888,7 +889,7 @@ } return cbfs_add_component(param.filename, param.name, - CBFS_COMPONENT_SELF, + CBFS_TYPE_SELF, param.baseaddress, param.headeroffset, cbfstool_convert_mkflatpayload); @@ -1577,10 +1578,7 @@ break; } case 'A': { - int algo = cbfs_parse_hash_algo(optarg); - if (algo >= 0) - param.hash = algo; - else { + if (!vb2_lookup_hash_alg(optarg, &param.hash)) { ERROR("Unknown hash algorithm '%s'.\n", optarg); return 1; diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h index f152104..8aac371 100644 --- a/util/cbfstool/common.h +++ b/util/cbfstool/common.h @@ -22,10 +22,10 @@ #include <string.h> #include <assert.h> +#include <commonlib/bsd/cbfs_serialized.h> #include <commonlib/helpers.h> #include <console/console.h> -/* Endianness */ #include "swab.h" #define IS_TOP_ALIGNED_ADDRESS(x) ((uint32_t)(x) > 0x80000000) @@ -160,47 +160,29 @@ typedef int (*decomp_func_ptr) (char *in, int in_len, char *out, int out_len, size_t *actual_size); -enum comp_algo { - CBFS_COMPRESS_NONE = 0, - CBFS_COMPRESS_LZMA = 1, - CBFS_COMPRESS_LZ4 = 2, -}; - -struct typedesc_t { - uint32_t type; - const char *name; -}; - -static const struct typedesc_t types_cbfs_compression[] = { - {CBFS_COMPRESS_NONE, "none"}, - {CBFS_COMPRESS_LZMA, "LZMA"}, - {CBFS_COMPRESS_LZ4, "LZ4"}, - {0, NULL}, -}; - -comp_func_ptr compression_function(enum comp_algo algo); -decomp_func_ptr decompression_function(enum comp_algo algo); +comp_func_ptr compression_function(enum cbfs_compression algo); +decomp_func_ptr decompression_function(enum cbfs_compression algo); uint64_t intfiletype(const char *name); /* cbfs-mkpayload.c */ int parse_elf_to_payload(const struct buffer *input, struct buffer *output, - enum comp_algo algo); + enum cbfs_compression algo); int parse_fv_to_payload(const struct buffer *input, struct buffer *output, - enum comp_algo algo); + enum cbfs_compression algo); int parse_fit_to_payload(const struct buffer *input, struct buffer *output, - enum comp_algo algo); + enum cbfs_compression algo); int parse_bzImage_to_payload(const struct buffer *input, struct buffer *output, const char *initrd, - char *cmdline, enum comp_algo algo); + char *cmdline, enum cbfs_compression algo); int parse_flat_binary_to_payload(const struct buffer *input, struct buffer *output, uint32_t loadaddress, uint32_t entrypoint, - enum comp_algo algo); + enum cbfs_compression algo); /* cbfs-mkstage.c */ int parse_elf_to_stage(const struct buffer *input, struct buffer *output, - enum comp_algo algo, uint32_t *location, + enum cbfs_compression algo, uint32_t *location, const char *ignore_section); /* location is TOP aligned. */ int parse_elf_to_xip_stage(const struct buffer *input, struct buffer *output, diff --git a/util/cbfstool/compress.c b/util/cbfstool/compress.c index 9938094..52ae6df 100644 --- a/util/cbfstool/compress.c +++ b/util/cbfstool/compress.c @@ -83,7 +83,7 @@ return 0; } -comp_func_ptr compression_function(enum comp_algo algo) +comp_func_ptr compression_function(enum cbfs_compression algo) { comp_func_ptr compress; switch (algo) { @@ -103,7 +103,7 @@ return compress; } -decomp_func_ptr decompression_function(enum comp_algo algo) +decomp_func_ptr decompression_function(enum cbfs_compression algo) { decomp_func_ptr decompress; switch (algo) { -- To view, visit https://review.coreboot.org/c/coreboot/+/41117 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22eae1bcd76d85fff17749617cfe4f1de55603f4 Gerrit-Change-Number: 41117 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Wim Vervoorn <wvervoorn(a)eltan.com> Gerrit-MessageType: newchange

4 20

Change in coreboot[master]: lib/trace: Remove TRACE support
by Kyösti Mälkki (Code Review) 03 Dec '20

03 Dec '20

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45757 ) Change subject: lib/trace: Remove TRACE support ...................................................................... lib/trace: Remove TRACE support Looks like the option is generally not compatible with garbage collections. Nothing is inlined, is_smp_boot() no longer evaluates to constant false and thus the symbols from secondary.S would need to be present for the build to pass after we set SMP=n. Change-Id: I0b6f310e15e6f4992cd054d288903fea8390e5cf Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M Makefile.inc M src/Kconfig M src/console/printk.c M src/console/vsprintf.c M src/drivers/uart/uart8250io.c D src/include/trace.h M src/lib/Makefile.inc D src/lib/trace.c 8 files changed, 0 insertions(+), 75 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/45757/1 diff --git a/Makefile.inc b/Makefile.inc index a43de5e..d2138a0 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -181,9 +181,6 @@ bootblock-generic-ccopts += -D__BOOTBLOCK__ romstage-generic-ccopts += -D__ROMSTAGE__ ramstage-generic-ccopts += -D__RAMSTAGE__ -ifeq ($(CONFIG_TRACE),y) -ramstage-c-ccopts += -finstrument-functions -endif ifeq ($(CONFIG_COVERAGE),y) ramstage-c-ccopts += -fprofile-arcs -ftest-coverage endif diff --git a/src/Kconfig b/src/Kconfig index e46a6e6..45d23ae 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -1092,16 +1092,6 @@ is present on Intel 6-series chipsets. endif -config TRACE - bool "Trace function calls" - default n - help - If enabled, every function will print information to console once - the function is entered. The syntax is ~0xaaaabbbb(0xccccdddd) - the 0xaaaabbbb is the actual function and 0xccccdddd is EIP - of calling function. Please note some printk related functions - are omitted from trace to have good looking console dumps. - config DEBUG_COVERAGE bool "Debug code coverage" default n diff --git a/src/console/printk.c b/src/console/printk.c index 4a3de47..85d9bfb 100644 --- a/src/console/printk.c +++ b/src/console/printk.c @@ -10,7 +10,6 @@ #include <console/vtxprintf.h> #include <smp/spinlock.h> #include <smp/node.h> -#include <trace.h> #include <timer.h> DECLARE_SPIN_LOCK(console_lock) @@ -81,7 +80,6 @@ if (log_this < CONSOLE_LOG_FAST) return 0; - DISABLE_TRACE; spin_lock(&console_lock); console_time_run(); @@ -96,7 +94,6 @@ console_time_stop(); spin_unlock(&console_lock); - ENABLE_TRACE; return i; } diff --git a/src/console/vsprintf.c b/src/console/vsprintf.c index d0c569b..06b9e49 100644 --- a/src/console/vsprintf.c +++ b/src/console/vsprintf.c @@ -2,7 +2,6 @@ #include <console/vtxprintf.h> #include <string.h> -#include <trace.h> struct vsnprintf_context { char *str_buf; @@ -24,16 +23,12 @@ int i; struct vsnprintf_context ctx; - DISABLE_TRACE; - ctx.str_buf = buf; ctx.buf_limit = size ? size - 1 : 0; i = vtxprintf(str_tx_byte, fmt, args, &ctx); if (size) *ctx.str_buf = '\0'; - ENABLE_TRACE; - return i; } diff --git a/src/drivers/uart/uart8250io.c b/src/drivers/uart/uart8250io.c index d0841de..aa8c969 100644 --- a/src/drivers/uart/uart8250io.c +++ b/src/drivers/uart/uart8250io.c @@ -3,7 +3,6 @@ #include <arch/io.h> #include <boot/coreboot_tables.h> #include <console/uart.h> -#include <trace.h> #include "uart8250reg.h" /* Should support 8250, 16450, 16550, 16550A type UARTs */ @@ -54,7 +53,6 @@ static void uart8250_init(unsigned int base_port, unsigned int divisor) { - DISABLE_TRACE; /* Disable interrupts */ outb(0x0, base_port + UART8250_IER); /* Enable FIFOs */ @@ -72,7 +70,6 @@ /* Set to 3 for 8N1 */ outb(CONFIG_TTYS0_LCS, base_port + UART8250_LCR); - ENABLE_TRACE; } static const unsigned int bases[] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 }; diff --git a/src/include/trace.h b/src/include/trace.h deleted file mode 100644 index ece1b21..0000000 --- a/src/include/trace.h +++ /dev/null @@ -1,28 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef __TRACE_H -#define __TRACE_H - -#if !ENV_ROMSTAGE_OR_BEFORE && CONFIG(TRACE) - -void __cyg_profile_func_enter(void *, void *) - __attribute__((no_instrument_function)); - -void __cyg_profile_func_exit(void *, void *) - __attribute__((no_instrument_function)); - -extern volatile int trace_dis; - -#define DISABLE_TRACE do { trace_dis = 1; } while (0); -#define ENABLE_TRACE do { trace_dis = 0; } while (0); -#define DISABLE_TRACE_ON_FUNCTION __attribute__((no_instrument_function)); - -#else /* !CONFIG_TRACE */ - -#define DISABLE_TRACE -#define ENABLE_TRACE -#define DISABLE_TRACE_ON_FUNCTION - -#endif - -#endif diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 73077f7..f1653a1 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -139,8 +139,6 @@ ramstage-$(CONFIG_CONSOLE_CBMEM) += cbmem_console.c ramstage-$(CONFIG_BOOTSPLASH) += bootsplash.c ramstage-$(CONFIG_BOOTSPLASH) += jpeg.c -ramstage-$(CONFIG_TRACE) += trace.c -postcar-$(CONFIG_TRACE) += trace.c ramstage-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c ramstage-$(CONFIG_COVERAGE) += libgcov.c ramstage-y += edid.c diff --git a/src/lib/trace.c b/src/lib/trace.c deleted file mode 100644 index a3db40b..0000000 --- a/src/lib/trace.c +++ /dev/null @@ -1,21 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <console/console.h> -#include <trace.h> - -int volatile trace_dis = 0; - -void __cyg_profile_func_enter(void *func, void *callsite) -{ - - if (trace_dis) - return; - - DISABLE_TRACE - printk(BIOS_INFO, "~%p(%p)\n", func, callsite); - ENABLE_TRACE -} - -void __cyg_profile_func_exit(void *func, void *callsite) -{ -} -- To view, visit https://review.coreboot.org/c/coreboot/+/45757 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0b6f310e15e6f4992cd054d288903fea8390e5cf Gerrit-Change-Number: 45757 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

4 8

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2020