Repository: chipsalliance/caliptra-sw
Pull request #547 (2023-07-26 20:34:22)

Add version numbers to firmware images
Caliptra has registers for reporting the firmware versions, but Caliptra firmware images do not have versions in their headers. Update the image header to add a version number and report it from RT firmware.

jhand2: > Can we get this PR from draft to official. We would like to write these registers from ROM

Resolved conflicts and promoted to a ready-for-review PR.

Patch:
diff --git a/builder/src/lib.rs b/builder/src/lib.rs
index 7759ce7c..0f58d18e 100644
--- a/builder/src/lib.rs
+++ b/builder/src/lib.rs
@@ -253,8 +253,10 @@ pub fn elf_size(elf_bytes: &[u8]) -> io::Result<u64> {
 }
 
 pub struct ImageOptions {
+    pub fmc_version: u32,
     pub fmc_min_svn: u32,
     pub fmc_svn: u32,
+    pub app_version: u32,
     pub app_min_svn: u32,
     pub app_svn: u32,
     pub vendor_config: ImageGeneratorVendorConfig,
@@ -263,8 +265,10 @@ pub struct ImageOptions {
 impl Default for ImageOptions {
     fn default() -> Self {
         Self {
+            fmc_version: Default::default(),
             fmc_min_svn: Default::default(),
             fmc_svn: Default::default(),
+            app_version: Default::default(),
             app_min_svn: Default::default(),
             app_svn: Default::default(),
             vendor_config: caliptra_image_fake_keys::VENDOR_CONFIG_KEY_0,
@@ -284,12 +288,14 @@ pub fn build_and_sign_image(
     let image = gen.generate(&ImageGeneratorConfig {
         fmc: ElfExecutable::new(
             &fmc_elf,
+            opts.fmc_version,
             opts.fmc_svn,
             opts.fmc_min_svn,
             image_revision_from_git_repo()?,
         )?,
         runtime: ElfExecutable::new(
             &app_elf,
+            opts.app_version,
             opts.app_svn,
             opts.app_min_svn,
             image_revision_from_git_repo()?,
diff --git a/drivers/src/soc_ifc.rs b/drivers/src/soc_ifc.rs
index 804643ab..a975b64b 100644
--- a/drivers/src/soc_ifc.rs
+++ b/drivers/src/soc_ifc.rs
@@ -136,7 +136,7 @@ impl SocIfc {
 
     /// Stop WDT1.
     ///
-    /// This is useful to call from a fatal-error-handling routine.  
+    /// This is useful to call from a fatal-error-handling routine.
     ///
     ///  # Safety
     ///
@@ -224,6 +224,16 @@ impl SocIfc {
             .cptra_flow_status()
             .write(|w| w.ready_for_runtime(true));
     }
+
+    pub fn set_fmc_fw_rev_id(&mut self, fmc_version: u32) {
+        let soc_ifc_regs = self.soc_ifc.regs_mut();
+        soc_ifc_regs.cptra_fw_rev_id().at(0).write(|_| fmc_version);
+    }
+
+    pub fn set_rt_fw_rev_id(&mut self, rt_version: u32) {
+        let soc_ifc_regs = self.soc_ifc.regs_mut();
+        soc_ifc_regs.cptra_fw_rev_id().at(1).write(|_| rt_version);
+    }
 }
 
 bitflags::bitflags! {
diff --git a/fmc/Makefile b/fmc/Makefile
index 6491d54b..1db017fe 100644
--- a/fmc/Makefile
+++ b/fmc/Makefile
@@ -72,10 +72,12 @@ build-fw-image: gen-certs build-emu build-test-rt
 		--ecc-pk-idx 3 \
 		--lms-pk-idx 3 \
 		--fmc $(TARGET_DIR)/caliptra-fmc \
+		--fmc-version 0 \
 		--fmc-svn 0 \
 		--fmc-min-svn 0 \
 		--fmc-rev $(GIT_REV) \
 		--rt $(TARGET_DIR)/caliptra-runtime \
+		--rt-version 0 \
 		--rt-svn 0 \
 		--rt-min-svn 0 \
 		--rt-rev $(GIT_REV) \
diff --git a/image/app/src/create/mod.rs b/image/app/src/create/mod.rs
index efa52e2f..8b8e838a 100644
--- a/image/app/src/create/mod.rs
+++ b/image/app/src/create/mod.rs
@@ -80,6 +80,10 @@ pub(crate) fn run_cmd(args: &ArgMatches) -> anyhow::Result<()> {
         .get_one::<PathBuf>("fmc")
         .with_context(|| "fmc arg not specified")?;
 
+    let fmc_version: &u32 = args
+        .get_one::<u32>("fmc-version")
+        .with_context(|| "fmc-version arg not specified")?;
+
     let fmc_svn: &u32 = args
         .get_one::<u32>("fmc-svn")
         .with_context(|| "fmc-svn arg not specified")?;
@@ -96,6 +100,10 @@ pub(crate) fn run_cmd(args: &ArgMatches) -> anyhow::Result<()> {
         .get_one::<PathBuf>("rt")
         .with_context(|| "rt arg not specified")?;
 
+    let runtime_version: &u32 = args
+        .get_one::<u32>("rt-version")
+        .with_context(|| "rt-version arg not specified")?;
+
     let runtime_svn: &u32 = args
         .get_one::<u32>("rt-svn")
         .with_context(|| "rt-svn arg not specified")?;
@@ -151,6 +159,7 @@ pub(crate) fn run_cmd(args: &ArgMatches) -> anyhow::Result<()> {
     let fmc_rev = hex::decode(fmc_rev)?;
     let fmc = ElfExecutable::open(
         fmc_path,
+        *fmc_version,
         *fmc_svn,
         *fmc_min_svn,
         fmc_rev[..IMAGE_REVISION_BYTE_SIZE].try_into()?,
@@ -159,6 +168,7 @@ pub(crate) fn run_cmd(args: &ArgMatches) -> anyhow::Result<()> {
     let runtime_rev = hex::decode(runtime_rev)?;
     let runtime = ElfExecutable::open(
         runtime_path,
+        *runtime_version,
         *runtime_svn,
         *runtime_min_svn,
         runtime_rev[..IMAGE_REVISION_BYTE_SIZE].try_into()?,
diff --git a/image/app/src/main.rs b/image/app/src/main.rs
index 63818e0a..a1a868bc 100644
--- a/image/app/src/main.rs
+++ b/image/app/src/main.rs
@@ -46,6 +46,11 @@ fn main() {
                 .required(false)
                 .value_parser(value_parser!(String)),
         )
+        .arg(
+            arg!(--"fmc-version" <U32> "FMC Firmware Version Number")
+                .required(true)
+                .value_parser(value_parser!(u32)),
+        )
         .arg(
             arg!(--"fmc-svn" <U32> "FMC Security Version Number")
                 .required(true)
@@ -66,6 +71,11 @@ fn main() {
                 .required(false)
                 .value_parser(value_parser!(String)),
         )
+        .arg(
+            arg!(--"rt-version" <U32> "Runtime Firmware Version Number")
+                .required(true)
+                .value_parser(value_parser!(u32)),
+        )
         .arg(
             arg!(--"rt-svn" <U32> "Runtime Security Version Number")
                 .required(true)
diff --git a/image/elf/src/lib.rs b/image/elf/src/lib.rs
index 676f5ae5..f14c3b06 100644
--- a/image/elf/src/lib.rs
+++ b/image/elf/src/lib.rs
@@ -22,6 +22,7 @@ use std::path::PathBuf;
 /// ELF Executable
 #[derive(Default)]
 pub struct ElfExecutable {
+    version: u32,
     svn: u32,
     min_svn: u32,
     rev: ImageRevision,
@@ -33,16 +34,18 @@ pub struct ElfExecutable {
 impl ElfExecutable {
     pub fn open(
         path: &PathBuf,
+        version: u32,
         svn: u32,
         min_svn: u32,
         rev: ImageRevision,
     ) -> anyhow::Result<Self> {
         let file_data = std::fs::read(path).with_context(|| "Failed to read file")?;
-        ElfExecutable::new(&file_data, svn, min_svn, rev)
+        ElfExecutable::new(&file_data, version, svn, min_svn, rev)
     }
     /// Create new instance of `ElfExecutable`.
     pub fn new(
         elf_bytes: &[u8],
+        version: u32,
         svn: u32,
         min_svn: u32,
         rev: ImageRevision,
@@ -64,6 +67,7 @@ impl ElfExecutable {
         let entry_point = elf_file.ehdr.e_entry as u32;
 
         Ok(Self {
+            version,
             svn,
             min_svn,
             rev,
@@ -100,6 +104,11 @@ impl ElfExecutable {
 }
 
 impl ImageGenratorExecutable for ElfExecutable {
+    /// Executable Version Number
+    fn version(&self) -> u32 {
+        self.version
+    }
+
     /// Executable Security Version Number
     fn svn(&self) -> u32 {
         self.svn
diff --git a/image/gen/src/generator.rs b/image/gen/src/generator.rs
index f6123097..a0ac1a63 100644
--- a/image/gen/src/generator.rs
+++ b/image/gen/src/generator.rs
@@ -248,6 +248,7 @@ impl<Crypto: ImageGeneratorCrypto> ImageGenerator<Crypto> {
             id: id.into(),
             r#type: r#type.into(),
             revision: *image.rev(),
+            version: image.version(),
             svn: image.svn(),
             min_svn: image.min_svn(),
             load_addr: image.load_addr(),
diff --git a/image/gen/src/lib.rs b/image/gen/src/lib.rs
index 2b23e8b3..680ca64b 100644
--- a/image/gen/src/lib.rs
+++ b/image/gen/src/lib.rs
@@ -20,6 +20,9 @@ use caliptra_image_types::*;
 
 /// Image Generator Executable
 pub trait ImageGenratorExecutable {
+    /// Executable Version Number
+    fn version(&self) -> u32;
+
     /// Executable Security Version Number
     fn svn(&self) -> u32;
 
diff --git a/image/types/src/lib.rs b/image/types/src/lib.rs
index 4e836006..012a86f4 100644
--- a/image/types/src/lib.rs
+++ b/image/types/src/lib.rs
@@ -369,6 +369,9 @@ pub struct ImageTocEntry {
     /// Commit revision
     pub revision: ImageRevision,
 
+    // Firmware release number
+    pub version: u32,
+
     /// Security Version Number
     pub svn: u32,
 
diff --git a/rom/dev/Makefile b/rom/dev/Makefile
index d7e0cd53..383fa35e 100644
--- a/rom/dev/Makefile
+++ b/rom/dev/Makefile
@@ -71,10 +71,12 @@ build-fw-image: gen-certs build-test-fmc build-test-rt
 		--ecc-pk-idx 3 \
 		--lms-pk-idx 3 \
 		--fmc $(TARGET_DIR)/caliptra-rom-test-fmc \
+		--fmc-version 0 \
 		--fmc-svn 0 \
 		--fmc-min-svn 0 \
 		--fmc-rev $(GIT_REV) \
 		--rt $(TARGET_DIR)/caliptra-rom-test-rt \
+		--rt-version 0 \
 		--rt-svn 0 \
 		--rt-min-svn 0 \
 		--rt-rev $(GIT_REV) \
diff --git a/rom/dev/src/flow/cold_reset/fw_processor.rs b/rom/dev/src/flow/cold_reset/fw_processor.rs
index 4c966359..4d6685dc 100644
--- a/rom/dev/src/flow/cold_reset/fw_processor.rs
+++ b/rom/dev/src/flow/cold_reset/fw_processor.rs
@@ -91,6 +91,10 @@ impl FirmwareProcessor {
         txn.complete(true)?;
         report_boot_status(FwProcessorFirmwareDownloadTxComplete.into());
 
+        // Update FW version registers
+        env.soc_ifc.set_fmc_fw_rev_id(manifest.fmc.version);
+        env.soc_ifc.set_rt_fw_rev_id(manifest.runtime.version);
+
         // Get the certificate validity info
         let (nb, nf) = Self::get_cert_validity_info(manifest);
 
diff --git a/rom/dev/src/flow/update_reset.rs b/rom/dev/src/flow/update_reset.rs
index e19e011a..4afb0316 100644
--- a/rom/dev/src/flow/update_reset.rs
+++ b/rom/dev/src/flow/update_reset.rs
@@ -81,6 +81,9 @@ impl UpdateResetFlow {
         Self::copy_regions();
         report_boot_status(UpdateResetOverwriteManifestComplete.into());
 
+        // Set RT version. FMC does not change.
+        env.soc_ifc.set_rt_fw_rev_id(manifest.runtime.version);
+
         cprintln!("[update-reset Success] --");
         report_boot_status(UpdateResetComplete.into());
 
diff --git a/sw-emulator/lib/periph/src/soc_reg.rs b/sw-emulator/lib/periph/src/soc_reg.rs
index ae7f92e9..0ddb9e80 100644
--- a/sw-emulator/lib/periph/src/soc_reg.rs
+++ b/sw-emulator/lib/periph/src/soc_reg.rs
@@ -442,6 +442,12 @@ struct SocRegistersImpl {
     #[register_array(offset = 0x00cc, write_fn = on_write_generic_output_wires)]
     cptra_generic_output_wires: [u32; CPTRA_GENERIC_OUTPUT_WIRES_SIZE / 4],
 
+    #[register(offset = 0x00d4)]
+    cptra_hw_rev_id: ReadOnlyRegister<u32>,
+
+    #[register_array(offset = 0x00d8)]
+    cptra_fw_rev_id: [u32; 2],
+
     #[register(offset = 0x00e0, write_fn = write_disabled)]
     cptra_hw_config: u32,
 
@@ -621,6 +627,8 @@ impl SocRegistersImpl {
             cptra_clk_gating_en: ReadOnlyRegister::new(0),
             cptra_generic_input_wires: Default::default(),
             cptra_generic_output_wires: Default::default(),
+            cptra_hw_rev_id: ReadOnlyRegister::new(0),
+            cptra_fw_rev_id: Default::default(),
             cptra_hw_config: 0,
             fuse_uds_seed: words_from_bytes_be(&Self::UDS),
             fuse_field_entropy: [0xffff_ffff; 8],


Test patch:
diff --git a/rom/dev/tests/test_fmcalias_derivation.rs b/rom/dev/tests/test_fmcalias_derivation.rs
index 89f0ea39..9d8087c0 100644
--- a/rom/dev/tests/test_fmcalias_derivation.rs
+++ b/rom/dev/tests/test_fmcalias_derivation.rs
@@ -345,8 +345,10 @@ fn test_fuse_log() {
         owner_config: Some(OWNER_CONFIG),
         fmc_svn: FMC_SVN,
         fmc_min_svn: FMC_MIN_SVN,
+        fmc_version: 0,
         app_svn: FMC_SVN,
         app_min_svn: FMC_MIN_SVN,
+        app_version: 0,
     };
     let image_bundle =
         caliptra_builder::build_and_sign_image(&TEST_FMC_WITH_UART, &APP_WITH_UART, image_options)
diff --git a/runtime/tests/common.rs b/runtime/tests/common.rs
index e2cc9737..c0c00c80 100644
--- a/runtime/tests/common.rs
+++ b/runtime/tests/common.rs
@@ -44,6 +44,8 @@ pub fn run_rt_test(test_bin_name: Option<&'static str>) -> DefaultHwModel {
 
     let mut image_options = ImageOptions::default();
     image_options.vendor_config.pl0_pauser = Some(0xFFFF0000);
+    image_options.fmc_version = 0xaaaaaaaa;
+    image_options.app_version = 0xbbbbbbbb;
     let image =
         caliptra_builder::build_and_sign_image(&FMC_WITH_UART, &runtime_fwid, image_options)
             .unwrap();
diff --git a/runtime/tests/integration_tests.rs b/runtime/tests/integration_tests.rs
index baf8fc52..7922c83d 100644
--- a/runtime/tests/integration_tests.rs
+++ b/runtime/tests/integration_tests.rs
@@ -6,7 +6,7 @@ use caliptra_drivers::Ecc384PubKey;
 use caliptra_hw_model::{HwModel, ModelError, ShaAccMode};
 use caliptra_runtime::{
     CommandId, EcdsaVerifyReq, FipsVersionCmd, FipsVersionResp, FwInfoResp, MailboxReqHeader,
-    MailboxRespHeader,
+    MailboxRespHeader, RtBootStatus,
 };
 use common::{run_rom_test, run_rt_test};
 use openssl::{
@@ -39,15 +39,16 @@ fn test_update() {
 
     model.step_until(|m| m.soc_mbox().status().read().mbox_fsm_ps().mbox_idle());
 
+    let image_options = ImageOptions {
+        app_version: 0xaabbccdd,
+        ..Default::default()
+    };
     // Make image to update to
-    let image = caliptra_builder::build_and_sign_image(
-        &FMC_WITH_UART,
-        &APP_WITH_UART,
-        ImageOptions::default(),
-    )
-    .unwrap()
-    .to_bytes()
-    .unwrap();
+    let image =
+        caliptra_builder::build_and_sign_image(&FMC_WITH_UART, &APP_WITH_UART, image_options)
+            .unwrap()
+            .to_bytes()
+            .unwrap();
 
     model
         .mailbox_execute(u32::from(CommandId::FIRMWARE_LOAD), &image)
@@ -56,6 +57,10 @@ fn test_update() {
     model
         .step_until_output_contains("Caliptra RT listening for mailbox commands...")
         .unwrap();
+
+    let fw_rev = model.soc_ifc().cptra_fw_rev_id().read();
+    assert_eq!(fw_rev[0], 0xaaaaaaaa);
+    assert_eq!(fw_rev[1], 0xaabbccdd);
 }
 
 #[test]
@@ -304,6 +309,18 @@ fn test_fips_cmd_api() {
     assert_eq!(resp, expected_err);
 }
 
+#[test]
+fn test_fw_version() {
+    let mut model = run_rt_test(None);
+    model.step_until(|m| {
+        m.soc_ifc().cptra_boot_status().read() == RtBootStatus::RtReadyForCommands.into()
+    });
+
+    let fw_rev = model.soc_ifc().cptra_fw_rev_id().read();
+    assert_eq!(fw_rev[0], 0xaaaaaaaa);
+    assert_eq!(fw_rev[1], 0xbbbbbbbb);
+}
+
 #[test]
 fn test_unimplemented_cmds() {
     let mut model = run_rom_test("mbox");