* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-29 14:12 [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature Ard Biesheuvel
@ 2020-03-30 11:29 ` Mark Rutland
2020-03-30 12:36 ` Ard Biesheuvel
2020-03-30 13:51 ` Will Deacon
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Mark Rutland @ 2020-03-30 11:29 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-arm-kernel, kernel-hardening, catalin.marinas, will
On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> In particular, it permits the segments to be mapped using level 2
> block entries when using 4k pages, which is expected to result in less
> TLB pressure.
>
> However, the mappings for the bulk of the kernel will use level 2
> entries anyway, and the misaligned fringes are organized such that they
> can take advantage of the contiguous bit, and use far fewer level 3
> entries than would be needed otherwise.
>
> This makes the value of this feature dubious at best, and since it is not
> enabled in defconfig or in the distro configs, it does not appear to be
> in wide use either. So let's just remove it.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
No strong feelings either way, but getting rid of code is usually good,
so:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/Kconfig.debug | 13 -------------
> arch/arm64/include/asm/memory.h | 12 +-----------
> drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> 3 files changed, 4 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index 1c906d932d6b..a1efa246c9ed 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -52,19 +52,6 @@ config DEBUG_WX
>
> If in doubt, say "Y".
>
> -config DEBUG_ALIGN_RODATA
> - depends on STRICT_KERNEL_RWX
> - bool "Align linker sections up to SECTION_SIZE"
> - help
> - If this option is enabled, sections that may potentially be marked as
> - read only or non-executable will be aligned up to the section size of
> - the kernel. This prevents sections from being split into pages and
> - avoids a potential TLB penalty. The downside is an increase in
> - alignment and potentially wasted space. Turn on this option if
> - performance is more important than memory pressure.
> -
> - If in doubt, say N.
> -
> config DEBUG_EFI
> depends on EFI && DEBUG_INFO
> bool "UEFI debugging"
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 4d94676e5a8b..3b34f7bde2f2 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -119,22 +119,12 @@
>
> /*
> * Alignment of kernel segments (e.g. .text, .data).
> - */
> -#if defined(CONFIG_DEBUG_ALIGN_RODATA)
> -/*
> - * 4 KB granule: 1 level 2 entry
> - * 16 KB granule: 128 level 3 entries, with contiguous bit
> - * 64 KB granule: 32 level 3 entries, with contiguous bit
> - */
> -#define SEGMENT_ALIGN SZ_2M
> -#else
> -/*
> + *
> * 4 KB granule: 16 level 3 entries, with contiguous bit
> * 16 KB granule: 4 level 3 entries, without contiguous bit
> * 64 KB granule: 1 level 3 entry
> */
> #define SEGMENT_ALIGN SZ_64K
> -#endif
>
> /*
> * Memory types available.
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index db0c1a9c1699..fc9f8ab533a7 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -75,14 +75,12 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>
> if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && phys_seed != 0) {
> /*
> - * If CONFIG_DEBUG_ALIGN_RODATA is not set, produce a
> - * displacement in the interval [0, MIN_KIMG_ALIGN) that
> - * doesn't violate this kernel's de-facto alignment
> + * Produce a displacement in the interval [0, MIN_KIMG_ALIGN)
> + * that doesn't violate this kernel's de-facto alignment
> * constraints.
> */
> u32 mask = (MIN_KIMG_ALIGN - 1) & ~(EFI_KIMG_ALIGN - 1);
> - u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ?
> - (phys_seed >> 32) & mask : TEXT_OFFSET;
> + u32 offset = (phys_seed >> 32) & mask;
>
> /*
> * With CONFIG_RANDOMIZE_TEXT_OFFSET=y, TEXT_OFFSET may not
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 11:29 ` Mark Rutland
@ 2020-03-30 12:36 ` Ard Biesheuvel
0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2020-03-30 12:36 UTC (permalink / raw)
To: Mark Rutland; +Cc: Linux ARM, kernel-hardening, Catalin Marinas, Will Deacon
On Mon, 30 Mar 2020 at 13:29, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> > When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> > different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> > etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> > In particular, it permits the segments to be mapped using level 2
> > block entries when using 4k pages, which is expected to result in less
> > TLB pressure.
> >
> > However, the mappings for the bulk of the kernel will use level 2
> > entries anyway, and the misaligned fringes are organized such that they
> > can take advantage of the contiguous bit, and use far fewer level 3
> > entries than would be needed otherwise.
> >
> > This makes the value of this feature dubious at best, and since it is not
> > enabled in defconfig or in the distro configs, it does not appear to be
> > in wide use either. So let's just remove it.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> No strong feelings either way, but getting rid of code is usually good,
> so:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
Thanks Mark.
This is related to [0], which increases the PE/COFF section alignment
to 64k so that a KASLR enabled kernel always lands at an address at
which it can execute without being moved around first. This is an
improvement in itself, but also provides 5 bits (log2(2M / 64k)) of
wiggle room for the virtual as well as the physical placement of the
kernel. CONFIG_DEBUG_ALIGN_RODATA kind of interferes with that, so I'd
like to get rid of it.
Catalin, Will: if you have no objections, I can include this in my
series for v5.8 and take it via the EFI tree.
Thanks,
Ard.
[0] https://lore.kernel.org/linux-efi/20200326165905.2240-1-ardb@kernel.org/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-29 14:12 [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature Ard Biesheuvel
2020-03-30 11:29 ` Mark Rutland
@ 2020-03-30 13:51 ` Will Deacon
2020-03-30 13:53 ` Ard Biesheuvel
2020-03-30 13:57 ` Laura Abbott
2020-04-02 11:15 ` Catalin Marinas
3 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2020-03-30 13:51 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, kernel-hardening, catalin.marinas, mark.rutland
On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> In particular, it permits the segments to be mapped using level 2
> block entries when using 4k pages, which is expected to result in less
> TLB pressure.
>
> However, the mappings for the bulk of the kernel will use level 2
> entries anyway, and the misaligned fringes are organized such that they
> can take advantage of the contiguous bit, and use far fewer level 3
> entries than would be needed otherwise.
>
> This makes the value of this feature dubious at best, and since it is not
> enabled in defconfig or in the distro configs, it does not appear to be
> in wide use either. So let's just remove it.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm64/Kconfig.debug | 13 -------------
> arch/arm64/include/asm/memory.h | 12 +-----------
> drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> 3 files changed, 4 insertions(+), 29 deletions(-)
Acked-by: Will Deacon <will@kernel.org>
But I would really like to go a step further and rip out the block mapping
support altogether so that we can fix non-coherent DMA aliases:
https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
Will
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 13:51 ` Will Deacon
@ 2020-03-30 13:53 ` Ard Biesheuvel
2020-03-30 13:59 ` Robin Murphy
2020-03-30 14:04 ` Will Deacon
0 siblings, 2 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2020-03-30 13:53 UTC (permalink / raw)
To: Will Deacon; +Cc: Linux ARM, kernel-hardening, Catalin Marinas, Mark Rutland
On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
>
> On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> > When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> > different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> > etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> > In particular, it permits the segments to be mapped using level 2
> > block entries when using 4k pages, which is expected to result in less
> > TLB pressure.
> >
> > However, the mappings for the bulk of the kernel will use level 2
> > entries anyway, and the misaligned fringes are organized such that they
> > can take advantage of the contiguous bit, and use far fewer level 3
> > entries than would be needed otherwise.
> >
> > This makes the value of this feature dubious at best, and since it is not
> > enabled in defconfig or in the distro configs, it does not appear to be
> > in wide use either. So let's just remove it.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > arch/arm64/Kconfig.debug | 13 -------------
> > arch/arm64/include/asm/memory.h | 12 +-----------
> > drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> > 3 files changed, 4 insertions(+), 29 deletions(-)
>
> Acked-by: Will Deacon <will@kernel.org>
>
> But I would really like to go a step further and rip out the block mapping
> support altogether so that we can fix non-coherent DMA aliases:
>
> https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
>
I'm not sure I follow - is this about mapping parts of the static
kernel Image for non-coherent DMA?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 13:53 ` Ard Biesheuvel
@ 2020-03-30 13:59 ` Robin Murphy
2020-03-30 14:04 ` Will Deacon
1 sibling, 0 replies; 19+ messages in thread
From: Robin Murphy @ 2020-03-30 13:59 UTC (permalink / raw)
To: Ard Biesheuvel, Will Deacon
Cc: Mark Rutland, Catalin Marinas, Linux ARM, kernel-hardening
On 2020-03-30 2:53 pm, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
>>
>> On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
>>> When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
>>> different permissions (r-x for .text, r-- for .rodata, rw- for .data,
>>> etc) are rounded up to 2 MiB so they can be mapped more efficiently.
>>> In particular, it permits the segments to be mapped using level 2
>>> block entries when using 4k pages, which is expected to result in less
>>> TLB pressure.
>>>
>>> However, the mappings for the bulk of the kernel will use level 2
>>> entries anyway, and the misaligned fringes are organized such that they
>>> can take advantage of the contiguous bit, and use far fewer level 3
>>> entries than would be needed otherwise.
>>>
>>> This makes the value of this feature dubious at best, and since it is not
>>> enabled in defconfig or in the distro configs, it does not appear to be
>>> in wide use either. So let's just remove it.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>> arch/arm64/Kconfig.debug | 13 -------------
>>> arch/arm64/include/asm/memory.h | 12 +-----------
>>> drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
>>> 3 files changed, 4 insertions(+), 29 deletions(-)
>>
>> Acked-by: Will Deacon <will@kernel.org>
>>
>> But I would really like to go a step further and rip out the block mapping
>> support altogether so that we can fix non-coherent DMA aliases:
>>
>> https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
>>
>
> I'm not sure I follow - is this about mapping parts of the static
> kernel Image for non-coherent DMA?
Yikes, I hope not!
The concern there is about block entries in the linear map; I'd assume
kernel text/data means not-linear-map, and is thus a different kettle of
fish anyway.
Robin.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 13:53 ` Ard Biesheuvel
2020-03-30 13:59 ` Robin Murphy
@ 2020-03-30 14:04 ` Will Deacon
2020-03-30 14:22 ` Ard Biesheuvel
1 sibling, 1 reply; 19+ messages in thread
From: Will Deacon @ 2020-03-30 14:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Linux ARM, kernel-hardening, Catalin Marinas, Mark Rutland
On Mon, Mar 30, 2020 at 03:53:04PM +0200, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
> >
> > On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> > > When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> > > different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> > > etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> > > In particular, it permits the segments to be mapped using level 2
> > > block entries when using 4k pages, which is expected to result in less
> > > TLB pressure.
> > >
> > > However, the mappings for the bulk of the kernel will use level 2
> > > entries anyway, and the misaligned fringes are organized such that they
> > > can take advantage of the contiguous bit, and use far fewer level 3
> > > entries than would be needed otherwise.
> > >
> > > This makes the value of this feature dubious at best, and since it is not
> > > enabled in defconfig or in the distro configs, it does not appear to be
> > > in wide use either. So let's just remove it.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > > arch/arm64/Kconfig.debug | 13 -------------
> > > arch/arm64/include/asm/memory.h | 12 +-----------
> > > drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> > > 3 files changed, 4 insertions(+), 29 deletions(-)
> >
> > Acked-by: Will Deacon <will@kernel.org>
> >
> > But I would really like to go a step further and rip out the block mapping
> > support altogether so that we can fix non-coherent DMA aliases:
> >
> > https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
> >
>
> I'm not sure I follow - is this about mapping parts of the static
> kernel Image for non-coherent DMA?
Sorry, it's not directly related to your patch, just that if we're removing
options relating to kernel mappings then I'd be quite keen on effectively
forcing page-granularity on the linear map, as is currently done by default
thanks to RODATA_FULL_DEFAULT_ENABLED, so that we can nobble cacheable
aliases for non-coherent streaming DMA mappings by hooking into Christoph's
series above.
This series just reminded me of it because it's another
"off-by-default-behaviour-for-block-mappings-probably-because-of-performance-
but-never-actually-measured" type of thing which really just gets in the
way.
Will
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 14:04 ` Will Deacon
@ 2020-03-30 14:22 ` Ard Biesheuvel
2020-03-30 14:28 ` Will Deacon
0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2020-03-30 14:22 UTC (permalink / raw)
To: Will Deacon; +Cc: Linux ARM, kernel-hardening, Catalin Marinas, Mark Rutland
On Mon, 30 Mar 2020 at 16:04, Will Deacon <will@kernel.org> wrote:
>
> On Mon, Mar 30, 2020 at 03:53:04PM +0200, Ard Biesheuvel wrote:
> > On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
> > >
> > > On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> > > > When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> > > > different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> > > > etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> > > > In particular, it permits the segments to be mapped using level 2
> > > > block entries when using 4k pages, which is expected to result in less
> > > > TLB pressure.
> > > >
> > > > However, the mappings for the bulk of the kernel will use level 2
> > > > entries anyway, and the misaligned fringes are organized such that they
> > > > can take advantage of the contiguous bit, and use far fewer level 3
> > > > entries than would be needed otherwise.
> > > >
> > > > This makes the value of this feature dubious at best, and since it is not
> > > > enabled in defconfig or in the distro configs, it does not appear to be
> > > > in wide use either. So let's just remove it.
> > > >
> > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > ---
> > > > arch/arm64/Kconfig.debug | 13 -------------
> > > > arch/arm64/include/asm/memory.h | 12 +-----------
> > > > drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> > > > 3 files changed, 4 insertions(+), 29 deletions(-)
> > >
> > > Acked-by: Will Deacon <will@kernel.org>
> > >
> > > But I would really like to go a step further and rip out the block mapping
> > > support altogether so that we can fix non-coherent DMA aliases:
> > >
> > > https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
> > >
> >
> > I'm not sure I follow - is this about mapping parts of the static
> > kernel Image for non-coherent DMA?
>
> Sorry, it's not directly related to your patch, just that if we're removing
> options relating to kernel mappings then I'd be quite keen on effectively
> forcing page-granularity on the linear map, as is currently done by default
> thanks to RODATA_FULL_DEFAULT_ENABLED, so that we can nobble cacheable
> aliases for non-coherent streaming DMA mappings by hooking into Christoph's
> series above.
>
Right. I don't remember seeing any complaints about
RODATA_FULL_DEFAULT_ENABLED, but maybe it's too early for that.
> This series just reminded me of it because it's another
> "off-by-default-behaviour-for-block-mappings-probably-because-of-performance-
> but-never-actually-measured" type of thing which really just gets in the
> way.
>
Well, even though I agree that the lack of actual numbers is a bit
disturbing here, I'd hate to penalize all systems even more than they
already are (due to ARCH_KMALLOC_MINALIGN == ARCH_DMA_MINALIGN) by
adding another workaround that is only needed on devices that have
non-coherent masters.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 14:22 ` Ard Biesheuvel
@ 2020-03-30 14:28 ` Will Deacon
2020-03-30 14:32 ` Ard Biesheuvel
0 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2020-03-30 14:28 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Linux ARM, kernel-hardening, Catalin Marinas, Mark Rutland
On Mon, Mar 30, 2020 at 04:22:24PM +0200, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 16:04, Will Deacon <will@kernel.org> wrote:
> >
> > On Mon, Mar 30, 2020 at 03:53:04PM +0200, Ard Biesheuvel wrote:
> > > On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
> > > >
> > > > On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> > > > > When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> > > > > different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> > > > > etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> > > > > In particular, it permits the segments to be mapped using level 2
> > > > > block entries when using 4k pages, which is expected to result in less
> > > > > TLB pressure.
> > > > >
> > > > > However, the mappings for the bulk of the kernel will use level 2
> > > > > entries anyway, and the misaligned fringes are organized such that they
> > > > > can take advantage of the contiguous bit, and use far fewer level 3
> > > > > entries than would be needed otherwise.
> > > > >
> > > > > This makes the value of this feature dubious at best, and since it is not
> > > > > enabled in defconfig or in the distro configs, it does not appear to be
> > > > > in wide use either. So let's just remove it.
> > > > >
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > ---
> > > > > arch/arm64/Kconfig.debug | 13 -------------
> > > > > arch/arm64/include/asm/memory.h | 12 +-----------
> > > > > drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> > > > > 3 files changed, 4 insertions(+), 29 deletions(-)
> > > >
> > > > Acked-by: Will Deacon <will@kernel.org>
> > > >
> > > > But I would really like to go a step further and rip out the block mapping
> > > > support altogether so that we can fix non-coherent DMA aliases:
> > > >
> > > > https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
> > > >
> > >
> > > I'm not sure I follow - is this about mapping parts of the static
> > > kernel Image for non-coherent DMA?
> >
> > Sorry, it's not directly related to your patch, just that if we're removing
> > options relating to kernel mappings then I'd be quite keen on effectively
> > forcing page-granularity on the linear map, as is currently done by default
> > thanks to RODATA_FULL_DEFAULT_ENABLED, so that we can nobble cacheable
> > aliases for non-coherent streaming DMA mappings by hooking into Christoph's
> > series above.
> >
>
> Right. I don't remember seeing any complaints about
> RODATA_FULL_DEFAULT_ENABLED, but maybe it's too early for that.
>
> > This series just reminded me of it because it's another
> > "off-by-default-behaviour-for-block-mappings-probably-because-of-performance-
> > but-never-actually-measured" type of thing which really just gets in the
> > way.
> >
>
> Well, even though I agree that the lack of actual numbers is a bit
> disturbing here, I'd hate to penalize all systems even more than they
> already are (due to ARCH_KMALLOC_MINALIGN == ARCH_DMA_MINALIGN) by
> adding another workaround that is only needed on devices that have
> non-coherent masters.
Fair enough, but I'd still like to see some numbers. If they're compelling,
then we could explore something like CONFIG_OF_DMA_DEFAULT_COHERENT, but
that doesn't really help the kconfig maze :(
Will
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 14:28 ` Will Deacon
@ 2020-03-30 14:32 ` Ard Biesheuvel
2020-04-02 11:30 ` Catalin Marinas
0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2020-03-30 14:32 UTC (permalink / raw)
To: Will Deacon; +Cc: Linux ARM, kernel-hardening, Catalin Marinas, Mark Rutland
On Mon, 30 Mar 2020 at 16:28, Will Deacon <will@kernel.org> wrote:
>
> On Mon, Mar 30, 2020 at 04:22:24PM +0200, Ard Biesheuvel wrote:
> > On Mon, 30 Mar 2020 at 16:04, Will Deacon <will@kernel.org> wrote:
> > >
> > > On Mon, Mar 30, 2020 at 03:53:04PM +0200, Ard Biesheuvel wrote:
> > > > On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
> > > > >
> > > > > On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> > > > > > When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> > > > > > different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> > > > > > etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> > > > > > In particular, it permits the segments to be mapped using level 2
> > > > > > block entries when using 4k pages, which is expected to result in less
> > > > > > TLB pressure.
> > > > > >
> > > > > > However, the mappings for the bulk of the kernel will use level 2
> > > > > > entries anyway, and the misaligned fringes are organized such that they
> > > > > > can take advantage of the contiguous bit, and use far fewer level 3
> > > > > > entries than would be needed otherwise.
> > > > > >
> > > > > > This makes the value of this feature dubious at best, and since it is not
> > > > > > enabled in defconfig or in the distro configs, it does not appear to be
> > > > > > in wide use either. So let's just remove it.
> > > > > >
> > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > > ---
> > > > > > arch/arm64/Kconfig.debug | 13 -------------
> > > > > > arch/arm64/include/asm/memory.h | 12 +-----------
> > > > > > drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> > > > > > 3 files changed, 4 insertions(+), 29 deletions(-)
> > > > >
> > > > > Acked-by: Will Deacon <will@kernel.org>
> > > > >
> > > > > But I would really like to go a step further and rip out the block mapping
> > > > > support altogether so that we can fix non-coherent DMA aliases:
> > > > >
> > > > > https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
> > > > >
> > > >
> > > > I'm not sure I follow - is this about mapping parts of the static
> > > > kernel Image for non-coherent DMA?
> > >
> > > Sorry, it's not directly related to your patch, just that if we're removing
> > > options relating to kernel mappings then I'd be quite keen on effectively
> > > forcing page-granularity on the linear map, as is currently done by default
> > > thanks to RODATA_FULL_DEFAULT_ENABLED, so that we can nobble cacheable
> > > aliases for non-coherent streaming DMA mappings by hooking into Christoph's
> > > series above.
> > >
> >
> > Right. I don't remember seeing any complaints about
> > RODATA_FULL_DEFAULT_ENABLED, but maybe it's too early for that.
> >
> > > This series just reminded me of it because it's another
> > > "off-by-default-behaviour-for-block-mappings-probably-because-of-performance-
> > > but-never-actually-measured" type of thing which really just gets in the
> > > way.
> > >
> >
> > Well, even though I agree that the lack of actual numbers is a bit
> > disturbing here, I'd hate to penalize all systems even more than they
> > already are (due to ARCH_KMALLOC_MINALIGN == ARCH_DMA_MINALIGN) by
> > adding another workaround that is only needed on devices that have
> > non-coherent masters.
>
> Fair enough, but I'd still like to see some numbers. If they're compelling,
> then we could explore something like CONFIG_OF_DMA_DEFAULT_COHERENT, but
> that doesn't really help the kconfig maze :(
>
Could we make this a runtime thing? E.g., remap the entire linear
region down to pages under stop_machine() the first time we probe a
device that uses non-coherent DMA?
(/me ducks)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-30 14:32 ` Ard Biesheuvel
@ 2020-04-02 11:30 ` Catalin Marinas
2020-04-02 12:17 ` Mark Rutland
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Catalin Marinas @ 2020-04-02 11:30 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Will Deacon, Linux ARM, kernel-hardening, Mark Rutland
On Mon, Mar 30, 2020 at 04:32:31PM +0200, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 16:28, Will Deacon <will@kernel.org> wrote:
> > > On Mon, 30 Mar 2020 at 16:04, Will Deacon <will@kernel.org> wrote:
> > > > On Mon, Mar 30, 2020 at 03:53:04PM +0200, Ard Biesheuvel wrote:
> > > > > On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
> > > > > > But I would really like to go a step further and rip out the block mapping
> > > > > > support altogether so that we can fix non-coherent DMA aliases:
> > > > > >
> > > > > > https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
> > > > >
> > > > > I'm not sure I follow - is this about mapping parts of the static
> > > > > kernel Image for non-coherent DMA?
> > > >
> > > > Sorry, it's not directly related to your patch, just that if we're removing
> > > > options relating to kernel mappings then I'd be quite keen on effectively
> > > > forcing page-granularity on the linear map, as is currently done by default
> > > > thanks to RODATA_FULL_DEFAULT_ENABLED, so that we can nobble cacheable
> > > > aliases for non-coherent streaming DMA mappings by hooking into Christoph's
> > > > series above.
Have we ever hit this issue in practice? At least from the CPU
perspective, we've assumed that a non-cacheable access would not hit in
the cache. Reading the ARM ARM rules, it doesn't seem to state this
explicitly but we can ask for clarification (I dug out an email from
2015, left unanswered).
Assuming that the CPU is behaving as we'd expect, are there other issues
with peripherals/SMMU?
> > Fair enough, but I'd still like to see some numbers. If they're compelling,
> > then we could explore something like CONFIG_OF_DMA_DEFAULT_COHERENT, but
> > that doesn't really help the kconfig maze :(
I'd prefer not to have a config option, we could easily break single
Image at some point.
> Could we make this a runtime thing? E.g., remap the entire linear
> region down to pages under stop_machine() the first time we probe a
> device that uses non-coherent DMA?
That could be pretty expensive at run-time. With the ARMv8.4-TTRem
feature, I wonder whether we could do this lazily when allocating
non-coherent DMA buffers.
(I still hope there isn't a problem at all with this mismatch ;)).
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-04-02 11:30 ` Catalin Marinas
@ 2020-04-02 12:17 ` Mark Rutland
2020-04-03 7:07 ` Will Deacon
2020-04-03 8:58 ` Ard Biesheuvel
2 siblings, 0 replies; 19+ messages in thread
From: Mark Rutland @ 2020-04-02 12:17 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Ard Biesheuvel, Will Deacon, Linux ARM, kernel-hardening
On Thu, Apr 02, 2020 at 12:30:33PM +0100, Catalin Marinas wrote:
> On Mon, Mar 30, 2020 at 04:32:31PM +0200, Ard Biesheuvel wrote:
> > Could we make this a runtime thing? E.g., remap the entire linear
> > region down to pages under stop_machine() the first time we probe a
> > device that uses non-coherent DMA?
>
> That could be pretty expensive at run-time. With the ARMv8.4-TTRem
> feature, I wonder whether we could do this lazily when allocating
> non-coherent DMA buffers.
It's worth noting that ARMv8.4-TTRem is optional and the "level 1" and
"level 2" behaviours still allows non-fatal faults to be taken while nT
is set (or until you perform the TLB invalidation). We can only safely
use it where we could use a full BBM sequence today.
Effectively TTRem is an optimized, where the CPU *might* be able to use
entries during the break stage, but is not guaranteed to do so.
Thanksm
Mark.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-04-02 11:30 ` Catalin Marinas
2020-04-02 12:17 ` Mark Rutland
@ 2020-04-03 7:07 ` Will Deacon
2020-04-03 8:58 ` Ard Biesheuvel
2 siblings, 0 replies; 19+ messages in thread
From: Will Deacon @ 2020-04-03 7:07 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Ard Biesheuvel, Linux ARM, kernel-hardening, Mark Rutland
On Thu, Apr 02, 2020 at 12:30:33PM +0100, Catalin Marinas wrote:
> On Mon, Mar 30, 2020 at 04:32:31PM +0200, Ard Biesheuvel wrote:
> > On Mon, 30 Mar 2020 at 16:28, Will Deacon <will@kernel.org> wrote:
> > > > On Mon, 30 Mar 2020 at 16:04, Will Deacon <will@kernel.org> wrote:
> > > > > On Mon, Mar 30, 2020 at 03:53:04PM +0200, Ard Biesheuvel wrote:
> > > > > > On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
> > > > > > > But I would really like to go a step further and rip out the block mapping
> > > > > > > support altogether so that we can fix non-coherent DMA aliases:
> > > > > > >
> > > > > > > https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
> > > > > >
> > > > > > I'm not sure I follow - is this about mapping parts of the static
> > > > > > kernel Image for non-coherent DMA?
> > > > >
> > > > > Sorry, it's not directly related to your patch, just that if we're removing
> > > > > options relating to kernel mappings then I'd be quite keen on effectively
> > > > > forcing page-granularity on the linear map, as is currently done by default
> > > > > thanks to RODATA_FULL_DEFAULT_ENABLED, so that we can nobble cacheable
> > > > > aliases for non-coherent streaming DMA mappings by hooking into Christoph's
> > > > > series above.
>
> Have we ever hit this issue in practice? At least from the CPU
> perspective, we've assumed that a non-cacheable access would not hit in
> the cache. Reading the ARM ARM rules, it doesn't seem to state this
> explicitly but we can ask for clarification (I dug out an email from
> 2015, left unanswered).
>
> Assuming that the CPU is behaving as we'd expect, are there other issues
> with peripherals/SMMU?
Any clarification would need to be architectural, I think, and not specific
to the CPU. Worth asking again though.
Will
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-04-02 11:30 ` Catalin Marinas
2020-04-02 12:17 ` Mark Rutland
2020-04-03 7:07 ` Will Deacon
@ 2020-04-03 8:58 ` Ard Biesheuvel
2020-05-05 10:44 ` Will Deacon
2 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2020-04-03 8:58 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Will Deacon, Linux ARM, kernel-hardening, Mark Rutland
On Thu, 2 Apr 2020 at 13:30, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, Mar 30, 2020 at 04:32:31PM +0200, Ard Biesheuvel wrote:
> > On Mon, 30 Mar 2020 at 16:28, Will Deacon <will@kernel.org> wrote:
> > > > On Mon, 30 Mar 2020 at 16:04, Will Deacon <will@kernel.org> wrote:
> > > > > On Mon, Mar 30, 2020 at 03:53:04PM +0200, Ard Biesheuvel wrote:
> > > > > > On Mon, 30 Mar 2020 at 15:51, Will Deacon <will@kernel.org> wrote:
> > > > > > > But I would really like to go a step further and rip out the block mapping
> > > > > > > support altogether so that we can fix non-coherent DMA aliases:
> > > > > > >
> > > > > > > https://lore.kernel.org/lkml/20200224194446.690816-1-hch@lst.de
> > > > > >
> > > > > > I'm not sure I follow - is this about mapping parts of the static
> > > > > > kernel Image for non-coherent DMA?
> > > > >
> > > > > Sorry, it's not directly related to your patch, just that if we're removing
> > > > > options relating to kernel mappings then I'd be quite keen on effectively
> > > > > forcing page-granularity on the linear map, as is currently done by default
> > > > > thanks to RODATA_FULL_DEFAULT_ENABLED, so that we can nobble cacheable
> > > > > aliases for non-coherent streaming DMA mappings by hooking into Christoph's
> > > > > series above.
>
> Have we ever hit this issue in practice? At least from the CPU
> perspective, we've assumed that a non-cacheable access would not hit in
> the cache. Reading the ARM ARM rules, it doesn't seem to state this
> explicitly but we can ask for clarification (I dug out an email from
> 2015, left unanswered).
>
There is some wording in D4.4.5 (Behavior of caches at reset) that
suggests that implementations may permit cache hits in regions that
are mapped Non-cacheable (although the paragraph in question talks
about global controls and not page table attributes)
> Assuming that the CPU is behaving as we'd expect, are there other issues
> with peripherals/SMMU?
>
There is the NoSnoop PCIe issue as well: PCIe masters that are DMA
coherent in general can generate transactions with non-cacheable
attributes. I guess this is mostly orthogonal, but I'm sure it would
be much easier to reason about correctness if it is guaranteed that no
mappings with mismatched attributes exist anywhere.
> > > Fair enough, but I'd still like to see some numbers. If they're compelling,
> > > then we could explore something like CONFIG_OF_DMA_DEFAULT_COHERENT, but
> > > that doesn't really help the kconfig maze :(
>
> I'd prefer not to have a config option, we could easily break single
> Image at some point.
>
> > Could we make this a runtime thing? E.g., remap the entire linear
> > region down to pages under stop_machine() the first time we probe a
> > device that uses non-coherent DMA?
>
> That could be pretty expensive at run-time. With the ARMv8.4-TTRem
> feature, I wonder whether we could do this lazily when allocating
> non-coherent DMA buffers.
>
> (I still hope there isn't a problem at all with this mismatch ;)).
>
Now that we have the pieces to easily remap the linear region down to
pages, and [apparently] some generic infrastructure to manage the
linear aliases, the only downside is the alleged performance hit
resulting from increased TLB pressure. This is obviously highly
micro-architecture dependent, but with Xgene1 and ThunderX1 out of the
picture, I wonder if the tradeoffs are different now. Maybe by now, we
should just suck it up (Note that we had no complaints afaik regarding
the fact that we map the linear map down to pages by default now)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-04-03 8:58 ` Ard Biesheuvel
@ 2020-05-05 10:44 ` Will Deacon
2020-05-07 13:43 ` Catalin Marinas
0 siblings, 1 reply; 19+ messages in thread
From: Will Deacon @ 2020-05-05 10:44 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Catalin Marinas, Linux ARM, kernel-hardening, Mark Rutland
On Fri, Apr 03, 2020 at 10:58:51AM +0200, Ard Biesheuvel wrote:
> On Thu, 2 Apr 2020 at 13:30, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Mon, Mar 30, 2020 at 04:32:31PM +0200, Ard Biesheuvel wrote:
> > > On Mon, 30 Mar 2020 at 16:28, Will Deacon <will@kernel.org> wrote:
> > > > Fair enough, but I'd still like to see some numbers. If they're compelling,
> > > > then we could explore something like CONFIG_OF_DMA_DEFAULT_COHERENT, but
> > > > that doesn't really help the kconfig maze :(
> >
> > I'd prefer not to have a config option, we could easily break single
> > Image at some point.
> >
> > > Could we make this a runtime thing? E.g., remap the entire linear
> > > region down to pages under stop_machine() the first time we probe a
> > > device that uses non-coherent DMA?
> >
> > That could be pretty expensive at run-time. With the ARMv8.4-TTRem
> > feature, I wonder whether we could do this lazily when allocating
> > non-coherent DMA buffers.
> >
> > (I still hope there isn't a problem at all with this mismatch ;)).
> >
>
> Now that we have the pieces to easily remap the linear region down to
> pages, and [apparently] some generic infrastructure to manage the
> linear aliases, the only downside is the alleged performance hit
> resulting from increased TLB pressure. This is obviously highly
> micro-architecture dependent, but with Xgene1 and ThunderX1 out of the
> picture, I wonder if the tradeoffs are different now. Maybe by now, we
> should just suck it up (Note that we had no complaints afaik regarding
> the fact that we map the linear map down to pages by default now)
I'd be in favour of that fwiw.
Catalin -- did you get anything back from the architects about the cache
hit behaviour?
Will
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-05-05 10:44 ` Will Deacon
@ 2020-05-07 13:43 ` Catalin Marinas
0 siblings, 0 replies; 19+ messages in thread
From: Catalin Marinas @ 2020-05-07 13:43 UTC (permalink / raw)
To: Will Deacon; +Cc: Ard Biesheuvel, Linux ARM, kernel-hardening, Mark Rutland
On Tue, May 05, 2020 at 11:44:06AM +0100, Will Deacon wrote:
> Catalin -- did you get anything back from the architects about the cache
> hit behaviour?
Any read from a non-cacheable alias would be coherent with writes using
the same non-cacheable attributes, irrespective of other cacheable
aliases (of course, subject to the cache lines having been previously
cleaned/invalidated to avoid dirty lines evictions).
So as long as the hardware works as per the ARM ARM (B2.8), we don't
need to unmap the non-cacheable DMA buffers from the linear map.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-29 14:12 [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature Ard Biesheuvel
2020-03-30 11:29 ` Mark Rutland
2020-03-30 13:51 ` Will Deacon
@ 2020-03-30 13:57 ` Laura Abbott
2020-04-02 11:15 ` Catalin Marinas
3 siblings, 0 replies; 19+ messages in thread
From: Laura Abbott @ 2020-03-30 13:57 UTC (permalink / raw)
To: Ard Biesheuvel, linux-arm-kernel
Cc: mark.rutland, catalin.marinas, will, kernel-hardening
On 3/29/20 10:12 AM, Ard Biesheuvel wrote:
> When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> In particular, it permits the segments to be mapped using level 2
> block entries when using 4k pages, which is expected to result in less
> TLB pressure.
>
> However, the mappings for the bulk of the kernel will use level 2
> entries anyway, and the misaligned fringes are organized such that they
> can take advantage of the contiguous bit, and use far fewer level 3
> entries than would be needed otherwise.
>
> This makes the value of this feature dubious at best, and since it is not
> enabled in defconfig or in the distro configs, it does not appear to be
> in wide use either. So let's just remove it.
>
This was supposed to avoid potential performance inconsistencies with
some memory being in pages vs other in block. It was always a minor
concern so if it's preventing other work:
Acked-by: Laura Abbott <labbott@kernel.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm64/Kconfig.debug | 13 -------------
> arch/arm64/include/asm/memory.h | 12 +-----------
> drivers/firmware/efi/libstub/arm64-stub.c | 8 +++-----
> 3 files changed, 4 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index 1c906d932d6b..a1efa246c9ed 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -52,19 +52,6 @@ config DEBUG_WX
>
> If in doubt, say "Y".
>
> -config DEBUG_ALIGN_RODATA
> - depends on STRICT_KERNEL_RWX
> - bool "Align linker sections up to SECTION_SIZE"
> - help
> - If this option is enabled, sections that may potentially be marked as
> - read only or non-executable will be aligned up to the section size of
> - the kernel. This prevents sections from being split into pages and
> - avoids a potential TLB penalty. The downside is an increase in
> - alignment and potentially wasted space. Turn on this option if
> - performance is more important than memory pressure.
> -
> - If in doubt, say N.
> -
> config DEBUG_EFI
> depends on EFI && DEBUG_INFO
> bool "UEFI debugging"
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 4d94676e5a8b..3b34f7bde2f2 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -119,22 +119,12 @@
>
> /*
> * Alignment of kernel segments (e.g. .text, .data).
> - */
> -#if defined(CONFIG_DEBUG_ALIGN_RODATA)
> -/*
> - * 4 KB granule: 1 level 2 entry
> - * 16 KB granule: 128 level 3 entries, with contiguous bit
> - * 64 KB granule: 32 level 3 entries, with contiguous bit
> - */
> -#define SEGMENT_ALIGN SZ_2M
> -#else
> -/*
> + *
> * 4 KB granule: 16 level 3 entries, with contiguous bit
> * 16 KB granule: 4 level 3 entries, without contiguous bit
> * 64 KB granule: 1 level 3 entry
> */
> #define SEGMENT_ALIGN SZ_64K
> -#endif
>
> /*
> * Memory types available.
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index db0c1a9c1699..fc9f8ab533a7 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -75,14 +75,12 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>
> if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && phys_seed != 0) {
> /*
> - * If CONFIG_DEBUG_ALIGN_RODATA is not set, produce a
> - * displacement in the interval [0, MIN_KIMG_ALIGN) that
> - * doesn't violate this kernel's de-facto alignment
> + * Produce a displacement in the interval [0, MIN_KIMG_ALIGN)
> + * that doesn't violate this kernel's de-facto alignment
> * constraints.
> */
> u32 mask = (MIN_KIMG_ALIGN - 1) & ~(EFI_KIMG_ALIGN - 1);
> - u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ?
> - (phys_seed >> 32) & mask : TEXT_OFFSET;
> + u32 offset = (phys_seed >> 32) & mask;
>
> /*
> * With CONFIG_RANDOMIZE_TEXT_OFFSET=y, TEXT_OFFSET may not
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-03-29 14:12 [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature Ard Biesheuvel
` (2 preceding siblings ...)
2020-03-30 13:57 ` Laura Abbott
@ 2020-04-02 11:15 ` Catalin Marinas
2020-04-02 11:24 ` Ard Biesheuvel
3 siblings, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2020-04-02 11:15 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-arm-kernel, kernel-hardening, will, mark.rutland
On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> In particular, it permits the segments to be mapped using level 2
> block entries when using 4k pages, which is expected to result in less
> TLB pressure.
>
> However, the mappings for the bulk of the kernel will use level 2
> entries anyway, and the misaligned fringes are organized such that they
> can take advantage of the contiguous bit, and use far fewer level 3
> entries than would be needed otherwise.
>
> This makes the value of this feature dubious at best, and since it is not
> enabled in defconfig or in the distro configs, it does not appear to be
> in wide use either. So let's just remove it.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Happy to take this patch via the arm64 tree for 5.7 (no new
functionality), unless you want it to go with your other relocation
login in the EFI stub patches.
--
Catalin
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH] arm64: remove CONFIG_DEBUG_ALIGN_RODATA feature
2020-04-02 11:15 ` Catalin Marinas
@ 2020-04-02 11:24 ` Ard Biesheuvel
0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2020-04-02 11:24 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Linux ARM, kernel-hardening, Will Deacon, Mark Rutland
On Thu, 2 Apr 2020 at 13:15, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Sun, Mar 29, 2020 at 04:12:58PM +0200, Ard Biesheuvel wrote:
> > When CONFIG_DEBUG_ALIGN_RODATA is enabled, kernel segments mapped with
> > different permissions (r-x for .text, r-- for .rodata, rw- for .data,
> > etc) are rounded up to 2 MiB so they can be mapped more efficiently.
> > In particular, it permits the segments to be mapped using level 2
> > block entries when using 4k pages, which is expected to result in less
> > TLB pressure.
> >
> > However, the mappings for the bulk of the kernel will use level 2
> > entries anyway, and the misaligned fringes are organized such that they
> > can take advantage of the contiguous bit, and use far fewer level 3
> > entries than would be needed otherwise.
> >
> > This makes the value of this feature dubious at best, and since it is not
> > enabled in defconfig or in the distro configs, it does not appear to be
> > in wide use either. So let's just remove it.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Happy to take this patch via the arm64 tree for 5.7 (no new
> functionality), unless you want it to go with your other relocation
> login in the EFI stub patches.
>
If you don't mind taking it for v5.7, please go ahead.
^ permalink raw reply [flat|nested] 19+ messages in thread