* [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
@ 2022-11-06 14:53 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2022-11-06 14:53 UTC (permalink / raw)
To: torvalds
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Ard Biesheuvel, Heinrich Schuchardt, Ilias Apalodimas
Currently, when mapping the EFI runtime regions in the EFI page tables,
we complain about misaligned regions in a rather noisy way, using
WARN().
Not only does this produce a lot of irrelevant clutter in the log, it is
factually incorrect, as misaligned runtime regions are actually allowed
by the EFI spec as long as they don't require conflicting memory types
within the same 64k page.
So let's drop the warning, and tweak the code so that we
- take both the start and end of the region into account when checking
for misalignment
- only revert to RWX mappings for non-code regions if misaligned code
regions are also known to exist.
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
Tested with uboot on QEMU/mach-virt using a 64k pagesize kernel build.
More details after the patch.
arch/arm64/kernel/efi.c | 52 +++++++++++++-------
1 file changed, 34 insertions(+), 18 deletions(-)
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index e1be6c429810d0d5..a908a37f03678b6b 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -12,6 +12,14 @@
#include <asm/efi.h>
+static bool region_is_misaligned(const efi_memory_desc_t *md)
+{
+ if (PAGE_SIZE == EFI_PAGE_SIZE)
+ return false;
+ return !PAGE_ALIGNED(md->phys_addr) ||
+ !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT);
+}
+
/*
* Only regions of type EFI_RUNTIME_SERVICES_CODE need to be
* executable, everything else can be mapped with the XN bits
@@ -25,14 +33,22 @@ static __init pteval_t create_mapping_protection(efi_memory_desc_t *md)
if (type == EFI_MEMORY_MAPPED_IO)
return PROT_DEVICE_nGnRE;
- if (WARN_ONCE(!PAGE_ALIGNED(md->phys_addr),
- "UEFI Runtime regions are not aligned to 64 KB -- buggy firmware?"))
+ if (region_is_misaligned(md)) {
+ static bool __initdata code_is_misaligned;
+
/*
- * If the region is not aligned to the page size of the OS, we
- * can not use strict permissions, since that would also affect
- * the mapping attributes of the adjacent regions.
+ * Regions that are not aligned to the OS page size cannot be
+ * mapped with strict permissions, as those might interfere
+ * with the permissions that are needed by the adjacent
+ * region's mapping. However, if we haven't encountered any
+ * misaligned runtime code regions so far, we can safely use
+ * non-executable permissions for non-code regions.
*/
- return pgprot_val(PAGE_KERNEL_EXEC);
+ code_is_misaligned |= (type == EFI_RUNTIME_SERVICES_CODE);
+
+ return code_is_misaligned ? pgprot_val(PAGE_KERNEL_EXEC)
+ : pgprot_val(PAGE_KERNEL);
+ }
/* R-- */
if ((attr & (EFI_MEMORY_XP | EFI_MEMORY_RO)) ==
@@ -63,19 +79,16 @@ int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
bool page_mappings_only = (md->type == EFI_RUNTIME_SERVICES_CODE ||
md->type == EFI_RUNTIME_SERVICES_DATA);
- if (!PAGE_ALIGNED(md->phys_addr) ||
- !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) {
- /*
- * If the end address of this region is not aligned to page
- * size, the mapping is rounded up, and may end up sharing a
- * page frame with the next UEFI memory region. If we create
- * a block entry now, we may need to split it again when mapping
- * the next region, and support for that is going to be removed
- * from the MMU routines. So avoid block mappings altogether in
- * that case.
- */
+ /*
+ * If this region is not aligned to the page size used by the OS, the
+ * mapping will be rounded outwards, and may end up sharing a page
+ * frame with an adjacent runtime memory region. Given that the page
+ * table descriptor covering the shared page will be rewritten when the
+ * adjacent region gets mapped, we must avoid block mappings here so we
+ * don't have to worry about splitting them when that happens.
+ */
+ if (region_is_misaligned(md))
page_mappings_only = true;
- }
create_pgd_mapping(mm, md->phys_addr, md->virt_addr,
md->num_pages << EFI_PAGE_SHIFT,
@@ -102,6 +115,9 @@ int __init efi_set_mapping_permissions(struct mm_struct *mm,
BUG_ON(md->type != EFI_RUNTIME_SERVICES_CODE &&
md->type != EFI_RUNTIME_SERVICES_DATA);
+ if (region_is_misaligned(md))
+ return 0;
+
/*
* Calling apply_to_page_range() is only safe on regions that are
* guaranteed to be mapped down to pages. Since we are only called
--
2.35.1
Output from a 64k page size kernel build booting via U-boot's EFI
implementation.
Note that the misaligned data regions are not mapped with executable
permissions with this patch applied. The code region is mapped with both
write and execute permissions, but this is unavoidable given that the
region covers statically allocated read-write data as well.
Note that these mappings are only live on a single CPU while a runtime
service call is in progress so none of this is critical in any case.
# cat /sys/kernel/debug/efi_page_tables
---[ UEFI runtime start ]---
0x0000000000000000-0x00000000e0000000 3584M PMD
0x00000000e0000000-0x00000000ffff0000 524224K PTE
0x00000000ffff0000-0x0000000100000000 64K PTE RW NX SHD AF NG UXN MEM/NORMAL
0x0000000100000000-0x0000000420000000 12800M PMD
0x0000000420000000-0x000000043dd60000 488832K PTE
0x000000043dd60000-0x000000043dd70000 64K PTE RW NX SHD AF NG UXN MEM/NORMAL
0x000000043dd70000-0x000000043ddc0000 320K PTE
0x000000043ddc0000-0x000000043ddd0000 64K PTE RW NX SHD AF NG UXN MEM/NORMAL
0x000000043ddd0000-0x000000043ff10000 34048K PTE
0x000000043ff10000-0x000000043ff20000 64K PTE RW x SHD AF NG UXN MEM/NORMAL
0x000000043ff20000-0x0000000440000000 896K PTE
efi: Processing EFI memory map:
efi: 0x000040000000-0x000047dfcfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x000047dfd000-0x000048002fff [ACPI Reclaim| | | | | | | | | | |WB| | | ]
efi: 0x000048003000-0x0000ffffdfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x0000ffffe000-0x0000ffffefff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x0000fffff000-0x0001c389ffff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x0001c38a0000-0x0001c73affff [Loader Code | | | | | | | | | | |WB| | | ]
efi: 0x0001c73b0000-0x0004368effff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x0004368f0000-0x00043a3fffff [Loader Code | | | | | | | | | | |WB| | | ]
efi: 0x00043a400000-0x00043db4ffff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x00043db50000-0x00043dd4ffff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd50000-0x00043dd5bfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x00043dd5c000-0x00043dd5cfff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd5d000-0x00043dd5dfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x00043dd5e000-0x00043dd5efff [ACPI Reclaim| | | | | | | | | | |WB| | | ]
efi: 0x00043dd5f000-0x00043dd5ffff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd60000-0x00043dd60fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043dd61000-0x00043dd61fff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd62000-0x00043dd63fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd64000-0x00043dd66fff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd67000-0x00043dd67fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd68000-0x00043dd97fff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd98000-0x00043dd98fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd99000-0x00043dd9bfff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd9c000-0x00043ddb5fff [Loader Code | | | | | | | | | | |WB| | | ]
efi: 0x00043ddb6000-0x00043ddc0fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddc1000-0x00043ddc1fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ddc2000-0x00043ddc2fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddc3000-0x00043ddc4fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ddc5000-0x00043ddc5fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddc6000-0x00043ddc9fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ddca000-0x00043ddd2fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddd3000-0x00043ff0ffff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ff10000-0x00043ff1ffff [Runtime Code|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ff20000-0x00043fffffff [Loader Data | | | | | | | | | | |WB| | | ]
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
@ 2022-11-06 14:53 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2022-11-06 14:53 UTC (permalink / raw)
To: torvalds
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Ard Biesheuvel, Heinrich Schuchardt, Ilias Apalodimas
Currently, when mapping the EFI runtime regions in the EFI page tables,
we complain about misaligned regions in a rather noisy way, using
WARN().
Not only does this produce a lot of irrelevant clutter in the log, it is
factually incorrect, as misaligned runtime regions are actually allowed
by the EFI spec as long as they don't require conflicting memory types
within the same 64k page.
So let's drop the warning, and tweak the code so that we
- take both the start and end of the region into account when checking
for misalignment
- only revert to RWX mappings for non-code regions if misaligned code
regions are also known to exist.
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
Tested with uboot on QEMU/mach-virt using a 64k pagesize kernel build.
More details after the patch.
arch/arm64/kernel/efi.c | 52 +++++++++++++-------
1 file changed, 34 insertions(+), 18 deletions(-)
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index e1be6c429810d0d5..a908a37f03678b6b 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -12,6 +12,14 @@
#include <asm/efi.h>
+static bool region_is_misaligned(const efi_memory_desc_t *md)
+{
+ if (PAGE_SIZE == EFI_PAGE_SIZE)
+ return false;
+ return !PAGE_ALIGNED(md->phys_addr) ||
+ !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT);
+}
+
/*
* Only regions of type EFI_RUNTIME_SERVICES_CODE need to be
* executable, everything else can be mapped with the XN bits
@@ -25,14 +33,22 @@ static __init pteval_t create_mapping_protection(efi_memory_desc_t *md)
if (type == EFI_MEMORY_MAPPED_IO)
return PROT_DEVICE_nGnRE;
- if (WARN_ONCE(!PAGE_ALIGNED(md->phys_addr),
- "UEFI Runtime regions are not aligned to 64 KB -- buggy firmware?"))
+ if (region_is_misaligned(md)) {
+ static bool __initdata code_is_misaligned;
+
/*
- * If the region is not aligned to the page size of the OS, we
- * can not use strict permissions, since that would also affect
- * the mapping attributes of the adjacent regions.
+ * Regions that are not aligned to the OS page size cannot be
+ * mapped with strict permissions, as those might interfere
+ * with the permissions that are needed by the adjacent
+ * region's mapping. However, if we haven't encountered any
+ * misaligned runtime code regions so far, we can safely use
+ * non-executable permissions for non-code regions.
*/
- return pgprot_val(PAGE_KERNEL_EXEC);
+ code_is_misaligned |= (type == EFI_RUNTIME_SERVICES_CODE);
+
+ return code_is_misaligned ? pgprot_val(PAGE_KERNEL_EXEC)
+ : pgprot_val(PAGE_KERNEL);
+ }
/* R-- */
if ((attr & (EFI_MEMORY_XP | EFI_MEMORY_RO)) ==
@@ -63,19 +79,16 @@ int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md)
bool page_mappings_only = (md->type == EFI_RUNTIME_SERVICES_CODE ||
md->type == EFI_RUNTIME_SERVICES_DATA);
- if (!PAGE_ALIGNED(md->phys_addr) ||
- !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) {
- /*
- * If the end address of this region is not aligned to page
- * size, the mapping is rounded up, and may end up sharing a
- * page frame with the next UEFI memory region. If we create
- * a block entry now, we may need to split it again when mapping
- * the next region, and support for that is going to be removed
- * from the MMU routines. So avoid block mappings altogether in
- * that case.
- */
+ /*
+ * If this region is not aligned to the page size used by the OS, the
+ * mapping will be rounded outwards, and may end up sharing a page
+ * frame with an adjacent runtime memory region. Given that the page
+ * table descriptor covering the shared page will be rewritten when the
+ * adjacent region gets mapped, we must avoid block mappings here so we
+ * don't have to worry about splitting them when that happens.
+ */
+ if (region_is_misaligned(md))
page_mappings_only = true;
- }
create_pgd_mapping(mm, md->phys_addr, md->virt_addr,
md->num_pages << EFI_PAGE_SHIFT,
@@ -102,6 +115,9 @@ int __init efi_set_mapping_permissions(struct mm_struct *mm,
BUG_ON(md->type != EFI_RUNTIME_SERVICES_CODE &&
md->type != EFI_RUNTIME_SERVICES_DATA);
+ if (region_is_misaligned(md))
+ return 0;
+
/*
* Calling apply_to_page_range() is only safe on regions that are
* guaranteed to be mapped down to pages. Since we are only called
--
2.35.1
Output from a 64k page size kernel build booting via U-boot's EFI
implementation.
Note that the misaligned data regions are not mapped with executable
permissions with this patch applied. The code region is mapped with both
write and execute permissions, but this is unavoidable given that the
region covers statically allocated read-write data as well.
Note that these mappings are only live on a single CPU while a runtime
service call is in progress so none of this is critical in any case.
# cat /sys/kernel/debug/efi_page_tables
---[ UEFI runtime start ]---
0x0000000000000000-0x00000000e0000000 3584M PMD
0x00000000e0000000-0x00000000ffff0000 524224K PTE
0x00000000ffff0000-0x0000000100000000 64K PTE RW NX SHD AF NG UXN MEM/NORMAL
0x0000000100000000-0x0000000420000000 12800M PMD
0x0000000420000000-0x000000043dd60000 488832K PTE
0x000000043dd60000-0x000000043dd70000 64K PTE RW NX SHD AF NG UXN MEM/NORMAL
0x000000043dd70000-0x000000043ddc0000 320K PTE
0x000000043ddc0000-0x000000043ddd0000 64K PTE RW NX SHD AF NG UXN MEM/NORMAL
0x000000043ddd0000-0x000000043ff10000 34048K PTE
0x000000043ff10000-0x000000043ff20000 64K PTE RW x SHD AF NG UXN MEM/NORMAL
0x000000043ff20000-0x0000000440000000 896K PTE
efi: Processing EFI memory map:
efi: 0x000040000000-0x000047dfcfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x000047dfd000-0x000048002fff [ACPI Reclaim| | | | | | | | | | |WB| | | ]
efi: 0x000048003000-0x0000ffffdfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x0000ffffe000-0x0000ffffefff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x0000fffff000-0x0001c389ffff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x0001c38a0000-0x0001c73affff [Loader Code | | | | | | | | | | |WB| | | ]
efi: 0x0001c73b0000-0x0004368effff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x0004368f0000-0x00043a3fffff [Loader Code | | | | | | | | | | |WB| | | ]
efi: 0x00043a400000-0x00043db4ffff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x00043db50000-0x00043dd4ffff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd50000-0x00043dd5bfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x00043dd5c000-0x00043dd5cfff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd5d000-0x00043dd5dfff [Conventional| | | | | | | | | | |WB| | | ]
efi: 0x00043dd5e000-0x00043dd5efff [ACPI Reclaim| | | | | | | | | | |WB| | | ]
efi: 0x00043dd5f000-0x00043dd5ffff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd60000-0x00043dd60fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043dd61000-0x00043dd61fff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd62000-0x00043dd63fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd64000-0x00043dd66fff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd67000-0x00043dd67fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd68000-0x00043dd97fff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd98000-0x00043dd98fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd99000-0x00043dd9bfff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043dd9c000-0x00043ddb5fff [Loader Code | | | | | | | | | | |WB| | | ]
efi: 0x00043ddb6000-0x00043ddc0fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddc1000-0x00043ddc1fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ddc2000-0x00043ddc2fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddc3000-0x00043ddc4fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ddc5000-0x00043ddc5fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddc6000-0x00043ddc9fff [Runtime Data|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ddca000-0x00043ddd2fff [Boot Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ddd3000-0x00043ff0ffff [Loader Data | | | | | | | | | | |WB| | | ]
efi: 0x00043ff10000-0x00043ff1ffff [Runtime Code|RUN| | | | | | | | | |WB| | | ]
efi: 0x00043ff20000-0x00043fffffff [Loader Data | | | | | | | | | | |WB| | | ]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
2022-11-06 14:53 ` Ard Biesheuvel
@ 2022-11-06 19:38 ` Linus Torvalds
-1 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2022-11-06 19:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Heinrich Schuchardt, Ilias Apalodimas
[ Note: in quoting the patch below, I removed the '-' lines, so the
quoted part is really just all that remains after the patch ]
On Sun, Nov 6, 2022 at 6:54 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> + if (region_is_misaligned(md)) {
> + static bool __initdata code_is_misaligned;
> +
> /*
> + * Regions that are not aligned to the OS page size cannot be
> + * mapped with strict permissions, as those might interfere
> + * with the permissions that are needed by the adjacent
> + * region's mapping. However, if we haven't encountered any
> + * misaligned runtime code regions so far, we can safely use
> + * non-executable permissions for non-code regions.
> */
> + code_is_misaligned |= (type == EFI_RUNTIME_SERVICES_CODE);
> +
> + return code_is_misaligned ? pgprot_val(PAGE_KERNEL_EXEC)
> + : pgprot_val(PAGE_KERNEL);
> + }
Ok, this seems like a nice improvement, in how it only does
PAGE_KERNEL_EXEC if any of the regions end up being code.
At the same time this is a much bigger change than just changing the
WARN_ONCE() to pr_warn_once(), so rather than me applying it directly
to my tree, I think I'd prefer it to go through the proper channels
and the usual way.
I'll still apply it to my private "this is the tree I actually boot on
my M2" testing tree, since that has all the other Asahi patches too.
That way I won't see the warning myself on that machine.
So "Acked-by" on the patch, and I hope I'll see it with a future arm64
or EFI pull request (and I'll holler loudly if it actually causes any
issues on my M2, but I obviously don't expect it to)
Thanks,
Linus
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
@ 2022-11-06 19:38 ` Linus Torvalds
0 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2022-11-06 19:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Heinrich Schuchardt, Ilias Apalodimas
[ Note: in quoting the patch below, I removed the '-' lines, so the
quoted part is really just all that remains after the patch ]
On Sun, Nov 6, 2022 at 6:54 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> + if (region_is_misaligned(md)) {
> + static bool __initdata code_is_misaligned;
> +
> /*
> + * Regions that are not aligned to the OS page size cannot be
> + * mapped with strict permissions, as those might interfere
> + * with the permissions that are needed by the adjacent
> + * region's mapping. However, if we haven't encountered any
> + * misaligned runtime code regions so far, we can safely use
> + * non-executable permissions for non-code regions.
> */
> + code_is_misaligned |= (type == EFI_RUNTIME_SERVICES_CODE);
> +
> + return code_is_misaligned ? pgprot_val(PAGE_KERNEL_EXEC)
> + : pgprot_val(PAGE_KERNEL);
> + }
Ok, this seems like a nice improvement, in how it only does
PAGE_KERNEL_EXEC if any of the regions end up being code.
At the same time this is a much bigger change than just changing the
WARN_ONCE() to pr_warn_once(), so rather than me applying it directly
to my tree, I think I'd prefer it to go through the proper channels
and the usual way.
I'll still apply it to my private "this is the tree I actually boot on
my M2" testing tree, since that has all the other Asahi patches too.
That way I won't see the warning myself on that machine.
So "Acked-by" on the patch, and I hope I'll see it with a future arm64
or EFI pull request (and I'll holler loudly if it actually causes any
issues on my M2, but I obviously don't expect it to)
Thanks,
Linus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
2022-11-06 19:38 ` Linus Torvalds
@ 2022-11-06 22:34 ` Ard Biesheuvel
-1 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2022-11-06 22:34 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Heinrich Schuchardt, Ilias Apalodimas
On Sun, 6 Nov 2022 at 20:38, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> [ Note: in quoting the patch below, I removed the '-' lines, so the
> quoted part is really just all that remains after the patch ]
>
> On Sun, Nov 6, 2022 at 6:54 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > + if (region_is_misaligned(md)) {
> > + static bool __initdata code_is_misaligned;
> > +
> > /*
> > + * Regions that are not aligned to the OS page size cannot be
> > + * mapped with strict permissions, as those might interfere
> > + * with the permissions that are needed by the adjacent
> > + * region's mapping. However, if we haven't encountered any
> > + * misaligned runtime code regions so far, we can safely use
> > + * non-executable permissions for non-code regions.
> > */
> > + code_is_misaligned |= (type == EFI_RUNTIME_SERVICES_CODE);
> > +
> > + return code_is_misaligned ? pgprot_val(PAGE_KERNEL_EXEC)
> > + : pgprot_val(PAGE_KERNEL);
> > + }
>
> Ok, this seems like a nice improvement, in how it only does
> PAGE_KERNEL_EXEC if any of the regions end up being code.
>
> At the same time this is a much bigger change than just changing the
> WARN_ONCE() to pr_warn_once(), so rather than me applying it directly
> to my tree, I think I'd prefer it to go through the proper channels
> and the usual way.
>
> I'll still apply it to my private "this is the tree I actually boot on
> my M2" testing tree, since that has all the other Asahi patches too.
> That way I won't see the warning myself on that machine.
>
> So "Acked-by" on the patch, and I hope I'll see it with a future arm64
> or EFI pull request (and I'll holler loudly if it actually causes any
> issues on my M2, but I obviously don't expect it to)
>
Thanks for the ack. I'll drop it in the EFI fixes branch and let it
soak in -next for the week.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
@ 2022-11-06 22:34 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2022-11-06 22:34 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Heinrich Schuchardt, Ilias Apalodimas
On Sun, 6 Nov 2022 at 20:38, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> [ Note: in quoting the patch below, I removed the '-' lines, so the
> quoted part is really just all that remains after the patch ]
>
> On Sun, Nov 6, 2022 at 6:54 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > + if (region_is_misaligned(md)) {
> > + static bool __initdata code_is_misaligned;
> > +
> > /*
> > + * Regions that are not aligned to the OS page size cannot be
> > + * mapped with strict permissions, as those might interfere
> > + * with the permissions that are needed by the adjacent
> > + * region's mapping. However, if we haven't encountered any
> > + * misaligned runtime code regions so far, we can safely use
> > + * non-executable permissions for non-code regions.
> > */
> > + code_is_misaligned |= (type == EFI_RUNTIME_SERVICES_CODE);
> > +
> > + return code_is_misaligned ? pgprot_val(PAGE_KERNEL_EXEC)
> > + : pgprot_val(PAGE_KERNEL);
> > + }
>
> Ok, this seems like a nice improvement, in how it only does
> PAGE_KERNEL_EXEC if any of the regions end up being code.
>
> At the same time this is a much bigger change than just changing the
> WARN_ONCE() to pr_warn_once(), so rather than me applying it directly
> to my tree, I think I'd prefer it to go through the proper channels
> and the usual way.
>
> I'll still apply it to my private "this is the tree I actually boot on
> my M2" testing tree, since that has all the other Asahi patches too.
> That way I won't see the warning myself on that machine.
>
> So "Acked-by" on the patch, and I hope I'll see it with a future arm64
> or EFI pull request (and I'll holler loudly if it actually causes any
> issues on my M2, but I obviously don't expect it to)
>
Thanks for the ack. I'll drop it in the EFI fixes branch and let it
soak in -next for the week.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
2022-11-06 22:34 ` Ard Biesheuvel
@ 2022-11-06 22:35 ` Linus Torvalds
-1 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2022-11-06 22:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Heinrich Schuchardt, Ilias Apalodimas
On Sun, Nov 6, 2022 at 2:34 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> > So "Acked-by" on the patch, and I hope I'll see it with a future arm64
> > or EFI pull request (and I'll holler loudly if it actually causes any
> > issues on my M2, but I obviously don't expect it to)
>
> Thanks for the ack. I'll drop it in the EFI fixes branch and let it
> soak in -next for the week.
.. and I guess I might as well make it explicit that yes, my M2 is
happy with the patch, rather than just that implicit silence about it.
Linus
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning
@ 2022-11-06 22:35 ` Linus Torvalds
0 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2022-11-06 22:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, linux-arm-kernel, catalin.marinas, will,
Heinrich Schuchardt, Ilias Apalodimas
On Sun, Nov 6, 2022 at 2:34 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> > So "Acked-by" on the patch, and I hope I'll see it with a future arm64
> > or EFI pull request (and I'll holler loudly if it actually causes any
> > issues on my M2, but I obviously don't expect it to)
>
> Thanks for the ack. I'll drop it in the EFI fixes branch and let it
> soak in -next for the week.
.. and I guess I might as well make it explicit that yes, my M2 is
happy with the patch, rather than just that implicit silence about it.
Linus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-06 22:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-06 14:53 [PATCH v2] arm64: efi: Fix handling of misaligned runtime regions and drop warning Ard Biesheuvel
2022-11-06 14:53 ` Ard Biesheuvel
2022-11-06 19:38 ` Linus Torvalds
2022-11-06 19:38 ` Linus Torvalds
2022-11-06 22:34 ` Ard Biesheuvel
2022-11-06 22:34 ` Ard Biesheuvel
2022-11-06 22:35 ` Linus Torvalds
2022-11-06 22:35 ` Linus Torvalds
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.