Re: [PATCH 1/2] ARM/efi: Remove duplicate permission settings

[Qiang Ma]

在 2025/10/23 16:30, Ard Biesheuvel 写道:
> On Thu, 23 Oct 2025 at 10:22, Qiang Ma <maqianga@xxxxxxxxxxxxx> wrote:
> > 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 <maqianga@xxxxxxxxxxxxx>
> 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?
--- 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))
+               desc.type = MT_MEMORY_RO;
        else
                desc.type = MT_DEVICE;

        create_mapping_late(mm, &desc, true);

-       /*
-        * If stricter permissions were specified, apply them now.
-        */
-       if (md->attribute & (EFI_MEMORY_RO | EFI_MEMORY_XP))
-               return efi_set_mapping_permissions(mm, md, false);
        return 0;
 }

The create_mapping_late() finds the corresponding page table attribute 
from mem_types through type.
Here it is MT_MEMORY_RO, and its corresponding prot_pte is:
      ...
        [MT_MEMORY_RO] = {
                .prot_pte  = L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY |
                             L_PTE_XN | L_PTE_RDONLY,
      ...

Finally, the page table is also set through the set_pte_ext().


Thanks.

> > ---
> >   arch/arm/kernel/efi.c | 5 -----
> >   1 file changed, 5 deletions(-)
> >
> > diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
> > index 6f9ec7d28a71..d2fca20d912e 100644
> > --- a/arch/arm/kernel/efi.c
> > +++ b/arch/arm/kernel/efi.c
> > @@ -70,11 +70,6 @@ int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
> >
> >          create_mapping_late(mm, &desc, true);
> >
> > -       /*
> > -        * If stricter permissions were specified, apply them now.
> > -        */
> > -       if (md->attribute & (EFI_MEMORY_RO | EFI_MEMORY_XP))
> > -               return efi_set_mapping_permissions(mm, md, false);
> >          return 0;
> >   }
> >
> > --
> > 2.20.1