Re: [PATCH 1/2] ARM/efi: Remove duplicate permission settings
From: Ard Biesheuvel <ardb@kernel.org>
Date: 2025-10-30 10:36:26
Also in:
linux-efi, lkml
On Thu, 30 Oct 2025 at 11:25, Qiang Ma [off-list ref] wrote:
在 2025/10/30 18:02, Ard Biesheuvel 写道:quoted
On Thu, 30 Oct 2025 at 08:37, Qiang Ma [off-list ref] wrote:quoted
在 2025/10/29 22:15, Ard Biesheuvel 写道:quoted
On Wed, 29 Oct 2025 at 10:55, Qiang Ma [off-list ref] wrote:quoted
在 2025/10/28 21:42, Ard Biesheuvel 写道:quoted
On Mon, 27 Oct 2025 at 04:46, Qiang Ma [off-list ref] wrote:quoted
在 2025/10/23 16:30, Ard Biesheuvel 写道:quoted
On Thu, 23 Oct 2025 at 10:22, Qiang Ma [off-list ref] wrote:quoted
In the efi_virtmap_init(), permission settings have been applied: static bool __init efi_virtmap_init(void) { ... for_each_efi_memory_desc(md) ... efi_create_mapping(&efi_mm, md); ... efi_memattr_apply_permissions(&efi_mm, efi_set_mapping_permissions); ... } Therefore, there is no need to apply it again in the efi_create_mapping(). Fixes: 9fc68b717c24 ("ARM/efi: Apply strict permissions for UEFI Runtime Services regions") Signed-off-by: Qiang Ma <redacted>No, efi_memattr_apply_permissions() uses the /optional/ memory attributes table, whereas efi_create_mapping() uses the permission attributes in the EFI memory map. The memory attributes table is optional, in which case any RO/XP attributes from the memory map should be used.I see. Then, can it be modified like this?Noquoted
--- a/arch/arm/kernel/efi.c +++ b/arch/arm/kernel/efi.c@@ -65,16 +65,13 @@ int __init efi_create_mapping(struct mm_struct *mm,efi_memory_desc_t *md) desc.type = MT_MEMORY_RWX_NONCACHED; else if (md->attribute & EFI_MEMORY_WC) desc.type = MT_DEVICE_WC; + else if (md->attribute & (EFI_MEMORY_RO | EFI_MEMORY_XP))This will be true for RO, XP or RO+XP.quoted
+ desc.type = MT_MEMORY_RO;This will apply RO permissions even to XP regions, which need to be writable.Thanks for your review. I see. I can introduce a new type MT_MEMORY_RO_XP, to describe RO+XP, and then we can use the RO+XP attribute to implement memory mapping.Why? The current code is working fine, no?Yes, the current code is running normally. The reasons for the modification are as follows: I noticed that the arm64/RISC-V efi_create_mapping() always return 0, but in the code where efi_virtmap_init() calls it, it is as follows: ret = efi_create_mapping(&efi_mm, md); if (ret) { pr_warn(" EFI remap %pa: failed to create mapping (%d)\n", &phys, ret); return false; } This return error print is unnecessary, so I want to remove it.So what is preventing you from removing this from the RISC-V version?The current idea is to first remove the unnecessary return print from arm/arm64,
Please leave the ARM code alone.
and then remove RISC-V later, as this RISC-V code is also adapted based on arm64.
RISC-V copied the ARM code and used it as a starting point. That does not mean it has to remain that way.