Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

specification: Define the interface unbinding flows #113

Merged
merged 1 commit into from
May 7, 2024

Conversation

sameo
Copy link
Collaborator

@sameo sameo commented Apr 28, 2024

Fixes #57

@sameo sameo added the v0.2.0 label Apr 28, 2024
@sameo sameo requested review from rsahita and jyao1 April 28, 2024 11:12
@sameo sameo force-pushed the topic/tdi-unbinding branch 3 times, most recently from ef13c22 to 6f3d217 Compare April 28, 2024 12:20
As the platform resources owner, only the host supervisor domain manager may
decide to unbind an interface from a TVM. At any moment, it can choose to do so
through the `sbi_covh_unbind_interface()` `COVH` ABI. The targeted TSM services
the request by moving the TDI from the TDISP `CONFIG_LOCKED` or `RUN` state back
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it handles ERROR state as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

participant TVM
participant IOMMU

note over TDI: CONFIG_LOCKED or RUN
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or ERROR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks.

VMM ->> TSM: [COVH] sbi_covh_unbind_interface()
TSM ->> TSM: Generate TDISP STOP_INTERFACE_REQUEST
TSM ->> VMM: [COVH] spdm_req(STOP_INTERFACE_REQUEST)
VMM ->> DSM: [DOE] SPDM_STOP_INTERFACE_REQUEST
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel the name SPDM_STOP_INTERFACE_REQUEST is confusing.

It is really a TDIPS protocol in SPDM session. But it is not SPDM protocol.
Maybe just call it spdm_req(STOP_INTERFACE_REQUEST)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, this is fixed now.

TDI ->> TDI: Abort or complete all pending DMA requests
TDI ->> TDI: Scrub all TDI secrets and confidential data
note over TDI: CONFIG_UNLOCKED
DSM ->> VMM: [DOE] SPDM_STOP_INTERFACE_RESPONSE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed as well.

@sameo sameo force-pushed the topic/tdi-unbinding branch from 6f3d217 to 3a3332a Compare May 5, 2024 17:03
Fixes riscv-non-isa#57

Signed-off-by: Samuel Ortiz <sameo@rivosinc.com>
@sameo sameo force-pushed the topic/tdi-unbinding branch from 3a3332a to edc2fc0 Compare May 5, 2024 17:11
@sameo sameo merged commit d886afd into riscv-non-isa:main May 7, 2024
2 checks passed
mapping the TSM initially created for the interface at
xref:binding-flow[TDI binding time].
8. The TSM removes all C-IOMMU, TVM G-stage mappings and the MMIO gpa -> hpa
mapping for the interface.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the late comment - does step 8 include removing all cached C-IOMMU ATC mappings for the interface being unbound?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. We're being explicit about it in #121

Copy link
Collaborator

@rsahita rsahita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with a minor comment/clarification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add section for TDI Interface removal
3 participants