Skip to content

head.S: move skl_info and bootloader_data fields back - #25

Open
m-iwanicki wants to merge 14 commits into
skl-loader-amdsl-noblobfrom
skl-loader-amdsl-noblob-sl_header-fix
Open

m-iwanicki wants to merge 14 commits into
skl-loader-amdsl-noblobfrom
skl-loader-amdsl-noblob-sl_header-fix

Conversation

@m-iwanicki

@m-iwanicki m-iwanicki commented Feb 19, 2026 •

Copy link
Copy Markdown

Fixes issue:

loader/slaunch/skl.c:117:slaunch: Possible SKL module measures bootloader data:
16424 (measured prefix) > 170 (data offset)
error: SKL module isn't correct.

Tested with tb-dev GRUB (meta-trenchboot)


More patches went in so they can be used in meta-trenchboot

Fixes issue:

```
loader/slaunch/skl.c:117:slaunch: Possible SKL module measures bootloader data:
16424 (measured prefix) > 170 (data offset)
error: SKL module isn't correct.
```

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
@krystian-hebel

Copy link
Copy Markdown
Member

I think this should be changed on GRUB side instead. The size of soc_flag field is out of our control, and may change without notice. By having skl_info and bootloader_data under known offsets, GRUB won't have to be aware of any changes made by AMD.

@SergiiDmytruk

Copy link
Copy Markdown
Member

I agree that updating this in GRUB a better place in the long run. Can use this modification for testing though since commits are WIP anyway and it should be easier than bumping GRUB.

@krystian-hebel

Copy link
Copy Markdown
Member

By the way, it may be useful to try to change those (presumed) CPUIDs to match the CPU used for testing. I didn't find any documentation as to what those are used by, it may be possible that PSP validates those somehow, but maybe they were meant for whatever loads the binary (GRUB in our case).

@m-iwanicki

Copy link
Copy Markdown
Author

it may be useful to try to change those (presumed) CPUIDs to match the CPU used for testing. I didn't find any documentation as to what those are used by, it may be possible that PSP validates those somehow

Changed when testing on MZ33-AR1 (AMD EPYC 9015). CPUID:

>>> import cpuid
>>> eax, *_ = cpuid.cpuid(1)
>>> f"{eax:#010x}"
'0x00b00f21'

I tested with 0x00b00f20 and 0x00b00f00. Results are the same as in TrenchBoot/trenchboot-issues#79 (comment). Maybe those are validated later if at all.

iwanicki92 and others added 13 commits September 21, 2026 23:29
Under the PSP DRTM service the extends of PCR 17 and 18 between SKINIT
and the DLME are the service's, kept in a log of its own, so the log the
DLME found could not replay to the PCRs. The AMDSL SKL now fetches that
log with DRTM_CMD_GET_TCG_LOGS after the OSSL extend and appends its
records, bar the SKINIT one it logged already, rewritten to the log's
banks: a digest the record carries for a bank is kept, a bank it lacks
gets the placeholder and the extend. Its log starts after the launch,
at the locality the service unlocks with it, so the TPM bank query
answers there.

This is the PSP half of "Log the TPM's banks and append the PSP's TCG
log". The bank half is #28.

Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
Signed-off-by: Michał Kopeć <michal.kopec@3mdeb.com>
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
convert_digest_list() steps over an SM3-256 digest by SM3256_SIZE, but
adds SHA1_SIZE to the size it returns. A TPM2_PCR_Extend with an SM3-256
digest is therefore built 12 bytes short, and the TPM fails it. Add
SM3256_SIZE instead.

Nothing extends an SM3-256 bank yet. The PCR bank support that follows
extends a placeholder digest into such a bank.

Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
.bss starts on the first page boundary after .skinit_reserved. Its
page-aligned data comes first: the page tables, the event log and the
IOMMU device table, 40 KiB in the 64-bit build. The small .bss variables
follow, about 1.2 KiB, and then bootloader_data. GRUB puts a 4 KiB SLRT
at bootloader_data and refuses an SKL that leaves less room before the
64 KiB end.

In a 64-bit DEBUG build, .skinit_reserved ends 0x380 bytes below 0x4000.
Once it crosses 0x4000, .bss starts at 0x5000, and the small variables
push bootloader_data past 0xf000.

Link the small variables into a section of their own before .bss, in the
space that the page alignment of .bss leaves free. The code can then
grow until it and the small variables reach 0x5000, about 2.8 KiB more.
The cost is room after bootloader_data. A 64-bit DEBUG build puts it at
0xf000, with exactly 4 KiB left instead of 6.8 KiB. An assertion fails
the link when less than 4 KiB is left.

