Re: [edk2-devel] [RFC 2/2] efi/memattr: add efi_mem_attr_table as a reserved region in 820_table_firmware

["Usama Arif via groups.io" <usamaarif642=gmail.com@groups.io>]

On 10/01/2025 07:32, Ard Biesheuvel wrote:
> On Thu, 9 Jan 2025 at 17:32, Usama Arif <usamaarif642@gmail.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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#121013): https://edk2.groups.io/g/devel/message/121013
Mute This Topic: https://groups.io/mt/110518541/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-