2021-12-18 08:57:49

by Gaosheng Cui

[permalink] [raw]
Subject: [PATCH -next 0/3] replace open coded VA->PA calculation

These patches replace an open coded calculation to obtain the physical
address of a far symbol with a call to the new ldr_l etc macro, and they
belong to the kaslr patch set of arm32.

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm-kaslr-latest

Ard Biesheuvel (3):
arm-soc: exynos: replace open coded VA->PA conversions
arm-soc: mvebu: replace open coded VA->PA conversion
arm-soc: various: replace open coded VA->PA calculation

arch/arm/mach-exynos/headsmp.S | 9 +--------
arch/arm/mach-exynos/sleep.S | 26 +++++---------------------
arch/arm/mach-mvebu/coherency_ll.S | 8 +-------
arch/arm/mach-spear/headsmp.S | 11 +++--------
arch/arm/plat-versatile/headsmp.S | 9 +--------
5 files changed, 11 insertions(+), 52 deletions(-)

--
2.30.0



2021-12-18 08:57:51

by Gaosheng Cui

[permalink] [raw]
Subject: [PATCH -next 3/3] arm-soc: various: replace open coded VA->PA calculation

From: Ard Biesheuvel <[email protected]>

This replaces a few copies of the open coded calculations of the
physical address in the secondary startup code of a couple of
platforms. This ensures these quantities are invariant under
runtime relocation.

Cc: Russell King <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Gaosheng Cui <[email protected]>
---
arch/arm/mach-spear/headsmp.S | 11 +++--------
arch/arm/plat-versatile/headsmp.S | 9 +--------
2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-spear/headsmp.S b/arch/arm/mach-spear/headsmp.S
index 96f89436ccf6..32ffc75ff332 100644
--- a/arch/arm/mach-spear/headsmp.S
+++ b/arch/arm/mach-spear/headsmp.S
@@ -10,6 +10,8 @@
#include <linux/linkage.h>
#include <linux/init.h>

+#include <asm/assembler.h>
+
__INIT

/*
@@ -20,10 +22,7 @@
ENTRY(spear13xx_secondary_startup)
mrc p15, 0, r0, c0, c0, 5
and r0, r0, #15
- adr r4, 1f
- ldmia r4, {r5, r6}
- sub r4, r4, r5
- add r6, r6, r4
+ adr_l r6, spear_pen_release
pen: ldr r7, [r6]
cmp r7, r0
bne pen
@@ -37,8 +36,4 @@ pen: ldr r7, [r6]
* should now contain the SVC stack for this core
*/
b secondary_startup
-
- .align
-1: .long .
- .long spear_pen_release
ENDPROC(spear13xx_secondary_startup)
diff --git a/arch/arm/plat-versatile/headsmp.S b/arch/arm/plat-versatile/headsmp.S
index 09d9fc30c8ca..cec71853b0b3 100644
--- a/arch/arm/plat-versatile/headsmp.S
+++ b/arch/arm/plat-versatile/headsmp.S
@@ -18,10 +18,7 @@ ENTRY(versatile_secondary_startup)
ARM_BE8(setend be)
mrc p15, 0, r0, c0, c0, 5
bic r0, #0xff000000
- adr r4, 1f
- ldmia r4, {r5, r6}
- sub r4, r4, r5
- add r6, r6, r4
+ adr_l r6, versatile_cpu_release
pen: ldr r7, [r6]
cmp r7, r0
bne pen
@@ -31,8 +28,4 @@ pen: ldr r7, [r6]
* should now contain the SVC stack for this core
*/
b secondary_startup
-
- .align
-1: .long .
- .long versatile_cpu_release
ENDPROC(versatile_secondary_startup)
--
2.30.0


2021-12-18 08:57:52

by Gaosheng Cui

[permalink] [raw]
Subject: [PATCH -next 1/3] arm-soc: exynos: replace open coded VA->PA conversions

