SrePkg: adopt the current FMP + capsule implementation

[kat-perez]

Sync SrePkg with the current implementation from the platform integration tree, making this repository the source for the package.

• SreFmpDeviceLib: capsule-delivered WIM apply path; the ESP-streaming path and its support code (SreImageSupport, SreCapsulePersistenceLib, CryptoPkg dependency, PcdSreStagingPath) are retired. A failed apply now aborts the open write session (no commit, write protection restored) so the other partition's apply and later storage operations are not blocked.
• SreStorage: reworked SreStorageNvmeLib / SreStorageNullLib / SreStorage.h; SreFmpDeviceLib.h moves to Include/Library/. Adds SreStorageWriteAbort for the failed-apply path.
• SrePkg.dec: adds gSreWimStagingGuid and gSreEsrtGuid.
• NvmeBpWrite: content-comparison-driven WRITE/VERIFY/NOOP decision at each boot.
• BuildBpFatImage.ps1: absolute-path normalization + SRE descriptor-region guard.
• Flash-BP1.md: fix a stale path reference.

Platform-specific capsule build/signing tooling is intentionally not included; the public capsule tooling in capsules/ is unchanged. VerifyCapsuleLayout.py and plan.md still describe the prior peeler-based design — updating them is follow-up work.

Note: an earlier revision of this PR also removed uefi/OdpPkg; that was dropped — OdpPkg remains the home for non-SRE ODP UEFI content. The duplicate gOdpPkgTokenSpaceGuid declaration between OdpPkg.dec and SrePkg.dec (different GUID values) is tracked as follow-up.

Co-authored-by: Ron Gurr rogurr@microsoft.com

[Copilot]

Pull request overview

This PR makes uefi/SrePkg the single source for the SRE UEFI package by removing the unused uefi/OdpPkg and syncing SRE’s storage + FMP update path to the current capsule-driven implementation.

Changes:

• Remove uefi/OdpPkg (demo Rust drivers, placeholders, and its DEC) and update docs that referenced it.
• Refactor SRE storage into a block-geometry API (SreStorageInfo + block read/write session) and update NVMe implementation accordingly.
• Rework SreFmpDeviceLib to write the capsule-provided image buffer directly to NVMe boot partitions and stamp a fixed-offset SRE_IMAGE_INFO trailer; remove the prior ESP-staged WIM + peeler/persistence components.

Reviewed changes

