EC capsules

[SergiiDmytruk]

• EC's version is derived from the timestamp, so 2025-04-25_c546244 becomes 0x7E90419 with commit hash being completely ignored and two releases on the same date looking identical (should not be a big deal). Could use BCD so the version looks like 0x20250425 instead which is more readable for humans.

• EC driver update could report success on failure to update, fixed that.

• Hit a case when EC capsule was put by coreboot into an SMM area and improved how coalesced buffer is selected.

• Not sure why it didn't come up earlier, but SPI bus commands sent to EC could be problematic because they omit the length byte. Maybe EC zeroes it at some point, but the code was updated to actually pass zero length.

• Made capsule.sh more suitable for use in scripts, which isn't used here immediately. See individual commits and their messages.

EDK PR: Dasharo/edk2#310
issue: Dasharo/dasharo-issues#1758
ref: ncm-2275

[mkopec]

Some observations from initial testing:

• When booting with AC disconnected, there is a popup saying that the EC update failed. However, when booting with AC attached, the EC does not update, but there is no pop-up anymore. Something is wrong in the EC update code in coreboot.
  • It should still work, for example for Heads users, but the EC update in coreboot should be disabled for UEFI builds, which will now update in the payload exclusively.
• fwupdmgr and Windows do not detect the new ESRT device

[philipanda]

EC's version is derived from the timestamp, so 2025-04-25_c546244 becomes 0x7E90419 with commit hash being completely ignored and two releases on the same date looking identical (should not be a big deal). Could use BCD so the version looks like 0x20250425 instead which is more readable for humans.

Why not use the commit hash and be safe?

In capsule updates Dasharo firmware is not being versioned by the date or commit SHA, but on a release basis, and we already do release tags on EC. Why not just use the Dasharo versioning scheme for EC too?

[SergiiDmytruk]

@mkopec

* When booting with AC disconnected, there is a popup saying that the EC update failed. However, when booting with AC attached, the EC does not update, but there is no pop-up anymore. Something is wrong in the EC update code in coreboot.

In coreboot?

  * It should still work, for example for Heads users, but the EC update in coreboot should be disabled for UEFI builds, which will now update in the payload exclusively.

Auto-updating in coreboot is mutually exclusive with EC capsules.

* fwupdmgr and Windows do not detect the new ESRT device

Have you enabled EC capsules in config? I didn't update any of them since I don't yet know which models need to be enabled and in what release. CONFIG_DRIVERS_EFI_EC_FW_GUID and CONFIG_DRIVERS_EFI_EC_FW_SIZE need to be set after disabling CONFIG_EC_DASHARO_EC_UPDATE.

---

@philipanda

Why not use the commit hash and be safe?

Commit hashes are pseudo-random and have no meaningful ordering defined for them, but date can be compared. Need ordering for fwupd and LSV.

In capsule updates Dasharo firmware is not being versioned by the date or commit SHA, but on a release basis, and we already do release tags on EC. Why not just use the Dasharo versioning scheme for EC too?

I was adapting the version already in use to 32-bit unsigned values, don't know if we want to make EC firmware report different version when enabling capsule updates.

[filipleple]

Approved the logic, but will test on HW today as well.

EDIT: @SergiiDmytruk nv4x laptop didn't boot after flashing FW with such diff enabling the feature:

 λ git df
diff --git a/configs/config.novacustom_nv4x_adl b/configs/config.novacustom_nv4x_adl
index 7296e90c9871..7298dc0ce7f4 100644
--- a/configs/config.novacustom_nv4x_adl
+++ b/configs/config.novacustom_nv4x_adl
@@ -16,7 +16,6 @@ CONFIG_EDK2_BOOTSPLASH_FILE="3rdparty/dasharo-blobs/novacustom/bootsplash.bmp"
 CONFIG_POWER_STATE_OFF_AFTER_FAILURE=y
 CONFIG_ALWAYS_ALLOW_ABOVE_4G_ALLOCATION=y
 CONFIG_ENABLE_EARLY_DMA_PROTECTION=y
-CONFIG_EC_DASHARO_EC_UPDATE=y
 CONFIG_HAVE_ME_BIN=y
 CONFIG_NO_GFX_INIT=y
 CONFIG_PCIEXP_HOTPLUG_PREFETCH_MEM_BELOW_4G=y
