Correcting SGX-ISA types and functions related to TDX support (Related to RTE-678)#914
Open
Correcting SGX-ISA types and functions related to TDX support (Related to RTE-678)#914
Conversation
Taowyoo
force-pushed
the
yx/better-name-of-report-mac-struct
branch
from
825b9db to
bdc9bdc
Compare
Taowyoo
requested review from
aditijannu,
gilanghamidy,
jethrogb and
raoulstrackx
gilanghamidy
reviewed
Apr 7, 2026
gilanghamidy
previously requested changes
Apr 7, 2026
gilanghamidy
reviewed
Apr 7, 2026
- Rename the `TdxReportMac` struct to `ReportMac` - Add `TdxReportMac` type alias for backward compatibility
Member
|
@Taowyoo I will help taking over the further revision, so you don't have to worry about it. :) |
gilanghamidy
force-pushed
the
yx/better-name-of-report-mac-struct
branch
5 times, most recently
from
b3ba000 to
35a04aa
Compare
- `verify` function now is part of `ReportMac` instead of `TdxReportV1`, although maintaining compatibility that the `verify` function still exists in `TdxReportV1` - Move the internal `tdx_arch` module to be part of entire `arch` module in the crate. - Move `ReportMac` to the main crate level instead of under `tdx` module namespace - Move all `TdxError` crates out as it is more relevant to `tdx-ql` crate. The error from SGX-ISA should be only the `ErrorCode` types. `TdxError` is not part of the ISA.
gilanghamidy
force-pushed
the
yx/better-name-of-report-mac-struct
branch
from
35a04aa to
eee73c8
Compare
gilanghamidy
dismissed
their stale review
I am taking over this PR, so it will require another reviewer.
jethrogb
reviewed
Apr 8, 2026
Member
jethrogb
left a comment
jethrogb
left a comment
There was a problem hiding this comment.
A lot of my comments are simply about consistency with how other types were already being defined in this crate. Please review all types that were recently added for consistency.
| } | ||
|
|
||
| impl ReportMac { | ||
| pub const UNPADDED_SIZE: usize = 256; |
Member
There was a problem hiding this comment.
Do we need that? Because the MAC itself is verified by everifyreport2, and software basically has no means to verify that. So we don't need the implementation like in the sgx_isa::Report type where there is verify function that uses a closure that splits the MAC and the rest of the data.
gilanghamidy
force-pushed
the
yx/better-name-of-report-mac-struct
branch
2 times, most recently
from
300d702 to
da8a740
Compare
gilanghamidy
force-pushed
the
yx/better-name-of-report-mac-struct
branch
from
da8a740 to
629ca86
Compare
gilanghamidy
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since REPORTMACSTRUCT is used by both SGX and TDX.
The naming should not strict to TDX.
This PR:
ReportMacStructand move to the main crate level instead of undertdxmodule namespace.verifyfunction now is part ofReportMacinstead ofTdxReportV1, although maintaining compatibility that theverifyfunction still exists inTdxReportV1tdx_archmodule to be part of entirearchmodule in the crate.archmodule to allow non-upstreamed new instruction sets to be used without breaking the entire module namespace in the mainlib.rsfile.TdxErrorcrates out as it is more relevant totdx-qlcrate. The error from SGX-ISA should be only theErrorCodetypes.TdxErroris not part of the ISA.ReportTypepossible known constants, instead of a singular constants.