Add Slot management by alistair23 · Pull Request #3668 · DMTF/libspdm

alistair23 · 2026-06-29T05:29:11Z

This builds on top of the pending PR for bank support in libspdm and utilises some parts of #3637, while also replacing it.

This replaces #3637 with a few minor fixes, but otherwise mostly just rebases #3637 on #3629 to have libspdm manage the banks instead of the HAL

jyao1 · 2026-06-29T14:31:49Z

 **/
 extern bool libspdm_update_local_cert_chain(
    void *spdm_context,
+    uint8_t bank_id,


I suggest we use uint8_t *bank_id, to support legacy SET_CERT API, where bank_id is not provided.

See my HAL update at 613af0a.

alistair23 · 2026-07-01T04:54:42Z

Ready to go!

jyao1 · 2026-07-01T05:57:55Z

A fundamental question: Since spdm_context does not include key_pair_info, how do you sync key_pair_info with the slot_management/bank information? How can you guarantee they are still in sync?

To me, it is weird design to give key_pair_info to integrator and only expand lots of bank data into spdm_context. The inconsistency worried me a lot.

alistair23 · 2026-07-02T03:27:10Z

A fundamental question: Since spdm_context does not include key_pair_info, how do you sync key_pair_info with the slot_management/bank information? How can you guarantee they are still in sync?

To me, it is weird design to give key_pair_info to integrator and only expand lots of bank data into spdm_context. The inconsistency worried me a lot.

Yeah, key_pair_info will need to be fixed and included in libspdm. Then all of the sync issues will be solved!

I agree it's a bit weird at the moment, but key_pair_info can be updated. It provides a huge simplification for the implementation

jyao1 · 2026-07-07T08:04:51Z

Yeah, key_pair_info will need to be fixed and included in libspdm. Then all of the sync issues will be solved!

I think the is the fundamental design choice.

Option 1: Put key_pair_info + slot_management into libspdm context (sync by libspdm)
Option 2: Leave key_pair_info + slot_management to the integrator (sync by device integrator. libspdm can provide sample lib for managing those data - see Slot Management — Relational Data Model Proposal #3646)

I prefer option 2.
The reason is that this management work belongs to device non-volatile state change, while libspdm_context is design to carry the required info for this connection. Even if we choose option 1 to extend the libspdm context includes these info, the device vendor still need to carry these non-volatile info somewhere. I treat it as duplicated effort.

alistair23 · 2026-07-13T02:23:13Z

The reason is that this management work belongs to device non-volatile state change, while libspdm_context is design to carry the required info for this connection.

Couldn't agree more. Which is why we need to store the bank information in libspdm_context, so that libspdm can handle the connection and slot management.

Even if we choose option 1 to extend the libspdm context includes these info, the device vendor still need to carry these non-volatile info somewhere. I treat it as duplicated effort.

The HAL will need to handle reading/writing the information from/to something non-volatile. There isn't much we can do about that. But with this approach that is all the HAL needs to do. Instead of managing the connection as well, like you try to do here: https://github.com/DMTF/libspdm/pull/3637/changes#diff-f223c3bf81dcc56487fc5e8f7b3f8538bdeacb2d7e3e20a04922582cbadd74b7R702 (which doesn't work as there are a range of sync issues).

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

If erasing a certificate that is in the libspdm state, but wasn't previously written to disk (as can happen when running the unit tests) then allow creating the file when opening it. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Add the LIBSPDM_ENABLE_CAPABILITY_SLOT_MGMT_CAP config switch, the internal common-lib declaration, and the SLOT_MANAGEMENT / SLOT_MANAGEMENT_RESP command-name map entries. Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: Claude Code:claude-opus-4-8

Add the requester functions for the SLOT_MANAGEMENT SubCodes: libspdm_slot_management_get_supported_subcodes, _get_bank_info, _get_bank_details, _get_certificate_chain, _manage_bank, _manage_slot, _get_csr, and _set_certificate. Each sends SLOT_MANAGEMENT with its SubCode and parses the SLOT_MANAGEMENT_RESP, gated by SLOT_MGMT_CAP. The GetCSR and SetCertificate APIs mirror libspdm_get_csr_ex and libspdm_set_certificate, with the added ability to address a Bank; _set_certificate also takes a KeyPairID (Table 147), zero when MULTI_KEY_CONN_RSP is false and non-zero otherwise. Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: Claude Code:claude-opus-4-8

Add libspdm_get_response_slot_management, which validates the request and dispatches on the SLOT_MANAGEMENT SubCode to a dedicated per-SubCode handler: SupportedSubCodes, GetBankInfo, GetBankDetails, GetCertificateChain, ManageBank, ManageSlot, GetCSR, and SetCertificate. Other SubCodes return ERROR(UnsupportedRequest). GetCertificateChain reads the certificate chain through the HAL, and the GetBankDetails slot digest is provided by the HAL over that same chain. SetCertificate conforms to SET_CERTIFICATE (DSP0274 Table 142). It rejects an unknown BankID up front with ERROR(InvalidRequest) (like the other SubCodes), then applies the same KeyPairID / CertModel validation base SET_CERTIFICATE does: with MULTI_KEY_CONN_RSP the KeyPairID shall be non-zero and CertModel a valid non-NONE model, otherwise both shall be zero and the effective model is derived from ALIAS_CERT_CAP. It verifies the certificate chain with the same libspdm_set_cert_verify_certchain() helper base uses (made non-static for reuse, gated by LIBSPDM_CERT_PARSE_SUPPORT; a failed verify returns ERROR(Unspecified) and retains the existing certificate), then reuses the SET_CERTIFICATE HAL hook libspdm_update_local_cert_chain, passing a pointer to the addressed BankID (the legacy SET_CERTIFICATE responder passes NULL); a HAL write failure is mapped Busy -> BUSY, otherwise -> Unspecified, matching the base SET_CERTIFICATE install path. GetCSR reuses the GET_CSR HAL hook libspdm_gen_csr unchanged (no Bank parameter; the CSR is generated from the negotiated algorithm). Register the handler in the responder dispatch table, gated by SLOT_MGMT_CAP. Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: Claude Code:claude-opus-4-8

Add requester tests for the SLOT_MANAGEMENT SubCodes: SupportedSubCodes (success and ERROR paths), GetBankInfo, GetBankDetails, GetCertificateChain, ManageBank, ManageSlot, GetCSR, and SetCertificate. Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: Claude Code:claude-opus-4-8

Add responder tests for the SLOT_MANAGEMENT SubCodes: SupportedSubCodes (success and error paths), GetBankInfo, GetBankDetails, GetCertificateChain, ManageBank (incl. consistency with GET_KEY_PAIR_INFO), ManageSlot (Erase), GetCSR, and SetCertificate. Signed-off-by: Jiewen Yao <jiewen.yao@intel.com> Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Include the spdm_context in the internal helper functions. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

alistair23 · 2026-07-22T20:23:41Z

Originally, we let the device handle that in sample lib. Code in libspdm is good.
Now, with this approach, libspdm itself is buggy because it does not have consistent state internally.

What is inconsistent?

Please also clarify: What test you have designed and run to ensure the slot/bank/algo/key_pair are in sync in any cases.

You don't think the current tests do that? What would you propose?

jyao1 · 2026-07-23T02:20:41Z

Originally, we let the device handle that in sample lib. Code in libspdm is good.
Now, with this approach, libspdm itself is buggy because it does not have consistent state internally.

What is inconsistent?

@alistair23
Since SPDM spec has different way to return same/similar info, the device must maintain the consistent state between slot/bank/algo/key_pair, if the libspdm uses different way to save the info.
For example:
DIGEST.KeyPairID is linked to KEY_PAIR_INFO.AssocCertSlotMask
DIGEST.KeyUsageMask is linked to KEY_PAIR_INFO.CurrentKeyUsage

Just 2 examples, the SPDM spec requires more. Please read the spec carefully to ensure you understand the link between them. There are others in slot_management command as well.

Optionally, you can refer to my document at https://github.com/jyao1/libspdm/blob/slot_management/doc/slot_management_database.md. (where I did consider the state sync.)

Anyway, if you just update one commend one time, then you break the state consistency across state.
Even worse, if you are not aware of this consistency requirement in SPDM spec, you even do not know you are making mistake.
That is exactly what I am worried about.

Please also clarify: What test you have designed and run to ensure the slot/bank/algo/key_pair are in sync in any cases.

You don't think the current tests do that? What would you propose?

This patch is very big. As such, I am not sure what test you have covered and I might miss something.

Since you are the owner to add the test, I would like to ask you. Please list all of cross state consistency related test for evaluation.

Again, please read the SPDM spec, to ensure the tests have fully covered all the scenarios for slot/bank/algo/key_pair change and the tests have fully covered the changed state check, since you are updating libspdm core code.
Just checking the command response according to the request is far from enough.

That will also help me to evaluate if there is something missing.

alistair23 · 2026-07-23T17:24:11Z

Since SPDM spec has different way to return same/similar info, the device must maintain the consistent state between slot/bank/algo/key_pair, if the libspdm uses different way to save the info. For example: DIGEST.KeyPairID is linked to KEY_PAIR_INFO.AssocCertSlotMask DIGEST.KeyUsageMask is linked to KEY_PAIR_INFO.CurrentKeyUsage

Just 2 examples, the SPDM spec requires more. Please read the spec carefully to ensure you understand the link between them. There are others in slot_management command as well.

It wasn't clear in the spec that DIGEST.KeyUsageMask is updated directly from the KeyPair.

The confusing part is that it isn't updated in libspdm today from the KeyPair and instead is set separately, so it seems like it is always out of date already. I have updated this PR to fix that issue by removing local_key_usage_bit_mask entirely.

I'm on holidays for two weeks. Maybe it would be best if you could read through the PR and comment explicitly on any areas where you think there are consistency issues? It's hard to address or respond to generic "consistency issues", but I can easily fix any bugs that you see.

AFAIK this PR doesn't add any consistency issues, in fact it removes a lot as it consolidates a bunch of competing information into a single source of truth. KeyPairs today for example are practically always out of sync and this PR does a pretty good job of cleaning that up.

This patch is very big. As such, I am not sure what test you have covered and I might miss something.

Yeah, I wanted to do it step by step, a little bit at a time to make it easy to review and test, but you didn't like that approach. So we now have a giant PR. I have added some more tests to compare consistency issues, hopefully that helps

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

We don't need to store the supported slot mask or the KeyPairIDs as we can calculate the information on the fly. Storing the information means it can become out of date, so let's just remove it. Resolves: DMTF#3638 Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Instead of manually setting the local_key_usage_bit_mask mask let's just get the value from the KeyPairs, that way the values are always kept up-to-date. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Signed-off-by: Alistair Francis <alistair.francis@wdc.com> Assisted-by: Devin:claude-sonet-5

jyao1 · 2026-07-24T04:05:31Z

I'm on holidays for two weeks. Maybe it would be best if you could read through the PR and comment explicitly on any areas where you think there are consistency issues? It's hard to address or respond to generic "consistency issues", but I can easily fix any bugs that you see.

That is exactly my concern. I feel like you are not aware of what consistency requirement in SPDM spec, and you rely my to find out that in your code. E.g. You fixed DIGEST/KEY_PAIR syncing problem after I point it out. But BANK and KEY_PAIR also need to keep syncing, I am not sure how you are doing that. (Please correct me if I am wrong.)
The development process should be: 1) read spec and understand the requirement, 2) propose a design and write test to ensure the spec requirement is followed, 3) then submit PR and ask for code review for final check.