From: Ard Biesheuvel <[email protected]>

This replaces a couple of open coded calculations to obtain the
physical address of a far symbol with calls to the new adr_l etc
macros.

Signed-off-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Gaosheng Cui <[email protected]>
---
arch/arm/mach-exynos/headsmp.S | 9 +--------
arch/arm/mach-exynos/sleep.S | 26 +++++---------------------
2 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-exynos/headsmp.S b/arch/arm/mach-exynos/headsmp.S
index 0ac2cb9a7355..be7cd0eebe1d 100644
--- a/arch/arm/mach-exynos/headsmp.S
+++ b/arch/arm/mach-exynos/headsmp.S
@@ -19,10 +19,7 @@ ENTRY(exynos4_secondary_startup)
ARM_BE8(setend be)
mrc p15, 0, r0, c0, c0, 5
and r0, r0, #15
- adr r4, 1f
- ldmia r4, {r5, r6}
- sub r4, r4, r5
- add r6, r6, r4
+ adr_l r6, exynos_pen_release
pen: ldr r7, [r6]
cmp r7, r0
bne pen
@@ -33,7 +30,3 @@ pen: ldr r7, [r6]
*/
b secondary_startup
ENDPROC(exynos4_secondary_startup)
-
- .align 2
-1: .long .
- .long exynos_pen_release
diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index ed93f91853b8..ed27515a4458 100644
--- a/arch/arm/mach-exynos/sleep.S
+++ b/arch/arm/mach-exynos/sleep.S
@@ -8,6 +8,7 @@

#include <linux/linkage.h>
#include <asm/asm-offsets.h>
+#include <asm/assembler.h>
#include <asm/hardware/cache-l2x0.h>
#include "smc.h"

@@ -54,19 +55,13 @@ ENTRY(exynos_cpu_resume_ns)
cmp r0, r1
bne skip_cp15

- adr r0, _cp15_save_power
- ldr r1, [r0]
- ldr r1, [r0, r1]
- adr r0, _cp15_save_diag
- ldr r2, [r0]
- ldr r2, [r0, r2]
+ ldr_l r1, cp15_save_power
+ ldr_l r2, cp15_save_diag
mov r0, #SMC_CMD_C15RESUME
dsb
smc #0
#ifdef CONFIG_CACHE_L2X0
- adr r0, 1f
- ldr r2, [r0]
- add r0, r2, r0
+ adr_l r0, l2x0_saved_regs

