xen/arch/x86: add TPR (TXT Protected Range) DMA protection support#29
xen/arch/x86: add TPR (TXT Protected Range) DMA protection support#29accek-itl wants to merge 4 commits into
Conversation
| * Find the TPR request element in the TXT heap extended data. | ||
| */ | ||
| static inline const struct txt_heap_tpr_req_element * | ||
| txt_find_tpr_req_element(const struct txt_os_sinit_data *os_sinit) |
There was a problem hiding this comment.
If you'll make this function take data element type as a parameter, it will be possible to reuse the function in tpm.c which is the only other place that searches for a data element.
P.S. Looks like tpm.c is actually doing it wrong: it interprets size relative to the data field which matches the comment on txt_ext_data_element in this file but contradicts the SDM. So it shouldn't actually work there and likely doesn't.
There was a problem hiding this comment.
Nice catch; it works by accident, because the event log element is the first one. I split this commit into multiple ones to separately extract the common os_sinit element search and apply it in TPM case. Only then add TPR support.
| { | ||
| const struct txt_heap_tpr_req_element *tpr_req; | ||
|
|
||
| tpr_req = txt_find_tpr_req_element(os_sinit); |
There was a problem hiding this comment.
Maybe add a comment that txt_verify_dma_protection() has already validated presence and contents of the element.
|
|
||
| tpr_req = txt_find_tpr_req_element(os_sinit); | ||
| lo_size = tpr_req->ranges[0].base + tpr_req->ranges[0].size; | ||
| if ( tpr_req->count >= 2 ) |
There was a problem hiding this comment.
| if ( tpr_req->count >= 2 ) | |
| if ( tpr_req->count == 2 ) |
or
| if ( tpr_req->count >= 2 ) | |
| if ( tpr_req->count > 1 ) |
| * txt_verify_pmr_ranges() makes sure the low range always starts at 0, so | ||
| * its size is also end address. | ||
| * txt_verify_dma_protection() makes sure the low range always starts at | ||
| * 0, so its size is also end address. |
There was a problem hiding this comment.
This comment should be in the else-branch above and the variable name should be lo_end instead of lo_size to make sense for both branches.
There was a problem hiding this comment.
Kept the assumption that lo_base is always 0, also in TPR flow, so keeping the comment here.
Adjusted the if-branch to just assign the size.
| 0x100000000ULL ) | ||
| txt_reset(SLAUNCH_ERROR_TPR_INVALID); | ||
|
|
||
| if ( tpr_req->count >= 2 ) |
There was a problem hiding this comment.
| if ( tpr_req->count >= 2 ) | |
| if ( tpr_req->count == 2 ) |
or
| if ( tpr_req->count >= 2 ) | |
| if ( tpr_req->count > 1 ) |
| * 0, so its size is also end address. | ||
| */ | ||
| if ( base + size <= os_sinit->vtd_pmr_lo_size ) | ||
| if ( base + size <= lo_size ) |
There was a problem hiding this comment.
This check makes sense only if lo_base is guaranteed to be zero. But it can't be with TPR due to overlaps with MMIO or something else, right? Then need to check lower bound too.
There was a problem hiding this comment.
I currently boot with lo TPR base hardcoded in grub to 0, just like it's for PMR path.
Tboot uses lowest region from e820, rounded down to 2M boundary, both for PMR and TPR, which in practice should be 0.
So I adjusted this code to assume and check that. Now both PMR and TPR branches have the same logic.
I think we can add non-zero base support later, if needed.
Do you know any reason/platform that needs non-zero lo_base?
| .long 0x42b651cb /* UUID3 */ | ||
| .long (.Lmle_header_end - mle_header) /* MLE header size */ | ||
| .long 0x00020002 /* MLE version 2.2 */ | ||
| .long 0x00020003 /* MLE version 2.3 */ |
There was a problem hiding this comment.
I hope this won't cause older SINITs to refuse processing this MLE.
There was a problem hiding this comment.
Yeah, me too. There's only min_mle_hdr_ver specified by SDG, so technically anything newer should always be compatible.
| if ( tpr_req->ranges[1].base + tpr_req->ranges[1].size <= | ||
| 0x100000000ULL ) |
There was a problem hiding this comment.
/* Hi range must start at or above 4G. */ check above leaves no chance for this condition to be true unless an integer overflow occurs.
Speaking of integer overflows, PMR code checks for them.
| if ( tpr_req->count > 2 ) | ||
| txt_reset(SLAUNCH_ERROR_TPR_UNSUPPORTED); | ||
|
|
||
| /* Lo range must not exceed 4G. */ |
There was a problem hiding this comment.
| /* Lo range must not exceed 4G. */ | |
| /* Low range must not exceed 4G. */ |
accek-itl
force-pushed
the
txt-tpr
branch
2 times, most recently
from
a45708d to
f7ebce8
Compare
Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Szymon Acedański <accek@invisiblethingslab.com>
Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Szymon Acedański <accek@invisiblethingslab.com>
Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Szymon Acedański <accek@invisiblethingslab.com>
Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Szymon Acedański <accek@invisiblethingslab.com>
2 participants
Assisted-by: Claude:claude-opus-4-6