In general, I do not feel it is a good idea to ask other people to find bug for you, then you fix it.
To me, the code review should be the last step, assuming everything is already correct, and just let other people confirm you are doing right thing.
So, please read SPDM spec and design test to cover the corner case for consistency between bank/slot/algo/key_pair.
I would like to review your design doc and test design description, before jump into the code.

AFAIK this PR doesn't add any consistency issues, in fact it removes a lot as it consolidates a bunch of competing information into a single source of truth. KeyPairs today for example are practically always out of sync and this PR does a pretty good job of cleaning that up.

I cannot agree with this part. And that is exactly my concern.
Original the consistency issue is in device sample. I agree it is problem (as I have submitted the issue), but at least Libspdm code lib is clean.
With your design, you moved the consistency issue into libspdm core lib. That changed the position completely. That is the reason that I am worried and ask for evidence of validation and production quality.

Yeah, I wanted to do it step by step, a little bit at a time to make it easy to review and test, but you didn't like that approach. So we now have a giant PR. I have added some more tests to compare consistency issues, hopefully that helps

OK. usually, for a clear architecture or a clear design, I prefer step by step. A small patch to resolve one specific problem.
But here, not the case, because you reject my ask for design review, then I have no way to understand your design. I have to ask you to provide everything.
A full design cannot be step by step. That will cause problem easily. Just like how easy we introduce the syncing issue for DIGEST and KeyPair. Now we have to deal with it.
And I want to prevent such thing from happening again for bank / slot_management.