/* Check that the address has been initialised. */
ldr r1, [r0, #L2X0_R_PHY_BASE]
@@ -85,9 +80,7 @@ ENTRY(exynos_cpu_resume_ns)
smc #0

/* Reload saved regs pointer because smc corrupts registers. */
- adr r0, 1f
- ldr r2, [r0]
- add r0, r2, r0
+ adr_l r0, l2x0_saved_regs

ldr r1, [r0, #L2X0_R_PWR_CTRL]
ldr r2, [r0, #L2X0_R_AUX_CTRL]
@@ -106,15 +99,6 @@ skip_cp15:
b cpu_resume
ENDPROC(exynos_cpu_resume_ns)

- .align
-_cp15_save_power:
- .long cp15_save_power - .
-_cp15_save_diag:
- .long cp15_save_diag - .
-#ifdef CONFIG_CACHE_L2X0
-1: .long l2x0_saved_regs - .
-#endif /* CONFIG_CACHE_L2X0 */
-
.data
.align 2
.globl cp15_save_diag
--
2.30.0


2021-12-20 15:40:03

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH -next 0/3] replace open coded VA->PA calculation

On Sat, Dec 18, 2021 at 9:58 AM Gaosheng Cui <[email protected]> wrote:
>
> These patches replace an open coded calculation to obtain the physical
> address of a far symbol with a call to the new ldr_l etc macro, and they
> belong to the kaslr patch set of arm32.
>
> Reference: https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm-kaslr-latest
>
> Ard Biesheuvel (3):
> arm-soc: exynos: replace open coded VA->PA conversions
> arm-soc: mvebu: replace open coded VA->PA conversion
> arm-soc: various: replace open coded VA->PA calculation

Usually these patches should go through the respective platform
maintainer trees,
and from there into the soc tree, but time is a little short here.

I could apply them directly with the maintainer Acks, but I don't understand
the significance of you sending them now. Is something broken without the
three patches? Are these the only ones missing from Ard's original series,
or is this preparation? Would you expect the patches to get backported to
stable kernels?

Arnd

2021-12-20 18:06:21

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH -next 0/3] replace open coded VA->PA calculation

On Mon, Dec 20, 2021 at 04:39:43PM +0100, Arnd Bergmann wrote:
> On Sat, Dec 18, 2021 at 9:58 AM Gaosheng Cui <[email protected]> wrote:
> >
> > These patches replace an open coded calculation to obtain the physical
> > address of a far symbol with a call to the new ldr_l etc macro, and they
> > belong to the kaslr patch set of arm32.
> >
> > Reference: https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm-kaslr-latest
> >
> > Ard Biesheuvel (3):
> > arm-soc: exynos: replace open coded VA->PA conversions
> > arm-soc: mvebu: replace open coded VA->PA conversion
> > arm-soc: various: replace open coded VA->PA calculation
>
> Usually these patches should go through the respective platform
> maintainer trees,
> and from there into the soc tree, but time is a little short here.
>
> I could apply them directly with the maintainer Acks

Sorry, but this is too low level for me to understand what is going
on, and so feel confident actually giving an ACK for the mvebu change.

Should the resulting assembly be exactly the same? Has the submitter
disassembled the object code and shown there is no actual difference
in the assembler output?

Andrew

2021-12-21 01:41:36

by Gaosheng Cui

[permalink] [raw]
Subject: Re: [PATCH -next 0/3] replace open coded VA->PA calculation

> I could apply them directly with the maintainer Acks, but I don't understand
> the significance of you sending them now. Is something broken without the
> three patches? Are these the only ones missing from Ard's original series,
> or is this preparation? Would you expect the patches to get backported to
> stable kernels?

Thanks for your reply.

This is preparation work for arm32 kaslr,and I want to continue to improve
the solution based on the work of Ard. These patches are relatively
independent, so I submit these patches first.

Gaosheng.

在 2021/12/20 23:39, Arnd Bergmann 写道:
> On Sat, Dec 18, 2021 at 9:58 AM Gaosheng Cui <[email protected]> wrote:
>> These patches replace an open coded calculation to obtain the physical
>> address of a far symbol with a call to the new ldr_l etc macro, and they
>> belong to the kaslr patch set of arm32.
>>
>> Reference: https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm-kaslr-latest
>>
>> Ard Biesheuvel (3):
>> arm-soc: exynos: replace open coded VA->PA conversions
>> arm-soc: mvebu: replace open coded VA->PA conversion
>> arm-soc: various: replace open coded VA->PA calculation
> Usually these patches should go through the respective platform
> maintainer trees,
> and from there into the soc tree, but time is a little short here.
>
> I could apply them directly with the maintainer Acks, but I don't understand
> the significance of you sending them now. Is something broken without the
> three patches? Are these the only ones missing from Ard's original series,
> or is this preparation? Would you expect the patches to get backported to
> stable kernels?
>
> Arnd
> .

2021-12-21 09:16:20

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH -next 0/3] replace open coded VA->PA calculation

On Tue, Dec 21, 2021 at 2:41 AM cuigaosheng <[email protected]> wrote:
>
> > I could apply them directly with the maintainer Acks, but I don't understand
> > the significance of you sending them now. Is something broken without the
> > three patches? Are these the only ones missing from Ard's original series,
> > or is this preparation? Would you expect the patches to get backported to
> > stable kernels?
>
> Thanks for your reply.
>
> This is preparation work for arm32 kaslr,and I want to continue to improve
> the solution based on the work of Ard. These patches are relatively
> independent, so I submit these patches first.

