Open krystian-hebel opened 1 year ago
About a half of currently listed issues was addressed in https://github.com/TrenchBoot/documentation/pull/25. The rest is planned to land in the specification together with changes specific to AMD processors.
@SergiiDmytruk I think there is no off-by-one error in "TPM Extend Operation". E_0
describes extend operation for Obj_0
and E_n
- for Obj_n
, not Obj_(n-1)
. Can we remove that from the list above?
I'm also not sure about using i
for index in "Measuring the Policy". Is this just to differentiate those from names used in "TPM Extend Operation"?
I think there is no off-by-one error in "TPM Extend Operation".
E_0
describes extend operation forObj_0
andE_n
- forObj_n
, notObj_(n-1)
. Can we remove that from the list above?
I meant that the way it's written there are (n + 1)
objects in the range [0;n]
(both ends included), instead of n
of them ([0;n)
or [1; n]
). Same in "Measuring the policy".
I'm also not sure about using
i
for index in "Measuring the Policy". Is this just to differentiate those from names used in "TPM Extend Operation"?
If there is only Entry_n = ...
and Entry_0, ..., Entry_n
, it looks like definitions of Entry_0
through Entry_{n-1}
are missing. i
usually suggests iteration, n
boundary.
If there is only
Entry_n = ...
andEntry_0, ..., Entry_n
, it looks like definitions ofEntry_0
throughEntry_{n-1}
are missing.
But for the policy it isn't defined through Entry_{n-1}
But for the policy it isn't defined through
Entry_{n-1}
How come? Isn't M_policy
a result of extending 0
and Entry_0
, then extending result with Entry_1
, etc. up to the last entry?
Isn't M_policy a result of extending 0 and Entry_0, then extending result with Entry_1, etc. up to the last entry?
Hmm, I understood it as concatenation of all entries (subset of their fields), but there is a different symbol for concatenation, so maybe you're right. @dpsmith @rossphilipson what are your opinions on this?
Hmm, I understood it as concatenation of all entries (subset of their fields), but there is a different symbol for concatenation, so maybe you're right.
The reason for my interpretation is definition of E()
in 5.1 which is recursive when applied to an array of objects.
This issue is made to gather all small fixes, improvements and other TODOs in one place before adding them to next specification revision. It will also be a place for conversation about proposed changes. After enough changes are listed (or a change significant enough to bump specification revision) we can push them all in one PR to keep version in source file synchronized with produced PDFs.
I (or anyone with permissions to edit the issue message) will periodically update the list below to have all the changes in one place. Feel free to put your suggestions in the comment.
struct slr_table *slrt;
- size of pointer should be explicitly specifiedSLR_ET_UNUSED
can be used for - https://github.com/TrenchBoot/grub/pull/13/files#r1354751740SLR_ET_UNSPECIFIED
- https://github.com/TrenchBoot/grub/pull/13/files#r1354869835~TXT_VARIABLE_MTRRS_LENGTH
andTPM_EVENT_INFO_LENGTH
are not specified (both are32
in implementation).{slr_policy_entry,slr_uefi_cfg_entry}::evt_info
must include trailing\0
or it can be omitted if label fills the whole buffer.2.2 Acronyms
doesn't expandSLRT
.SLR_POLICY_*
andSLR_ET_*
aren't explained. WithSLR_ET_SLRT
marked as required.SLR_ET_*
aren't always self-describing, they were made with Linux in mind and some names should reflect this - https://github.com/TrenchBoot/grub/pull/14#discussion_r13868367900..n
instead of0..n-1
).|
denotes concatenation instead of bitwise OR.SLR_ET_SLRT
should reference "Appendix A".Entry_n = PCR_n | EntityType_n | EventInfo_n
in "Measuring the Policy" should usei
for index.kernel_info
for Linux, whatever we'll choose for Multiboot2).mtrr_vcnt array
in description ofmtrr_vcnt
should saymtrr_pair array
instead.grub_uint64_t dlme_base;
andgrub_uint32_t dlme_size;
tostruct slr_entry_dl_info
.dlme_entry
instruct slr_entry_dl_info
togrub_uint32_t
(see).~