Copilot reviewed 31 out of 34 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
uefi/SrePkg/SrePkg.dec	Adds GUIDs for WIM staging identifier and ESRT capsule GUID.
uefi/SrePkg/Library/SreStorage/SreStorageNvmeLib.c	Major NVMe boot-partition read/write redesign, adds geometry + alignment handling.
uefi/SrePkg/Library/SreStorage/SreStorageNullLib.c	Updates NULL instance to new storage API shape (still stubbed/unsupported).
uefi/SrePkg/Library/SreFmpDeviceLib/SreImageSupport.h	Removes staged-WIM helper header (no longer used).
uefi/SrePkg/Library/SreFmpDeviceLib/SreImageSupport.c	Removes staged-WIM streaming + hash verification implementation (no longer used).
uefi/SrePkg/Library/SreFmpDeviceLib/SreFmpDeviceLib.inf	Drops staged-WIM sources/deps and updates library dependencies accordingly.
uefi/SrePkg/Library/SreFmpDeviceLib/SreFmpDeviceLib.c	Implements capsule-buffer-to-storage streaming, fixed-offset image info stamping, version retrieval updates.
uefi/SrePkg/Library/SreCapsulePersistenceLib/SreCapsulePersistenceLib.inf	Removes custom capsule persistence/peeler library instance.
uefi/SrePkg/Library/SreCapsulePersistenceLib/SreCapsulePersistenceLib.c	Removes peeler implementation that staged WIM to ESP and rebuilt descriptor-only capsule.
uefi/SrePkg/Library/SreCapsulePersistenceLib/CapsulePersistence.h	Removes copied capsule persistence interface (no longer needed).
uefi/SrePkg/Library/SreCapsulePersistenceLib/CapsulePersistence.c	Removes copied capsule persistence disk engine (no longer needed).
uefi/SrePkg/Include/SreFmpDeviceLib.h	Removes old descriptor-only header (moved/replaced).
uefi/SrePkg/Include/Library/SreStorage.h	Updates public storage API from byte-range IO to block-based geometry + sessions.
uefi/SrePkg/Include/Library/SreFmpDeviceLib.h	Adds new public header for fixed-offset SRE_IMAGE_INFO and partition enum.
uefi/SrePkg/Application/NvmeBpWrite/NvmeBpWrite.c	Adjusts behavior around prior NVRAM result variable usage (currently inconsistent with comments).
uefi/SrePkg/Application/NvmeBpWrite/Flash-BP1.md	Updates doc reference from OdpPkg path to SrePkg path.
uefi/SrePkg/Application/BpRecoveryLoader/BuildBpFatImage.ps1	Normalizes output path and adds descriptor-region guard checks for layout safety.
uefi/OdpPkg/OdpPkg.dec	Removes unused OdpPkg DEC.
uefi/OdpPkg/Library/README.md	Removes placeholder README.
uefi/OdpPkg/Include/README.md	Removes placeholder README.
uefi/OdpPkg/Drivers/ODP_RustDxeDemo/src/main.rs	Removes Rust DXE demo driver source.
uefi/OdpPkg/Drivers/ODP_RustDxeDemo/README.md	Removes Rust DXE demo documentation.
uefi/OdpPkg/Drivers/ODP_RustDxeDemo/Cargo.toml	Removes Rust DXE demo manifest.
uefi/OdpPkg/Drivers/ODP_RustDxeDemo/Cargo.lock	Removes Rust DXE demo lockfile.
uefi/OdpPkg/Drivers/ODP_PatinaSmbiosDemo/src/platform.rs	Removes Patina SMBIOS demo platform source.
uefi/OdpPkg/Drivers/ODP_PatinaSmbiosDemo/src/main.rs	Removes Patina SMBIOS demo driver shell.
uefi/OdpPkg/Drivers/ODP_PatinaSmbiosDemo/rust-toolchain.toml	Removes demo toolchain pin.
uefi/OdpPkg/Drivers/ODP_PatinaSmbiosDemo/README.md	Removes demo documentation.
uefi/OdpPkg/Drivers/ODP_PatinaSmbiosDemo/ODP_PatinaSmbiosDemo.depex	Removes demo depex file.
uefi/OdpPkg/Drivers/ODP_PatinaSmbiosDemo/Cargo.toml	Removes demo manifest.
uefi/OdpPkg/Drivers/ODP_PatinaSmbiosDemo/Cargo.lock	Removes demo lockfile.
uefi/crates/patina_tianocore/README.md	Updates README to remove references to a follow-up PR example under OdpPkg.
README.md	Updates top-level repo documentation to point at SrePkg instead of OdpPkg.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

uefi/SrePkg/Include/Library/SreStorage.h:17

• This public header uses EFI types (EFI_STATUS/UINTN) and PARTITION_INDEX but doesn’t include a header that defines them. That makes the API brittle for any consumer that includes <Library/SreStorage.h> without having already included Uefi.h and the PARTITION_INDEX definition.

// Return the geometry of the SRE storage area.  If the device is not present or
// supported this returns EFI_UNSUPPORTED and any other call will also fail.
//
// BlockCount           - Number of blocks in the storage area
// BlockSize            - Size of a block in bytes

uefi/SrePkg/SrePkg.dec:24

• The new gSreWimStagingGuid comment still refers to the (now retired) “peeler” design. Given this PR switches to capsule-delivered apply and explicitly notes peeler-based docs are follow-up work, this GUID comment should be updated so it doesn’t misdocument the current flow.

  # Staging UpdateImageTypeId carried by the WIM payload. No FmpDxe instance
  # claims it; it exists only so the peeler can identify the WIM and so stock
  # FmpDxe would harmlessly skip it if the peeler were ever bypassed.

uefi/SrePkg/Include/Library/SreFmpDeviceLib.h:11

• The file header comment still describes the retired “descriptor-only payload + staged WIM + streaming” design (size/hash/ESP staging, never loaded as a single buffer). In this PR, SetImage() receives the in-memory capsule payload and the only on-disk metadata is SRE_IMAGE_INFO at a fixed offset, so this header comment is now misleading and should be updated.

  This header defines the small "capsule descriptor" that the SRE Firmware
  Management Protocol (FMP) payload carries. The descriptor is what flows
  through the stock capsule path (Capsule-On-Disk -> PEI -> DxeCapsuleLib ->
  FmpDxe -> FmpDeviceLib::SetImage). It is only a few bytes: it records the
  size and hash of the large recovery WIM and where to find it. The ~1 GB WIM