clang with LTO puts each variable in a section named after it, such as
.bss.tis_buff. The page-aligned data moves to .bss..page_data, so the new
section can take every .bss.<name> section but that one.

The small .bss would share a LOAD segment with the code, and ld 2.39 and
later warn about the RWX segment, which --fatal-warnings turns into an
error. A maximum page size of 16 bytes puts them in separate segments.
skl.bin is a flat binary, so the segments do not change it.

Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
The event log declares SHA-1 and SHA-256 whatever banks the TPM has.
Linux resets on a log whose banks differ from the TPM's banks, because
its extend needs one digest per allocated bank, in the TPM's order. So a
TPM with only SHA-256, or with SHA-384 as well, fails every launch.

Ask the TPM for its PCR allocation with TPM2_GetCapability, and declare
those banks in that order. Each record carries SHA-1 and SHA-256 where
the TPM has them. Other banks get a placeholder digest (01 followed by
zeros), as Linux and Xen log for such banks. For the records the SKL
measures itself, it extends the same placeholder into those banks, so
those records replay. The SKINIT record does not replay there, because
SKINIT extends PCR 17 with the real digest in every bank.

If the query fails, or finds no bank of an algorithm the SKL knows, the
log declares SHA-1 and SHA-256, the banks most discrete TPMs have
allocated by default. DEBUG builds print the query's return code. The
query fails in these cases:

- on a CRB TPM, because crb_recv() does not read the reply
- in the AMDSL build, which starts the log while the PSP keeps
  locality 2 locked
- on a TPM with more than five banks

A bank of an algorithm the SKL does not know is left out of the log.

test-event_log.c checks the header and records for two, one and three
banks and the fallback. Run it with "make run-test-event_log". It stubs
the GetCapability query, so it does not cover the reply parser.

Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
MSI PRO B850 (1022:15C7) - change from PSP_NONE to PSP_V3
MZ33-AR1 (1022:156E) - add device and set to ASP (and expand
                       psp_version_t enum)

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
pci_init() kept only the low 32 bits of MSR C001_0058. On a platform
that places the MMIO configuration space above 4 GiB the enable bit is
set and bits 31:20 are zero, so SKL selected the MMIO accessor with a
base of 0 and every configuration read returned low physical memory.

On a Gigabyte MZ33-AR1 (EPYC 9015, firmware R19_F13) the MSR reads
0x00003FFB00000021, which puts the configuration space at
0x3FFB00000000. discover_psp() enumerated device IDs that match no
hardware and get_psp_bar_addr() returned 0, so the PSP was never found.
GRUB, which uses 0xCF8/0xCFC on the same platform, finds the PSP and
its BAR.

Read the whole 64-bit MSR value, derive the region size from BusRange,
and use the MMIO accessor only when the region lies below 4 GiB. That
is the range SKL can address: the 32-bit build runs without paging and
the 64-bit build identity-maps the first 4 GiB. Otherwise use type 1
accesses, and print a note so the chosen path shows up in the log.

Fixes: TrenchBoot/trenchboot-issues#81
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
AMD's SKL signing flow rounds the measured length in the SL header up
to a multiple of 16 bytes and places the "$AS1" signature header at
that offset. header_tool.py does the rounding in Python, and
AmdSlSigningTool repeats it in place before hashing (SignPSS rounds the
byte at offset 2 up to the next 0x10 and carries into offset 3).

The openssl-signed image on this branch aligns _end_of_measured to
4 bytes only. The length field then ends 8 bytes past a 16-byte
boundary (0x4028 in the image tested on the Gigabyte MZ33-AR1, 0x40c8
with the toolchain here) and the signature header sits at an offset a
tool-signed image never has. That test ended in DRTM_LAUNCH_ERROR with
PCRs 18 to 20 capped, which is what the PSP does when SKL fails
authentication.

Align .rodata and _end_of_measured to 16 bytes so the signature header
follows the measured region at a multiple of 16, as in the tool output.

Discard toolchain .note sections at link time. ld places orphan notes
right after .rodata, which puts them between the measured region and
the signature header. The Makefile already strips them from skl.bin, so
the flat binary keeps the same content and loses the gap.

Assert both properties so a layout drift fails the build.

