[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <B41D3199-8054-4B2C-94D6-508D1DE4C8B3@zytor.com>
Date: Sat, 29 Mar 2025 18:13:15 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Ross Philipson <ross.philipson@...cle.com>, linux-kernel@...r.kernel.org,
x86@...nel.org, linux-integrity@...r.kernel.org,
linux-doc@...r.kernel.org, linux-crypto@...r.kernel.org,
kexec@...ts.infradead.org, linux-efi@...r.kernel.org,
iommu@...ts.linux.dev
CC: ross.philipson@...cle.com, dpsmith@...rtussolutions.com,
tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, ardb@...nel.org, mjg59@...f.ucam.org,
James.Bottomley@...senpartnership.com, peterhuewe@....de,
jarkko@...nel.org, jgg@...pe.ca, luto@...capital.net,
nivedita@...m.mit.edu, herbert@...dor.apana.org.au,
davem@...emloft.net, corbet@....net, ebiederm@...ssion.com,
dwmw2@...radead.org, baolu.lu@...ux.intel.com,
kanth.ghatraju@...cle.com, andrew.cooper3@...rix.com,
trenchboot-devel@...glegroups.com
Subject: Re: [PATCH v13 19/19] x86/efi: EFI stub DRTM launch support for Secure Launch
On March 28, 2025 4:08:14 PM PDT, Ross Philipson <ross.philipson@...cle.com> 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 <ross.philipson@...cle.com>
>Reviewed-by: Ard Biesheuvel <ardb@...nel.org>
>---
> drivers/firmware/efi/libstub/efistub.h | 8 +++
> drivers/firmware/efi/libstub/x86-stub.c | 94 +++++++++++++++++++++++++
> 2 files changed, 102 insertions(+)
>
>diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
>index d96d4494070d..bbbc4b327ce1 100644
>--- a/drivers/firmware/efi/libstub/efistub.h
>+++ b/drivers/firmware/efi/libstub/efistub.h
>@@ -135,6 +135,14 @@ void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
> *hi = upper_32_bits(data);
> }
>
>+static inline
>+void efi_set_u64_form(u32 lo, u32 hi, u64 *data)
>+{
>+ u64 upper = hi;
>+
>+ *data = lo | upper << 32;
>+}
>+
> /*
> * Allocation types for calls to boottime->allocate_pages.
> */
>diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
>index 863910e9eefc..033133e7d953 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>
>@@ -798,6 +800,93 @@ 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;
>+ bool updated = false;
>+ int i;
>+
>+ txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO);
>+ if (!txt_info)
>+ return false;
>+
>+ txt_info->boot_params_addr = (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 +
>+ offsetof(struct boot_params, hdr));
>+ u64 cmdline_ptr;
>+
>+ 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;
>+ efi_set_u64_form(boot_params->hdr.cmd_line_ptr, boot_params->ext_cmd_line_ptr,
>+ &cmdline_ptr);
>+ 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)
> {
>@@ -925,6 +1014,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.
efi_set_u64_form()?
What the heck is that? If it actually involves two u32 packed into a 64 field, why not simply do two stores?
Powered by blists - more mailing lists