On 13.05.2025 19:05, Sergii Dmytruk wrote:
> From: Krystian Hebel <krystian.he...@3mdeb.com>
>
> The file contains TXT register spaces base address, registers offsets,
> error codes and inline functions for accessing structures stored on
> TXT heap.
>
> Signed-off-by: Krystian Hebel <krystian.he...@3mdeb.com>
> Signed-off-by: Sergii Dmytruk <sergii.dmyt...@3mdeb.com>
> ---
> xen/arch/x86/include/asm/intel-txt.h | 277 +++++++++++++++++++++++++++
> xen/arch/x86/tboot.c | 20 +-
> 2 files changed, 279 insertions(+), 18 deletions(-)
> create mode 100644 xen/arch/x86/include/asm/intel-txt.h
>
> diff --git a/xen/arch/x86/include/asm/intel-txt.h
> b/xen/arch/x86/include/asm/intel-txt.h
> new file mode 100644
> index 0000000000..07be43f557
> --- /dev/null
> +++ b/xen/arch/x86/include/asm/intel-txt.h
> @@ -0,0 +1,277 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef ASM__X86__INTEL_TXT_H
> +#define ASM__X86__INTEL_TXT_H
> +
> +/*
> + * TXT configuration registers (offsets from TXT_{PUB,
> PRIV}_CONFIG_REGS_BASE)
> + */
> +#define TXT_PUB_CONFIG_REGS_BASE 0xfed30000U
> +#define TXT_PRIV_CONFIG_REGS_BASE 0xfed20000U
> +
> +/*
> + * The same set of registers is exposed twice (with different permissions)
> and
> + * they are allocated continuously with page alignment.
> + */
> +#define NR_TXT_CONFIG_SIZE \
> + (TXT_PUB_CONFIG_REGS_BASE - TXT_PRIV_CONFIG_REGS_BASE)
What does the NR_ in the identifier try to express?
> +/*
> + * Secure Launch defined OS/MLE TXT Heap table
> + */
> +struct txt_os_mle_data {
> + uint32_t version;
> + uint32_t reserved;
> + uint64_t slrt;
> + uint64_t txt_info;
> + uint32_t ap_wake_block;
> + uint32_t ap_wake_block_size;
> + uint8_t mle_scratch[64];
> +} __packed;
This being x86-specific, what's the __packed intended to achieve here?
> +/*
> + * TXT specification defined BIOS data TXT Heap table
> + */
> +struct txt_bios_data {
> + uint32_t version; /* Currently 5 for TPM 1.2 and 6 for TPM 2.0 */
> + uint32_t bios_sinit_size;
> + uint64_t reserved1;
> + uint64_t reserved2;
> + uint32_t num_logical_procs;
> + /* Versions >= 3 && < 5 */
> + uint32_t sinit_flags;
> + /* Versions >= 5 with updates in version 6 */
> + uint32_t mle_flags;
> + /* Versions >= 4 */
> + /* Ext Data Elements */
> +} __packed;
It does affect sizeof() here, which I'm unsure is going to matter.
> +/*
> + * TXT specification defined OS/SINIT TXT Heap table
> + */
> +struct txt_os_sinit_data {
> + uint32_t version; /* Currently 6 for TPM 1.2 and 7 for TPM 2.0 */
> + uint32_t flags; /* Reserved in version 6 */
> + uint64_t mle_ptab;
> + uint64_t mle_size;
> + uint64_t mle_hdr_base;
> + uint64_t vtd_pmr_lo_base;
> + uint64_t vtd_pmr_lo_size;
> + uint64_t vtd_pmr_hi_base;
> + uint64_t vtd_pmr_hi_size;
> + uint64_t lcp_po_base;
> + uint64_t lcp_po_size;
> + uint32_t capabilities;
> + /* Version = 5 */
> + uint64_t efi_rsdt_ptr; /* RSD*P* in versions >= 6 */
> + /* Versions >= 6 */
> + /* Ext Data Elements */
> +} __packed;
It does really affect the layout here, so can't be removed in this case.
> +/*
> + * Functions to extract data from the Intel TXT Heap Memory. The layout
> + * of the heap is as follows:
> + * +------------------------------------+
> + * | Size of Bios Data table (uint64_t) |
> + * +------------------------------------+
> + * | Bios Data table |
> + * +------------------------------------+
> + * | Size of OS MLE table (uint64_t) |
> + * +------------------------------------+
> + * | OS MLE table |
> + * +-------------------------------- +
> + * | Size of OS SINIT table (uint64_t) |
> + * +------------------------------------+
> + * | OS SINIT table |
> + * +------------------------------------+
> + * | Size of SINIT MLE table (uint64_t) |
> + * +------------------------------------+
> + * | SINIT MLE table |
> + * +------------------------------------+
> + *
> + * NOTE: the table size fields include the 8 byte size field itself.
> + */
> +static inline uint64_t txt_bios_data_size(void *heap)
Here, below, and in general: Please try to have code be const-correct, i.e.
use pointers-to-const wherever applicable.
> +{
> + return *((uint64_t *)heap) - sizeof(uint64_t);
For readability it generally helps if excess parentheses like the ones here
are omitted.
> +}
> +
> +static inline void *txt_bios_data_start(void *heap)
> +{
> + return heap + sizeof(uint64_t);
> +}
> +
> +static inline uint64_t txt_os_mle_data_size(void *heap)
> +{
> + return *((uint64_t *)(txt_bios_data_start(heap) +
> + txt_bios_data_size(heap))) -
> + sizeof(uint64_t);
This line wants indenting a little further, such that the"sizeof" aligns
with the start of the expression. (Same elsewhere below.)
Jan