Let me clarify my goal again: a clean architecture to resolve any data syncing issue between slot/bank/algo/key_pair according to SPDM specification.

jyao1 reviewed Jun 29, 2026

View reviewed changes

alistair23 force-pushed the alistair/bank-support-wip branch 13 times, most recently from 0d3da72 to b02f4f8 Compare July 1, 2026 04:06

alistair23 marked this pull request as ready for review July 1, 2026 04:53

alistair23 requested a review from steven-bellock as a code owner July 1, 2026 04:53

alistair23 mentioned this pull request Jul 1, 2026

add Slot management #3637

Open

jyao1 reviewed Jul 1, 2026

View reviewed changes

Comment thread include/internal/libspdm_common_lib.h

jyao1 reviewed Jul 1, 2026

View reviewed changes

Comment thread include/internal/libspdm_common_lib.h

jyao1 reviewed Jul 1, 2026

View reviewed changes

Comment thread include/library/spdm_common_lib.h

alistair23 force-pushed the alistair/bank-support-wip branch 5 times, most recently from 37f3a05 to 214b10b Compare July 20, 2026 19:55

alistair23 and others added 9 commits July 22, 2026 11:25

libspdm_com_context_data: Allow setting SLOT_MANAGEMENT subcodes

786a624

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

libspdm_com_context_data: Allow setting bank crypto algs

