Fix version translation for VersionedRotBootInfo (#2186)

Translate MGS 1-based versions to RoT 0-based versions for the SP/RoT
`versioned_rot_boot_info()` call and clamp the version to be the lesser
of the MGS's and SP's `RotBootInfo::HIGHEST_KNOWN_VERSION`.

The version argument of the VersionedRotBootInfo message was being used
directly for the sprot `versioned_rot_boot_info` call. These are
actually two different name spaces. This change helps when an upcoming
PR introduces new MGS and RoT varients to support the `BootDecisionLog`.

Without this change, a new MGS and RoT running against an old SP will
have some cases where an MGS request is understood by the RoT but the
RoT response cannot be deserialized by the SP and results in sending an
error to the MGS.

Issue #2185

Author: lzrd

drv/lpc55-update-api/src/lib.rs

@@ -161,6 +161,9 @@ pub enum VersionedRotBootInfo {
     V1(RotBootInfo),
     V2(RotBootInfoV2),
 }
+impl VersionedRotBootInfo {
+    pub const HIGHEST_KNOWN_VERSION: u8 = 2;
+}
 
 #[derive(Clone, Copy, Serialize, Deserialize, SerializedSize)]
 pub enum RotPage {

task/control-plane-agent/src/mgs_common.rs

@@ -13,7 +13,6 @@ use drv_sprot_api::{
     CabooseOrSprotError,
     Fwid as SpFwid,
     ImageError as SpImageError,
-    ImageVersion as SpImageVersion,
     RotBootInfo as SpRotBootInfo,
     RotBootInfoV2 as SpRotBootInfoV2,
     RotComponent as SpRotComponent,
@@ -612,6 +611,25 @@ impl MgsCommon {
         Ok(())
     }
 
+    /// Returns the `GwRotBootInfo` version requested by MGS, or falls back to
+    /// the highest supported version that does not exceed the request.
+    ///
+    /// This "best-effort" behavior simplifies the client's logic. Later
+    /// response versions are improvements and the control plane benefits from
+    /// the most complete information for update planning. We provide the best
+    /// available data directly rather than forcing the client to handle errors
+    /// and search for a supported version. Callers that require strict
+    /// versioning can treat version mismatches as an error.
+    ///
+    /// The supported versions are constrained by this SP and the RoT firmware.
+    /// Some version skew between the control plane (MGS), SP, and RoT is
+    /// expected.
+    /// Temporary version skew is **unavoidable** during normal system updates
+    /// but can also result from update failures or they can occur during
+    /// development.
+    /// The fallback logic is designed to handle these mismatches gracefully.
+    /// Introducing a new `GwRotBootInfo` variant requires care, as an
+    /// unhandled mismatch can break the update process.
     pub(crate) fn versioned_rot_boot_info(
         &mut self,
         version: u8,
@@ -620,17 +638,66 @@ impl MgsCommon {
             version
         }));
 
-        match self.sprot.versioned_rot_boot_info(version)? {
-            SpVersionedRotBootInfo::V1(v1) => {
-                Ok(GwRotBootInfo::V1(MgsRotState::from(v1).0))
+        // For full context on the version mapping, see the RoT's
+        // `versioned_rot_boot_info` function in `drv/lpc55-update-server`
+        // and the corresponding MGS data structures.
+
+        // Ensure that the version mapping logic below is
+        // intentionally reviewed whenever a new `GwRotBootInfo` version
+        // or SpVersionedRotBootInfo variant is added.
+        // If these assertions fail, both the `match version` mapping and the
+        // `match self.sprot...` response construction must be updated.
+
+        // Force update of this code if new MGS variants are introduced
+        const _HIGHEST_KNOWN_MGS_VERSION: u8 = 3;
+        static_assertions::const_assert_eq!(
+            GwRotBootInfo::HIGHEST_KNOWN_VERSION,
+            _HIGHEST_KNOWN_MGS_VERSION
+        );
+
+        // Force update of this code if new ROT variants are introduced
+        const HIGHEST_KNOWN_ROT_VERSION: u8 = 2;
+        static_assertions::const_assert_eq!(
+            SpVersionedRotBootInfo::HIGHEST_KNOWN_VERSION,
+            HIGHEST_KNOWN_ROT_VERSION
+        );
+
+        // Map the MGS RotBootInfo 1-based versions to RoT 0-based versions.
+        let rot_version = match version {
+            // There is no version -1 in the RoT version number scheme.
+            0 => return Err(GwSpError::RequestUnsupportedForComponent),
+
+            // See Issue #2193: To maintain compatibility with older systems,
+            // we intentionally preserve a legacy bug where MGS V1 was passed
+            // unaltered to the RoT instead of being mapped to RoT V0.
+            // 1 => 0, // This is the correct mapping.
+            1 => 1, // This is the bug-compatible mapping
+
+            // The remaining mappings are consistent with the design.
+            2 => 1,
+            3 => 2,
+            // Clamp the version to the highest known by the SP.
+            _ => HIGHEST_KNOWN_ROT_VERSION,
+        };
+
+        match self.sprot.versioned_rot_boot_info(rot_version)? {
+            SpVersionedRotBootInfo::V1(rot_v1) => {
+                if version == 1 {
+                    // The MGS request is for a deprecated RoT response.
+                    // Maintain bug compatibility until issue #2193 is resolved.
+                    // Construct an MGS V1 from the more recent RoT V1 struct.
+                    Ok(GwRotBootInfo::V1(MgsRotState::from(rot_v1).0))
+                } else {
+                    // The RoT V1 response corresponds to the MGS V2 or is the
+                    // highest that the RoT could provide.
+                    Ok(GwRotBootInfo::V2(MgsRotStateV2::from(rot_v1).0))
+                }
+            }
+            // The RoT V2 response corresponds to the MGS V3 or is the highest
+            // version the RoT could provide.
+            SpVersionedRotBootInfo::V2(rot_v2) => {
+                Ok(GwRotBootInfo::V3(MgsRotStateV3::from(rot_v2).0))
             }
-            SpVersionedRotBootInfo::V2(v2) => match version {
-                2 => Ok(GwRotBootInfo::V2(MgsRotStateV2::from(v2).0)),
-                // RoT's V2 is MGS V3 and the highest version that we can offer today.
-                _ => Ok(GwRotBootInfo::V3(MgsRotStateV3::from(v2).0)),
-            },
-            // New variants that this code doesn't know about yet will
-            // result in a deserialization error.
         }
     }
 
@@ -693,6 +760,7 @@ impl From<SpSlotId> for MgsRotSlotId {
     }
 }
 
+// Note: this struct and impl go away when issue #2193 is resolved.
 struct MgsRotState(GwRotState);
 
 impl From<SpRotBootInfo> for MgsRotState {
@@ -703,21 +771,19 @@ impl From<SpRotBootInfo> for MgsRotState {
                     active: MgsRotSlotId::from(v1.active).0,
                     slot_a: v1.slot_a_sha3_256_digest.map(|digest| {
                         GwRotImageDetails {
-                            version: MgsImageVersion::from(SpImageVersion {
+                            version: GwImageVersion {
                                 version: 0,
                                 epoch: 0,
-                            })
-                            .0,
+                            },
                             digest,
                         }
                     }),
                     slot_b: v1.slot_b_sha3_256_digest.map(|digest| {
                         GwRotImageDetails {
-                            version: MgsImageVersion::from(SpImageVersion {
+                            version: GwImageVersion {
                                 version: 0,
                                 epoch: 0,
-                            })
-                            .0,
+                            },
                             digest,
                         }
                     }),
@@ -781,17 +847,6 @@ impl From<SpRotBootInfoV2> for MgsRotStateV2 {
     }
 }
 
-struct MgsImageVersion(GwImageVersion);
-
-impl From<SpImageVersion> for MgsImageVersion {
-    fn from(iv: SpImageVersion) -> Self {
-        MgsImageVersion(GwImageVersion {
-            version: iv.version,
-            epoch: iv.epoch,
-        })
-    }
-}
-
 struct MgsRotStateV3(GwRotStateV3);
 
 impl From<SpRotBootInfoV2> for MgsRotStateV3 {