Changes for Heads on NovaCustom V560TU #609

mkopec · 2025-01-15T16:05:52Z

Fix building with FSP from dasharo-blobs
Add PR0 lockdown in SMM
Enable S3 on MTL
Fixes for HAP on MTL
Always pull FSP submodule, to ensure SplitFspBin is present
Move IOE_P2SB below 4G to reduce MTRR fragmentation (faster boot on 64 and 96 GB RAM configs)
Always put I2C and framebuffer below 4G when built for IA32, to make sure we can use them in coreboot
Fix for FSP GOP not initializing if an external display with res < 1920x1200 is connected
JPEG patches for bootsplash, previously maintaned downstream in Heads

…t vc dir Signed-off-by: Michał Kopeć <[email protected]>

In a server platform many silicon specific register lock operations are by default in FSP space. CHIPSET_LOCKDOWN_FSP provides an option to make sure the codes could be used out-of-box to build products. Change-Id: I8efcc1f27446be8e35f51e2568c4af6f8165486b Signed-off-by: Shuo Liu <[email protected]> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82081 Reviewed-by: Angel Pons <[email protected]> Tested-by: build bot (Jenkins) <[email protected]> Reviewed-by: Patrick Rudolph <[email protected]>

Heads payload uses APM_CNT_FINALIZE SMI to set and lock down the SPI controller with PR0 flash protection for pre-Skylake platforms. Add new option to skip LPC and FAST SPI lock down in coreboot and move it to APM_CNT_FINALIZE SMI handler. Reuse the INTEL_CHIPSET_LOCKDOWN option to prevent issuing APM_CNT_FINALIZE SMI on normal boot path, like it was done on pre-Skylake platforms. As the locking on modern SOCs became more complicated, separate the SPI and LPC locking into new modules to make linking to SMM easier. The expected configuration to leverage the feautre is to unselect INTEL_CHIPSET_LOCKDOWN and select SOC_INTEL_COMMON_SPI_LOCKDOWN_SMM. Testing various microarchitectures happens on heads repository: linuxboot/heads#1818 TEST=Lock the SPI flash using APM_CNT_FINALIZE in heads on Alder Lake (Protectli VP66xx) and Comet Lake (Protectli VP46xx) platforms. Check if flash is unlocked in the heads recovery console. Check if flash is locked in the kexec'ed OS. Change-Id: Icbcc6fcde90e5b0a999aacb720e2e3dc2748c838 Signed-off-by: Michał Żygowski <[email protected]>

Signed-off-by: Michał Kopeć <[email protected]>

mkopec · 2025-01-15T16:49:50Z

Should go in before linuxboot/heads#1846

miczyg1

While we are at limiting the necessary resources to 32bit, can you please limit it on the CSE HECI device as well? That way the allocation could work also for other builds than heads, where ME is active.

miczyg1 · 2025-01-16T09:14:28Z

src/soc/intel/common/block/graphics/graphics.c

+		/* Place framebuffer below 4G to allow drawing bootsplash */
+		struct resource *res_bar2 = find_resource(dev, PCI_BASE_ADDRESS_2);
+		res_bar2->limit = 0xffffffff;
+		res_bar2->flags &= ~(IORESOURCE_PCI64 | IORESOURCE_ABOVE_4G);


Suggested change

res_bar2->flags &= ~(IORESOURCE_PCI64 | IORESOURCE_ABOVE_4G);

res_bar2->flags &= ~IORESOURCE_ABOVE_4G;

Should be enough. The BAR is still 64bit and we should keep that info. The resource limit set to UINT32_MAX (0xffffffff) should make coreboot allocate it under 4G. Please try it and apply in other places too.

Applied: b890d38

miczyg1 · 2025-01-16T09:19:43Z

src/soc/intel/meteorlake/Kconfig

@@ -255,7 +255,7 @@ config PCR_BASE_ADDRESS

 config IOE_PCR_BASE_ADDRESS
 	hex
-	default 0x3fff0000000
+	default 0x60000000


Is it really necessary? That address 0x3fff0000000 will always be UC MMIO anyways and should not influence the MTRR allocation... I mean even if you have 8GB of memory or 96GB of memory, maximum possible address of RAM for 96GB would be 0x1800000000 (excluding any MMIO holes). If we add 2GB MMIO hole in 32bit space, then we would have 0x1880000000. Anything above it will have to be UC, always, no matter the IOE_P2SB BAR is in 32bit or 64bit space. Could you please try it?

Here's my cbmem without 5863a5f : mtrr_96gb_before.log

