On Mon, 2025-04-21 at 09:26 -0700, Ross Philipson wrote:
> Introduce the main Secure Launch header file used in the early SL stub
> and the early setup code.
>
> This header file contains the following categories:
> - Secure Launch implementation specific structures and definitions.
> - Intel TXT architecture specific DRTM structures, definitions and functions
> used by Secure Launch.
> - DRTM TPM event logging definitions and helper functions.
Looking at the actual code in this patch, seems >90% code in the
<linux/slaunch.h> is Intel specific, e.g., TXT specific macro/structure
definitions. It doesn't seem to be the right way to organize the code.
E.g., following the current pattern, when we need to add support for another TXT
similar vendor-specific technology, we will need to add yet-another set of
macro/structure definitions for that technology to <linux/slaunch.h> as well.
That would be a giant mess IMHO.
Could we make <linux/slaunch.h> only contain the common things, and move Intel
specific things to some x86 header(s), e.g., <asm/intel-txt.h> or <asm/txt.h>?
[...]
> +/*
> + * External functions available in mainline kernel.
> + */
> +void slaunch_setup_txt(void);
> +void slaunch_fixup_jump_vector(void);
> +u32 slaunch_get_flags(void);
> +struct sl_ap_wake_info *slaunch_get_ap_wake_info(void);
> +struct acpi_table_header *slaunch_get_dmar_table(struct acpi_table_header
> *dmar);
> +void __noreturn slaunch_txt_reset(void __iomem *txt,
> + const char *msg, u64 error);
> +void slaunch_finalize(int do_sexit);
> +
> +static inline bool slaunch_is_txt_launch(void)
> +{
> + u32 mask = SL_FLAG_ACTIVE | SL_FLAG_ARCH_TXT;
> +
> + return (slaunch_get_flags() & mask) == mask;
> +}
> +
> +#else
> +
> +static inline void slaunch_setup_txt(void)
> +{
> +}
> +
> +static inline void slaunch_fixup_jump_vector(void)
> +{
> +}
> +
> +static inline u32 slaunch_get_flags(void)
> +{
> + return 0;
> +}
> +
> +static inline struct acpi_table_header *slaunch_get_dmar_table(struct
> acpi_table_header *dmar)
> +{
> + return dmar;
> +}
> +
> +static inline void slaunch_finalize(int do_sexit)
> +{
> +}
> +
> +static inline bool slaunch_is_txt_launch(void)
> +{
> + return false;
> +}
>
.. btw it's not clear which part of the code is common code.
Looking at the abvoe code, it seems those functions are common functions called
from common code. E.g., slaunch_finalize() is called from kernel/kexec_core.c,
which means it is a concept in the kernel common code and must be available for
all ARCHs (I haven't read how other functions are called, though).
But the name slaunch_setup_txt(), slaunch_get_dmar_table() and
slaunch_is_txt_launch() are quite Intel specific. So it seems to me this patch
_tries_ to support Secure Launch at the arch agnostic level but it doesn't do a
good job at the abstraction?
