coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1796 discussions

Change in coreboot[master]: Documentation: Mark up register names as code
by Paul Menzel (Code Review) 09 Feb '20

09 Feb '20

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38721 ) Change subject: Documentation: Mark up register names as code ...................................................................... Documentation: Mark up register names as code Change-Id: I708385bca8edcd74b0d4c0a3ecc181b6ccd30c2b Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M Documentation/mainboard/lenovo/ivb_internal_flashing.md 1 file changed, 15 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/38721/1 diff --git a/Documentation/mainboard/lenovo/ivb_internal_flashing.md b/Documentation/mainboard/lenovo/ivb_internal_flashing.md index 355cf98..e6b597b 100644 --- a/Documentation/mainboard/lenovo/ivb_internal_flashing.md +++ b/Documentation/mainboard/lenovo/ivb_internal_flashing.md @@ -5,9 +5,9 @@ Old versions of stock BIOS for these models have several security issues. In order to flash coreboot internally, two of them are of interest. -**First** is the fact the SMM_BWP and BLE are not enabled in BIOS +**First** is the fact the `SMM_BWP` and `BLE` are not enabled in BIOS versions released before 2014. We have tested many versions on T430 and -X230 and found out that SMM_BWP=1 only since the update, the changelog +X230 and found out that `SMM_BWP=1` only since the update, the changelog of which contains following line: > (New) Improved the UEFI BIOS security feature. @@ -159,14 +159,14 @@ Configuration Registers. When set to 1, PR0-PR4 registers cannot be written. Once set to 1, cannot be changed anymore. -To be able to flash, we need SMM_BWP=0, BIOSWE=1, BLE=0, FLOCKDN=0 or +To be able to flash, we need `SMM_BWP=0`, `BIOSWE=1`, `BLE=0`, `FLOCKDN=0` or SPI protected ranges (PRx) to have a WP bit set to 0. -Let's see what we have. Examine HSFS register: +Let's see what we have. Examine `HSFS` register: sudo chipsec_main -m chipsec.modules.common.spi_lock -You should see that FLOCKDN=1: +You should see that `FLOCKDN=1`: [x][ ======================================================================= [x][ Module: SPI Flash Controller Configuration Locks @@ -181,11 +181,11 @@ [14] FDV = 1 << Flash Descriptor Valid [15] FLOCKDN = 1 << Flash Configuration Lock-Down -Then check BIOS_CNTL and PR0-PR4: +Then check `BIOS_CNTL` and PR0-PR4: sudo chipsec_main -m common.bios_wp -Good news: on old BIOS versions, SMM_BWP=0 and BLE=0. +Good news: on old BIOS versions, `SMM_BWP=0` and `BLE=0`. Bad news: there are 4 write protected SPI ranges: @@ -215,8 +215,8 @@ sudo chipsec_util mmio dump SPIBAR -You will see SPIBAR address (0xFED1F800) and registers (for example, -00000004 is HSFS): +You will see `SPIBAR` address (0xFED1F800) and registers (for example, +`00000004` is `HSFS`): [mmio] MMIO register range [0x00000000FED1F800:0x00000000FED1F800+00000200]: +00000000: 0BFF0500 @@ -224,11 +224,11 @@ ... As you can see, the only thing we need is to unset WP bit on PR0-PR4. -But that cannot be done once FLOCKDN is set to 1. +But that cannot be done once `FLOCKDN` is set to 1. Now the fun part! -FLOCKDN may only be cleared by a hardware reset, which includes S3 +`FLOCKDN` may only be cleared by a hardware reset, which includes S3 state. On S3 resume boot path, the chipset configuration has to be restored and it's done by executing so-called S3 Boot Scripts. You can dump these scripts by executing: @@ -236,7 +236,7 @@ sudo chipsec_util uefi s3bootscript There are many entries. Along them, you can find instructions to write -to HSFS (remember, we know that SPIBAR is 0xFED1F800): +to `HSFS` (remember, we know that `SPIBAR` is 0xFED1F800): Entry at offset 0x2B8F (len = 0x17, header len = 0x0): Data: @@ -251,7 +251,7 @@ These scripts are stored in memory. The vulnerability is that we can overwrite this memory, change these instructions and they will be -executed on S3 resume. Once we patch that instruction to not set FLOCKDN +executed on S3 resume. Once we patch that instruction to not set `FLOCKDN` bit, we will be able to write to PR0-PR4 registers. ## Creating a backup @@ -274,7 +274,7 @@ ## Removing protections (practice) -The original boot script writes 0xE009 to HSFS. FLOCKDN is 15th bit, so +The original boot script writes 0xE009 to `HSFS`. `FLOCKDN` is 15th bit, so let's write 0x6009 instead: sudo chipsec_main -m tools.uefi.s3script_modify -a replace_op,mmio_wr,0xFED1F804,0x6009,0x2 @@ -297,7 +297,7 @@ [*] After sleep/resume, check the value of register 0xFED1F804 is 0x6009 [+] PASSED: The script has been modified. Go to sleep.. -Now go to S3, then resume and check FLOCKDN. It should be 0: +Now go to S3, then resume and check `FLOCKDN`. It should be 0: sudo chipsec_main -m chipsec.modules.common.spi_lock -- To view, visit https://review.coreboot.org/c/coreboot/+/38721 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I708385bca8edcd74b0d4c0a3ecc181b6ccd30c2b Gerrit-Change-Number: 38721 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: Documentation: Intend code blocks instead of using ```
by Paul Menzel (Code Review) 09 Feb '20

09 Feb '20

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38720 ) Change subject: Documentation: Intend code blocks instead of using ``` ...................................................................... Documentation: Intend code blocks instead of using ``` Both versions are correct, but especially for one liners intending them with four spaces instead of using ``` blocks helps readablily of the source file. Change-Id: Ie2543c8c4cccefd74e966f784e651ed7dc3a9252 Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M Documentation/mainboard/lenovo/ivb_internal_flashing.md 1 file changed, 166 insertions(+), 177 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/38720/1 diff --git a/Documentation/mainboard/lenovo/ivb_internal_flashing.md b/Documentation/mainboard/lenovo/ivb_internal_flashing.md index d0ac3cd..355cf98 100644 --- a/Documentation/mainboard/lenovo/ivb_internal_flashing.md +++ b/Documentation/mainboard/lenovo/ivb_internal_flashing.md @@ -63,55 +63,51 @@ downloaded. Extract an El Torito image: -``` -geteltorito -o ./bios.img g1uj41us.iso -``` + + geteltorito -o ./bios.img g1uj41us.iso + Mount the partition in that image: -``` -sudo mount -t vfat ./bios.img /mnt -o loop,offset=16384 -``` + + sudo mount -t vfat ./bios.img /mnt -o loop,offset=16384 + List files, find the `AUTOEXEC.BAT` file and the `FLASH` directory: -``` -ls /mnt -ls /mnt/FLASH -``` + + ls /mnt + ls /mnt/FLASH Inside the `FLASH` directory, there should be a directory called `G1ET93WW` or similar (exact name depends on your ThinkPad model and BIOS version). See what's inside: -``` -ls /mnt/FLASH/G1ET93WW -``` + + ls /mnt/FLASH/G1ET93WW + There must be a file with `.FL1` extension called `$01D2000.FL1` or something similar. Now open the `AUTOEXEC.BAT` file: -``` -sudo vim /mnt/AUTOEXEC.BAT -``` + + sudo vim /mnt/AUTOEXEC.BAT + You will see a list of commands: -``` -@ECHO OFF -PROMPT $p$g -cd c:\flash -command.com -``` + + @ECHO OFF + PROMPT $p$g + cd c:\flash + command.com + Replace the last line (`command.com`) with this (change path to the `.FL1` file according to yours): -``` -dosflash.exe /sd /file G1ET93WW\$01D2000.FL1 -``` + + dosflash.exe /sd /file G1ET93WW\$01D2000.FL1 Save the file, then unmount the partition: -``` -sudo unmount /mnt -``` + + sudo unmount /mnt Write this image to a USB drive (replace `/dev/sdX` with your USB drive device name): -``` -sudo dd if=./bios.img of=/dev/sdX bs=1M -``` + + sudo dd if=./bios.img of=/dev/sdX bs=1M Now reboot and press F1 to enter BIOS settings. Open the **Startup** tab and set the startup mode to **Legacy** (or **Both**/**Legacy First**): @@ -167,69 +163,66 @@ SPI protected ranges (PRx) to have a WP bit set to 0. Let's see what we have. Examine HSFS register: -``` -sudo chipsec_main -m chipsec.modules.common.spi_lock -``` + + sudo chipsec_main -m chipsec.modules.common.spi_lock + You should see that FLOCKDN=1: -``` -[x][ ======================================================================= -[x][ Module: SPI Flash Controller Configuration Locks -[x][ ======================================================================= -[*] HSFS = 0xE009 << Hardware Sequencing Flash Status Register (SPIBAR + 0x4) - [00] FDONE = 1 << Flash Cycle Done - [01] FCERR = 0 << Flash Cycle Error - [02] AEL = 0 << Access Error Log - [03] BERASE = 1 << Block/Sector Erase Size - [05] SCIP = 0 << SPI cycle in progress - [13] FDOPSS = 1 << Flash Descriptor Override Pin-Strap Status - [14] FDV = 1 << Flash Descriptor Valid - [15] FLOCKDN = 1 << Flash Configuration Lock-Down -``` + + [x][ ======================================================================= + [x][ Module: SPI Flash Controller Configuration Locks + [x][ ======================================================================= + [*] HSFS = 0xE009 << Hardware Sequencing Flash Status Register (SPIBAR + 0x4) + [00] FDONE = 1 << Flash Cycle Done + [01] FCERR = 0 << Flash Cycle Error + [02] AEL = 0 << Access Error Log + [03] BERASE = 1 << Block/Sector Erase Size + [05] SCIP = 0 << SPI cycle in progress + [13] FDOPSS = 1 << Flash Descriptor Override Pin-Strap Status + [14] FDV = 1 << Flash Descriptor Valid + [15] FLOCKDN = 1 << Flash Configuration Lock-Down Then check BIOS_CNTL and PR0-PR4: -``` -sudo chipsec_main -m common.bios_wp -``` + + sudo chipsec_main -m common.bios_wp + Good news: on old BIOS versions, SMM_BWP=0 and BLE=0. Bad news: there are 4 write protected SPI ranges: -``` -[x][ ======================================================================= -[x][ Module: BIOS Region Write Protection -[x][ ======================================================================= -[*] BC = 0x 8 << BIOS Control (b:d.f 00:31.0 + 0xDC) - [00] BIOSWE = 0 << BIOS Write Enable - [01] BLE = 0 << BIOS Lock Enable - [02] SRC = 2 << SPI Read Configuration - [04] TSS = 0 << Top Swap Status - [05] SMM_BWP = 0 << SMM BIOS Write Protection -[-] BIOS region write protection is disabled! + [x][ ======================================================================= + [x][ Module: BIOS Region Write Protection + [x][ ======================================================================= + [*] BC = 0x 8 << BIOS Control (b:d.f 00:31.0 + 0xDC) + [00] BIOSWE = 0 << BIOS Write Enable + [01] BLE = 0 << BIOS Lock Enable + [02] SRC = 2 << SPI Read Configuration + [04] TSS = 0 << Top Swap Status + [05] SMM_BWP = 0 << SMM BIOS Write Protection + [-] BIOS region write protection is disabled! -[*] BIOS Region: Base = 0x00500000, Limit = 0x00BFFFFF -SPI Protected Ranges ------------------------------------------------------------- -PRx (offset) | Value | Base | Limit | WP? | RP? ------------------------------------------------------------- -PR0 (74) | 00000000 | 00000000 | 00000000 | 0 | 0 -PR1 (78) | 8BFF0B40 | 00B40000 | 00BFFFFF | 1 | 0 -PR2 (7C) | 8B100B10 | 00B10000 | 00B10FFF | 1 | 0 -PR3 (80) | 8ADE0AD0 | 00AD0000 | 00ADEFFF | 1 | 0 -PR4 (84) | 8AAF0800 | 00800000 | 00AAFFFF | 1 | 0 -``` + [*] BIOS Region: Base = 0x00500000, Limit = 0x00BFFFFF + SPI Protected Ranges + ------------------------------------------------------------ + PRx (offset) | Value | Base | Limit | WP? | RP? + ------------------------------------------------------------ + PR0 (74) | 00000000 | 00000000 | 00000000 | 0 | 0 + PR1 (78) | 8BFF0B40 | 00B40000 | 00BFFFFF | 1 | 0 + PR2 (7C) | 8B100B10 | 00B10000 | 00B10FFF | 1 | 0 + PR3 (80) | 8ADE0AD0 | 00AD0000 | 00ADEFFF | 1 | 0 + PR4 (84) | 8AAF0800 | 00800000 | 00AAFFFF | 1 | 0 Other way to examine SPI configuration registers is to just dump SPIBAR: -``` -sudo chipsec_util mmio dump SPIBAR -``` + + sudo chipsec_util mmio dump SPIBAR + You will see SPIBAR address (0xFED1F800) and registers (for example, 00000004 is HSFS): -``` -[mmio] MMIO register range [0x00000000FED1F800:0x00000000FED1F800+00000200]: -+00000000: 0BFF0500 -+00000004: 0004E009 -... -``` + + [mmio] MMIO register range [0x00000000FED1F800:0x00000000FED1F800+00000200]: + +00000000: 0BFF0500 + +00000004: 0004E009 + ... + As you can see, the only thing we need is to unset WP bit on PR0-PR4. But that cannot be done once FLOCKDN is set to 1. @@ -239,23 +232,23 @@ state. On S3 resume boot path, the chipset configuration has to be restored and it's done by executing so-called S3 Boot Scripts. You can dump these scripts by executing: -``` -sudo chipsec_util uefi s3bootscript -``` + + sudo chipsec_util uefi s3bootscript + There are many entries. Along them, you can find instructions to write to HSFS (remember, we know that SPIBAR is 0xFED1F800): -``` -Entry at offset 0x2B8F (len = 0x17, header len = 0x0): -Data: -02 00 17 02 00 00 00 01 00 00 00 04 f8 d1 fe 00 | -00 00 00 09 e0 04 00 | -Decoded: - Opcode : S3_BOOTSCRIPT_MEM_WRITE (0x0002) - Width : 0x02 (4 bytes) - Address: 0xFED1F804 - Count : 0x1 - Values : 0x0004E009 -``` + + Entry at offset 0x2B8F (len = 0x17, header len = 0x0): + Data: + 02 00 17 02 00 00 00 01 00 00 00 04 f8 d1 fe 00 | + 00 00 00 09 e0 04 00 | + Decoded: + Opcode : S3_BOOTSCRIPT_MEM_WRITE (0x0002) + Width : 0x02 (4 bytes) + Address: 0xFED1F804 + Count : 0x1 + Values : 0x0004E009 + These scripts are stored in memory. The vulnerability is that we can overwrite this memory, change these instructions and they will be executed on S3 resume. Once we patch that instruction to not set FLOCKDN @@ -268,14 +261,13 @@ The `me` region is locked, so an attempt to create a full dump will fail. But you can back up the `bios`: -``` -sudo flashrom -p internal -r bios_backup.rom --ifd -i bios -``` + + sudo flashrom -p internal -r bios_backup.rom --ifd -i bios If you will ever need to flash it back, use `--ifd -i bios` as well: -``` -sudo flashrom -p <YOUR_PROGRAMMER> -w bios_backup.rom --ifd -i bios -``` + + sudo flashrom -p <YOUR_PROGRAMMER> -w bios_backup.rom --ifd -i bios + **Caution:** if you will omit `--ifd -i bios` for flashing, you will brick your machine, because your backup has `FF`s in place of `fd` and `me` regions. Flash only `bios` region! @@ -284,83 +276,80 @@ The original boot script writes 0xE009 to HSFS. FLOCKDN is 15th bit, so let's write 0x6009 instead: -``` -sudo chipsec_main -m tools.uefi.s3script_modify -a replace_op,mmio_wr,0xFED1F804,0x6009,0x2 -``` + + sudo chipsec_main -m tools.uefi.s3script_modify -a replace_op,mmio_wr,0xFED1F804,0x6009,0x2 + You will get a lot of output and in the end you should see something like this: -``` -[*] Modifying S3 boot script entry at address 0x00000000DAF49B8F.. -[mem] 0x00000000DAF49B8F -[*] Original entry: - 2 0 17 2 0 0 0 1 0 0 0 4 f8 d1 fe 0 | - 0 0 0 9 e0 4 0 | -[mem] buffer len = 0x17 to PA = 0x00000000DAF49B8F - 2 0 17 2 0 0 0 1 0 0 0 4 f8 d1 fe 0 | - 0 0 0 9 60 0 0 | ` -[mem] 0x00000000DAF49B8F -[*] Modified entry: - 2 0 17 2 0 0 0 1 0 0 0 4 f8 d1 fe 0 | - 0 0 0 9 60 0 0 | ` -[*] After sleep/resume, check the value of register 0xFED1F804 is 0x6009 -[+] PASSED: The script has been modified. Go to sleep.. -``` -Now go to S3, then resume and check FLOCKDN. It should be 0: -``` -sudo chipsec_main -m chipsec.modules.common.spi_lock -``` -``` -... -[x][ ======================================================================= -[x][ Module: SPI Flash Controller Configuration Locks -[x][ ======================================================================= -[*] HSFS = 0x6008 << Hardware Sequencing Flash Status Register (SPIBAR + 0x4) - [00] FDONE = 0 << Flash Cycle Done - [01] FCERR = 0 << Flash Cycle Error - [02] AEL = 0 << Access Error Log - [03] BERASE = 1 << Block/Sector Erase Size - [05] SCIP = 0 << SPI cycle in progress - [13] FDOPSS = 1 << Flash Descriptor Override Pin-Strap Status - [14] FDV = 1 << Flash Descriptor Valid - [15] FLOCKDN = 0 << Flash Configuration Lock-Down -[-] SPI Flash Controller configuration is not locked -[-] FAILED: SPI Flash Controller not locked correctly. -... -``` -Remove WP from protected ranges: -``` -sudo chipsec_util mmio write SPIBAR 0x74 0x4 0xAAF0800 -sudo chipsec_util mmio write SPIBAR 0x78 0x4 0xADE0AD0 -sudo chipsec_util mmio write SPIBAR 0x7C 0x4 0xB100B10 -sudo chipsec_util mmio write SPIBAR 0x80 0x4 0xBFF0B40 -``` -Verify that it worked: -``` -sudo chipsec_main -m common.bios_wp -``` -``` -[x][ ======================================================================= -[x][ Module: BIOS Region Write Protection -[x][ ======================================================================= -[*] BC = 0x 9 << BIOS Control (b:d.f 00:31.0 + 0xDC) - [00] BIOSWE = 1 << BIOS Write Enable - [01] BLE = 0 << BIOS Lock Enable - [02] SRC = 2 << SPI Read Configuration - [04] TSS = 0 << Top Swap Status - [05] SMM_BWP = 0 << SMM BIOS Write Protection -[-] BIOS region write protection is disabled! -[*] BIOS Region: Base = 0x00500000, Limit = 0x00BFFFFF -SPI Protected Ranges ------------------------------------------------------------- -PRx (offset) | Value | Base | Limit | WP? | RP? ------------------------------------------------------------- -PR0 (74) | 0AAF0800 | 00800000 | 00AAF000 | 0 | 0 -PR1 (78) | 0ADE0AD0 | 00AD0000 | 00ADE000 | 0 | 0 -PR2 (7C) | 0B100B10 | 00B10000 | 00B10000 | 0 | 0 -PR3 (80) | 0BFF0B40 | 00B40000 | 00BFF000 | 0 | 0 -PR4 (84) | 00000000 | 00000000 | 00000000 | 0 | 0 -``` + [*] Modifying S3 boot script entry at address 0x00000000DAF49B8F.. + [mem] 0x00000000DAF49B8F + [*] Original entry: + 2 0 17 2 0 0 0 1 0 0 0 4 f8 d1 fe 0 | + 0 0 0 9 e0 4 0 | + [mem] buffer len = 0x17 to PA = 0x00000000DAF49B8F + 2 0 17 2 0 0 0 1 0 0 0 4 f8 d1 fe 0 | + 0 0 0 9 60 0 0 | ` + [mem] 0x00000000DAF49B8F + [*] Modified entry: + 2 0 17 2 0 0 0 1 0 0 0 4 f8 d1 fe 0 | + 0 0 0 9 60 0 0 | ` + [*] After sleep/resume, check the value of register 0xFED1F804 is 0x6009 + [+] PASSED: The script has been modified. Go to sleep.. + +Now go to S3, then resume and check FLOCKDN. It should be 0: + + sudo chipsec_main -m chipsec.modules.common.spi_lock + + ... + [x][ ======================================================================= + [x][ Module: SPI Flash Controller Configuration Locks + [x][ ======================================================================= + [*] HSFS = 0x6008 << Hardware Sequencing Flash Status Register (SPIBAR + 0x4) + [00] FDONE = 0 << Flash Cycle Done + [01] FCERR = 0 << Flash Cycle Error + [02] AEL = 0 << Access Error Log + [03] BERASE = 1 << Block/Sector Erase Size + [05] SCIP = 0 << SPI cycle in progress + [13] FDOPSS = 1 << Flash Descriptor Override Pin-Strap Status + [14] FDV = 1 << Flash Descriptor Valid + [15] FLOCKDN = 0 << Flash Configuration Lock-Down + [-] SPI Flash Controller configuration is not locked + [-] FAILED: SPI Flash Controller not locked correctly. + ... + +Remove WP from protected ranges: + + sudo chipsec_util mmio write SPIBAR 0x74 0x4 0xAAF0800 + sudo chipsec_util mmio write SPIBAR 0x78 0x4 0xADE0AD0 + sudo chipsec_util mmio write SPIBAR 0x7C 0x4 0xB100B10 + sudo chipsec_util mmio write SPIBAR 0x80 0x4 0xBFF0B40 + +Verify that it worked: + + sudo chipsec_main -m common.bios_wp + + [x][ ======================================================================= + [x][ Module: BIOS Region Write Protection + [x][ ======================================================================= + [*] BC = 0x 9 << BIOS Control (b:d.f 00:31.0 + 0xDC) + [00] BIOSWE = 1 << BIOS Write Enable + [01] BLE = 0 << BIOS Lock Enable + [02] SRC = 2 << SPI Read Configuration + [04] TSS = 0 << Top Swap Status + [05] SMM_BWP = 0 << SMM BIOS Write Protection + [-] BIOS region write protection is disabled! + + [*] BIOS Region: Base = 0x00500000, Limit = 0x00BFFFFF + SPI Protected Ranges + ------------------------------------------------------------ + PRx (offset) | Value | Base | Limit | WP? | RP? + ------------------------------------------------------------ + PR0 (74) | 0AAF0800 | 00800000 | 00AAF000 | 0 | 0 + PR1 (78) | 0ADE0AD0 | 00AD0000 | 00ADE000 | 0 | 0 + PR2 (7C) | 0B100B10 | 00B10000 | 00B10000 | 0 | 0 + PR3 (80) | 0BFF0B40 | 00B40000 | 00BFF000 | 0 | 0 + PR4 (84) | 00000000 | 00000000 | 00000000 | 0 | 0 Bingo! -- To view, visit https://review.coreboot.org/c/coreboot/+/38720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie2543c8c4cccefd74e966f784e651ed7dc3a9252 Gerrit-Change-Number: 38720 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

3 6

Change in coreboot[master]: mb/google/octopus: Override VBT selection for Bipship
by Tony Huang (Code Review) 09 Feb '20

09 Feb '20

Tony Huang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38716 ) Change subject: mb/google/octopus: Override VBT selection for Bipship ...................................................................... mb/google/octopus: Override VBT selection for Bipship Share the same vbt_blooguard.bin to disalbe DRRS support. BUG=b:148892903, b:147021309 BRANCH=octopus TEST=emerge-octopus coreboot chromeos-bootimage check i915_drrs_status shows DRRS supported NO when SKU ID is bipship. Change-Id: I61f12d4ddea17a05255751fde2a5ce822dd2e782 Signed-off-by: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> --- M src/mainboard/google/octopus/variants/bloog/variant.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/38716/1 diff --git a/src/mainboard/google/octopus/variants/bloog/variant.c b/src/mainboard/google/octopus/variants/bloog/variant.c index 18f44b9..99f967c 100644 --- a/src/mainboard/google/octopus/variants/bloog/variant.c +++ b/src/mainboard/google/octopus/variants/bloog/variant.c @@ -30,6 +30,8 @@ SKU_50_BLOOGUARD = 50, /* kb blit, USI Stylus */ SKU_51_BLOOGUARD = 51, /* no kb blit, no USI Stylus */ SKU_52_BLOOGUARD = 52, /* no kb blit, USI Stylus */ + SKU_53_BIPSHIP = 53, /* no kb blit, TS, 360, no Stylus, no rare-cam */ + SKU_54_BIPSHIP = 54, /* kb blit, TS, 360, no Stylus, no rare-cam */ SKU_65_BLOOGLET = 65, /* TS, kb blit */ SKU_66_BLOOGLET = 66, /* TS, no kb blit */ SKU_67_BLOOGLET = 67, /* non-TS, kb blit */ @@ -64,6 +66,7 @@ if (sku_id == SKU_49_BLOOGUARD || sku_id == SKU_50_BLOOGUARD || sku_id == SKU_51_BLOOGUARD || sku_id == SKU_52_BLOOGUARD || + sku_id == SKU_53_BIPSHIP || sku_id == SKU_54_BIPSHIP || sku_id == SKU_65_BLOOGLET || sku_id == SKU_66_BLOOGLET || sku_id == SKU_67_BLOOGLET || sku_id == SKU_68_BLOOGLET) return "vbt_blooguard.bin"; -- To view, visit https://review.coreboot.org/c/coreboot/+/38716 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I61f12d4ddea17a05255751fde2a5ce822dd2e782 Gerrit-Change-Number: 38716 Gerrit-PatchSet: 1 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-MessageType: newchange

5 21

Change in coreboot[master]: util/ifdtool: Support modification of single Flash Descriptor
by Marcello Sylvester Bauer (Code Review) 09 Feb '20

09 Feb '20

Marcello Sylvester Bauer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38711 ) Change subject: util/ifdtool: Support modification of single Flash Descriptor ...................................................................... util/ifdtool: Support modification of single Flash Descriptor Add the capability to update the Flash Descriptor module directly instead of raising a Segmentation Fault. In this way it will be possible to add a Kconfig options to modify the ifd descriptor at build-time. Change-Id: Id3db09291af2bd2e759c283e316afd5da1fb4ca7 Signed-off-by: Marcello Sylvester Bauer <sylv(a)sylv.io> --- M util/ifdtool/ifdtool.c 1 file changed, 23 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/38711/1 diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c index 0b6b210..fa2a564 100644 --- a/util/ifdtool/ifdtool.c +++ b/util/ifdtool/ifdtool.c @@ -1263,6 +1263,7 @@ region_t new_regions[MAX_REGIONS]; int new_extent = 0; char *new_image; + bool descriptor_only; /* load current descriptor map and regions */ frba_t *frba = find_frba(image, size); @@ -1336,8 +1337,17 @@ new_extent = new_regions[i].limit; } + /* check if the image is actually a Flash Descriptor module */ + descriptor_only = false; + /* compare image size to descriptor */ + if (size == new_regions[0].size) { + printf("The image is a single Flash Descriptor module:\n"); + printf(" Only the module will be modified\n"); + descriptor_only = true; + } + new_extent = next_pow2(new_extent - 1); - if (new_extent != size) { + if (!descriptor_only && new_extent != size) { printf("The image has changed in size.\n"); printf("The old image is %d bytes.\n", size); printf("The new image is %d bytes.\n", new_extent); @@ -1367,6 +1377,12 @@ offset_current = current->size - new->size; } + if (size < current->base + offset_current + copy_size) { + printf("Skip Descriptor %d (%s) (region missing in the old image)\n", i, + region_name(i)); + continue; + }; + printf("Copy Descriptor %d (%s) (%d bytes)\n", i, region_name(i), copy_size); printf(" from %08x+%08x:%08x (%10d)\n", current->base, @@ -1384,10 +1400,15 @@ if (!frba) exit(EXIT_FAILURE); + printf("Modify Flash Descriptor regions\n"); for (i = 1; i < max_regions; i++) set_region(frba, i, &new_regions[i]); - write_image(filename, new_image, new_extent); + if (descriptor_only) { + write_image(filename, new_image, size); + } else { + write_image(filename, new_image, new_extent); + }; free(new_image); } -- To view, visit https://review.coreboot.org/c/coreboot/+/38711 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id3db09291af2bd2e759c283e316afd5da1fb4ca7 Gerrit-Change-Number: 38711 Gerrit-PatchSet: 1 Gerrit-Owner: Marcello Sylvester Bauer <sylv(a)sylv.io> Gerrit-MessageType: newchange

4 9

Change in coreboot[master]: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_r...
by Sridhar Siricilla (Code Review) 09 Feb '20

09 Feb '20

Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37584 ) Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ...................................................................... soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. Test=Verified on hatch board. Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 6 files changed, 25 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/1 diff --git a/src/soc/intel/cannonlake/reset.c b/src/soc/intel/cannonlake/reset.c index 4758faf..6889bb0 100644 --- a/src/soc/intel/cannonlake/reset.c +++ b/src/soc/intel/cannonlake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (send_heci_reset_req_message(GLOBAL_RESET)) return; /* global reset if CSE fail to reset */ diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index b60d888..f40e328 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -583,7 +583,7 @@ * Sends GLOBAL_RESET_REQ cmd to CSE.The reset type can be GLOBAL_RESET/ * HOST_RESET_ONLY/CSE_RESET_ONLY. */ -int send_heci_reset_req_message(uint8_t rst_type) +int send_heci_reset_req_message(enum rst_req_type rst_type) { int status; struct mkhi_hdr reply; @@ -602,27 +602,25 @@ }; size_t reply_size; + printk(BIOS_DEBUG, "HECI: Global Reset(Type:%d) Command\n", rst_type); if (!((rst_type == GLOBAL_RESET) || - (rst_type == HOST_RESET_ONLY) || (rst_type == CSE_RESET_ONLY))) - return -1; + (rst_type == HOST_RESET_ONLY) || (rst_type == CSE_RESET_ONLY))) { + printk(BIOS_ERR, "HECI: Unsupported reset type is requested\n"); + return 0; + } heci_reset(); reply_size = sizeof(reply); memset(&reply, 0, reply_size); - printk(BIOS_DEBUG, "HECI: Global Reset(Type:%d) Command\n", rst_type); if (rst_type == CSE_RESET_ONLY) - status = heci_send_receive(&msg, sizeof(msg), NULL, 0); + status = heci_send(&msg, sizeof(msg), BIOS_HOST_ADDR, HECI_MKHI_ADDR); else - status = heci_send_receive(&msg, sizeof(msg), &reply, - &reply_size); + status = heci_send_receive(&msg, sizeof(msg), &reply, &reply_size); - if (status != 1) - return -1; - - printk(BIOS_DEBUG, "HECI: Global Reset success!\n"); - return 0; + printk(BIOS_DEBUG, "HECI: Global Reset %s!\n", status ? "success" : "failure"); + return status; } /* Sends HMRFPO Enable command to CSE */ diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index 53f5493..9ca1a5f 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -120,12 +120,19 @@ */ uint8_t wait_cse_sec_override_mode(void); +/* Command GLOBAL_RESET_REQ Reset Types */ +enum rst_req_type { + GLOBAL_RESET = 1, + HOST_RESET_ONLY = 2, + CSE_RESET_ONLY = 3, +}; + /* - * Sends GLOBAL_RESET_REQ cmd to CSE.The reset type can be - * GLOBAL_RESET/HOST_RESET_ONLY/CSE_RESET_ONLY. - * Returns -1 on failure a 0 on success. + * Sends GLOBAL_RESET_REQ cmd to CSE. + * The reset type can be one of the above defined reset type. + * Returns 0 on failure a 1 on success. */ -int send_heci_reset_req_message(uint8_t rst_type); +int send_heci_reset_req_message(enum rst_req_type rst_type); /* * Sends HMRFPO_ENABLE command. @@ -156,11 +163,6 @@ #define BIOS_HOST_ADDR 0x00 #define HECI_MKHI_ADDR 0x07 -/* Command GLOBAL_RESET_REQ Reset Types */ -#define GLOBAL_RESET 1 -#define HOST_RESET_ONLY 2 -#define CSE_RESET_ONLY 3 - /*HMRFPO Status types */ #define MKHI_HMRFPO_DISABLED 0 #define MKHI_HMRFPO_LOCKED 1 diff --git a/src/soc/intel/icelake/reset.c b/src/soc/intel/icelake/reset.c index 5526a42..5ac8f6f 100644 --- a/src/soc/intel/icelake/reset.c +++ b/src/soc/intel/icelake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (send_heci_reset_req_message(GLOBAL_RESET)) return; /* global reset if CSE fail to reset */ diff --git a/src/soc/intel/skylake/reset.c b/src/soc/intel/skylake/reset.c index 8f5bf30..b16e11c 100644 --- a/src/soc/intel/skylake/reset.c +++ b/src/soc/intel/skylake/reset.c @@ -37,7 +37,7 @@ void do_global_reset(void) { - if (send_global_reset() != 0) { + if (!send_global_reset()) { /* If ME unable to reset platform then * force global reset using PMC CF9GR register*/ do_force_global_reset(); diff --git a/src/soc/intel/tigerlake/reset.c b/src/soc/intel/tigerlake/reset.c index 674cf68..d3d3340 100644 --- a/src/soc/intel/tigerlake/reset.c +++ b/src/soc/intel/tigerlake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (send_heci_reset_req_message(GLOBAL_RESET)) return; /* global reset if CSE fail to reset */ -- To view, visit https://review.coreboot.org/c/coreboot/+/37584 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Gerrit-Change-Number: 37584 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: newchange

3 15

Change in coreboot[master]: soc/intel/common: Add description to HMRFPO status
by Sridhar Siricilla (Code Review) 09 Feb '20

09 Feb '20

Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38480 ) Change subject: soc/intel/common: Add description to HMRFPO status ...................................................................... soc/intel/common: Add description to HMRFPO status Below changes are implemented: 1. Fix typos. 2. Rename 'padding' field of hmrfpo_get_status_resp struct to 'reserved' to match with ME BWG Guide. 3. Add documentation for HMRFPO Status. TEST=Build and boot hatch Change-Id: I4db9bdf7386c48e17ed0373cf334ccff358d1951 Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 10 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/38480/1 diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index 978f44f..5873d70 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -690,7 +690,7 @@ /* * Sends HMRFPO Get Status command to CSE to get the HMRFPO status. - * The status can be DISABLES/LOCKED/ENABLED + * The status can be DISABLED/LOCKED/ENABLED */ int cse_hmrfpo_get_status(void) { @@ -701,7 +701,7 @@ struct hmrfpo_get_status_resp { struct mkhi_hdr hdr; uint8_t status; - uint8_t padding[3]; + uint8_t reserved[3]; } __packed; struct hmrfpo_get_status_msg msg = { diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index 6233f7d..1377bd4 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -144,8 +144,16 @@ #define CSE_RESET_ONLY 3 /* HMRFPO Status types */ +/* Host can't access ME region */ #define MKHI_HMRFPO_DISABLED 0 + +/* + * ME Firmware locked down HMRFPO Feature. + * Host can't access ME region. + */ #define MKHI_HMRFPO_LOCKED 1 + +/* Host can access ME region */ #define MKHI_HMRFPO_ENABLED 2 /* -- To view, visit https://review.coreboot.org/c/coreboot/+/38480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4db9bdf7386c48e17ed0373cf334ccff358d1951 Gerrit-Change-Number: 38480 Gerrit-PatchSet: 1 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: newchange

3 2

Change in coreboot[master]: soc/intel/{common,skl,cnl,icl,apl,tgl}: Move HFSTS1 register definiti...
by Patrick Georgi (Code Review) 09 Feb '20

09 Feb '20

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/{common,skl,cnl,icl,apl,tgl}: Move HFSTS1 register definition to SoC ...................................................................... soc/intel/{common,skl,cnl,icl,apl,tgl}: Move HFSTS1 register definition to SoC Below changes are implemented: 1. Move HFSTS1 register definition to SoC since HFSTS1 register definition is specific to a SoC. Moving structure back to SoC specific to avoid unnecessay SoC specific macros in the common code. 2. Define a set of APIs in common code since CSE operation modes and working states are same across SoCs. cse_is_hfs1_com_normal(void) cse_is_hfs1_com_secover_mei_msg(void) cse_is_hfs1_com_soft_temp_disable(void) cse_is_hfs1_cws_normal(void) 3. Modify existing code to use callbacks to get data of me_hfs1 structure. TEST=Build and Boot hatch, soraka, tglrvp, bobba and iclrvp boards. Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/35546 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Furquan Shaikh <furquan(a)google.com> --- A src/soc/intel/apollolake/include/soc/me.h M src/soc/intel/cannonlake/include/soc/me.h M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h A src/soc/intel/icelake/include/soc/me.h M src/soc/intel/skylake/include/soc/me.h A src/soc/intel/tigerlake/include/soc/me.h 7 files changed, 235 insertions(+), 38 deletions(-) Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved diff --git a/src/soc/intel/apollolake/include/soc/me.h b/src/soc/intel/apollolake/include/soc/me.h new file mode 100644 index 0000000..7ac4dee --- /dev/null +++ b/src/soc/intel/apollolake/include/soc/me.h @@ -0,0 +1,43 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _APOLLOLAKE_ME_H_ +#define _APOLLOLAKE_ME_H_ + +/* ME Host Firmware Status register 1 */ +union me_hfsts1 { + u32 data; + struct { + u32 working_state: 4; + u32 mfg_mode: 1; + u32 fpt_bad: 1; + u32 operation_state: 3; + u32 fw_init_complete: 1; + u32 ft_bup_ld_flr: 1; + u32 update_in_progress: 1; + u32 error_code: 4; + u32 operation_mode: 4; + u32 reset_count: 4; + u32 boot_options_present: 1; + u32 bist_finished: 1; + u32 hw_bist_passed: 1; + u32 bist_reset_request: 1; + u32 current_power_source: 2; + u32 reserved: 1; + u32 d0i3_support_valid: 1; + } __packed fields; +}; + +#endif /* _APOLLOLAKE_ME_H_ */ diff --git a/src/soc/intel/cannonlake/include/soc/me.h b/src/soc/intel/cannonlake/include/soc/me.h index 5b411d3..041769b 100644 --- a/src/soc/intel/cannonlake/include/soc/me.h +++ b/src/soc/intel/cannonlake/include/soc/me.h @@ -16,6 +16,34 @@ #ifndef _CANNONLAKE_ME_H_ #define _CANNONLAKE_ME_H_ +/* ME Host Firmware Status register 1 */ +union me_hfsts1 { + u32 data; + struct { + u32 working_state: 4; + u32 mfg_mode: 1; + u32 fpt_bad: 1; + u32 operation_state: 3; + u32 fw_init_complete: 1; + u32 ft_bup_ld_flr: 1; + u32 update_in_progress: 1; + u32 error_code: 4; + u32 operation_mode: 4; + u32 reset_count: 4; + u32 boot_options_present: 1; +#if CONFIG(SOC_INTEL_COMETLAKE) + u32 invoke_enhance_dbg_mode:1; +#else + u32 reserved0: 1; +#endif + u32 bist_test_state: 1; + u32 bist_reset_request: 1; + u32 current_power_source: 2; + u32 reserved1: 1; + u32 d0i3_support_valid: 1; + } __packed fields; +}; + void dump_me_status(void *unused); #endif /* _CANNONLAKE_ME_H_ */ diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index 4b9d1a4..440b59b 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -24,6 +24,7 @@ #include <intelblocks/cse.h> #include <soc/iomap.h> #include <soc/pci_devs.h> +#include <soc/me.h> #include <string.h> #include <timer.h> @@ -238,17 +239,35 @@ return csr & CSR_READY; } -/* - * Checks if CSE is in ME_HFS1_COM_SECOVER_MEI_MSG operation mode. This is the mode where - * CSE will allow reflashing of CSE region. - */ -static uint8_t check_cse_sec_override_mode(void) +static bool cse_check_hfs1_com(int mode) { union me_hfsts1 hfs1; hfs1.data = me_read_config32(PCI_ME_HFSTS1); - if (hfs1.fields.operation_mode == ME_HFS1_COM_SECOVER_MEI_MSG) - return 1; - return 0; + return hfs1.fields.operation_mode == mode; +} + +bool cse_is_hfs1_cws_normal(void) +{ + union me_hfsts1 hfs1; + hfs1.data = me_read_config32(PCI_ME_HFSTS1); + if (hfs1.fields.working_state == ME_HFS1_CWS_NORMAL) + return true; + return false; +} + +bool cse_is_hfs1_com_normal(void) +{ + return cse_check_hfs1_com(ME_HFS1_COM_NORMAL); +} + +bool cse_is_hfs1_com_secover_mei_msg(void) +{ + return cse_check_hfs1_com(ME_HFS1_COM_SECOVER_MEI_MSG); +} + +bool cse_is_hfs1_com_soft_temp_disable(void) +{ + return cse_check_hfs1_com(ME_HFS1_COM_SOFT_TEMP_DISABLE); } /* Makes the host ready to communicate with CSE */ @@ -266,7 +285,7 @@ { struct stopwatch sw; stopwatch_init_msecs_expire(&sw, HECI_DELAY_READY); - while (!check_cse_sec_override_mode()) { + while (!cse_is_hfs1_com_secover_mei_msg()) { udelay(HECI_DELAY); if (stopwatch_expired(&sw)) return 0; @@ -632,18 +651,15 @@ struct hmrfpo_enable_resp resp; size_t resp_size = sizeof(struct hmrfpo_enable_resp); - union me_hfsts1 hfs1; printk(BIOS_DEBUG, "HECI: Send HMRFPO Enable Command\n"); - hfs1.data = me_read_config32(PCI_ME_HFSTS1); /* * This command can be run only if: * - Working state is normal and * - Operation mode is normal or temporary disable mode. */ - if (hfs1.fields.working_state != ME_HFS1_CWS_NORMAL || - (hfs1.fields.operation_mode != ME_HFS1_COM_NORMAL && - hfs1.fields.operation_mode != ME_HFS1_COM_SOFT_TEMP_DISABLE)) { + if (!cse_is_hfs1_cws_normal() || + (!cse_is_hfs1_com_normal() && !cse_is_hfs1_com_soft_temp_disable())) { printk(BIOS_ERR, "HECI: ME not in required Mode\n"); goto failed; } diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index 515f1a4..4673070 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -51,30 +51,6 @@ PCI_ME_HFSTS6 = 0x6C, }; -/* ME Host Firmware Status register 1 */ -union me_hfsts1 { - u32 data; - struct { - u32 working_state: 4; - u32 mfg_mode: 1; - u32 fpt_bad: 1; - u32 operation_state: 3; - u32 fw_init_complete: 1; - u32 ft_bup_ld_flr: 1; - u32 update_in_progress: 1; - u32 error_code: 4; - u32 operation_mode: 4; - u32 reset_count: 4; - u32 boot_options_present: 1; - u32 reserved1: 1; - u32 bist_test_state: 1; - u32 bist_reset_request: 1; - u32 current_power_source: 2; - u32 d3_support_valid: 1; - u32 d0i3_support_valid: 1; - } __packed fields; -}; - /* HECI Message Header */ struct mkhi_hdr { uint8_t group_id; @@ -172,4 +148,28 @@ #define MKHI_HMRFPO_LOCKED 1 #define MKHI_HMRFPO_ENABLED 2 +/* + * Checks current working operation state is normal or not. + * Returns true if CSE's current working state is normal, otherwise false. + */ +bool cse_is_hfs1_cws_normal(void); + +/* + * Checks CSE's current operation mode is normal or not. + * Returns true if CSE's current operation mode is normal, otherwise false. + */ +bool cse_is_hfs1_com_normal(void); + +/* + * Checks CSE's current operation mode is SECOVER_MEI_MSG or not. + * Returns true if CSE's current operation mode is SECOVER_MEI_MSG, otherwise false. + */ +bool cse_is_hfs1_com_secover_mei_msg(void); + +/* + * Checks CSE's current operation mode is Soft Disable Mode or not. + * Returns true if CSE's current operation mode is Soft Disable Mode, otherwise false. + */ +bool cse_is_hfs1_com_soft_temp_disable(void); + #endif // SOC_INTEL_COMMON_CSE_H diff --git a/src/soc/intel/icelake/include/soc/me.h b/src/soc/intel/icelake/include/soc/me.h new file mode 100644 index 0000000..b1646a2 --- /dev/null +++ b/src/soc/intel/icelake/include/soc/me.h @@ -0,0 +1,43 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _ICELAKE_ME_H_ +#define _ICELAKE_ME_H_ + +/* ME Host Firmware Status register 1 */ +union me_hfsts1 { + u32 data; + struct { + u32 working_state: 4; + u32 mfg_mode: 1; + u32 fpt_bad: 1; + u32 operation_state: 3; + u32 fw_init_complete: 1; + u32 ft_bup_ld_flr: 1; + u32 update_in_progress: 1; + u32 error_code: 4; + u32 operation_mode: 4; + u32 reset_count: 4; + u32 boot_options_present: 1; + u32 reserved1: 1; + u32 bist_test_state: 1; + u32 bist_reset_request: 1; + u32 current_power_source: 2; + u32 reserved: 1; + u32 d0i3_support_valid: 1; + } __packed fields; +}; + +#endif /* _ICELAKE_ME_H_ */ diff --git a/src/soc/intel/skylake/include/soc/me.h b/src/soc/intel/skylake/include/soc/me.h index c1fdc81..30de219 100644 --- a/src/soc/intel/skylake/include/soc/me.h +++ b/src/soc/intel/skylake/include/soc/me.h @@ -123,6 +123,30 @@ #define ME_HFS2_PMEVENT_CM3_CM3PG 0xe #define ME_HFS2_PMEVENT_CM0PG_CM0 0xf +/* ME Host Firmware Status register 1 */ +union me_hfsts1 { + u32 data; + struct { + u32 working_state: 4; + u32 mfg_mode: 1; + u32 fpt_bad: 1; + u32 operation_state: 3; + u32 fw_init_complete: 1; + u32 ft_bup_ld_flr: 1; + u32 update_in_progress: 1; + u32 error_code: 4; + u32 operation_mode: 4; + u32 reset_count: 4; + u32 boot_options_present: 1; + u32 reserved1: 1; + u32 bist_test_state: 1; + u32 bist_reset_request: 1; + u32 current_power_source: 2; + u32 d3_support_valid: 1; + u32 d0i3_support_valid: 1; + } __packed fields; +}; + union me_hfs2 { u32 data; struct { diff --git a/src/soc/intel/tigerlake/include/soc/me.h b/src/soc/intel/tigerlake/include/soc/me.h new file mode 100644 index 0000000..3baa004 --- /dev/null +++ b/src/soc/intel/tigerlake/include/soc/me.h @@ -0,0 +1,43 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2020 Intel Corporation. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _TIGERLAKE_ME_H_ +#define _TIGERLAKE_ME_H_ + +/* ME Host Firmware Status register 1 */ +union me_hfsts1 { + u32 data; + struct { + u32 working_state: 4; + u32 spi_protection_mode: 1; + u32 fpt_bad: 1; + u32 operation_state: 3; + u32 fw_init_complete: 1; + u32 ft_bup_ld_flr: 1; + u32 update_in_progress: 1; + u32 error_code: 4; + u32 operation_mode: 4; + u32 reset_count: 4; + u32 boot_options_present: 1; + u32 invoke_enhance_dbg_mode: 1; + u32 bist_test_state: 1; + u32 bist_reset_request: 1; + u32 current_power_source: 2; + u32 reserved: 1; + u32 d0i3_support_valid: 1; + } __packed fields; +}; + +#endif /* _TIGERLAKE_ME_H_ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/35546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Gerrit-Change-Number: 35546 Gerrit-PatchSet: 29 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <riz.pro(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: cpu/x86/smm: Add overflow check
by Nico Huber (Code Review) 09 Feb '20

09 Feb '20

Hello Kyösti Mälkki, Aaron Durbin, Arthur Heymans, cedarhouse1(a)comcast.net, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/38763 to review the following change. Change subject: cpu/x86/smm: Add overflow check ...................................................................... cpu/x86/smm: Add overflow check Rather bail out than run into undefined behavior. Change-Id: Ife26a0abed0ce6bcafe1e7cd8f499618631c4df4 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/cpu/x86/smm/smm_module_loader.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/38763/1 diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index a421436..81020a4 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -202,6 +202,8 @@ /* Adjust remaining size to account for save state. */ total_save_state_size = params->per_cpu_save_state_size * params->num_concurrent_save_states; + if (total_save_state_size > size) + return -1; size -= total_save_state_size; /* The save state size encroached over the first SMM entry point. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/38763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ife26a0abed0ce6bcafe1e7cd8f499618631c4df4 Gerrit-Change-Number: 38763 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: cedarhouse1(a)comcast.net Gerrit-MessageType: newchange

6 7

Change in coreboot[master]: soc/intel/common/block/cse: Add boot partition related APIs
by Sridhar Siricilla (Code Review) 09 Feb '20

09 Feb '20

Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, V Sowmya, Nico Huber, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/35402 to look at the new patch set (#61). Change subject: soc/intel/common/block/cse: Add boot partition related APIs ...................................................................... soc/intel/common/block/cse: Add boot partition related APIs The CSE region is logically divided into 3 boot partitions when redundancy is enabled. These boot partitions are represented by BP1, BP2 and BP3. In chrome platforms, CSE can boot from either BP1 or BP2. The CSE image layout appears as below.. ------------- -------------------- -------------------------- |CSE REGION | => | RO | RW | DATA | => | BP1 | BP2 + BP3 | DATA | ------------- -------------------- -------------------------- In order to support CSE FW update to RW region, below APIs help coreboot to get info about the boot partitions, and allows coreboot to set CSE to boot from required boot partition (either BP1(RO) or BP2). GET_BOOT_PARTITION_INFO - provides info on available partitions in the CSE region. The API provides info on boot partitions like start/end offsets of a partition within CSE region, and their version and partition status. SET_BOOT_PARTITION_INFO - Sets next boot partition to boot for CSE. With the HECI API, firmware can notify CSE to boot from BP1 or BP2 on next boot. BUG=b:145809764 Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/cse/Makefile.inc A src/soc/intel/common/block/cse/cse_bp.c M src/soc/intel/common/block/include/intelblocks/cse.h 3 files changed, 505 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/35402/61 -- To view, visit https://review.coreboot.org/c/coreboot/+/35402 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Gerrit-Change-Number: 35402 Gerrit-PatchSet: 61 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: superio/nuvoton/nct5539d/acpi: fix # comment in superio.asl
by Pavel Sayekat (Code Review) 09 Feb '20

09 Feb '20

Pavel Sayekat has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38782 ) Change subject: superio/nuvoton/nct5539d/acpi: fix # comment in superio.asl ...................................................................... superio/nuvoton/nct5539d/acpi: fix # comment in superio.asl Change-Id: Ic2ba1f9b744014f97d318671bf86468f4d6c6469 Signed-off-by: Pavel Sayekat <pavelsayekat(a)gmail.com> --- M src/superio/nuvoton/nct5539d/acpi/superio.asl 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/38782/1 diff --git a/src/superio/nuvoton/nct5539d/acpi/superio.asl b/src/superio/nuvoton/nct5539d/acpi/superio.asl index 70a84a2..73bc938 100644 --- a/src/superio/nuvoton/nct5539d/acpi/superio.asl +++ b/src/superio/nuvoton/nct5539d/acpi/superio.asl @@ -1,7 +1,8 @@ +/* # SPDX-License-Identifier: GPL-2.0-only # This file is part of the coreboot project. -/* + * Include this file into a mainboard's DSDT _SB device tree and it will * expose the NCT5539D SuperIO and some of its functionality. * -- To view, visit https://review.coreboot.org/c/coreboot/+/38782 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic2ba1f9b744014f97d318671bf86468f4d6c6469 Gerrit-Change-Number: 38782 Gerrit-PatchSet: 1 Gerrit-Owner: Pavel Sayekat Gerrit-MessageType: newchange

5 14

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020