The other fields the signing tool writes match the header emitted by
head.S: "$AS1", signed size equal to length + 0x100, SigOption 1, the
key ID, the size with signature at 0x6c, and the total length at
offset 0x18 of the SL header. The public key blob matches
testPublicKey.bin, and the signature over length + 0x100 bytes
verifies with a digest-length PSS salt, which is what the tool uses.

Related: TrenchBoot/trenchboot-issues#79
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
The PSP verifies the SKL signature with RSA-PSS, SHA-384 and a salt of
48 bytes, the hash length (AMD publication 58453, DRTM Service
Integration Guide, appendix G.1). AmdSlSigningTool signs the same way.

openssl dgst picks the salt length from its own default when none is
given. OpenSSL 3.1 and newer use the digest length, older releases use
the maximum salt the modulus allows, 462 bytes here. An image signed on
such a host carries a signature the PSP will not accept.

Pass rsa_pss_saltlen:digest so the signature matches the PSP on every
OpenSSL version.

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
The SOC flag structure names the CPU families and models the image is
built for, as CPUID leaf 1 EAX with the low byte cleared. The list ends
at family 19h. The Gigabyte MZ33-AR1 carries an EPYC 9015, family 1Ah
model 02h, CPUID 0x00B00F21, and its ASP answers DRTM_CMD_LAUNCH with
DRTM_LAUNCH_ERROR for an image whose header, signature and key token
otherwise match AMD's signing tool output. The list is the one field
left that does not describe that CPU.

The structure has to keep eight entries. GRUB reads skl_info_offset and
bootloader_data_offset from fixed offsets 0x42 and 0x44, and boot.h
declares the same layout, so a ninth entry would move both fields.
Family 1Ah model 00h replaces family 17h model 60h, the oldest entry.

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
The DRTM service opens locality 3 to read and extend PCR 17 during the
launch command. TIS grants a higher locality only while no lower one is
active, so the SKL relinquishes its locality before issuing the command
and requests locality 2 again afterwards. The service keeps localities 1
and 2 locked until the launch succeeds, which is why PCR reads at
locality 2 returned -EAGAIN both before and after the command.

tis_request_locality() treats an all-ones TPM_ACCESS read as not granted.
Reserved bit 6 is set in that value, and it is what a locality the
chipset does not decode for the host returns.

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
tis_recv() reads all but the last byte of the response body into the
buffer and then reads the last byte into the first byte of that same
body, so the final byte of every response is whatever the buffer held
before. A PCR read on the MZ33-AR1 returned digests whose last byte was
zero for that reason. The last byte now lands after the bytes already
received.

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
The mailbox wait gives the PSP 50 retries of 100 ms for every command.
Hashing the DLME is the one command whose work scales with the payload,
and on the Gigabyte MZ33-AR1 a 7.6 MB Xen image ran past the five
seconds after a successful launch, so the SKL reported "command failed
to complete" and moved on. The extend now waits up to 1200 retries. The
other commands keep the short wait.

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
tis_recv() waits TIMEOUT_D once after STS_GO, and gives up if no
response is there by then. TIMEOUT_D is 30 ms on TPM 2.0 and 750 ms on
TPM 1.2. It bounds a register access, not a command, and commands take
longer. Linux's TPM driver allows 750 ms for TPM2_GetCapability and
TPM2_PCR_Extend (TPM2_DURATION_MEDIUM), and 2 s for slower commands
such as TPM2_GetRandom (TPM2_DURATION_LONG).

Poll dataAvail every millisecond for up to 2 s instead, which covers
both, on both TPM families. This lengthens the TPM 1.2 wait from 750 ms
to 2 s. It prepares for a PCR bank query with TPM2_GetCapability, whose
response can take longer than 30 ms. The family argument is no longer
used and is marked unused, as in crb_recv().

Signed-off-by: iwanicki92 <iwanicki92@gmail.com>
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
mkopec pushed a commit to zarhus/meta-trenchboot that referenced this pull request Nov 4, 2026
skl-loader-amdsl-noblob-sl_header-fix is skl-loader-amdsl-noblob with
TrenchBoot/secure-kernel-loader#25, which moves the skl_info and
bootloader_data fields back so GRUB no longer rejects the SKL module
for measuring bootloader data, and the AMD PSP fixes from the MSI PRO
B850 and Gigabyte MZ33-AR1 bring-up:

- The MSI PRO B850 and MZ33-AR1 PSPs are listed with their versions.
- PCI configuration space is read with type 1 accesses when the MMIO
  window lies above 4G.
- The measured length is aligned to 16 bytes and the image is signed
  with a PSS salt of the digest length, as the PSP expects.
