| Message ID | 20240826223835.3928819-21-ross.philipson@oracle.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | x86: Trenchboot secure dynamic launch Linux kernel support | expand |
On Tue, 27 Aug 2024 at 00:44, 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 | 98 +++++++++++++++++++++++++ > 2 files changed, 106 insertions(+) > > diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h > index d33ccbc4a2c6..baf42d6d0796 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 f8e465da344d..04786c1b3b5d 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> > @@ -923,6 +925,99 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry) > return efi_adjust_memory_range_protection(addr, kernel_text_size); > } > > +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 = (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; > + > + if (!IS_ENABLED(CONFIG_SECURE_LAUNCH)) > + return; > + > + /* > + * 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(); > +} > + > static void __noreturn enter_kernel(unsigned long kernel_addr, > struct boot_params *boot_params) > { > @@ -1050,6 +1145,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle, > goto fail; > } > > + /* If a Secure Launch is in progress, this never returns */ > + efi_secure_launch(boot_params); > + > /* > * 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 8/27/24 3:28 AM, Ard Biesheuvel wrote: > On Tue, 27 Aug 2024 at 00:44, 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> Thank you for the two reviews Ross > >> --- >> drivers/firmware/efi/libstub/efistub.h | 8 ++ >> drivers/firmware/efi/libstub/x86-stub.c | 98 +++++++++++++++++++++++++ >> 2 files changed, 106 insertions(+) >> >> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h >> index d33ccbc4a2c6..baf42d6d0796 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 f8e465da344d..04786c1b3b5d 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> >> @@ -923,6 +925,99 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry) >> return efi_adjust_memory_range_protection(addr, kernel_text_size); >> } >> >> +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 = (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; >> + >> + if (!IS_ENABLED(CONFIG_SECURE_LAUNCH)) >> + return; >> + >> + /* >> + * 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(); >> +} >> + >> static void __noreturn enter_kernel(unsigned long kernel_addr, >> struct boot_params *boot_params) >> { >> @@ -1050,6 +1145,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle, >> goto fail; >> } >> >> + /* If a Secure Launch is in progress, this never returns */ >> + efi_secure_launch(boot_params); >> + >> /* >> * 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 >>
Hi Ross, kernel test robot noticed the following build warnings: [auto build test WARNING on tip/x86/core] [also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5] [cannot apply to herbert-crypto-2.6/master next-20240827] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225 base: tip/x86/core patch link: https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson%40oracle.com patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20240828/202408280656.66ZxoOOL-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408280656.66ZxoOOL-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408280656.66ZxoOOL-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/firmware/efi/libstub/x86-stub.c: In function 'efi_secure_launch_update_boot_params': >> drivers/firmware/efi/libstub/x86-stub.c:941:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 941 | os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap); | ^ drivers/firmware/efi/libstub/x86-stub.c:945:36: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 945 | os_mle->boot_params_addr = (u64)boot_params; | ^ drivers/firmware/efi/libstub/x86-stub.c:953:60: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 953 | policy->policy_entries[i].entity = (u64)boot_params; | ^ drivers/firmware/efi/libstub/x86-stub.c:980:56: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 980 | boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr); | ^ drivers/firmware/efi/libstub/x86-stub.c: In function 'efi_secure_launch': drivers/firmware/efi/libstub/x86-stub.c:1014:28: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 1014 | handler_callback = (dl_handler_func)dlinfo->dl_handler; | ^ vim +941 drivers/firmware/efi/libstub/x86-stub.c 927 928 static bool efi_secure_launch_update_boot_params(struct slr_table *slrt, 929 struct boot_params *boot_params) 930 { 931 struct slr_entry_intel_info *txt_info; 932 struct slr_entry_policy *policy; 933 struct txt_os_mle_data *os_mle; 934 bool updated = false; 935 int i; 936 937 txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO); 938 if (!txt_info) 939 return false; 940 > 941 os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap); 942 if (!os_mle) 943 return false; 944 945 os_mle->boot_params_addr = (u64)boot_params; 946 947 policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY); 948 if (!policy) 949 return false; 950 951 for (i = 0; i < policy->nr_entries; i++) { 952 if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) { 953 policy->policy_entries[i].entity = (u64)boot_params; 954 updated = true; 955 break; 956 } 957 } 958 959 /* 960 * If this is a PE entry into EFI stub the mocked up boot params will 961 * be missing some of the setup header data needed for the second stage 962 * of the Secure Launch boot. 963 */ 964 if (image) { 965 struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + 966 offsetof(struct boot_params, hdr)); 967 u64 cmdline_ptr; 968 969 boot_params->hdr.setup_sects = hdr->setup_sects; 970 boot_params->hdr.syssize = hdr->syssize; 971 boot_params->hdr.version = hdr->version; 972 boot_params->hdr.loadflags = hdr->loadflags; 973 boot_params->hdr.kernel_alignment = hdr->kernel_alignment; 974 boot_params->hdr.min_alignment = hdr->min_alignment; 975 boot_params->hdr.xloadflags = hdr->xloadflags; 976 boot_params->hdr.init_size = hdr->init_size; 977 boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset; 978 efi_set_u64_form(boot_params->hdr.cmd_line_ptr, boot_params->ext_cmd_line_ptr, 979 &cmdline_ptr); 980 boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr); 981 } 982 983 return updated; 984 } 985
Hi Ross, kernel test robot noticed the following build warnings: [auto build test WARNING on tip/x86/core] [also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5] [cannot apply to herbert-crypto-2.6/master next-20240828] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225 base: tip/x86/core patch link: https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson%40oracle.com patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch config: i386-randconfig-062-20240828 (https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408290030.FEbUhHbr-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/firmware/efi/libstub/x86-stub.c:945:41: sparse: sparse: non size-preserving pointer to integer cast drivers/firmware/efi/libstub/x86-stub.c:953:65: sparse: sparse: non size-preserving pointer to integer cast >> drivers/firmware/efi/libstub/x86-stub.c:980:70: sparse: sparse: non size-preserving integer to pointer cast drivers/firmware/efi/libstub/x86-stub.c:1014:45: sparse: sparse: non size-preserving integer to pointer cast vim +945 drivers/firmware/efi/libstub/x86-stub.c 927 928 static bool efi_secure_launch_update_boot_params(struct slr_table *slrt, 929 struct boot_params *boot_params) 930 { 931 struct slr_entry_intel_info *txt_info; 932 struct slr_entry_policy *policy; 933 struct txt_os_mle_data *os_mle; 934 bool updated = false; 935 int i; 936 937 txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO); 938 if (!txt_info) 939 return false; 940 941 os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap); 942 if (!os_mle) 943 return false; 944 > 945 os_mle->boot_params_addr = (u64)boot_params; 946 947 policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY); 948 if (!policy) 949 return false; 950 951 for (i = 0; i < policy->nr_entries; i++) { 952 if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) { 953 policy->policy_entries[i].entity = (u64)boot_params; 954 updated = true; 955 break; 956 } 957 } 958 959 /* 960 * If this is a PE entry into EFI stub the mocked up boot params will 961 * be missing some of the setup header data needed for the second stage 962 * of the Secure Launch boot. 963 */ 964 if (image) { 965 struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + 966 offsetof(struct boot_params, hdr)); 967 u64 cmdline_ptr; 968 969 boot_params->hdr.setup_sects = hdr->setup_sects; 970 boot_params->hdr.syssize = hdr->syssize; 971 boot_params->hdr.version = hdr->version; 972 boot_params->hdr.loadflags = hdr->loadflags; 973 boot_params->hdr.kernel_alignment = hdr->kernel_alignment; 974 boot_params->hdr.min_alignment = hdr->min_alignment; 975 boot_params->hdr.xloadflags = hdr->xloadflags; 976 boot_params->hdr.init_size = hdr->init_size; 977 boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset; 978 efi_set_u64_form(boot_params->hdr.cmd_line_ptr, boot_params->ext_cmd_line_ptr, 979 &cmdline_ptr); > 980 boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr); 981 } 982 983 return updated; 984 } 985
On Wed, 28 Aug 2024 at 19:09, kernel test robot <lkp@intel.com> wrote: > > Hi Ross, > > kernel test robot noticed the following build warnings: > > [auto build test WARNING on tip/x86/core] > [also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5] > [cannot apply to herbert-crypto-2.6/master next-20240828] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225 > base: tip/x86/core > patch link: https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson%40oracle.com > patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch > config: i386-randconfig-062-20240828 (https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/config) This is a i386 32-bit build, which makes no sense: this stuff should just declare 'depends on 64BIT' > compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202408290030.FEbUhHbr-lkp@intel.com/ > > sparse warnings: (new ones prefixed by >>) > >> drivers/firmware/efi/libstub/x86-stub.c:945:41: sparse: sparse: non size-preserving pointer to integer cast > drivers/firmware/efi/libstub/x86-stub.c:953:65: sparse: sparse: non size-preserving pointer to integer cast > >> drivers/firmware/efi/libstub/x86-stub.c:980:70: sparse: sparse: non size-preserving integer to pointer cast > drivers/firmware/efi/libstub/x86-stub.c:1014:45: sparse: sparse: non size-preserving integer to pointer cast > > vim +945 drivers/firmware/efi/libstub/x86-stub.c > > 927 > 928 static bool efi_secure_launch_update_boot_params(struct slr_table *slrt, > 929 struct boot_params *boot_params) > 930 { > 931 struct slr_entry_intel_info *txt_info; > 932 struct slr_entry_policy *policy; > 933 struct txt_os_mle_data *os_mle; > 934 bool updated = false; > 935 int i; > 936 > 937 txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO); > 938 if (!txt_info) > 939 return false; > 940 > 941 os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap); > 942 if (!os_mle) > 943 return false; > 944 > > 945 os_mle->boot_params_addr = (u64)boot_params; > 946 > 947 policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY); > 948 if (!policy) > 949 return false; > 950 > 951 for (i = 0; i < policy->nr_entries; i++) { > 952 if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) { > 953 policy->policy_entries[i].entity = (u64)boot_params; > 954 updated = true; > 955 break; > 956 } > 957 } > 958 > 959 /* > 960 * If this is a PE entry into EFI stub the mocked up boot params will > 961 * be missing some of the setup header data needed for the second stage > 962 * of the Secure Launch boot. > 963 */ > 964 if (image) { > 965 struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + > 966 offsetof(struct boot_params, hdr)); > 967 u64 cmdline_ptr; > 968 > 969 boot_params->hdr.setup_sects = hdr->setup_sects; > 970 boot_params->hdr.syssize = hdr->syssize; > 971 boot_params->hdr.version = hdr->version; > 972 boot_params->hdr.loadflags = hdr->loadflags; > 973 boot_params->hdr.kernel_alignment = hdr->kernel_alignment; > 974 boot_params->hdr.min_alignment = hdr->min_alignment; > 975 boot_params->hdr.xloadflags = hdr->xloadflags; > 976 boot_params->hdr.init_size = hdr->init_size; > 977 boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset; > 978 efi_set_u64_form(boot_params->hdr.cmd_line_ptr, boot_params->ext_cmd_line_ptr, > 979 &cmdline_ptr); > > 980 boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr); > 981 } > 982 > 983 return updated; > 984 } > 985 > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki >
On 8/28/24 10:14 AM, Ard Biesheuvel wrote: > On Wed, 28 Aug 2024 at 19:09, kernel test robot <lkp@intel.com> wrote: >> >> Hi Ross, >> >> kernel test robot noticed the following build warnings: >> >> [auto build test WARNING on tip/x86/core] >> [also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5] >> [cannot apply to herbert-crypto-2.6/master next-20240828] >> [If your patch is applied to the wrong git tree, kindly drop us a note. >> And when submitting patch, we suggest to use '--base' as documented in >> https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch*_base_tree_information__;Iw!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIxuz-LAC$ ] >> >> url: https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI7Z6SQKy$ >> base: tip/x86/core >> patch link: https://urldefense.com/v3/__https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson*40oracle.com__;JQ!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIzWfs1XZ$ >> patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch >> config: i386-randconfig-062-20240828 (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/config__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIwkYG0TY$ ) > > > This is a i386 32-bit build, which makes no sense: this stuff should > just declare 'depends on 64BIT' Our config entry already has 'depends on X86_64' which in turn depends on 64BIT. I would think that would be enough. Do you think it needs an explicit 'depends on 64BIT' in our entry as well? Thanks Ross > > >> compiler: clang version 18.1.5 (https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI2SDLdTN$ 617a15a9eac96088ae5e9134248d8236e34b91b1) >> reproduce (this is a W=1 build): (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/reproduce__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI5MJDdIG$ ) >> >> If you fix the issue in a separate patch/commit (i.e. not just a new version of >> the same patch/commit), kindly add following tags >> | Reported-by: kernel test robot <lkp@intel.com> >> | Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-kbuild-all/202408290030.FEbUhHbr-lkp@intel.com/__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI-MitiqR$ >> >> sparse warnings: (new ones prefixed by >>) >>>> drivers/firmware/efi/libstub/x86-stub.c:945:41: sparse: sparse: non size-preserving pointer to integer cast >> drivers/firmware/efi/libstub/x86-stub.c:953:65: sparse: sparse: non size-preserving pointer to integer cast >>>> drivers/firmware/efi/libstub/x86-stub.c:980:70: sparse: sparse: non size-preserving integer to pointer cast >> drivers/firmware/efi/libstub/x86-stub.c:1014:45: sparse: sparse: non size-preserving integer to pointer cast >> >> vim +945 drivers/firmware/efi/libstub/x86-stub.c >> >> 927 >> 928 static bool efi_secure_launch_update_boot_params(struct slr_table *slrt, >> 929 struct boot_params *boot_params) >> 930 { >> 931 struct slr_entry_intel_info *txt_info; >> 932 struct slr_entry_policy *policy; >> 933 struct txt_os_mle_data *os_mle; >> 934 bool updated = false; >> 935 int i; >> 936 >> 937 txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO); >> 938 if (!txt_info) >> 939 return false; >> 940 >> 941 os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap); >> 942 if (!os_mle) >> 943 return false; >> 944 >> > 945 os_mle->boot_params_addr = (u64)boot_params; >> 946 >> 947 policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY); >> 948 if (!policy) >> 949 return false; >> 950 >> 951 for (i = 0; i < policy->nr_entries; i++) { >> 952 if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) { >> 953 policy->policy_entries[i].entity = (u64)boot_params; >> 954 updated = true; >> 955 break; >> 956 } >> 957 } >> 958 >> 959 /* >> 960 * If this is a PE entry into EFI stub the mocked up boot params will >> 961 * be missing some of the setup header data needed for the second stage >> 962 * of the Secure Launch boot. >> 963 */ >> 964 if (image) { >> 965 struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + >> 966 offsetof(struct boot_params, hdr)); >> 967 u64 cmdline_ptr; >> 968 >> 969 boot_params->hdr.setup_sects = hdr->setup_sects; >> 970 boot_params->hdr.syssize = hdr->syssize; >> 971 boot_params->hdr.version = hdr->version; >> 972 boot_params->hdr.loadflags = hdr->loadflags; >> 973 boot_params->hdr.kernel_alignment = hdr->kernel_alignment; >> 974 boot_params->hdr.min_alignment = hdr->min_alignment; >> 975 boot_params->hdr.xloadflags = hdr->xloadflags; >> 976 boot_params->hdr.init_size = hdr->init_size; >> 977 boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset; >> 978 efi_set_u64_form(boot_params->hdr.cmd_line_ptr, boot_params->ext_cmd_line_ptr, >> 979 &cmdline_ptr); >> > 980 boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr); >> 981 } >> 982 >> 983 return updated; >> 984 } >> 985 >> >> -- >> 0-DAY CI Kernel Test Service >> https://urldefense.com/v3/__https://github.com/intel/lkp-tests/wiki__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIy5kGTJf$ >>
On Wed, Aug 28, 2024 at 01:19:16PM -0700, ross.philipson@oracle.com wrote: > On 8/28/24 10:14 AM, Ard Biesheuvel wrote: > > On Wed, 28 Aug 2024 at 19:09, kernel test robot <lkp@intel.com> wrote: > > > > > > Hi Ross, > > > > > > kernel test robot noticed the following build warnings: > > > > > > [auto build test WARNING on tip/x86/core] > > > [also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5] > > > [cannot apply to herbert-crypto-2.6/master next-20240828] > > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > > And when submitting patch, we suggest to use '--base' as documented in > > > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch*_base_tree_information__;Iw!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIxuz-LAC$ ] > > > > > > url: https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI7Z6SQKy$ > > > base: tip/x86/core > > > patch link: https://urldefense.com/v3/__https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson*40oracle.com__;JQ!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIzWfs1XZ$ > > > patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch > > > config: i386-randconfig-062-20240828 (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/config__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIwkYG0TY$ ) > > > > > > This is a i386 32-bit build, which makes no sense: this stuff should > > just declare 'depends on 64BIT' > > Our config entry already has 'depends on X86_64' which in turn depends on > 64BIT. I would think that would be enough. Do you think it needs an explicit > 'depends on 64BIT' in our entry as well? The error is in x86-stub.c, which is pre-existing and compiled for 32 bit as well, so you need more than a "depends" here. > > > compiler: clang version 18.1.5 (https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI2SDLdTN$ 617a15a9eac96088ae5e9134248d8236e34b91b1) > > > reproduce (this is a W=1 build): (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/reproduce__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI5MJDdIG$ ) > > > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > > the same patch/commit), kindly add following tags > > > | Reported-by: kernel test robot <lkp@intel.com> > > > | Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-kbuild-all/202408290030.FEbUhHbr-lkp@intel.com/__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI-MitiqR$ > > > > > > sparse warnings: (new ones prefixed by >>) > > > > > drivers/firmware/efi/libstub/x86-stub.c:945:41: sparse: sparse: non size-preserving pointer to integer cast > > > drivers/firmware/efi/libstub/x86-stub.c:953:65: sparse: sparse: non size-preserving pointer to integer cast > > > > > drivers/firmware/efi/libstub/x86-stub.c:980:70: sparse: sparse: non size-preserving integer to pointer cast > > > drivers/firmware/efi/libstub/x86-stub.c:1014:45: sparse: sparse: non size-preserving integer to pointer cast > > > > > > vim +945 drivers/firmware/efi/libstub/x86-stub.c > > > > > > 927 > > > 928 static bool efi_secure_launch_update_boot_params(struct slr_table *slrt, > > > 929 struct boot_params *boot_params) > > > 930 { > > > 931 struct slr_entry_intel_info *txt_info; > > > 932 struct slr_entry_policy *policy; > > > 933 struct txt_os_mle_data *os_mle; > > > 934 bool updated = false; > > > 935 int i; > > > 936 > > > 937 txt_info = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_INTEL_INFO); > > > 938 if (!txt_info) > > > 939 return false; > > > 940 > > > 941 os_mle = txt_os_mle_data_start((void *)txt_info->txt_heap); > > > 942 if (!os_mle) > > > 943 return false; > > > 944 > > > > 945 os_mle->boot_params_addr = (u64)boot_params; > > > 946 > > > 947 policy = slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_ENTRY_POLICY); > > > 948 if (!policy) > > > 949 return false; > > > 950 > > > 951 for (i = 0; i < policy->nr_entries; i++) { > > > 952 if (policy->policy_entries[i].entity_type == SLR_ET_BOOT_PARAMS) { > > > 953 policy->policy_entries[i].entity = (u64)boot_params; > > > 954 updated = true; > > > 955 break; > > > 956 } > > > 957 } > > > 958 > > > 959 /* > > > 960 * If this is a PE entry into EFI stub the mocked up boot params will > > > 961 * be missing some of the setup header data needed for the second stage > > > 962 * of the Secure Launch boot. > > > 963 */ > > > 964 if (image) { > > > 965 struct setup_header *hdr = (struct setup_header *)((u8 *)image->image_base + > > > 966 offsetof(struct boot_params, hdr)); > > > 967 u64 cmdline_ptr; > > > 968 > > > 969 boot_params->hdr.setup_sects = hdr->setup_sects; > > > 970 boot_params->hdr.syssize = hdr->syssize; > > > 971 boot_params->hdr.version = hdr->version; > > > 972 boot_params->hdr.loadflags = hdr->loadflags; > > > 973 boot_params->hdr.kernel_alignment = hdr->kernel_alignment; > > > 974 boot_params->hdr.min_alignment = hdr->min_alignment; > > > 975 boot_params->hdr.xloadflags = hdr->xloadflags; > > > 976 boot_params->hdr.init_size = hdr->init_size; > > > 977 boot_params->hdr.kernel_info_offset = hdr->kernel_info_offset; > > > 978 efi_set_u64_form(boot_params->hdr.cmd_line_ptr, boot_params->ext_cmd_line_ptr, > > > 979 &cmdline_ptr); > > > > 980 boot_params->hdr.cmdline_size = strlen((const char *)cmdline_ptr); > > > 981 } > > > 982 > > > 983 return updated; > > > 984 } > > > 985 > > > > > > -- > > > 0-DAY CI Kernel Test Service > > > https://urldefense.com/v3/__https://github.com/intel/lkp-tests/wiki__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIy5kGTJf$ > > > > > J.
On Thu, 29 Aug 2024 at 15:24, Jonathan McDowell <noodles@earth.li> wrote: > > On Wed, Aug 28, 2024 at 01:19:16PM -0700, ross.philipson@oracle.com wrote: > > On 8/28/24 10:14 AM, Ard Biesheuvel wrote: > > > On Wed, 28 Aug 2024 at 19:09, kernel test robot <lkp@intel.com> wrote: > > > > > > > > Hi Ross, > > > > > > > > kernel test robot noticed the following build warnings: > > > > > > > > [auto build test WARNING on tip/x86/core] > > > > [also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5] > > > > [cannot apply to herbert-crypto-2.6/master next-20240828] > > > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > > > And when submitting patch, we suggest to use '--base' as documented in > > > > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch*_base_tree_information__;Iw!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIxuz-LAC$ ] > > > > > > > > url: https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI7Z6SQKy$ > > > > base: tip/x86/core > > > > patch link: https://urldefense.com/v3/__https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson*40oracle.com__;JQ!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIzWfs1XZ$ > > > > patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch > > > > config: i386-randconfig-062-20240828 (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/config__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIwkYG0TY$ ) > > > > > > > > > This is a i386 32-bit build, which makes no sense: this stuff should > > > just declare 'depends on 64BIT' > > > > Our config entry already has 'depends on X86_64' which in turn depends on > > 64BIT. I would think that would be enough. Do you think it needs an explicit > > 'depends on 64BIT' in our entry as well? > > The error is in x86-stub.c, which is pre-existing and compiled for 32 > bit as well, so you need more than a "depends" here. > Ugh, that means this is my fault - apologies. Replacing the #ifdef with IS_ENABLED() makes the code visible to the 32-bit compiler, even though the code is disregarded. I'd still prefer IS_ENABLED(), but this would require the code in question to live in a separate compilation unit (which depends on CONFIG_SECURE_LAUNCH). If that is too fiddly, feel free to bring back the #ifdef CONFIG_SECURE_LAUNCH here (and retain my R-b)
On 8/29/24 6:28 AM, Ard Biesheuvel wrote: > On Thu, 29 Aug 2024 at 15:24, Jonathan McDowell <noodles@earth.li> wrote: >> >> On Wed, Aug 28, 2024 at 01:19:16PM -0700, ross.philipson@oracle.com wrote: >>> On 8/28/24 10:14 AM, Ard Biesheuvel wrote: >>>> On Wed, 28 Aug 2024 at 19:09, kernel test robot <lkp@intel.com> wrote: >>>>> >>>>> Hi Ross, >>>>> >>>>> kernel test robot noticed the following build warnings: >>>>> >>>>> [auto build test WARNING on tip/x86/core] >>>>> [also build test WARNING on char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master efi/next linus/master v6.11-rc5] >>>>> [cannot apply to herbert-crypto-2.6/master next-20240828] >>>>> [If your patch is applied to the wrong git tree, kindly drop us a note. >>>>> And when submitting patch, we suggest to use '--base' as documented in >>>>> https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch*_base_tree_information__;Iw!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIxuz-LAC$ ] >>>>> >>>>> url: https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/Documentation-x86-Secure-Launch-kernel-documentation/20240827-065225__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmI7Z6SQKy$ >>>>> base: tip/x86/core >>>>> patch link: https://urldefense.com/v3/__https://lore.kernel.org/r/20240826223835.3928819-21-ross.philipson*40oracle.com__;JQ!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIzWfs1XZ$ >>>>> patch subject: [PATCH v10 20/20] x86/efi: EFI stub DRTM launch support for Secure Launch >>>>> config: i386-randconfig-062-20240828 (https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20240829/202408290030.FEbUhHbr-lkp@intel.com/config__;!!ACWV5N9M2RV99hQ!KhkZK77BXRIR4F24tKkUeIlIrdqXtUW2vcnDV74c_5BmrQBQaQ4FqcDKKv9LB3HQUocTGkrmIwkYG0TY$ ) >>>> >>>> >>>> This is a i386 32-bit build, which makes no sense: this stuff should >>>> just declare 'depends on 64BIT' >>> >>> Our config entry already has 'depends on X86_64' which in turn depends on >>> 64BIT. I would think that would be enough. Do you think it needs an explicit >>> 'depends on 64BIT' in our entry as well? >> >> The error is in x86-stub.c, which is pre-existing and compiled for 32 >> bit as well, so you need more than a "depends" here. >> > > Ugh, that means this is my fault - apologies. Replacing the #ifdef > with IS_ENABLED() makes the code visible to the 32-bit compiler, even > though the code is disregarded. > > I'd still prefer IS_ENABLED(), but this would require the code in > question to live in a separate compilation unit (which depends on > CONFIG_SECURE_LAUNCH). If that is too fiddly, feel free to bring back > the #ifdef CONFIG_SECURE_LAUNCH here (and retain my R-b) Thanks to both of you for the followup. If there was a very large amount of new code here, I would consider making a separate compilation unit. I can't really say if this is a wider issue with the the EFI libstub code but if it ware broken up later, it would make sense to move our bits to where 64bit only code lives. Given that it is a small block of code though, I am inclined to just go back to #ifdef's for now. Thanks Ross
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h index d33ccbc4a2c6..baf42d6d0796 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 f8e465da344d..04786c1b3b5d 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> @@ -923,6 +925,99 @@ static efi_status_t efi_decompress_kernel(unsigned long *kernel_entry) return efi_adjust_memory_range_protection(addr, kernel_text_size); } +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 = (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; + + if (!IS_ENABLED(CONFIG_SECURE_LAUNCH)) + return; + + /* + * 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(); +} + static void __noreturn enter_kernel(unsigned long kernel_addr, struct boot_params *boot_params) { @@ -1050,6 +1145,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle, goto fail; } + /* If a Secure Launch is in progress, this never returns */ + efi_secure_launch(boot_params); + /* * Call the SEV init code while still running with the firmware's * GDT/IDT, so #VC exceptions will be handled by EFI.
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> --- drivers/firmware/efi/libstub/efistub.h | 8 ++ drivers/firmware/efi/libstub/x86-stub.c | 98 +++++++++++++++++++++++++ 2 files changed, 106 insertions(+)