@@ -26,6 +25,9 @@ CONFIG_DRIVERS_EFI_FW_INFO=y
 CONFIG_DRIVERS_EFI_MAIN_FW_GUID="b79b7279-79a4-47cf-ab2f-8636c541f2d0"
 CONFIG_DRIVERS_EFI_MAIN_FW_VERSION=0x01080006
 CONFIG_DRIVERS_EFI_MAIN_FW_LSV=0x01080006
+CONFIG_DRIVERS_EFI_EC_FW_GUID="79e7f788-d24c-4cb2-b1b8-ed9cd5e0c2e0"
+CONFIG_DRIVERS_EFI_EC_FW_LSV=0x00000000
+CONFIG_DRIVERS_EFI_EC_FW_SIZE=0x20000
 CONFIG_DRIVERS_EFI_UPDATE_CAPSULES=y
 CONFIG_DRIVERS_GENERIC_CBFS_SERIAL=y
 CONFIG_DRIVERS_GENERIC_CBFS_UUID=y

 λ strings novacustom_nv4x_adl_v1.8.0-rc6.rom | tail
(...)
novacustom_v56x_mtl_igpu_v1.0.2-rc1-19-g8c97e27df1ed-dirty
NovaCustom
nv40pz

Fans are spinning and e.g. keyboard backlight works, but the screen stays black. The firmware was built with ec.rom built from the latest Dasharo/ec commit, so it would trigger an EC sync normally. I'm guessing that something broke in the logic around it.

Unfortunately, the laptop unit I picked doesn't have any debug output soldered in, so I can't recover the logs from it

[SergiiDmytruk]

@filipleple, so you didn't use an EC capsule and the laptop failed to boot right after flashing?

Fans are spinning and e.g. keyboard backlight works, but the screen stays black.

Sounds like it didn't get to EDK, but the only new code which gets run in this configuration is in src/drivers/efi/info.c and so far I fail to see why it would hang.

The firmware was built with ec.rom built from the latest Dasharo/ec commit, so it would trigger an EC sync normally. I'm guessing that something broke in the logic around it.

This contradicts the removal of CONFIG_EC_DASHARO_EC_UPDATE=y. CONFIG_EC_DASHARO_EC_UPDATE_FILE depends on CONFIG_EC_DASHARO_EC_UPDATE. And without CONFIG_EC_DASHARO_EC_UPDATE=y coreboot doesn't do any updates:

coreboot/src/ec/dasharo/ec/dasharo_ec.c

Lines 872 to 873 in 8c97e27

	if (!CONFIG(EC_DASHARO_EC_UPDATE))
	return;

Maybe check config embedded in the ROM file to see if configuration matches the expectation. make olddefconfig on the config with your patch looks correct (no CONFIG_EC_DASHARO_EC_UPDATE or CONFIG_EC_DASHARO_EC_UPDATE_FILE).

[filipleple]

@SergiiDmytruk correct, I didn't use a capsule yet, only a full flash of the rom built with a config aiming at enabling the changes. I tried to cold boot a couple times, EC reset by holding down the power button, no luck. I'll try to read the ROM back and examine the config now.

[SergiiDmytruk]

Explored 3 possible reasons for the hang assuming the configuration is as expected:

• ESRT parsing in EDK (in case it happens before display is initialized). Should be fine, most of the code is as before and there are no new loops.
• Querying EC version at a bad moment in coreboot. get_smbios_strings() does the same a bit earlier (SMBIOS tables are constructed right before coreboot table), so there should be no change in EC's state or other things affecting communication with it.
• EC version parsing going wrong in EC. ec_version_str buffer is intentionally large to avoid an overflow. There is a chance that it's not NUL-terminated, but parsing would stop at a non-digit in random data instead of hanging.

[filipleple]

I've retrieved the config, it looks as expected, that is the current nv4x config with the EC capsule feature enabled.
config.txt

[SergiiDmytruk]

I've retrieved the config, it looks as expected, that is the current nv4x config with the EC capsule feature enabled. config.txt

Yes, the config is exactly as it should be.

I've noticed two more things in the code:

• There are additional interactions with EC in mainboard's code. These use simple commands which are unrelated to SPI and should not be affected by any changes here.
• Usable memory map in efi/drivers/capsules.c is built even in the absence of capsules and is executed on every boot. My code there looks sane and is kinda trivial, but maybe memranges_* functions have a bug related to managing the data structure. Will take a closer look.

[SergiiDmytruk]

[...] maybe memranges_* functions have a bug related to managing the data structure. Will take a closer look.

Doesn't seem like there are issues with that code. Threw operations on random ranges at it and couldn't produce a hang. Inspecting the code didn't reveal bugs either.