[rogurr]

I'm not convinced this is the best way to handle this considering the read/write and write enable locks are really putting the device into an upload state or normal state and just the one lock is really a write-once lock. I'd like to think about this and make sure we handle things properly.

Can you write this up this as a bug and assign to me?

[felipebalbi]

Could you update the comment to contain the version format?

Does this read as 0.0.1? How many bits for each of major/minor/patch?

[kat-perez]

@rogurr Can you help speak to the design intent and I can update the doc comment

[rogurr]

UEFI has a president for using a UINT32 number that is incremented every time a structure needs to change. This is just an integer tag so that if the internal layout of the structure containing information changes, we know and can handle it accordingly. This is not the version of the UEFI which does have a major/minor/build format which is a U32 in a U8.U16.U8 format.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (4)

uefi/SrePkg/Include/Library/SreStorage.h:11

• SreStorage.h now exposes APIs that take PARTITION_INDEX but no longer defines that type or includes the header that defines it, creating a fragile include-order dependency for all consumers.

#ifndef _SRE_STORAGE_H_
#define _SRE_STORAGE_H_

uefi/SrePkg/SrePkg.dec:24

• The new gSreWimStagingGuid comment still references the (now-retired) peeler-based design, which makes this GUID’s purpose unclear in the current capsule-delivered flow. Update the comment to describe the GUID’s role without depending on the old peeler terminology.

  # Staging UpdateImageTypeId carried by the WIM payload. No FmpDxe instance
  # claims it; it exists only so the peeler can identify the WIM and so stock
  # FmpDxe would harmlessly skip it if the peeler were ever bypassed.

uefi/SrePkg/Library/SreStorage/SreStorageNvmeLib.c:354

• mBlockCount can compute to 0 when the controller reports a large FWUG (BlockSize) relative to BPINFO BPSZ. In that case the constructor marks the device supported, but later reads/writes will behave inconsistently (e.g., reads reject every BlockIndex, writes immediately hit END_OF_MEDIA). Treat a 0 block count as unsupported.

  mBlockCount = (UINTN)Bpinfo * SIZE_128KB / mBlockSize;

  // Supported
  mIsSupported = TRUE;
  DEBUG ((DEBUG_INFO, "[SreStorageNvmeLib] Boot partition support = TRUE\n"));

uefi/SrePkg/Application/NvmeBpWrite/NvmeBpWrite.c:342

