Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36418 )
Change subject: [NOT FOR MERGE]: Demo adding near_reset_vector symbol
......................................................................
[NOT FOR MERGE]: Demo adding near_reset_vector symbol
In https://review.coreboot.org/c/coreboot/+/35035/8/src/arch/x86/early_dram.ld
we had a discussion about whether an approach like for RESET_VECTOR_IN_RAM
could potentially be used for a bootblock that needed to grow >64KB (to keep
the first 16-bit jmp within reach).
This is a mockup of the test I ran to verify that would be practical. Using
this code I built a google/grunt and recorded the following with readelf.
Section Headers:
[Nr] Name Type Addr Off Size ES Flg Lk Inf Al
[ 0] NULL 00000000 000000 000000 00 0 0 0
[ 1] .text PROGBITS ffff0000 000060 0059d0 00 AX 0 0 32
[ 2] .rel.text REL 00000000 01335c 001378 08 I 10 1 4
[ 3] .car.data NOBITS 00030000 000000 005860 00 WA 0 0 4
[ 4] .near_reset_vecto PROGBITS fffffe00 00fe60 0000bb 00 AX 0 0 4
[ 5] .rel.near_reset_v REL 00000000 0146d4 000040 08 I 10 4 4
[ 6] .reset PROGBITS fffffff0 010050 000010 00 AX 0 0 1
[ 7] .rel.reset REL 00000000 014714 000008 08 I 10 6 4
[ 8] .id PROGBITS ffffff4a 00ffaa 000036 00 A 0 0 1
[ 9] .gnu_debuglink PROGBITS 00000000 010060 000014 00 0 0 4
[10] .symtab SYMTAB 00000000 010074 001a80 10 11 243 4
[11] .strtab STRTAB 00000000 011af4 001868 00 0 0 1
[12] .shstrtab STRTAB 00000000 01471c 000064 00 0 0 1
Note, however, that I did not grow the bootblock.elf file; only inserted the
section and relocated the _start16bit code.
fffffe00 <_start16bit>:
fffffe00: fa cli
fffffe01: 66 89 c5 mov %ax,%bp
fffffe04: b0 01 mov $0x1,%al
fffffe06: e6 80 out %al,$0x80
fffffe08: 66 31 c0 xor %ax,%ax
fffffe0b: 0f 22 d8 mov %eax,%cr3
fffffe0e: 8c c8 mov %cs,%eax
fffffe10: c1 e0 04 shl $0x4,%eax
fffffe13: bb 4c fe 29 c3 mov $0xc329fe4c,%ebx
fffffe18: 2e 0f 01 1f lidtl %cs:(%edi)
fffffe1c: bb 44 fe 29 c3 mov $0xc329fe44,%ebx
fffffe21: 2e 66 0f 01 17 lgdtw %cs:(%edi)
fffffe26: 0f 20 c0 mov %cr0,%eax
fffffe29: 66 25 d1 ff and $0xffd1,%ax
fffffe2d: fa cli
fffffe2e: 7f 66 jg fffffe96 <__protected_start+0xb>
fffffe30: 0d 01 00 00 60 or $0x60000001,%eax
fffffe35: 0f 22 c0 mov %eax,%cr0
fffffe38: 66 89 e8 mov %bp,%ax
fffffe3b: 66 ea 8b fe ff ff ljmpw $0xffff,$0xfe8b
fffffe41: 08 00 or %al,(%eax)
fffffe43: 90 nop
Growing the elf should be reasonably straightforward. Right now reset16.ld
has a built-in assumption of 64KB. For RESET_VECTOR_IN_RAM I've added a
size symbol to Kconfig (yet to land still) for locating the reset vector
and near_reset_vector at the very top, and so that the calculations are
expected to match the instructions given to the PSP.
Change-Id: I6e7888c778e1d1cc426e4160543f4a4662ebf834
Signed-off-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
---
M src/cpu/x86/16bit/entry16.inc
M src/cpu/x86/16bit/reset16.ld
2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/36418/1
diff --git a/src/cpu/x86/16bit/entry16.inc b/src/cpu/x86/16bit/entry16.inc
index 9e00c55..6062abd 100644
--- a/src/cpu/x86/16bit/entry16.inc
+++ b/src/cpu/x86/16bit/entry16.inc
@@ -37,6 +37,9 @@
.align 4096
#endif
.code16
+
+.section ".near_reset_vector", "ax", %progbits
+
.globl _start16bit
.type _start16bit, @function
diff --git a/src/cpu/x86/16bit/reset16.ld b/src/cpu/x86/16bit/reset16.ld
index c57cc96..31716e0 100644
--- a/src/cpu/x86/16bit/reset16.ld
+++ b/src/cpu/x86/16bit/reset16.ld
@@ -17,6 +17,14 @@
*/
SECTIONS {
+ _ROMTOP = 0xfffffff0;
+ _NEAR_RESET_VECTOR = _ROMTOP + 0x10 - 0x200;
+
+ . = _NEAR_RESET_VECTOR;
+ .near_reset_vector . : {
+ *(.near_reset_vector);
+ }
+
/* Trigger an error if I have an unuseable start address */
_bogus = ASSERT(_start16bit >= 0xffff0000, "_start16bit too low. Please report.");
_ROMTOP = 0xfffffff0;
--
To view, visit https://review.coreboot.org/c/coreboot/+/36418
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6e7888c778e1d1cc426e4160543f4a4662ebf834
Gerrit-Change-Number: 36418
Gerrit-PatchSet: 1
Gerrit-Owner: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Hello Matt DeVillier, Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/39031
to review the following change.
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS
......................................................................
cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS
Make the variable override for CPU_MICROCODE_CBFS_EXTERNAL_BINS local to
the target. Otherwise, `cpu_microcode_bin +=` lines that are evaluated
after `src/cpu/Makefile.inc` still append to it.
Change-Id: If81f307afc325ff3c1e987e9483ed5e45fdc403e
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M src/cpu/Makefile.inc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/39031/1
diff --git a/src/cpu/Makefile.inc b/src/cpu/Makefile.inc
index 0289be0..92e47aa 100644
--- a/src/cpu/Makefile.inc
+++ b/src/cpu/Makefile.inc
@@ -28,7 +28,7 @@
endif
ifeq ($(CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS),y)
-cpu_microcode_bins := $(call strip_quotes,$(CONFIG_CPU_UCODE_BINARIES))
+$(obj)/cpu_microcode_blob.bin: cpu_microcode_bins := $(call strip_quotes,$(CONFIG_CPU_UCODE_BINARIES))
endif
# otherwise `cpu_microcode_bins` should be filled by platform makefiles
--
To view, visit https://review.coreboot.org/c/coreboot/+/39031
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If81f307afc325ff3c1e987e9483ed5e45fdc403e
Gerrit-Change-Number: 39031
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newchange
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39259 )
Change subject: soc/intel/common/block/tco: clear TCO1_STS register, too
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39259/2//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/39259/2//COMMIT_MSG@16
PS2, Line 16: the SMI handler by unconditionally (re)setting it.
I don't see how your change is different than previous code in this regard. Could you elaborate? Was it related to tco1_sts?
--
To view, visit https://review.coreboot.org/c/coreboot/+/39259
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia57c203a672fdd0095355a7e2a0e01aaa6657968
Gerrit-Change-Number: 39259
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <anpetrov(a)fb.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.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: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 04 Mar 2020 21:00:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39264 )
Change subject: soc/intel/common/block: tco: enable intruder SMI
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39264/5/src/soc/intel/common/block…
File src/soc/intel/common/block/smbus/tco.c:
https://review.coreboot.org/c/coreboot/+/39264/5/src/soc/intel/common/block…
PS5, Line 105: /* Make TCO issue an SMI on INTRD_DET assertion */
This should be driven by config policy as well -- falls under the broader TCO SMI option.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39264
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3bc62c79ca3dc9e8896d9e2b9abdc14cfa46a9e7
Gerrit-Change-Number: 39264
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 04 Mar 2020 20:53:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39262 )
Change subject: intel/soc: skl,apl,cnl,icl,tgl,common: enable TCO SMIs
......................................................................
Patch Set 5:
> Patch Set 5:
>
> > Patch Set 4:
> >
> > > Patch Set 4:
> > >
> > > > Patch Set 4: Code-Review-1
> > > >
> > > > This should be made into an option. From a Chrome OS perspective we do not want to take SMIs for these events. It leads to having more complex handlers and the associated policy with them.
> > >
> > > What is Chrome OS using instead?
> >
> > We don't use TCO SMIs for anything. For intruder specifically we don't currently plumb anything up. For example, I don't want to take an SMI for a TCO timer expiration -- just want a reset on double expiration. Maybe we will in the future, but I don't want to enable TCO SMIs by default.
>
> Hm, but why can't you just ignore the fact that maybe SMI are raised? It doesn't interfere with anything, does it? Currently there is no functionality implemented but it's just a stub to make that possible.
>
> Don't get me wrong, I'm not against having an option for that. I just want to understand how that could affect Chrome OS or others.
I don't like taking unnecessary SMIs as it steals cycles and it adds more attack surface area.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39262
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If63effe74ac59b5d051a6454bc6375bb89605215
Gerrit-Change-Number: 39262
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 20:45:51 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39265 )
Change subject: soc/intel/common/block/smm: add case intrusion to SMI handler
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39265/5/src/soc/intel/common/block…
File src/soc/intel/common/block/smm/smihandler.c:
https://review.coreboot.org/c/coreboot/+/39265/5/src/soc/intel/common/block…
PS5, Line 444: if (tco_sts & (TCO_INTRD_DET << 16)) { /* INTRUDER# assertion */
Shouldn't all these events have optional callback instead of just printing things?
--
To view, visit https://review.coreboot.org/c/coreboot/+/39265
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifad675bb09215ada760efebdcd915958febf5778
Gerrit-Change-Number: 39265
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 04 Mar 2020 20:43:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39265 )
Change subject: soc/intel/common/block/smm: add case intrusion to SMI handler
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39265/4/src/soc/intel/common/block…
File src/soc/intel/common/block/smm/smihandler.c:
https://review.coreboot.org/c/coreboot/+/39265/4/src/soc/intel/common/block…
PS4, Line 446: printk(BIOS_CRIT, "Intrusion detected.\n");
> well, the function is named smihandler_... […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/39265
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifad675bb09215ada760efebdcd915958febf5778
Gerrit-Change-Number: 39265
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 04 Mar 2020 19:37:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Michael Niewöhner
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39262 )
Change subject: intel/soc: skl,apl,cnl,icl,tgl,common: enable TCO SMIs
......................................................................
Patch Set 5:
> Patch Set 4:
>
> > Patch Set 4:
> >
> > > Patch Set 4: Code-Review-1
> > >
> > > This should be made into an option. From a Chrome OS perspective we do not want to take SMIs for these events. It leads to having more complex handlers and the associated policy with them.
> >
> > What is Chrome OS using instead?
>
> We don't use TCO SMIs for anything. For intruder specifically we don't currently plumb anything up. For example, I don't want to take an SMI for a TCO timer expiration -- just want a reset on double expiration. Maybe we will in the future, but I don't want to enable TCO SMIs by default.
Hm, but why can't you just ignore the fact that maybe SMI are raised? It doesn't interfere with anything, does it? Currently there is no functionality implemented but it's just a stub to make that possible.
Don't get me wrong, I'm not against having an option for that. I just want to understand how that could affect Chrome OS or others.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39262
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If63effe74ac59b5d051a6454bc6375bb89605215
Gerrit-Change-Number: 39262
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Comment-Date: Wed, 04 Mar 2020 19:36:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment