This support allows the DRTM launch to be initiated after an EFI stub
launch of the Linux kernel is done. This is accomplished by providing
a handler to jump to when a Secure Launch is in progress. This has to be
called after the EFI stub does Exit Boot Services.
Signed-off-by: Ross Philipson <[email protected]>
---
drivers/firmware/efi/libstub/x86-stub.c | 98 +++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index d5a8182cf2e1..a1143d006202 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -9,6 +9,8 @@
#include <linux/efi.h>
#include <linux/pci.h>
#include <linux/stddef.h>
+#include <linux/slr_table.h>
+#include <linux/slaunch.h>
#include <asm/efi.h>
#include <asm/e820/types.h>
@@ -830,6 +832,97 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry)
return efi_adjust_memory_range_protection(addr, kernel_text_size);
}
+#if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
+static bool efi_secure_launch_update_boot_params(struct slr_table *slrt,
+ struct boot_params *boot_params)
+{
+ struct slr_entry_intel_info *txt_info;
+ struct slr_entry_policy *policy;
+ struct txt_os_mle_data *os_mle;
+ bool updated = false;
+ int i;
+
+ txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO);
+ if (!txt_info)
+ return false;
+
+ os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap);
+ if (!os_mle)
+ return false;
+
+ os_mle->boot_params_addr = (u32)(u64)boot_params;
+
+ policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY);
+ if (!policy)
+ return false;
+
+ for (i = 0; i < policy->nr_entries; i++) {
+ if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) {
+ policy->policy_entries[i].entity = (u64)boot_params;
+ updated = true;
+ break;
+ }
+ }
+
+ /*
+ * If this is a PE entry into EFI stub the mocked up boot params will
+ * be missing some of the setup header data needed for the second stage
+ * of the Secure Launch boot.
+ */
+ if (image) {
+ struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + 0x1f1);
+ u64 cmdline_ptr, hi_val;
+
+ boot_params->hdr.setup_sects = hdr->setup_sects;
+ boot_params->hdr.syssize = hdr->syssize;
+ boot_params->hdr.version = hdr->version;
+ boot_params->hdr.loadflags = hdr->loadflags;
+ boot_params->hdr.kernel_alignment = hdr->kernel_alignment;
+ boot_params->hdr.min_alignment = hdr->min_alignment;
+ boot_params->hdr.xloadflags = hdr->xloadflags;
+ boot_params->hdr.init_size = hdr->init_size;
+ boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset;
+ hi_val = boot_params->ext_cmd_line_ptr;
+ cmdline_ptr = boot_params->hdr.cmd_line_ptr | hi_val << 32;
+ boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr);;
+ }
+
+ return updated;
+}
+
+static void efi_secure_launch(struct boot_params *boot_params)
+{
+ struct slr_entry_dl_info *dlinfo;
+ efi_guid_t guid = SLR_TABLE_GUID;
+ dl_handler_func handler_callback;
+ struct slr_table *slrt;
+
+ /*
+ * The presence of this table indicated a Secure Launch
+ * is being requested.
+ */
+ slrt = (struct slr_table *)get_efi_config_table(guid);
+ if (!slrt || slrt->magic != SLR_TABLE_MAGIC)
+ return;
+
+ /*
+ * Since the EFI stub library creates its own boot_params on entry, the
+ * SLRT and TXT heap have to be updated with this version.
+ */
+ if (!efi_secure_launch_update_boot_params(slrt, boot_params))
+ return;
+
+ /* Jump through DL stub to initiate Secure Launch */
+ dlinfo = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_DL_INFO);
+
+ handler_callback = (dl_handler_func)dlinfo->dl_handler;
+
+ handler_callback(&dlinfo->bl_context);
+
+ unreachable();
+}
+#endif
+
static void __noreturn enter_kernel(unsigned long kernel_addr,
struct boot_params *boot_params)
{
@@ -957,6 +1050,11 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
goto fail;
}
+#if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
+ /* If a Secure Launch is in progress, this never returns */
+ efi_secure_launch(boot_params);
+#endif
+
/*
* Call the SEV init code while still running with the firmware's
* GDT/IDT, so #VC exceptions will be handled by EFI.
--
2.39.3
On Fri, 31 May 2024 at 03:32, Ross Philipson <[email protected]> wrote:
>
> This support allows the DRTM launch to be initiated after an EFI stub
> launch of the Linux kernel is done. This is accomplished by providing
> a handler to jump to when a Secure Launch is in progress. This has to be
> called after the EFI stub does Exit Boot Services.
>
> Signed-off-by: Ross Philipson <[email protected]>
Just some minor remarks below. The overall approach in this patch
looks fine now.
> ---
> drivers/firmware/efi/libstub/x86-stub.c | 98 +++++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
>
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index d5a8182cf2e1..a1143d006202 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -9,6 +9,8 @@
> #include <linux/efi.h>
> #include <linux/pci.h>
> #include <linux/stddef.h>
> +#include <linux/slr_table.h>
> +#include <linux/slaunch.h>
>
> #include <asm/efi.h>
> #include <asm/e820/types.h>
> @@ -830,6 +832,97 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry)
> return efi_adjust_memory_range_protection(addr, kernel_text_size);
> }
>
> +#if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
IS_ENABLED() is mostly used for C conditionals not CPP ones.
It would be nice if this #if could be dropped, and replaced with ... (see below)
> +static bool efi_secure_launch_update_boot_params(struct slr_table *slrt,
> + struct boot_params *boot_params)
> +{
> + struct slr_entry_intel_info *txt_info;
> + struct slr_entry_policy *policy;
> + struct txt_os_mle_data *os_mle;
> + bool updated = false;
> + int i;
> +
> + txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO);
> + if (!txt_info)
> + return false;
> +
> + os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap);
> + if (!os_mle)
> + return false;
> +
> + os_mle->boot_params_addr = (u32)(u64)boot_params;
> +
Why is this safe?
> + policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY);
> + if (!policy)
> + return false;
> +
> + for (i = 0; i < policy->nr_entries; i++) {
> + if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) {
> + policy->policy_entries[i].entity = (u64)boot_params;
> + updated = true;
> + break;
> + }
> + }
> +
> + /*
> + * If this is a PE entry into EFI stub the mocked up boot params will
> + * be missing some of the setup header data needed for the second stage
> + * of the Secure Launch boot.
> + */
> + if (image) {
> + struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + 0x1f1);
Could we use something other than a bare 0x1f1 constant here? struct
boot_params has a struct setup_header at the correct offset, so with
some casting of offsetof() use, we can make this look a lot more self
explanatory.
> + u64 cmdline_ptr, hi_val;
> +
> + boot_params->hdr.setup_sects = hdr->setup_sects;
> + boot_params->hdr.syssize = hdr->syssize;
> + boot_params->hdr.version = hdr->version;
> + boot_params->hdr.loadflags = hdr->loadflags;
> + boot_params->hdr.kernel_alignment = hdr->kernel_alignment;
> + boot_params->hdr.min_alignment = hdr->min_alignment;
> + boot_params->hdr.xloadflags = hdr->xloadflags;
> + boot_params->hdr.init_size = hdr->init_size;
> + boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset;
> + hi_val = boot_params->ext_cmd_line_ptr;
We have efi_set_u64_split() for this.
> + cmdline_ptr = boot_params->hdr.cmd_line_ptr | hi_val << 32;
> + boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr);;
> + }
> +
> + return updated;
> +}
> +
> +static void efi_secure_launch(struct boot_params *boot_params)
> +{
> + struct slr_entry_dl_info *dlinfo;
> + efi_guid_t guid = SLR_TABLE_GUID;
> + dl_handler_func handler_callback;
> + struct slr_table *slrt;
> +
... a C conditional here, e.g.,
if (!IS_ENABLED(CONFIG_SECURE_LAUNCH))
return;
The difference is that all the code will get compile test coverage
every time, instead of only in configs that enable
CONFIG_SECURE_LAUNCH.
This significantly reduces the risk that your stuff will get broken
inadvertently.
> + /*
> + * The presence of this table indicated a Secure Launch
> + * is being requested.
> + */
> + slrt = (struct slr_table *)get_efi_config_table(guid);
> + if (!slrt || slrt->magic != SLR_TABLE_MAGIC)
> + return;
> +
> + /*
> + * Since the EFI stub library creates its own boot_params on entry, the
> + * SLRT and TXT heap have to be updated with this version.
> + */
> + if (!efi_secure_launch_update_boot_params(slrt, boot_params))
> + return;
> +
> + /* Jump through DL stub to initiate Secure Launch */
> + dlinfo = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_DL_INFO);
> +
> + handler_callback = (dl_handler_func)dlinfo->dl_handler;
> +
> + handler_callback(&dlinfo->bl_context);
> +
> + unreachable();
> +}
> +#endif
> +
> static void __noreturn enter_kernel(unsigned long kernel_addr,
> struct boot_params *boot_params)
> {
> @@ -957,6 +1050,11 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> goto fail;
> }
>
> +#if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
... and drop this #if as well.
> + /* If a Secure Launch is in progress, this never returns */
> + efi_secure_launch(boot_params);
> +#endif
> +
> /*
> * Call the SEV init code while still running with the firmware's
> * GDT/IDT, so #VC exceptions will be handled by EFI.
> --
> 2.39.3
>
On 5/31/24 4:09 AM, Ard Biesheuvel wrote:
> On Fri, 31 May 2024 at 03:32, Ross Philipson <[email protected]> wrote:
>>
>> This support allows the DRTM launch to be initiated after an EFI stub
>> launch of the Linux kernel is done. This is accomplished by providing
>> a handler to jump to when a Secure Launch is in progress. This has to be
>> called after the EFI stub does Exit Boot Services.
>>
>> Signed-off-by: Ross Philipson <[email protected]>
>
> Just some minor remarks below. The overall approach in this patch
> looks fine now.
>
>
>> ---
>> drivers/firmware/efi/libstub/x86-stub.c | 98 +++++++++++++++++++++++++
>> 1 file changed, 98 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
>> index d5a8182cf2e1..a1143d006202 100644
>> --- a/drivers/firmware/efi/libstub/x86-stub.c
>> +++ b/drivers/firmware/efi/libstub/x86-stub.c
>> @@ -9,6 +9,8 @@
>> #include <linux/efi.h>
>> #include <linux/pci.h>
>> #include <linux/stddef.h>
>> +#include <linux/slr_table.h>
>> +#include <linux/slaunch.h>
>>
>> #include <asm/efi.h>
>> #include <asm/e820/types.h>
>> @@ -830,6 +832,97 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry)
>> return efi_adjust_memory_range_protection(addr, kernel_text_size);
>> }
>>
>> +#if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
>
> IS_ENABLED() is mostly used for C conditionals not CPP ones.
>
> It would be nice if this #if could be dropped, and replaced with ... (see below)
>
>
>> +static bool efi_secure_launch_update_boot_params(struct slr_table *slrt,
>> + struct boot_params *boot_params)
>> +{
>> + struct slr_entry_intel_info *txt_info;
>> + struct slr_entry_policy *policy;
>> + struct txt_os_mle_data *os_mle;
>> + bool updated = false;
>> + int i;
>> +
>> + txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO);
>> + if (!txt_info)
>> + return false;
>> +
>> + os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap);
>> + if (!os_mle)
>> + return false;
>> +
>> + os_mle->boot_params_addr = (u32)(u64)boot_params;
>> +
>
> Why is this safe?
The size of the boot_params_addr is a holdover from the legacy boot
world when boot params were always loaded at a low address. We will
increase the size of the field.
>
>> + policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY);
>> + if (!policy)
>> + return false;
>> +
>> + for (i = 0; i < policy->nr_entries; i++) {
>> + if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) {
>> + policy->policy_entries[i].entity = (u64)boot_params;
>> + updated = true;
>> + break;
>> + }
>> + }
>> +
>> + /*
>> + * If this is a PE entry into EFI stub the mocked up boot params will
>> + * be missing some of the setup header data needed for the second stage
>> + * of the Secure Launch boot.
>> + */
>> + if (image) {
>> + struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + 0x1f1);
>
> Could we use something other than a bare 0x1f1 constant here? struct
> boot_params has a struct setup_header at the correct offset, so with
> some casting of offsetof() use, we can make this look a lot more self
> explanatory.
Yes we can do this.
>
>
>> + u64 cmdline_ptr, hi_val;
>> +
>> + boot_params->hdr.setup_sects = hdr->setup_sects;
>> + boot_params->hdr.syssize = hdr->syssize;
>> + boot_params->hdr.version = hdr->version;
>> + boot_params->hdr.loadflags = hdr->loadflags;
>> + boot_params->hdr.kernel_alignment = hdr->kernel_alignment;
>> + boot_params->hdr.min_alignment = hdr->min_alignment;
>> + boot_params->hdr.xloadflags = hdr->xloadflags;
>> + boot_params->hdr.init_size = hdr->init_size;
>> + boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset;
>> + hi_val = boot_params->ext_cmd_line_ptr;
>
> We have efi_set_u64_split() for this.
Ok I will use that then.
>
>> + cmdline_ptr = boot_params->hdr.cmd_line_ptr | hi_val << 32;
>> + boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr);;
>> + }
>> +
>> + return updated;
>> +}
>> +
>> +static void efi_secure_launch(struct boot_params *boot_params)
>> +{
>> + struct slr_entry_dl_info *dlinfo;
>> + efi_guid_t guid = SLR_TABLE_GUID;
>> + dl_handler_func handler_callback;
>> + struct slr_table *slrt;
>> +
>
> ... a C conditional here, e.g.,
>
> if (!IS_ENABLED(CONFIG_SECURE_LAUNCH))
> return;
>
> The difference is that all the code will get compile test coverage
> every time, instead of only in configs that enable
> CONFIG_SECURE_LAUNCH.
>
> This significantly reduces the risk that your stuff will get broken
> inadvertently.
Understood, I will address these as you suggest.
>
>> + /*
>> + * The presence of this table indicated a Secure Launch
>> + * is being requested.
>> + */
>> + slrt = (struct slr_table *)get_efi_config_table(guid);
>> + if (!slrt || slrt->magic != SLR_TABLE_MAGIC)
>> + return;
>> +
>> + /*
>> + * Since the EFI stub library creates its own boot_params on entry, the
>> + * SLRT and TXT heap have to be updated with this version.
>> + */
>> + if (!efi_secure_launch_update_boot_params(slrt, boot_params))
>> + return;
>> +
>> + /* Jump through DL stub to initiate Secure Launch */
>> + dlinfo = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_DL_INFO);
>> +
>> + handler_callback = (dl_handler_func)dlinfo->dl_handler;
>> +
>> + handler_callback(&dlinfo->bl_context);
>> +
>> + unreachable();
>> +}
>> +#endif
>> +
>> static void __noreturn enter_kernel(unsigned long kernel_addr,
>> struct boot_params *boot_params)
>> {
>> @@ -957,6 +1050,11 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
>> goto fail;
>> }
>>
>> +#if (IS_ENABLED(CONFIG_SECURE_LAUNCH))
>
> ... and drop this #if as well.
Yes.
Thanks
Ross
>
>> + /* If a Secure Launch is in progress, this never returns */
>> + efi_secure_launch(boot_params);
>> +#endif
>> +
>> /*
>> * Call the SEV init code while still running with the firmware's
>> * GDT/IDT, so #VC exceptions will be handled by EFI.
>> --
>> 2.39.3
>>