Patch Set 7:

(1 comment)

Patch Set 7:

(1 comment)

Patch Set 3:

Patch Set 3:

Is the STM a fully-fledged payload now? If not, it should presumably continue to be located in security/intel/stm

The STM is not by the coreboot definition a payload.

This patch is intended to replace the blob located in 3rdparty/blobs/cpu/intel/stm with source that allows the STM to be built as part of the coreboot build.

The code in security/intel/stm currently takes the stm blob and moves it to the MSEG. It also initializes some data structures, etc. so that when the STM is started, it can find the SMI handler and the BIOS resource list.

Since the STM is no longer a blob, it seems that it should not be in its current location in blobs. Earlier, it was suggested that the STM build be located in the payloads directory, but that suggestion was some time ago.

I can see a couple of options for locating the STM build aside from payloads:

(1) Somewhere under 3rdparty, but not in blobs.
(2) under security/intel/stm

So, my question is: Where should the STM be located when it is built as part of coreboot?

Thanks for the explanation and background. I agree that 3rdparty/stm is a good place for the submodule.

Has this been tested yet? Build currently fails for me with:
`src/security/intel/stm/SmmStm.c: In function 'add_pi_resource':
src/security/intel/stm/SmmStm.c:480:21: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
480 | printk(BIOS_DEBUG, "STM: ResourceSize - 0x%08lx\n", resource_size);`

If this wasn't intended, this can be fixed by changing the format to `%x`.

Interesting, this was building and running on my Purism Laptop and builds okay under gerrit. I checked my coreboot console output to verify. However, I will fix this as suggested.

It was with GCC 10.2.1 on Fedora. I've seen the warning about using other toolchains, so, if this is operating as expected, I can drop the issue. However, it may be due to GCC 10 as a whole. I'll test coreboot's GCC 10 (CB:42251), but if so, I can bring this up over there.

View Change

1 comment:

To view, visit change 44687. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8c6e0c85edd4e2b0658791553bd9947656e8c796
Gerrit-Change-Number: 44687
Gerrit-PatchSet: 7
Gerrit-Owner: Eugene Myers <cedarhouse1@comcast.net>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00@gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie@chromium.org>
Gerrit-Reviewer: Eugene Myers <cedarhouse@comcast.net>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich@gmail.com>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Comment-Date: Wed, 09 Sep 2020 01:26:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Eugene Myers <cedarhouse1@comcast.net>
Comment-In-Reply-To: Benjamin Doron <benjamin.doron00@gmail.com>
Gerrit-MessageType: comment