[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d3ee6521-fae3-41b2-a759-e454681dde6f@gmail.com>
Date: Fri, 10 Jan 2025 14:31:14 +0000
From: Usama Arif <usamaarif642@...il.com>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-efi@...r.kernel.org, devel@...2.groups.io,
kexec@...ts.infradead.org, hannes@...xchg.org, dyoung@...hat.com,
x86@...nel.org, linux-kernel@...r.kernel.org, leitao@...ian.org,
gourry@...rry.net, kernel-team@...a.com
Subject: Re: [RFC 2/2] efi/memattr: add efi_mem_attr_table as a reserved
region in 820_table_firmware
On 10/01/2025 07:32, Ard Biesheuvel wrote:
> On Thu, 9 Jan 2025 at 17:32, Usama Arif <usamaarif642@...il.com> 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@gmail.com/
>> 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
Powered by blists - more mailing lists