Re: [PATCH v13 19/19] x86/efi: EFI stub DRTM launch support for Secure Launch

[ross.philipson@oracle.com]

On 3/29/25 6:13 PM, H. Peter Anvin wrote:
> On March 28, 2025 4:08:14 PM PDT, Ross Philipson <ross.philipson@oracle.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@oracle.com>
>> Reviewed-by: Ard Biesheuvel <ardb@kernel.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?
> 

Well the story is this. The EFI maintainers asked me to use the 
efi_set_u64_split() type functions (this one splits a u64 into 2 u32). I 
went to look and there was no function that did the opposite action so I 
added it. The original function was called efi_set_u64_split() so 
efi_set_u64_form() was what I came up with. I can name it anything that 
is desired.

Thanks
Ross