With: mtrr_96gb_after.log

I think the difference comes down to the mask of MTRR 8

miczyg1 · 2025-01-16T09:20:38Z

src/soc/intel/meteorlake/include/soc/iomap.h

@@ -66,9 +66,10 @@
 #define IOE_P2SB_BAR		IOE_PCR_ABOVE_4G_BASE_ADDR
 #define IOE_P2SB_SIZE		(256 * MiB)

-#define IOM_BASE_ADDR		0x3fff0aa0000
+/* IOE_P2SB_BAR + 0xaa0000, but iasl refuses to perform arithmetics  */


What? Can you point to the faulting code?

Similar bug to https://review.coreboot.org/c/coreboot/+/84216

In my case setting this to (IOE_P2SB_BAR + 0xaa0000) results in this iasl error:

dsdt.asl 1374: 0x1600,,,) Error 6050 - ^ Length is not equal to fixed Min/Max window

coming from here:

coreboot/src/soc/intel/meteorlake/acpi/tcss.asl

Lines 332 to 335 in 4e2fa99

QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,

NonCacheable, ReadWrite, 0x0,

IOM_BASE_ADDR, IOM_BASE_ADDR_MAX, 0x0,

IOM_BASE_SIZE,,,)

Perhaps it's worth adding such references (if known) to the commit description

Right, amended: d047598

Signed-off-by: Michał Kopeć <[email protected]>

mkopec · 2025-01-16T11:05:37Z

While we are at limiting the necessary resources to 32bit, can you please limit it on the CSE HECI device as well? That way the allocation could work also for other builds than heads, where ME is active.

Done 5a313c7 (builds, but untested on a platform with ME enabled)

Without this patch, if RESOURCE_ALLOCATION_TOP_DOWN is enabled and coreboot is compiled for IA32, the HECI device becomes inaccessible after resource allocation. Signed-off-by: Michał Kopeć <[email protected]>

Signed-off-by: Michał Kopeć <[email protected]>

mkopec · 2025-01-16T11:57:37Z

@miczyg1 can you take another look?

This frees up two MTRRs for SPI flash caching IOM_BASE_ADDR has to be hardcoded, otherwise iasl throws an error while building ``` dsdt.asl 1374: 0x1600,,,) Error 6050 - ^ Length is not equal to fixed Min/Max window ``` It seems like IASL can't perform arithmetic operations inside QWordMemory definitions. See https://review.coreboot.org/c/coreboot/+/84216 Signed-off-by: Michał Kopeć <[email protected]>

Since commit 1d029b4 ("lib/jpeg: Replace decoder with Wuffs' implementation"), a relatively large heap allocation is needed to decode many JPEGs for use as work area. The prior decoder did not need this, but also had many limitations in the JPEGs it could decode, was not as memory-safe and quickly crashed under fuzzing. This commit keeps using Wuffs' JPEG decoder, but it no longer requires any heap allocation (and thus configuring the heap size depending on how big a bootsplash image you want to support). Change-Id: Ie4c52520cbce498539517c4898ff765365a6beba Signed-off-by: Nigel Tao <[email protected]> Reviewed-on: https://review.coreboot.org/c/coreboot/+/83895 Tested-by: build bot (Jenkins) <[email protected]> Reviewed-by: Nico Huber <[email protected]> Reviewed-by: Felix Singer <[email protected]> Reviewed-by: Jonathon Hall <[email protected]>

We were previously at Wuffs 0.4.0-alpha.2. The C file was copied from https://github.com/google/wuffs-mirror-release-c and its hash matches https://github.com/google/wuffs-mirror-release-c/blob/90e4d81a6a8b7b601e8e568da32a105d7f7705e5/sync.txt#L9-L10 $ sha256sum src/vendorcode/wuffs/wuffs-v0.4.c 6c22caff4af929112601379a73f72461bc4719a5215366bcc90d599cbc442bb6 src/vendorcode/wuffs/wuffs-v0.4.c Change-Id: Ie90d989384e0db2b23d7d1b3d9a57920ac8a95a2 Signed-off-by: Nigel Tao <[email protected]> Reviewed-on: https://review.coreboot.org/c/coreboot/+/83894 Reviewed-by: Felix Singer <[email protected]> Reviewed-by: Nico Huber <[email protected]> Tested-by: build bot (Jenkins) <[email protected]>

Signed-off-by: Michał Kopeć <[email protected]>

mkopec and others added 7 commits January 15, 2025 17:01

vc/intel/fsp/fsp2_0/iot/meteorlake: add missing MemInfoHob from clien…

