[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <006327B65B574934+440b408c-499d-4ec9-837e-703991ad51fc@uniontech.com>
Date: Thu, 30 Oct 2025 18:44:58 +0800
From: Qiang Ma <maqianga@...ontech.com>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux@...linux.org.uk, linux-efi@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] ARM/efi: Remove duplicate permission settings
在 2025/10/30 18:36, Ard Biesheuvel 写道:
> On Thu, 30 Oct 2025 at 11:25, Qiang Ma <maqianga@...ontech.com> wrote:
>>
>> 在 2025/10/30 18:02, Ard Biesheuvel 写道:
>>> On Thu, 30 Oct 2025 at 08:37, Qiang Ma <maqianga@...ontech.com> wrote:
>>>> 在 2025/10/29 22:15, Ard Biesheuvel 写道:
>>>>> On Wed, 29 Oct 2025 at 10:55, Qiang Ma <maqianga@...ontech.com> wrote:
>>>>>> 在 2025/10/28 21:42, Ard Biesheuvel 写道:
>>>>>>> On Mon, 27 Oct 2025 at 04:46, Qiang Ma <maqianga@...ontech.com> wrote:
>>>>>>>> 在 2025/10/23 16:30, Ard Biesheuvel 写道:
>>>>>>>>> On Thu, 23 Oct 2025 at 10:22, Qiang Ma <maqianga@...ontech.com> 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@...ontech.com>
>>>>>>>>> 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?
>>>>>>> No
>>>>>>>
>>>>>>>> --- 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.
>>>>>>>
>>>>>>>> +               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.
I see.
>> 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.
I can propose a patch specifically for RISC-V.
Powered by blists - more mailing lists
 
