On 10/01/2025 07:32, Ard Biesheuvel wrote: > On Thu, 9 Jan 2025 at 17:32, Usama Arif <usamaarif642@xxxxxxxxx> wrote: >> >> >> >> On 09/01/2025 16:15, Ard Biesheuvel wrote: >> I think in the end whoevers' responsibility it is, the easiest path forward >> seems to be in kernel? (and not firmware or libstub) >> > > Agreed. But as I pointed out in the other thread, the memory > attributes table only augments the memory map with permission > information, and can be disregarded, and given how badly we mangle the > memory map on x86, maybe this is the right choice here. > >>> >>>> The next ideal place would be in libstub. However, it looks like >>>> InstallMemoryAttributesTable [2] is not available as a boot service >>>> call option [3], [4], and install_configuration_table does not >>>> seem to work as a valid substitute. >>>> >>> >>> To do what, exactly? >>> >> >> To change the memory type from System RAM to either reserved or >> something more appropriate, i.e. any type that is not touched by >> kexec or any other userspace. >> >> Basically the example code I attached at the end of the cover letter in >> https://lore.kernel.org/all/20250108215957.3437660-1-usamaarif642@xxxxxxxxx/ >> It could be EFI_ACPI_RECLAIM_MEMORY or EFI_RESERVED_TYPE, both of which aren't >> touched by kexec. >> > > This is a kexec problem (on x86 only) so let's fix it there. I don't believe we can accurately tell if we are booting from a cold boot or kexec. There is bootloader_type available for x86, but not sure if we should rely on that. I think a way forward would be to move it behind a Kconfig option, something like below, which defaults to n for x86. Anyone who needs it can enable it. What do you think? diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index aa95f77d7a30..31deb0a5371e 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -83,7 +83,9 @@ static const unsigned long * const efi_tables[] = { &efi_config_table, &efi.esrt, &prop_phys, +#ifdef CONFIG_EFI_MEMATTR &efi_mem_attr_table, +#endif #ifdef CONFIG_EFI_RCI2_TABLE &rci2_table_phys, #endif diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index 72f2537d90ca..b8ecb318768c 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -287,6 +287,13 @@ config EFI_EMBEDDED_FIRMWARE bool select CRYPTO_LIB_SHA256 +config EFI_MEMATTR + bool "EFI Memory attributes table" + default n if X86_64 + help + EFI Memory Attributes table describes memory protections that may + be applied to the EFI Runtime code and data regions by the kernel. + endmenu config UEFI_CPER diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile index a2d0009560d0..c593ec0d9940 100644 --- a/drivers/firmware/efi/Makefile +++ b/drivers/firmware/efi/Makefile @@ -11,7 +11,9 @@ KASAN_SANITIZE_runtime-wrappers.o := n obj-$(CONFIG_ACPI_BGRT) += efi-bgrt.o -obj-$(CONFIG_EFI) += efi.o vars.o reboot.o memattr.o tpm.o +obj-$(CONFIG_EFI) += efi.o vars.o reboot.o tpm.o +obj-$(CONFIG_EFI_MEMATTR) += memattr.o + obj-$(CONFIG_EFI) += memmap.o ifneq ($(CONFIG_EFI_CAPSULE_LOADER),) obj-$(CONFIG_EFI) += capsule.o diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index fdf07dd6f459..f359179083d5 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -596,7 +596,9 @@ static const efi_config_table_type_t common_tables[] __initconst = { {SMBIOS_TABLE_GUID, &efi.smbios, "SMBIOS" }, {SMBIOS3_TABLE_GUID, &efi.smbios3, "SMBIOS 3.0" }, {EFI_SYSTEM_RESOURCE_TABLE_GUID, &efi.esrt, "ESRT" }, +#ifdef CONFIG_EFI_MEMATTR {EFI_MEMORY_ATTRIBUTES_TABLE_GUID, &efi_mem_attr_table, "MEMATTR" }, +#endif {LINUX_EFI_RANDOM_SEED_TABLE_GUID, &efi_rng_seed, "RNG" }, {LINUX_EFI_TPM_EVENT_LOG_GUID, &efi.tpm_log, "TPMEventLog" }, {EFI_TCG2_FINAL_EVENTS_TABLE_GUID, &efi.tpm_final_log, "TPMFinalLog" }, diff --git a/include/linux/efi.h b/include/linux/efi.h index 9c239cdff771..4cf5ebe014e2 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -783,9 +783,21 @@ extern unsigned long efi_mem_attr_table; */ typedef int (*efi_memattr_perm_setter)(struct mm_struct *, efi_memory_desc_t *, bool); +#ifdef CONFIG_EFI_MEMATTR extern int efi_memattr_init(void); extern int efi_memattr_apply_permissions(struct mm_struct *mm, efi_memattr_perm_setter fn); +#else +static inline int efi_memattr_init(void) +{ + return 0; +} +static inline int efi_memattr_apply_permissions(struct mm_struct *mm, + efi_memattr_perm_setter fn) +{ + return 0; +} +#endif /* * efi_memdesc_ptr - get the n-th EFI memmap descriptor