e0129e9

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

os_stub: spdm_device_secret_lib_sample: Include spdm_context

5941980

Include the spdm_context in the internal helper functions. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

alistair23 force-pushed the alistair/bank-support-wip branch 4 times, most recently from 21a3575 to e0ad362 Compare July 22, 2026 20:22

alistair23 force-pushed the alistair/bank-support-wip branch from e0ad362 to f651aea Compare July 22, 2026 20:26

alistair23 force-pushed the alistair/bank-support-wip branch 2 times, most recently from 3d22169 to cf3e100 Compare July 23, 2026 17:23

alistair23 force-pushed the alistair/bank-support-wip branch 4 times, most recently from fedda3d to dca9c55 Compare July 23, 2026 17:39

alistair23 added 5 commits July 23, 2026 14:01

Store KeyPair as part of libspdm context

def6326

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

hal: responder: Add a ManageBank callback

d8cabb0

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Remove the local local_key_usage_bit_mask

94fc5cc

Instead of manually setting the local_key_usage_bit_mask mask let's just get the value from the KeyPairs, that way the values are always kept up-to-date. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

unit_test: Add tests for KeyPair and SLOT_MANAGEMENT consistency checks

4d868f8

Signed-off-by: Alistair Francis <alistair.francis@wdc.com> Assisted-by: Devin:claude-sonet-5

alistair23 force-pushed the alistair/bank-support-wip branch from dca9c55 to 4d868f8 Compare July 23, 2026 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Slot management - #3668

Add Slot management#3668
alistair23 wants to merge 18 commits into
DMTF:mainfrom
alistair23:alistair/bank-support-wip

alistair23 commented Jun 29, 2026 •

edited

Loading

Uh oh!

jyao1 Jun 29, 2026

Uh oh!

alistair23 commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyao1 commented Jul 1, 2026

Uh oh!

alistair23 commented Jul 2, 2026

Uh oh!

jyao1 commented Jul 7, 2026

Uh oh!

alistair23 commented Jul 13, 2026

Uh oh!

alistair23 commented Jul 22, 2026

Uh oh!

jyao1 commented Jul 23, 2026 •

edited

Loading

Uh oh!

alistair23 commented Jul 23, 2026

Uh oh!

jyao1 commented Jul 24, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

alistair23 commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyao1 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

alistair23 commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyao1 commented Jul 1, 2026

Uh oh!

alistair23 commented Jul 2, 2026

Uh oh!

jyao1 commented Jul 7, 2026

Uh oh!

alistair23 commented Jul 13, 2026

Uh oh!

alistair23 commented Jul 22, 2026

Uh oh!

jyao1 commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alistair23 commented Jul 23, 2026

Uh oh!

jyao1 commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alistair23 commented Jun 29, 2026 •

edited

Loading

jyao1 commented Jul 23, 2026 •

edited

Loading

jyao1 commented Jul 24, 2026 •

edited

Loading