Drtm payload 25.12 by hanetzer · Pull Request #926 · Dasharo/coreboot

hanetzer · 2026-06-19T19:48:02Z

Replacement of #916, can't change my branch, just the target. Hoping to avoid losing the comments there. Sorry for the noise.

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

…r IOMMU Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

Move the GNB IOAPIC enabling to northbridge init where the IOAPIC is initialized. Remove the comment about IoapicSbFeatureEn, this bit is not touched here. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

… register Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

Introduce PCI_DEVID macro which allows to construct PCI device addresses or range with bus numbers. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

…e area Use the IORESOURCE_SOFT_RESERVE attribute to reserve CC6 save state DRAM. Using regualr reserved memory makes it hard on EDK2 to detect if it is MMIO or reserved DRAM, as TOM2 is equal to the base of the CC6 save state area, not its limit. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

Per family 16h models 30h-3fh BKDG the IoapicSbFeatureEn must be configured according to the interrupt routing mode selecte by OS. If OS chose APIC mode, the IoapicSbFeatureEn must be cleared. Otherwise, it must be set, meaning PIC mode is used. Add a hook to _PIC method to call SoC/northbridge specific code to set/clear the bit to configure GNB IOAPIC properly. ACPI specification says that _PIC method is optional and can be called by OSPM to provide the interrupt routing mode information to the firmware. However, if the method is not called, the firmware must assume PIC mode is used. AGESA sets the IoapicSbFeatureEn already to be compliant with ACPI. Previously, coreboot cleared the bit unconditionally and left a comment to move that part to DSDT. The hook allows to clear the IoapicSbFeatureEn bit if OS chooses APIC mode for interrupt routing. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

Needed early in build to use sklt.h header Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

Modeled after BL31's code from arm64 Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

SergiiDmytruk

Reviewed the changes in more detail.

SergiiDmytruk · 2026-06-20T16:27:09Z

 #endif

-External (\_SB.NAPE, MethodObj)
+External (\_SB.PCI0.NAPE, MethodObj)


Can't simply rename this, the change is in upstream and will break AMD Turin (see soc/amd/turin_poc/acpi/ioapic_routing.asl).

Getting rid of PCI0 will affect other methods, so maybe permit both names and pick one via CondRefOf.

I pulled it from apu2_fix_iommu and rebased it as-is, in fact everything before the drtm stuff is from that branch. if its not needed at all I could just drop those commits.

SergiiDmytruk · 2026-06-20T16:29:05Z

+	If (CondRefOf (\_SB.PCI0.NAPE))
 	{
-		\_SB.NAPE()
+		\_SB.PCI0.NAPE(Arg0 & 1)


For compatibility, the parameter should be dropped in favour of using PICM.

SergiiDmytruk · 2026-06-20T16:38:53Z

 #define CAP_OFFSET_10_MSI_NUM_PPR_SHIFT		27
 #define CAP_OFFSET_10_MSI_NUM_PPR		(0x1f << CAP_OFFSET_10_MSI_NUM_PPR_SHIFT)

+#define PCI_DEVID(BUS, DEV, FN) ( \


src/include/device/pci_def.h seems like a better place for this macro, that's where PCI_DEVFN is defined.

honestly unsure if I even need the stuff before the drtm commits at this point. I should give that a test, try to focus effort in one area.

SergiiDmytruk · 2026-06-20T17:56:53Z

+	slrt->magic = 0x4452544d;
+	slrt->revision = 0x1;
+	slrt->architecture = 0x2;


Better to use constants like in https://github.com/TrenchBoot/grub/blob/tb-dev/include/grub/slr_table.h.

Yeah definitely, was mostly just 'banging it out' to get it to actually start talking to me.

SergiiDmytruk · 2026-06-20T17:57:39Z

+	slrt->size = 16;
+	slrt->max_size = 0;


slrt->size is sizeof(*slrt) and slrt->max_size should be 64*KiB - bootloader_data_offset (64*KiB - (skl - (void *)slrt) above is a more complicated way of computing the same).

SergiiDmytruk · 2026-06-20T17:58:36Z

+	 */
+	_Static_assert(sizeof(skl) == 4);
+
+	hexdump(prog, sizeof(*prog));


Don't see a need to keep this.

the assert or the hexdump?

The hexdump() invocation. The lines above it are shown by GitHub as a context.

SergiiDmytruk · 2026-06-20T18:01:29Z

+	skl = memalign(64*KiB, 64*KiB);
+
+	cbfs_load(CONFIG_CBFS_PREFIX "/drtm_payload", skl, 64*KiB);


Both functions can fail. This function should die() in such cases, because simply returning will apparently result in repeated invocation with possibly weird side-effects.

SergiiDmytruk · 2026-06-20T18:09:32Z

+[submodule "3rdparty/secure-kernel-loader"]
+	path = 3rdparty/secure-kernel-loader
+	url = https://github.com/hanetzer/trenchboot-secure-kernel-loader


If we don't need to have SKL as a submodule, I think it's worth avoiding adding one. Adding submodules results in them being checked out for no use and also require setting up a mirror in upstream coreboot.

yeah I can probably drop this and some related bits, this was when I was trying for explicitly using slrt.h from skl itself; when its done via payloads/external it doesn't exist early enough for compiling the ramstage.

miczyg1 and others added 21 commits June 18, 2026 16:25

northbridge/amd/pi/00730F01/acpi/northbridge.asl: Add IOMMU device

41fcf3f

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

src/mainboard/pcengines/apu2/acpi/routing.asl: Add PCI INT routing fo…

3235ebb

…r IOMMU Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

northbridge/amd/pi/00730F01/northbridge.c: Disable Guest AVIC

8aa2a6e

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

src/mainboard/pcengines/apu2/mainboard.c: Assign INTB# to IRQ17

a17cb4a

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

src/mainboard/pcengines/apu2/mainboard.c: Clear the right bits in ECC…

050089c

… register Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

src/northbridge/amd/pi/00730F01/northbridge.c: Add PCI_DEVID macro

2a38f5e

Introduce PCI_DEVID macro which allows to construct PCI device addresses or range with bus numbers. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

src/northbridge/amd/pi/00730F01/northbridge.c: Rework IVRS generation

938e709

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>

cpu/amd/pi: add skeleton for DRTM

6999308

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>

cpu/amd/pi/skinit: load and start DRTM payload

66316b5

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>

cpu/amd/skinit.c: pass minimal set of info required to boot to SKL

3cbfa32

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>

cpu/amd/skinit: print MSRs connected with SMM

5934f03

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>

configs/config.pcengines_apu2_drtm_payload: add file

c3c209b

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>

configs/config.pcengines_apu2_drtm_payload: enable UEFI shell

c4d995f

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>

amd/common: move skl here

dfe5fb4

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

3rdparty: add secure-kernel-loader

1d60c88

Needed early in build to use sklt.h header Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

skl: build from submodule

958f8ed

Modeled after BL31's code from arm64 Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

skinit: use slrt.h from secure-kernel-loader

e2bbbcd

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

skinit: move to slrt tags

a2678b4

Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>

hanetzer force-pushed the drtm_payload-25.12 branch from 8abdcd7 to a2678b4 Compare June 19, 2026 20:55

SergiiDmytruk reviewed Jun 20, 2026

View reviewed changes

		skl = memalign(64KiB, 64KiB);

		cbfs_load(CONFIG_CBFS_PREFIX "/drtm_payload", skl, 64*KiB);

Uh oh!

Conversation

hanetzer commented Jun 19, 2026

Uh oh!

SergiiDmytruk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants