Change in coreboot[master]: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms
Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms PBET timer has to be stopped before APs are launched and initialized. Otherwise the platform will reset. The PBET expiration time may be very low so stop timer as quickly as possible. The expiration time is defined in Boot Guard manifests. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com> Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d --- M src/cpu/x86/16bit/entry16.inc 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/43395/1 diff --git a/src/cpu/x86/16bit/entry16.inc b/src/cpu/x86/16bit/entry16.inc index babed02..c555c5c 100644 --- a/src/cpu/x86/16bit/entry16.inc +++ b/src/cpu/x86/16bit/entry16.inc @@ -29,6 +29,7 @@ #include <cpu/x86/post_code.h> #define BOOTGUARD_SACM_INFO 0x13a +#define BOOTGUARD_PBEC 0x139 /* Symbol _start16bit must be aligned to 4kB to start AP CPUs with * Startup IPI message without RAM. @@ -118,6 +119,14 @@ andl $0x7FFAFFD1, %ebx /* PG,AM,WP,NE,TS,EM,MP = 0 */ orl $0x60000001, %ebx /* CD, NW, PE = 1 */ #if CONFIG(INTEL_BOOTGUARD) + /* + * Stop PBET timer. It is recommended to stop the PBET timer + * regardless of Boot Guard status. + */ + movl $BOOTGUARD_PBEC, %ecx + movl $0, %edx + movl $1, %eax + wrmsr /* DO NOT disable cache if Intel BootGuard is supported */ movl $BOOTGUARD_SACM_INFO, %ecx -- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-MessageType: newchange
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... File src/cpu/x86/16bit/entry16.inc: https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... PS1, Line 129: wrmsr I wonder if you really need to do this in assembly. -- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-Comment-Date: Mon, 13 Jul 2020 09:20:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: Code-Review+1 (3 comments) https://review.coreboot.org/c/coreboot/+/43395/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/43395/1//COMMIT_MSG@7 PS1, Line 7: PBET timer nit: this is saying "Protect BIOS Environment Timer timer" https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... File src/cpu/x86/16bit/entry16.inc: https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... PS1, Line 32: #define BOOTGUARD_PBEC 0x139 I'd order the MSR definitions in numerically increasing order (0x139 < 0x13a) https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... PS1, Line 129: wrmsr
I wonder if you really need to do this in assembly. As coreboot may take an arbitrarily long time to execute (because debugging stuff and the like), it's better to do this really early.
-- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-Comment-Date: Mon, 13 Jul 2020 09:35:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... File src/cpu/x86/16bit/entry16.inc: https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... PS1, Line 129: wrmsr
I wonder if you really need to do this in assembly. FSP-M is supposed to have an option to stop PBET and the minimum expiration time is 60 seconds according the spec/ On the second hand whet PBET value is set to 0, the timer whouls not run, but still BIOS is responsible to write to this MSR. Due to this confusion I think it is better to stop it here.
-- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-Comment-Date: Mon, 13 Jul 2020 09:37:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-MessageType: comment
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... File src/cpu/x86/16bit/entry16.inc: https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... PS1, Line 129: wrmsr
FSP-M is supposed to have an option to stop PBET and the minimum expiration time is 60 seconds accor […] I am aware of BOOTBLOCK_DEBUG_SPINLOOP. Other than that, I was just surprised this is done before FSP-T, when apparently Intel design was to do this inside FSP-M. Unless Michal meant to say "option in FSP-T" above.
My suggestion would have been bootblock_soc_early_init() but I don't mind if this is done here. -- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-Comment-Date: Mon, 13 Jul 2020 10:00:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski@3mdeb.com> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... File src/cpu/x86/16bit/entry16.inc: https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... PS1, Line 129: wrmsr
I am aware of BOOTBLOCK_DEBUG_SPINLOOP. […] Unfotunately no, it is *FSP-M* that has an option to stop PBET.
-- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-Comment-Date: Mon, 13 Jul 2020 10:06:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski@3mdeb.com> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... File src/cpu/x86/16bit/entry16.inc: https://review.coreboot.org/c/coreboot/+/43395/1/src/cpu/x86/16bit/entry16.i... PS1, Line 123: recommended Please add a reference to the documentation. -- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 13 Jul 2020 10:40:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43395 ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/43395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 13 Jul 2020 12:04:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43395?usp=email ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/43395?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss@mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: abandon
Daniel Schaefer has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/43395?usp=email ) Change subject: cpu/x86/16bit/entry16.inc: Stop PBET timer on Boot Guard platforms ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: For future reference, looks like this got implemented in: https://review.coreboot.org/c/coreboot/+/36682 -- To view, visit https://review.coreboot.org/c/coreboot/+/43395?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I545a36d60597fe37a30b8207336ae7fa7831674d Gerrit-Change-Number: 43395 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski <michal.zygowski@3mdeb.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss@mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Daniel Schaefer <dhs@frame.work> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Comment-Date: Fri, 07 Aug 2026 21:14:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
participants (7)
-
Angel Pons (Code Review) -
Daniel Schaefer (Code Review)
-
Kyösti Mälkki (Code Review)
-
Michael Niewöhner (Code Review)
-
Michał Żygowski (Code Review)
-
Paul Menzel (Code Review)
-
Stefan Reinauer (Code Review)