- Family 1Ah is listed in the SOC flags.
- The SKL holds no TPM locality during DRTM_CMD_LAUNCH, treats an
  all-ones TPM_ACCESS read as not granted and stores the last byte of a
  TIS response at the end of the buffer.
- The SKL waits up to two minutes for DRTM_CMD_EXTEND_OSSL_DIGEST.

Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
Signed-off-by: Michał Kopeć <michal.kopec@3mdeb.com>
@iwanicki92

iwanicki92 commented Nov 4, 2026 •

Copy link
Copy Markdown

By the way, it may be useful to try to change those (presumed) CPUIDs to match the CPU used for testing. I didn't find any documentation as to what those are used by, it may be possible that PSP validates those somehow, but maybe they were meant for whatever loads the binary (GRUB in our case).

@krystian-hebel

These values most likely come from AMD's own Secure Loader. The list at 60df73b, before f4c7714 in this PR swaps 860f00 for b00f00, is byte-identical to the one in the SL inside amddrtm.dll from the AMD EPYC DRTM Boot Driver 1.10.2.0 (Lenovo package sysboard_chipset.tsv3-amddrtm-1.10.2.0-006.zip). It has the same 8 entries in the same unsorted order. AMD's SLs place the list right after the offset field, at 0x1e, as head.S in this PR does. Other driver versions carry different lists: 1.10.4.9 has a00f00 a10f00 a80f00 aa0f00 b00f00 b10f00. The commit that added the values (4d6c8d1) also calls them "AMD-supplied".

On what uses them: AMD's Windows driver does, which fits your guess about whatever loads the binary. The driver is amddrtm.dll, which holds the SL and issues SKINIT itself, much like GRUB does for SKL. It reads the list through the offset at header + 0x1c and compares each entry with CPUID 0x80000001 EAX & 0x0FFF0F00, and it stops with an error if none match. That mask drops the base model as well as the stepping, so each entry covers 16 models.

AMD's SL also holds the same CPUs in its own code. The SL talks to the PSP through the DRTM mailbox, and it looks up the mailbox offset (0x10a20) by CPU. I checked this in the 1.10.4.9 SL, the one whose list is quoted above: its lookup checks for exactly the 6 CPUs in its header. On any other CPU the lookup finds no offset, so the SL polls a wrong address until it gives up and resets. So this looks like the list of CPUs the SL supports.

The PSP most likely doesn't verify it, at least where the firmware has no SL key allowlist. I launched an SL of my own with 0 in that offset field, so with no list at all, and DRTM_CMD_LAUNCH accepted it. This can't be tested on firmware with an allowlist: it takes only SLs signed with a listed key, and their signature covers the list.

@krystian-hebel

Copy link
Copy Markdown
Member

It reads the list through the offset at header + 0x1c

So, unless I misunderstood, it should be safe to move this elsewhere, and make room for bootloader_data and skl_info at constant offsets regardless of size of list, right? Of course it would have to be handled in the similar way on GRUB side and in boot.h, but I think this would be easier to maintain int he long run than limiting the number of supported families.

@iwanicki92

Copy link
Copy Markdown

Yes, as far as I can tell, with one gap. The driver only follows the offset: it adds the 16-bit value at header + 0x1c to the header's address and reads the count and entries from there. I checked this in 1.10.2.0, and there is no hardcoded 0x1e and no check on the value. It maps 64 KiB of the image, so the list has to stay inside that. GRUB loads SKL rather than this driver, so the driver wouldn't see this list on our path anyway.

The gap is the PSP. It accepted an SL with no list at all, and SLs with a list at 0x1e that holds the running CPU. I haven't launched one with a list somewhere other than 0x1e, or one whose list leaves out the running CPU, so I don't know how the PSP treats those (there are a lot of different PSP/DRTM TA versions, and each might differ).

4d6c8d1 also says SKINIT fails without these header values, which doesn't match what I saw. My SL sets only the entry point and length, with the rest of the header zeroed up to bootloader_data, so it has no GUID, no offset and no list. SKINIT still launched it, and PCR 17 matched its measurement. That commit adds all the values at once, so it doesn't show which of them mattered. It may also depend on the platform, so the best way to find out is to ask the commit's author which platform it was tested on.

head.S on this PR's base branch, skl-loader-amdsl-noblob, already keeps those two fields at constant offsets: skl_info is at 0x1e and bootloader_data at 0x20, both before the list at 0x22, so the list can grow without moving them. GRUB and boot.h could use those offsets instead of 0x42 and 0x44.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants