[keymgr_dpe] Replace keymgr in EG - PR4 BootStageOwnerInt / Top dep. parameter

[rroth-lowrisc]

PR4 - BootStageOwnerInt / Top dep. parameter

This PR is the fourth in a series which will replace the keymgr with the kemgr_dpe in earlgrey. The replacement was approved by this RFC.

Merge-Dependencies

• [keymgr_dpe] Replace keymgr in EG - PR3 blocklevel dv #30617
• [keymgr_dpe] Replace keymgr in EG - PR1 otp_ctrl #30615

Relevant Commits

Last two commits

Description

The goal of this PR is twofold:

• Introduce the third bootstage required for the integration of the keymgr_dpe into earlgrey as approved by the RFC.
• Make all necessary parameters top dependent which diverge between darjeeling and earlgrey (to avoid templating the IP)

After this PR is approved, both commits should be squashed together into a single commit.

[gautschimi]

Thanks for this PR. It looks good from my side. Only a few minor comments/questions

[rroth-lowrisc]

The last [CI-Blocker] commit is intentional to prevent the CI from running (at least partially) as discussed with @vogelpi . The reason is to avoid unnecessary runtime of the CI.

I will remove the commit as soon as I get review from the DV team.

[gautschimi]

@rroth-lowrisc if you rebase this branch to master we don't have to reauthorize the changes. (because they are already in master)

[rroth-lowrisc]

@rroth-lowrisc if you rebase this branch to master we don't have to reauthorize the changes. (because they are already in master)

I rebased this PR and #30617 to the latest master

[andreaskurth]

theory_of_operation.md could do with a few updates as well; suggestion:

diff --git a/hw/ip/keymgr_dpe/doc/theory_of_operation.md b/hw/ip/keymgr_dpe/doc/theory_of_operation.md
index fb4adae053..13c334f945 100644
--- a/hw/ip/keymgr_dpe/doc/theory_of_operation.md
+++ b/hw/ip/keymgr_dpe/doc/theory_of_operation.md
@@ -28,7 +28,7 @@ In order to address the relationships among keymgr_dpe slots and their stored DP
 
 ## Key Manager Slots
 
-Keymgr_dpe consists of `DpeNumSlots` slots, which is a generic parameter.
+Keymgr_dpe consists of `NumInstHwSlot` slots, which is a top-level parameter (instantiated up to `NumMaxHwSlot`).
 Each of these key manager slots can store a DPE context, i.e. all DICE-related information for a particular boot stage. That includes a secret key along with additional context information described below.
 The secret key size is fixed to 256-bit.
 
@@ -45,7 +45,8 @@ When a slot is active, `boot_stage` refers to its DPE context boot stage.
 For flexibility, `boot_stage` is a simple unsigned integer that is incremented from the parent's `boot_stage` during advance calls.
 Its actual mapping to boot stages such as ROM, BL0 or Kernel can be determined by SW.
 Hence, keymgr_dpe is oblivious to this mapping between counter values and the boot stages, though with an exception.
-The exception is that the first two stages are treated specially by RTL during KDF advance calls, as they consume other HW-backed inputs that come from keymgr_dpe’s peripheral inputs.
+The exception is that the initial boot stages are treated specially by RTL during KDF advance calls, as they consume other HW-backed inputs that come from keymgr_dpe’s peripheral inputs.
+How many stages consume such HW-backed inputs, and which inputs they consume, depends on the `NumBootStages` parameter (see [Advance](#advance)).
 From SW's point of view, they are still treated as any arbitrary DICE layer in that SW can provide further inputs through `SW_CDI_INPUT` CSR.
 
 A slot's `max_key_version` receives its value from `MAX_KEY_VERSION` register during a previous advance call that ends up populating this slot with DPE context.
@@ -132,11 +133,20 @@ If the OTP creator root key is not valid during the latching cycle, keymgr_dpe m
 
 Further advance calls use the key stored in the specified `CONTROL_SHADOWED.SLOT_SRC_SEL` slot (equally referred to as _parent_ or _source_ slot) , and the result of the derivation updates the slot specified by `CONTROL_SHADOWED.SLOT_DST_SEL`  (referred to as _destination_ or _child_ slot).
 Assuming that `key_policy`, `boot_stage` or `valid` bits of the parent context permit, the child secret is derived from the parent secret through a key derivation function during advance operation.
-`child_key = KDF(parent_key, message)`, where the message input might take few forms depending on the parent slot's `boot_stage` value.
-In particular:
-* If `boot_stage = 0` for the parent, then `message = SW_CDI_INPUT || hw_revision_seed || device_identifier || health_st_measurement || rom_descriptors || creator_seed`. See [KDF Details](#kdf-details) for more details on HW-backed inputs.
-* If `boot_stage = 1` for the parent, then `message = SW_CDI_INPUT || owner_seed`.
-* If `boot_stage > 1` for the parent, then `message = SW_CDI_INPUT`.
+`child_key = KDF(parent_key, message)`, where the message input takes different forms depending on the parent slot's `boot_stage` value.
+Which HW-backed inputs are consumed at each stage is controlled by the `NumBootStages` parameter, and in particular whether the `creator_seed` is consumed together with the other creator inputs (two-stage configuration) or by a dedicated intermediate owner stage (three-stage configuration).
+See [KDF Details](#kdf-details) for more details on HW-backed inputs.
+
+With `NumBootStages = 3`, the `creator_seed` is consumed by a dedicated `OwnerInt` (owner intermediate) stage:
+* If `boot_stage = 0` (Creator) for the parent, then `message = SW_CDI_INPUT || device_identifier || health_st_measurement || rom_descriptors || hw_revision_seed`.
+* If `boot_stage = 1` (OwnerInt) for the parent, then `message = SW_CDI_INPUT || creator_seed`.
+* If `boot_stage = 2` (Owner) for the parent, then `message = SW_CDI_INPUT || owner_seed`.
+* If `boot_stage > 2` for the parent, then `message = SW_CDI_INPUT`.
+
+With `NumBootStages = 2`, the `OwnerInt` stage is omitted; the `creator_seed` is consumed together with the other creator inputs, and an advance from `Creator` increments `boot_stage` directly to `Owner` (i.e. `boot_stage = 1` is not used):
+* If `boot_stage = 0` (Creator) for the parent, then `message = SW_CDI_INPUT || hw_revision_seed || device_identifier || health_st_measurement || rom_descriptors || creator_seed`.
+* If `boot_stage = 2` (Owner) for the parent, then `message = SW_CDI_INPUT || owner_seed`.
+* If `boot_stage > 2` for the parent, then `message = SW_CDI_INPUT`.
 
 At the end of a successful advance operation, the following updates are made for the slot selected by `SLOT_DST_SEL`:
 * `valid` bit is set to 1.
@@ -161,7 +171,7 @@ When there is no fault and the enable signal is active by life cycle controller,
   * If `retain_parent = true`, then the source and the destination slots are different.
   * If `retain_parent = true`, then the destination slot is not valid (i.e. `valid = 0`).
   * If `retain_parent = false`, then the source and the destination slots are the same.
-  * `boot_stage` of the source slot has not reached to the maximum value supported by HW (i.e. `boot_stage + 1 < DpeNumBootStages`.
+  * `boot_stage` of the source slot has not reached to the maximum value supported by HW (i.e. `boot_stage + 1 < NumBootStages`.
 
 
 ### Versioned Key Generation
@@ -212,7 +222,7 @@ During advance operations, KDF inputs are 0 padded to `AdvDataWidth` bits. Depen
 * `hw_revision_seed` is a 256-bit netlist constant.
 * `device_identifier` is a 256-bit non-secret device identifier. This value is received from peripheral OTP port.
 * `health_st_measurement` is a 128-bit domain separator (i.e. diversification constant) that depends on the life cycle stage. This value is received from peripheral LC port.
-* `rom_descriptors` are two hash values for ROM0, ROM1. Each digest is 256-bits. These values are received from their respective ROM controllers.
+* `rom_descriptors` are `NumRomDigestInputs` hash values received from the respective ROM controller(s). Each digest is 256-bits.
 * `creator_seed` is 256-bit creator secret received from the `SECRET2` OTP partition..
 * `owner_seed` is 256-bit owner secret received from the `SECRET3` OTP partition.

[andreaskurth]

The invalid_creator_seed/invalid_owner_seed register fields currently don't seem to be predicted/checked nor stimulated by DV. Do you have them on the testplan? If not, let's create an issue to track this (the issue should be created before this PR gets merged, but the resolution of the issue is not urgent).

[rroth-lowrisc]

I will add a test in the testplan, the PR is already open so the overhead is nearly zero

[rroth-lowrisc]

Currently the testplan checks only if I have invalid hw input data e.g. all 0's or all 1's

The current testplan has a test which verifies if the signals are valid, it only has to check the debug register too.

[rroth-lowrisc]

theory_of_operation.md could do with a few updates as well; suggestion:

diff --git a/hw/ip/keymgr_dpe/doc/theory_of_operation.md b/hw/ip/keymgr_dpe/doc/theory_of_operation.md
index fb4adae053..13c334f945 100644
--- a/hw/ip/keymgr_dpe/doc/theory_of_operation.md
+++ b/hw/ip/keymgr_dpe/doc/theory_of_operation.md
@@ -28,7 +28,7 @@ In order to address the relationships among keymgr_dpe slots and their stored DP
 
 ## Key Manager Slots
 
-Keymgr_dpe consists of `DpeNumSlots` slots, which is a generic parameter.
+Keymgr_dpe consists of `NumInstHwSlot` slots, which is a top-level parameter (instantiated up to `NumMaxHwSlot`).
 Each of these key manager slots can store a DPE context, i.e. all DICE-related information for a particular boot stage. That includes a secret key along with additional context information described below.
 The secret key size is fixed to 256-bit.
 
@@ -45,7 +45,8 @@ When a slot is active, `boot_stage` refers to its DPE context boot stage.
 For flexibility, `boot_stage` is a simple unsigned integer that is incremented from the parent's `boot_stage` during advance calls.
 Its actual mapping to boot stages such as ROM, BL0 or Kernel can be determined by SW.
 Hence, keymgr_dpe is oblivious to this mapping between counter values and the boot stages, though with an exception.
-The exception is that the first two stages are treated specially by RTL during KDF advance calls, as they consume other HW-backed inputs that come from keymgr_dpe’s peripheral inputs.
+The exception is that the initial boot stages are treated specially by RTL during KDF advance calls, as they consume other HW-backed inputs that come from keymgr_dpe’s peripheral inputs.
+How many stages consume such HW-backed inputs, and which inputs they consume, depends on the `NumBootStages` parameter (see [Advance](#advance)).
 From SW's point of view, they are still treated as any arbitrary DICE layer in that SW can provide further inputs through `SW_CDI_INPUT` CSR.
 
 A slot's `max_key_version` receives its value from `MAX_KEY_VERSION` register during a previous advance call that ends up populating this slot with DPE context.
@@ -132,11 +133,20 @@ If the OTP creator root key is not valid during the latching cycle, keymgr_dpe m
 
 Further advance calls use the key stored in the specified `CONTROL_SHADOWED.SLOT_SRC_SEL` slot (equally referred to as _parent_ or _source_ slot) , and the result of the derivation updates the slot specified by `CONTROL_SHADOWED.SLOT_DST_SEL`  (referred to as _destination_ or _child_ slot).
 Assuming that `key_policy`, `boot_stage` or `valid` bits of the parent context permit, the child secret is derived from the parent secret through a key derivation function during advance operation.
-`child_key = KDF(parent_key, message)`, where the message input might take few forms depending on the parent slot's `boot_stage` value.
-In particular:
-* If `boot_stage = 0` for the parent, then `message = SW_CDI_INPUT || hw_revision_seed || device_identifier || health_st_measurement || rom_descriptors || creator_seed`. See [KDF Details](#kdf-details) for more details on HW-backed inputs.
-* If `boot_stage = 1` for the parent, then `message = SW_CDI_INPUT || owner_seed`.
-* If `boot_stage > 1` for the parent, then `message = SW_CDI_INPUT`.
+`child_key = KDF(parent_key, message)`, where the message input takes different forms depending on the parent slot's `boot_stage` value.
+Which HW-backed inputs are consumed at each stage is controlled by the `NumBootStages` parameter, and in particular whether the `creator_seed` is consumed together with the other creator inputs (two-stage configuration) or by a dedicated intermediate owner stage (three-stage configuration).
+See [KDF Details](#kdf-details) for more details on HW-backed inputs.
+
+With `NumBootStages = 3`, the `creator_seed` is consumed by a dedicated `OwnerInt` (owner intermediate) stage:
+* If `boot_stage = 0` (Creator) for the parent, then `message = SW_CDI_INPUT || device_identifier || health_st_measurement || rom_descriptors || hw_revision_seed`.
+* If `boot_stage = 1` (OwnerInt) for the parent, then `message = SW_CDI_INPUT || creator_seed`.
+* If `boot_stage = 2` (Owner) for the parent, then `message = SW_CDI_INPUT || owner_seed`.
+* If `boot_stage > 2` for the parent, then `message = SW_CDI_INPUT`.
+
+With `NumBootStages = 2`, the `OwnerInt` stage is omitted; the `creator_seed` is consumed together with the other creator inputs, and an advance from `Creator` increments `boot_stage` directly to `Owner` (i.e. `boot_stage = 1` is not used):
+* If `boot_stage = 0` (Creator) for the parent, then `message = SW_CDI_INPUT || hw_revision_seed || device_identifier || health_st_measurement || rom_descriptors || creator_seed`.
+* If `boot_stage = 2` (Owner) for the parent, then `message = SW_CDI_INPUT || owner_seed`.
+* If `boot_stage > 2` for the parent, then `message = SW_CDI_INPUT`.
 
 At the end of a successful advance operation, the following updates are made for the slot selected by `SLOT_DST_SEL`:
 * `valid` bit is set to 1.
@@ -161,7 +171,7 @@ When there is no fault and the enable signal is active by life cycle controller,
   * If `retain_parent = true`, then the source and the destination slots are different.
   * If `retain_parent = true`, then the destination slot is not valid (i.e. `valid = 0`).
   * If `retain_parent = false`, then the source and the destination slots are the same.
-  * `boot_stage` of the source slot has not reached to the maximum value supported by HW (i.e. `boot_stage + 1 < DpeNumBootStages`.
+  * `boot_stage` of the source slot has not reached to the maximum value supported by HW (i.e. `boot_stage + 1 < NumBootStages`.
 
 
 ### Versioned Key Generation
@@ -212,7 +222,7 @@ During advance operations, KDF inputs are 0 padded to `AdvDataWidth` bits. Depen
 * `hw_revision_seed` is a 256-bit netlist constant.
 * `device_identifier` is a 256-bit non-secret device identifier. This value is received from peripheral OTP port.
 * `health_st_measurement` is a 128-bit domain separator (i.e. diversification constant) that depends on the life cycle stage. This value is received from peripheral LC port.
-* `rom_descriptors` are two hash values for ROM0, ROM1. Each digest is 256-bits. These values are received from their respective ROM controllers.
+* `rom_descriptors` are `NumRomDigestInputs` hash values received from the respective ROM controller(s). Each digest is 256-bits.
 * `creator_seed` is 256-bit creator secret received from the `SECRET2` OTP partition..
 * `owner_seed` is 256-bit owner secret received from the `SECRET3` OTP partition.

My original goal was to write a later on a documentation PR which introduces all the new features. But probably better to append a minimalistic version when changing the RTL.

[andreaskurth]

My original goal was to write a later on a documentation PR which introduces all the new features. But probably better to append a minimalistic version when changing the RTL.

Yes, let's please change everything that would become wrong or outdated in the documentation in the same PR as the RTL. Otherwise, we incur too much risk of not doing the required documentation updates and/or getting confused by doc-code mismatches in the meantime.

I agree that substantial extensions of the documentation to cover new features in greater detail can wait until follow-up PRs, but whenever we are planning to do that, we need an issue collecting a summary of the pending extensions to track the remaining work.

[rroth-lowrisc]

Thanks for the review @andreaskurth
I squashed these two commits together to avoid introducing breaking commits.
AscentLint for darjeeling is passing (keymgr_dpe is currently only included in the darjeeling configuration)

[andreaskurth]

LGTM, thanks @rroth-lowrisc!

Obviously, please rebase on master after #30617 has been merged and before merging this PR :-)

[rroth-lowrisc]

@martin-velay I implemented your review
Could you check the file chip_sw_keymgr_dpe_key_derivation_vseq.sv again?
I had there some additional (minor) changes since you reviewed the file.

[martin-velay]

It looks good, I haven't got time to review it deeply though and the Darjeeling top vseq would need a complete rework at some point anyway.

[andreaskurth]

Thanks @rroth-lowrisc!