e8a3f7e

…t vc dir Signed-off-by: Michał Kopeć <[email protected]>

soc/intel/mtl/fsp_params.c: wire up sleep mode option

092267f

Signed-off-by: Michał Kopeć <[email protected]>

clevo/mtl-h/ramstage.c: implement sleep mode option

9f5143b

Signed-off-by: Michał Kopeć <[email protected]>

soc/intel/cmn/blk/cse/cse.c: skip me_enable while ME is in Debug mode

8d960e9

Signed-off-by: Michał Kopeć <[email protected]>

Makefile.mk: Always pull FSP submodule

3138e53

Signed-off-by: Michał Kopeć <[email protected]>

macpijan requested a review from miczyg1 January 15, 2025 16:06

miczyg1 requested changes Jan 16, 2025

View reviewed changes

mkopec added 2 commits January 16, 2025 12:03

soc/intel/cmn/blk/graphics: ensure framebuffer is allocated <4G

b890d38

Signed-off-by: Michał Kopeć <[email protected]>

soc/intel/cmn/blk/i2c: ensure resources are allocated <4G

c4edf10

Signed-off-by: Michał Kopeć <[email protected]>

mkopec force-pushed the mtl_heads branch from e67e84b to 773b49c Compare January 16, 2025 11:03

mkopec added 2 commits January 16, 2025 12:18

soc/intel/cmn/blk/cse/cse.c: ensure resources are allocated <4G

5a313c7

Without this patch, if RESOURCE_ALLOCATION_TOP_DOWN is enabled and coreboot is compiled for IA32, the HECI device becomes inaccessible after resource allocation. Signed-off-by: Michał Kopeć <[email protected]>

mb/clevo/mtl-h: Set MMIO size to 2.5GB

40e8d1a

Signed-off-by: Michał Kopeć <[email protected]>

mkopec force-pushed the mtl_heads branch from 773b49c to 3bf6b21 Compare January 16, 2025 11:18

mkopec and others added 4 commits January 16, 2025 12:59

mb/clevo/mtl-h/variants/igpu/data.vbt: Disable secondary display feature

048ca83

Signed-off-by: Michał Kopeć <[email protected]>

mkopec force-pushed the mtl_heads branch from 3bf6b21 to 048ca83 Compare January 16, 2025 12:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes for Heads on NovaCustom V560TU #609

Changes for Heads on NovaCustom V560TU #609

mkopec commented Jan 15, 2025 •

edited

Loading

mkopec commented Jan 15, 2025

miczyg1 left a comment

miczyg1 Jan 16, 2025

mkopec Jan 16, 2025

miczyg1 Jan 16, 2025 •

edited

Loading

mkopec Jan 16, 2025 •

edited

Loading

miczyg1 Jan 16, 2025

mkopec Jan 16, 2025 •

edited

Loading

macpijan Jan 16, 2025

mkopec Jan 16, 2025

mkopec commented Jan 16, 2025 •

edited

Loading

mkopec commented Jan 16, 2025

	res_bar2->flags &= ~(IORESOURCE_PCI64 \| IORESOURCE_ABOVE_4G);
	res_bar2->flags &= ~IORESOURCE_ABOVE_4G;

	QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
	NonCacheable, ReadWrite, 0x0,
	IOM_BASE_ADDR, IOM_BASE_ADDR_MAX, 0x0,
	IOM_BASE_SIZE,,,)

Changes for Heads on NovaCustom V560TU #609

Are you sure you want to change the base?

Changes for Heads on NovaCustom V560TU #609

Conversation

mkopec commented Jan 15, 2025 • edited Loading

mkopec commented Jan 15, 2025

miczyg1 left a comment

Choose a reason for hiding this comment

miczyg1 Jan 16, 2025

Choose a reason for hiding this comment

mkopec Jan 16, 2025

Choose a reason for hiding this comment

miczyg1 Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

mkopec Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

miczyg1 Jan 16, 2025

Choose a reason for hiding this comment

mkopec Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

macpijan Jan 16, 2025

Choose a reason for hiding this comment

mkopec Jan 16, 2025

Choose a reason for hiding this comment

mkopec commented Jan 16, 2025 • edited Loading

mkopec commented Jan 16, 2025

mkopec commented Jan 15, 2025 •

edited

Loading

miczyg1 Jan 16, 2025 •

edited

Loading

mkopec Jan 16, 2025 •

edited

Loading

mkopec Jan 16, 2025 •

edited

Loading

mkopec commented Jan 16, 2025 •

edited

Loading