• WriteResultToNvRam() no longer checks the return status of SetVariable(). If this write fails (e.g., NVRAM full / access denied), the host loses the primary way to observe the run outcome and BP write-protection readback, and the failure becomes silent. Capture and log the error as before.

  gResult.Magic   = BP_RESULT_MAGIC;
  gResult.Version = BP_RESULT_VERSION;
  gRT->SetVariable (
         L"NvmeBpResult",
         &gNvmeBpResultGuid,
         EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
         sizeof (gResult),

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Suppressed comments (4)

uefi/SrePkg/Application/BpRecoveryLoader/BuildBpFatImage.ps1:122

• The overlap check treats a FAT partition that starts exactly at the end of the reserved descriptor window as an error. The reserved window is expressed as a half-open interval [start..end), so equality is safe; only a start offset strictly below the end overlaps.

    if ($part.Offset -le ($SreDescriptorOffset + $SreDescriptorBytes)) {

uefi/SrePkg/Application/NvmeBpWrite/NvmeBpWrite.c:340

• WriteResultToNvRam() no longer checks the return status of SetVariable(). If the write fails (quota, auth, storage error), the tool silently loses the only host-readable outcome signal, making failures harder to diagnose.

  gResult.Magic   = BP_RESULT_MAGIC;
  gResult.Version = BP_RESULT_VERSION;
  gRT->SetVariable (
         L"NvmeBpResult",
         &gNvmeBpResultGuid,

uefi/SrePkg/SrePkg.dec:24

• These new GUID comments still describe a "peeler" identifying the WIM payload, but the PR description explicitly says the peeler-based design is retired. Keeping this wording makes the .dec documentation contradictory and confusing for future readers.

  # Staging UpdateImageTypeId carried by the WIM payload. No FmpDxe instance
  # claims it; it exists only so the peeler can identify the WIM and so stock
  # FmpDxe would harmlessly skip it if the peeler were ever bypassed.

uefi/SrePkg/Include/Library/SreFmpDeviceLib.h:11

• The file header comment still describes the old peeler/staged-WIM design ("small capsule descriptor", "WIM ... is NOT part of this descriptor"), but this PR removes the staging path and ApplyWimToSreStorage now streams the in-memory capsule payload directly. This comment is now misleading.

  This header defines the small "capsule descriptor" that the SRE Firmware
  Management Protocol (FMP) payload carries. The descriptor is what flows
  through the stock capsule path (Capsule-On-Disk -> PEI -> DxeCapsuleLib ->
  FmpDxe -> FmpDeviceLib::SetImage). It is only a few bytes: it records the
  size and hash of the large recovery WIM and where to find it. The ~1 GB WIM

[rogurr]

Again due to how things are handled I would like to examine this more carefully. I'll take the above issue, this, and the misspelling, update the private branch where this was originally created, then ping you again when finished. Maybe just set this PR as draft for now?

[rogurr]

The check is the right thing to do since there is a change if the NVME manufacturer is not reporting things properly. However the flow seemed mis-guided since the granularity no support and any support were being handled in the call, so a fix is being added to this change to move the default value override.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (7)

uefi/SrePkg/Library/SreStorage/SreStorageNvmeLib.c:379

• NvmeSetLockState() programs CDW11 with only the requested partition’s 3-bit field (Config = LockState << Shift) and zeros the rest. Because FID=0x85 encodes both BP0 and BP1 states in the same DWORD, this clobbers the other partition’s write-protection state (e.g., unlocking A implicitly unlocks B; locking A then locking B leaves A unlocked). Preserve the other field via a GET_FEATURES read-modify-write before SET_FEATURES.

  UINT32  Shift;
  UINT32  Config;

  if (PartitionIndex > SrePartition_B) {
    return EFI_INVALID_PARAMETER;

uefi/SrePkg/Include/Library/SreStorage.h:12

• SreStorage.h uses PARTITION_INDEX in its public API but does not include (or define) the type. This makes the header order-dependent (e.g., including SreStorage.h before SreFmpDeviceLib.h breaks consumers). Include the defining header so SreStorage.h is self-contained.

#ifndef _SRE_STORAGE_H_
#define _SRE_STORAGE_H_


//

uefi/SrePkg/Include/Library/SreStorage.h:74

• The SreStorageWriteBlock() doc still refers to a DataSize parameter and EFI_BAD_BUFFER_SIZE, but the function signature no longer provides a size and always writes exactly one BlockSize buffer. Update the comment to match the current API contract.

// Write the input data to the currently indexed block and move the internal block pointer to the
// next block
//
// BlockBuffer - Buffer containing data to write to the storage area, all bytes from the block are written.
//               Must be BlockSize bytes and aligned to BlockBufferAlignment as reported by SreStorageInfo.
//
// EFI_BAD_BUFFER_SIZE - If DataSize != BlockSize
// EFI_END_OF_MEDIA    - If internal indexed block is at end of storage area
// EFI_UNSUPPORTED - If the SRE storage device is not present or supported

uefi/SrePkg/Include/Library/SreFmpDeviceLib.h:11

• The file header comment still describes the old ESP-staged WIM “capsule descriptor” design (WimSize/WimHash, staged file, etc.), but this header now defines SRE_IMAGE_INFO stored at a fixed offset in the boot partition and PARTITION_INDEX. Please update the comment so it matches the current capsule-delivered apply + on-flash version metadata design.

  This header defines the small "capsule descriptor" that the SRE Firmware
  Management Protocol (FMP) payload carries. The descriptor is what flows
  through the stock capsule path (Capsule-On-Disk -> PEI -> DxeCapsuleLib ->
  FmpDxe -> FmpDeviceLib::SetImage). It is only a few bytes: it records the
  size and hash of the large recovery WIM and where to find it. The ~1 GB WIM

uefi/SrePkg/Library/SreFmpDeviceLib/SreFmpDeviceLib.c:56

• SreProgress() uses a static LastPercent that persists across separate update attempts within the same boot. If SetImage is invoked more than once, the first progress callback (0% for partition A) can be suppressed because LastPercent still holds the prior run’s final value. Reset LastPercent when a new update begins.

  STATIC UINTN LastPercent = (UINTN)-1;
  UINTN Percent;

  if (Progress != NULL) {
    Percent = ((BlockIndex * 50) / BlockCount) + (PartitionIndex * 50);

uefi/SrePkg/Application/BpRecoveryLoader/BuildBpFatImage.ps1:122

• The descriptor-window overlap check treats a partition that starts exactly at the end of the reserved window as overlapping. Since the window is described as half-open ([0x4400..0x4400+Bytes)), the comparison should be strict (<), not <=, otherwise a boundary-aligned layout is rejected unnecessarily.

    if ($part.Offset -le ($SreDescriptorOffset + $SreDescriptorBytes)) {

uefi/SrePkg/Application/NvmeBpWrite/NvmeBpWrite.c:340

• WriteResultToNvRam() no longer checks the return status of SetVariable(). If this write fails (quota, permissions, storage error), the host-side readback can observe stale NvmeBpResult data with no in-band indication. Consider restoring the status check + warning log so failures are diagnosable.

  gResult.Magic   = BP_RESULT_MAGIC;
  gResult.Version = BP_RESULT_VERSION;
  gRT->SetVariable (
         L"NvmeBpResult",
         &gNvmeBpResultGuid,

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (5)

uefi/SrePkg/Include/Library/SreStorage.h:12

• SreStorage.h uses PARTITION_INDEX in its public prototypes but does not define the type or include the header that defines it, making this header non-self-contained and forcing include-order requirements on all consumers.

#ifndef _SRE_STORAGE_H_
#define _SRE_STORAGE_H_


//

uefi/SrePkg/Include/Library/SreStorage.h:74

• SreStorageWriteBlock() no longer has a DataSize parameter, so the documentation claiming EFI_BAD_BUFFER_SIZE on DataSize != BlockSize is no longer applicable and is misleading for callers.

//               Must be BlockSize bytes and aligned to BlockBufferAlignment as reported by SreStorageInfo.
//
// EFI_BAD_BUFFER_SIZE - If DataSize != BlockSize
// EFI_END_OF_MEDIA    - If internal indexed block is at end of storage area
// EFI_UNSUPPORTED - If the SRE storage device is not present or supported

uefi/SrePkg/Include/Library/SreFmpDeviceLib.h:8

• The file-level comment still describes the older "capsule descriptor + staged WIM" design, but this header now defines the on-flash SRE_IMAGE_INFO trailer and PARTITION_INDEX. This mismatch makes the contract and intended usage unclear for consumers.

  This header defines the small "capsule descriptor" that the SRE Firmware
  Management Protocol (FMP) payload carries. The descriptor is what flows
  through the stock capsule path (Capsule-On-Disk -> PEI -> DxeCapsuleLib ->
  FmpDxe -> FmpDeviceLib::SetImage). It is only a few bytes: it records the
  size and hash of the large recovery WIM and where to find it. The ~1 GB WIM

uefi/SrePkg/Application/NvmeBpWrite/NvmeBpWrite.c:342

• WriteResultToNvRam() now ignores the EFI_STATUS from SetVariable. If this fails (e.g., NVRAM full/locked), the tool silently loses its primary host-readable result channel, making failures much harder to diagnose.

  gRT->SetVariable (
         L"NvmeBpResult",
         &gNvmeBpResultGuid,
         EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
         sizeof (gResult),

uefi/SrePkg/Library/SreFmpDeviceLib/SreFmpDeviceLib.c:515

• FmpDeviceCheckImageWithStatus() still refers to a "staged WIM" in comments/logs, but the current apply path uses the capsule image buffer directly. Updating the terminology avoids confusion when debugging update failures.

  // Check that the staged WIM will fit into the SRE boot partition
  Status = SreStorageInfo (&BlockCount, &BlockSize, &BlockBufferAlignment);
  if (EFI_ERROR (Status)) {
    DEBUG ((DEBUG_ERROR, "[SRE %a] failed to read the boot partition size - %r\n", __FUNCTION__, Status));
    return Status;

[rogurr]

I saw you purposefully left out the capsules directory. The SreCapsuleBuilder.cmd is the only file that has OEM specific information we should not include here. Maybe replace the 1 line with text that will break the build, but clearly shows this is an auto sign tool that runs on a binary blob?

The SreCapsuleBuilder.py file is generic and shows how a capsule calls the EDK helpers to build the INF, CAP, etc.

And all 4 files currently in the capsules directory are outdated and can be removed.

[rogurr]

There is also a file in the SrePkg root called "SRE Capsule Design Notes.md" that replaces the generic 'plan.md' file and is up to date. If you add that file you should be able to remove the comments in the PR about still needing to remove the old design notation.