Eugene Myers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44687 )
Change subject: 3rdparty: Add STM as a submodule ......................................................................
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.
https://review.coreboot.org/c/coreboot/+/44687/7/.gitmodules File .gitmodules:
https://review.coreboot.org/c/coreboot/+/44687/7/.gitmodules@56 PS7, Line 56: STM
"STM. […]
It takes either ../STM or ../STM.git. The coreboot STM repository page in its clone line uses 'STM' as below:
git clone "ssh://EugeneDMyers@review.coreboot.org:29418/STM"
I have no issue with adding 'git' but decided to go with what was on the repository page.