From fae85934ac938e737b6cf9166b3c985fada5fddb Mon Sep 17 00:00:00 2001 From: Fatih Balli Date: Mon, 29 Apr 2024 13:37:30 +0000 Subject: [PATCH] [keymgr] Reuse `revision_seed` in advance calls A cheap solution to the issue raised in #22565. The focus is on: * Not introducing extra seeds that incur extra area. * Cleaner diversifications for Keymgr calls, so that we can claim no message collisons for different use cases. Signed-off-by: Fatih Balli --- hw/ip/keymgr/dv/env/keymgr_scoreboard.sv | 20 ++++++++++++------- hw/ip/keymgr/rtl/keymgr.sv | 8 ++++---- .../chip_sw_keymgr_key_derivation_vseq.sv | 18 +++++++++-------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv index 112544e7c86de6..dda7f7308147fa 100644 --- a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv +++ b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv @@ -13,24 +13,26 @@ class keymgr_scoreboard extends cip_base_scoreboard #( typedef struct packed { bit [keymgr_reg_pkg::NumSwBindingReg-1:0][TL_DW-1:0] SoftwareBinding; - bit [keymgr_pkg::KeyWidth-1:0] HardwareRevisionSecret; bit [keymgr_pkg::DevIdWidth-1:0] DeviceIdentifier; bit [keymgr_pkg::HealthStateWidth-1:0] HealthMeasurement; bit [keymgr_pkg::KeyWidth-1:0] RomDigest; bit [keymgr_pkg::KeyWidth-1:0] DiversificationKey; + bit [keymgr_pkg::KeyWidth-1:0] HardwareRevisionSecret; } adv_creator_data_t; typedef struct packed { // some portions are unused, which are 0s - bit [keymgr_pkg::AdvDataWidth-keymgr_pkg::KeyWidth-keymgr_pkg::SwBindingWidth-1:0] unused; + bit [keymgr_pkg::AdvDataWidth-2*keymgr_pkg::KeyWidth-keymgr_pkg::SwBindingWidth-1:0] unused; bit [keymgr_reg_pkg::NumSwBindingReg-1:0][TL_DW-1:0] SoftwareBinding; - bit [keymgr_pkg::KeyWidth-1:0] OwnerRootSecret; + bit [keymgr_pkg::KeyWidth-1:0] OwnerRootSecret; + bit [keymgr_pkg::KeyWidth-1:0] HardwareRevisionSecret; } adv_owner_int_data_t; typedef struct packed { // some portions are unused, which are 0s - bit [keymgr_pkg::AdvDataWidth-keymgr_pkg::SwBindingWidth-1:0] unused; + bit [keymgr_pkg::AdvDataWidth-keymgr_pkg::KeyWidth-keymgr_pkg::SwBindingWidth-1:0] unused; bit [keymgr_reg_pkg::NumSwBindingReg-1:0][TL_DW-1:0] SoftwareBinding; + bit [keymgr_pkg::KeyWidth-1:0] HardwareRevisionSecret; } adv_owner_data_t; typedef struct packed { @@ -991,20 +993,20 @@ class keymgr_scoreboard extends cip_base_scoreboard #( if (exp_match) `DV_CHECK_EQ(byte_data_q.size, keymgr_pkg::AdvDataWidth / 8) act = {<<8{byte_data_q}}; + exp.HardwareRevisionSecret = keymgr_pkg::RndCnstRevisionSeedDefault; exp.DiversificationKey = cfg.keymgr_vif.flash.seeds[flash_ctrl_pkg::CreatorSeedIdx]; exp.RomDigest = cfg.keymgr_vif.rom_digest.data; exp.HealthMeasurement = cfg.keymgr_vif.keymgr_div; exp.DeviceIdentifier = cfg.keymgr_vif.otp_device_id; - exp.HardwareRevisionSecret = keymgr_pkg::RndCnstRevisionSeedDefault; get_sw_binding_mirrored_value(cdi_type, exp.SoftwareBinding); // The order of the string creation must match the design + `CREATE_CMP_STR(HardwareRevisionSecret) `CREATE_CMP_STR(DiversificationKey) `CREATE_CMP_STR(RomDigest) `CREATE_CMP_STR(HealthMeasurement) `CREATE_CMP_STR(DeviceIdentifier) - `CREATE_CMP_STR(HardwareRevisionSecret) `CREATE_CMP_STR(SoftwareBinding) if (exp_match) begin @@ -1024,11 +1026,13 @@ class keymgr_scoreboard extends cip_base_scoreboard #( act = {<<8{byte_data_q}}; + exp.HardwareRevisionSecret = keymgr_pkg::RndCnstRevisionSeedDefault; exp.OwnerRootSecret = cfg.keymgr_vif.flash.seeds[flash_ctrl_pkg::OwnerSeedIdx]; get_sw_binding_mirrored_value(cdi_type, exp.SoftwareBinding); - `CREATE_CMP_STR(unused) + `CREATE_CMP_STR(HardwareRevisionSecret) `CREATE_CMP_STR(OwnerRootSecret) + `CREATE_CMP_STR(unused) for (int i = 0; i < keymgr_reg_pkg::NumSwBindingReg; i++) begin `CREATE_CMP_STR(SoftwareBinding[i]) end @@ -1050,8 +1054,10 @@ class keymgr_scoreboard extends cip_base_scoreboard #( act = {<<8{byte_data_q}}; + exp.HardwareRevisionSecret = keymgr_pkg::RndCnstRevisionSeedDefault; get_sw_binding_mirrored_value(cdi_type, exp.SoftwareBinding); + `CREATE_CMP_STR(HardwareRevisionSecret) `CREATE_CMP_STR(unused) for (int i=0; i < keymgr_reg_pkg::NumSwBindingReg; i++) begin `CREATE_CMP_STR(SoftwareBinding[i]) diff --git a/hw/ip/keymgr/rtl/keymgr.sv b/hw/ip/keymgr/rtl/keymgr.sv index 2955090d49c48d..4508a1d21d2f9c 100644 --- a/hw/ip/keymgr/rtl/keymgr.sv +++ b/hw/ip/keymgr/rtl/keymgr.sv @@ -434,11 +434,11 @@ module keymgr // rom_digests, // creator_seed}); assign adv_matrix[Creator] = AdvDataWidth'({sw_binding, - revision_seed, otp_device_id_i, lc_keymgr_div_i, rom_digest_i.data, - creator_seed}); + creator_seed, + revision_seed}); assign adv_dvalid[Creator] = creator_seed_vld & devid_vld & @@ -457,11 +457,11 @@ module keymgr otp_key_i.owner_seed_valid}; assign owner_seed = flash_i.seeds[flash_ctrl_pkg::OwnerSeedIdx]; end - assign adv_matrix[OwnerInt] = AdvDataWidth'({sw_binding,owner_seed}); + assign adv_matrix[OwnerInt] = AdvDataWidth'({sw_binding, owner_seed, revision_seed}); assign adv_dvalid[OwnerInt] = owner_seed_vld; // Advance to owner_key - assign adv_matrix[Owner] = AdvDataWidth'(sw_binding); + assign adv_matrix[Owner] = AdvDataWidth'({sw_binding, revision_seed}); assign adv_dvalid[Owner] = 1'b1; // Generate Identity operation input construction diff --git a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_keymgr_key_derivation_vseq.sv b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_keymgr_key_derivation_vseq.sv index a0f90dda43b125..63cef76ee983ac 100644 --- a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_keymgr_key_derivation_vseq.sv +++ b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_keymgr_key_derivation_vseq.sv @@ -30,18 +30,19 @@ class chip_sw_keymgr_key_derivation_vseq extends chip_sw_base_vseq; typedef struct packed { bit [keymgr_reg_pkg::NumSwBindingReg-1:0][TL_DW-1:0] SoftwareBinding; - bit [keymgr_pkg::KeyWidth-1:0] HardwareRevisionSecret; - bit [keymgr_pkg::DevIdWidth-1:0] DeviceIdentifier; - bit [keymgr_pkg::HealthStateWidth-1:0] HealthMeasurement; - bit [keymgr_pkg::KeyWidth-1:0] RomDigest; - bit [keymgr_pkg::KeyWidth-1:0] DiversificationKey; + bit [keymgr_pkg::DevIdWidth-1:0] DeviceIdentifier; + bit [keymgr_pkg::HealthStateWidth-1:0] HealthMeasurement; + bit [keymgr_pkg::KeyWidth-1:0] RomDigest; + bit [keymgr_pkg::KeyWidth-1:0] DiversificationKey; + bit [keymgr_pkg::KeyWidth-1:0] HardwareRevisionSecret; } adv_creator_data_t; typedef struct packed { // some portions are unused, which are 0s - bit [keymgr_pkg::AdvDataWidth-keymgr_pkg::KeyWidth-keymgr_pkg::SwBindingWidth-1:0] unused; + bit [keymgr_pkg::AdvDataWidth-2*keymgr_pkg::KeyWidth-keymgr_pkg::SwBindingWidth-1:0] unused; bit [keymgr_reg_pkg::NumSwBindingReg-1:0][TL_DW-1:0] SoftwareBinding; - bit [keymgr_pkg::KeyWidth-1:0] OwnerRootSecret; + bit [keymgr_pkg::KeyWidth-1:0] OwnerRootSecret; + bit [keymgr_pkg::KeyWidth-1:0] HardwareRevisionSecret; } adv_owner_int_data_t; typedef struct packed { @@ -221,7 +222,6 @@ class chip_sw_keymgr_key_derivation_vseq extends chip_sw_base_vseq; virtual task get_creator_data(output bit [keymgr_pkg::AdvDataWidth-1:0] creator_data_out); adv_creator_data_t creator_data; creator_data.SoftwareBinding = CreatorSwBinding; - creator_data.HardwareRevisionSecret = top_earlgrey_rnd_cnst_pkg::RndCnstKeymgrRevisionSeed; for (int i = 0; i < keymgr_pkg::DevIdWidth / TL_DW; i++) begin bit [TL_DW-1:0] rdata = csr_peek(ral.lc_ctrl.device_id[i]); @@ -247,6 +247,7 @@ class chip_sw_keymgr_key_derivation_vseq extends chip_sw_base_vseq; UVM_LOW) creator_data.DiversificationKey = CreatorFlashSeeds; + creator_data.HardwareRevisionSecret = top_earlgrey_rnd_cnst_pkg::RndCnstKeymgrRevisionSeed; creator_data_out = keymgr_pkg::AdvDataWidth'(creator_data); endtask @@ -254,6 +255,7 @@ class chip_sw_keymgr_key_derivation_vseq extends chip_sw_base_vseq; adv_owner_int_data_t owner_int_data; owner_int_data.SoftwareBinding = OwnerIntSwBinding; owner_int_data.OwnerRootSecret = OwnerFlashSeeds; + owner_int_data.HardwareRevisionSecret = top_earlgrey_rnd_cnst_pkg::RndCnstKeymgrRevisionSeed; return keymgr_pkg::AdvDataWidth'(owner_int_data); endfunction