The approach of merging support incrementally is good in principle, but in this
case I think we first need to agree on the overall direction first.
How far have you
come rebasing Ard's patches, do you have KASLR working yet? This is information
that should go into the [PATCH 0/3] cover letter.

Do you have a particular target platform in mind?

I think for CPUs that can use LPAE, we want to eventually move to the 4G:4G
memory model, which in turn depends on having the kernel in vmalloc space, as
implemented by Linus Walleij in
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/log/?h=kernel-in-vmalloc-v5.14-rc1

With this work, the randomization will look quite different, on the one hand it
leaves less room for relocating the kernel within the smaller 256MB vmalloc
space, while on the other hand it does open the possibility of complete
randomization by scrambling the virt-to-phys mapping in vmalloc space,
using linear virtual addresses to refer to a randomized set of
physical addresses.
(this is just a wild idea that one could implement, nobody has actual plans for
it at the moment, and it comes with additional runtime overhead).

Arnd

2021-12-22 02:31:26

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH -next 0/3] replace open coded VA->PA calculation

On Tue, Dec 21, 2021 at 10:16 AM Arnd Bergmann <[email protected]> wrote:

> I think for CPUs that can use LPAE, we want to eventually move to the 4G:4G
> memory model, which in turn depends on having the kernel in vmalloc space, as
> implemented by Linus Walleij in
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/log/?h=kernel-in-vmalloc-v5.14-rc1

Yeah I'm still working on this series. The 4/4 split works pretty well,
but it breaks KASan and I was in the process of fixing that when I left
for parental leave.

The reason it breaks would be related to KASan not expecting
executable code in the vmalloc area, and since the shadowing
algorithm is a simple pointer offset, and since we go to lengths
to chisel shadow memory out of lowmem at a fixed offset from
TEXT_OFFSET, we have a problem.

The patch "KASAN horror" shows what I am trying to do to fix it,
it's "just" some hard work missing.

Yours,
Linus Walleij

2021-12-24 04:06:02

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH -next 0/3] replace open coded VA->PA calculation

On Wed, Dec 22, 2021 at 10:30 AM Ard Biesheuvel <[email protected]> wrote:
> On Wed, 22 Dec 2021 at 03:31, Linus Walleij <[email protected]> wrote:
> > On Tue, Dec 21, 2021 at 10:16 AM Arnd Bergmann <[email protected]> wrote:
> >
> > > I think for CPUs that can use LPAE, we want to eventually move to the 4G:4G
> > > memory model, which in turn depends on having the kernel in vmalloc space, as
> > > implemented by Linus Walleij in
> > > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/log/?h=kernel-in-vmalloc-v5.14-rc1
> >
> > Yeah I'm still working on this series. The 4/4 split works pretty well,

Actually it's just kernel-in-vmalloc, 4/4 comes after that.

> > but it breaks KASan and I was in the process of fixing that when I left
> > for parental leave.
> >
> > The reason it breaks would be related to KASan not expecting
> > executable code in the vmalloc area, and since the shadowing
> > algorithm is a simple pointer offset, and since we go to lengths
> > to chisel shadow memory out of lowmem at a fixed offset from
> > TEXT_OFFSET, we have a problem.
> >
>
> Vmap'ed stacks actually has a similar problem, which is why it is
> disabled when KAsan is enabled. But this can be fixed by enabling arch
> support for KASAN_VMALLOC, and I suspect it may address the vmap'ed
> kernel as well.

Yep after seeing the other convo on the topic I realized that this
is indeed the same as I'm seeing. I can't disable KASAN just
because the kernel is in VMALLOC though, so I suppose when
I finally get back to this I have to fix KASAN_VMALLOC too
if noone beats me to it.

(It'd be great if someone could beat me to it...)

Yours,
Linus Walleij