[PATCH] KVM: arm64: Don't use cbz/adr with external symbols
allmodconfig + CONFIG_LTO_CLANG_THIN=y fails to build due to following linker errors: ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21CC): relocation R_AARCH64_CONDBR19 out of range: 2031220 is not in [-1048576, 1048575]; references hyp_panic >>> defined in vmlinux.o ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21E0): relocation R_AARCH64_ADR_PREL_LO21 out of range: 2031200 is not in [-1048576, 1048575]; references hyp_panic >>> defined in vmlinux.o This is because with LTO, the compiler ends up placing hyp_panic() more than 1MB away from __guest_enter(). Use an unconditional branch and adr_l instead to fix the issue. Link: https://github.com/ClangBuiltLinux/linux/issues/1317 Reported-by: Nathan Chancellor Suggested-by: Marc Zyngier Suggested-by: Ard Biesheuvel Signed-off-by: Sami Tolvanen --- arch/arm64/kvm/hyp/entry.S | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S index b0afad7a99c6..c62265951467 100644 --- a/arch/arm64/kvm/hyp/entry.S +++ b/arch/arm64/kvm/hyp/entry.S @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) // If the hyp context is loaded, go straight to hyp_panic get_loaded_vcpu x0, x1 - cbz x0, hyp_panic + cbnzx0, 1f + b hyp_panic +1: // The hyp context is saved so make sure it is restored to allow // hyp_panic to run at hyp and, subsequently, panic to run in the host. // This makes use of __guest_exit to avoid duplication but sets the @@ -94,7 +96,7 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) // current state is saved to the guest context but it will only be // accurate if the guest had been completely restored. adr_this_cpu x0, kvm_hyp_ctxt, x1 - adr x1, hyp_panic + adr_l x1, hyp_panic str x1, [x0, #CPU_XREG_OFFSET(30)] get_vcpu_ptrx1, x0 base-commit: 280d542f6ffac0e6d65dc267f92191d509b13b64 -- 2.30.1.766.gb4fecdf3b7-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] KVM: arm64: Disable LTO in hyp
On Thu, Mar 4, 2021 at 2:34 PM Sami Tolvanen wrote:
>
> On Thu, Mar 4, 2021 at 2:17 PM Marc Zyngier wrote:
> >
> > On Thu, 04 Mar 2021 21:25:41 +,
> > Sami Tolvanen wrote:
> > >
> > > On Thu, Mar 4, 2021 at 11:15 AM Marc Zyngier wrote:
> > > >
> > > > On Thu, 04 Mar 2021 18:45:44 +,
> > > > Sami Tolvanen wrote:
> > > > >
> > > > > allmodconfig + CONFIG_LTO_CLANG_THIN=y fails to build due to following
> > > > > linker errors:
> > > > >
> > > > > ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21CC):
> > > >
> > > > I assume this message is only an oddity, right? Because
> > > > __guest_enter() is as far as you can imagine from irqbypass.c...
> > >
> > > I'm not sure what's up with the filename in the error message. Fangrui
> > > or Nick probably have a better idea.
> > >
> > > > > relocation R_AARCH64_CONDBR19 out of range: 2031220 is not in
> > > > > [-1048576, 1048575]; references hyp_panic
> > > > > >>> defined in vmlinux.o
> > > > >
> > > > > ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21E0):
> > > > > relocation R_AARCH64_ADR_PREL_LO21 out of range: 2031200 is not in
> > > > > [-1048576, 1048575]; references hyp_panic
> > > > > >>> defined in vmlinux.o
> > > > >
> > > > > As LTO is not really necessary for the hypervisor code, disable it for
> > > > > the hyp directory to fix the build.
> > > >
> > > > Can you shed some light on what the problem is exactly?
> > >
> > > I assume hyp_panic() ends up being placed too far from __guest_enter()
> > > when the kernel is large enough. Possibly something to do with LLVM
> > > always splitting functions into separate sections with LTO. I'm not
> > > sure why the linker cannot shuffle things around to make everyone
> > > happy in this case, but I confirmed that this patch also fixes the
> > > build issue for me:
> > >
> > > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c
> > > b/arch/arm64/kvm/hyp/vhe/switch.c
> > > index af8e940d0f03..128197b7c794 100644
> > > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > > @@ -214,7 +214,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64
> > > par)
> > > }
> > > NOKPROBE_SYMBOL(__hyp_call_panic);
> > >
> > > -void __noreturn hyp_panic(void)
> > > +void __noreturn hyp_panic(void) __section(".text")
> > > {
> > > u64 spsr = read_sysreg_el2(SYS_SPSR);
> > > u64 elr = read_sysreg_el2(SYS_ELR);
> > >
> >
> > We're getting into black-magic territory here. Why wouldn't hyp_panic
> > be in the .text section already?
>
> It's not quite black magic. LLVM essentially flips on
> -ffunction-sections with LTO and therefore, hyp_panic() will be in
> .text.hyp_panic in vmlinux.o, while __guest_enter() will be in .text.
> Everything ends up in .text when we link vmlinux, of course.
>
> $ readelf --sections vmlinux.o | grep hyp_panic
> [3936] .text.hyp_panic PROGBITS 004b56e4
Note that disabling LTO here has essentially the same effect as using
__section(".text"). It stops the compiler from splitting these
functions into .text.* sections and makes it less likely that
hyp_panic() ends up too far away from __guest_enter().
If neither of these workarounds sound appealing, I suppose we could
alternatively change hyp/entry.S to use adr_l for hyp_panic. Thoughts?
Sami
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] KVM: arm64: Disable LTO in hyp
On Thu, Mar 4, 2021 at 2:17 PM Marc Zyngier wrote:
>
> On Thu, 04 Mar 2021 21:25:41 +0000,
> Sami Tolvanen wrote:
> >
> > On Thu, Mar 4, 2021 at 11:15 AM Marc Zyngier wrote:
> > >
> > > On Thu, 04 Mar 2021 18:45:44 +,
> > > Sami Tolvanen wrote:
> > > >
> > > > allmodconfig + CONFIG_LTO_CLANG_THIN=y fails to build due to following
> > > > linker errors:
> > > >
> > > > ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21CC):
> > >
> > > I assume this message is only an oddity, right? Because
> > > __guest_enter() is as far as you can imagine from irqbypass.c...
> >
> > I'm not sure what's up with the filename in the error message. Fangrui
> > or Nick probably have a better idea.
> >
> > > > relocation R_AARCH64_CONDBR19 out of range: 2031220 is not in
> > > > [-1048576, 1048575]; references hyp_panic
> > > > >>> defined in vmlinux.o
> > > >
> > > > ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21E0):
> > > > relocation R_AARCH64_ADR_PREL_LO21 out of range: 2031200 is not in
> > > > [-1048576, 1048575]; references hyp_panic
> > > > >>> defined in vmlinux.o
> > > >
> > > > As LTO is not really necessary for the hypervisor code, disable it for
> > > > the hyp directory to fix the build.
> > >
> > > Can you shed some light on what the problem is exactly?
> >
> > I assume hyp_panic() ends up being placed too far from __guest_enter()
> > when the kernel is large enough. Possibly something to do with LLVM
> > always splitting functions into separate sections with LTO. I'm not
> > sure why the linker cannot shuffle things around to make everyone
> > happy in this case, but I confirmed that this patch also fixes the
> > build issue for me:
> >
> > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c
> > b/arch/arm64/kvm/hyp/vhe/switch.c
> > index af8e940d0f03..128197b7c794 100644
> > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > @@ -214,7 +214,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64 par)
> > }
> > NOKPROBE_SYMBOL(__hyp_call_panic);
> >
> > -void __noreturn hyp_panic(void)
> > +void __noreturn hyp_panic(void) __section(".text")
> > {
> > u64 spsr = read_sysreg_el2(SYS_SPSR);
> > u64 elr = read_sysreg_el2(SYS_ELR);
> >
>
> We're getting into black-magic territory here. Why wouldn't hyp_panic
> be in the .text section already?
It's not quite black magic. LLVM essentially flips on
-ffunction-sections with LTO and therefore, hyp_panic() will be in
.text.hyp_panic in vmlinux.o, while __guest_enter() will be in .text.
Everything ends up in .text when we link vmlinux, of course.
$ readelf --sections vmlinux.o | grep hyp_panic
[3936] .text.hyp_panic PROGBITS 004b56e4
Sami
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] KVM: arm64: Disable LTO in hyp
On Fri, Mar 5, 2021 at 6:22 AM Ard Biesheuvel wrote:
>
> On Fri, 5 Mar 2021 at 12:38, Marc Zyngier wrote:
> >
> > On Fri, 05 Mar 2021 02:38:17 +,
> > Sami Tolvanen wrote:
> > >
> > > On Thu, Mar 4, 2021 at 2:34 PM Sami Tolvanen
> > > wrote:
> > > >
> > > > On Thu, Mar 4, 2021 at 2:17 PM Marc Zyngier wrote:
> > > > >
> > > > > On Thu, 04 Mar 2021 21:25:41 +,
> > > > > Sami Tolvanen wrote:
> >
> > [...]
> >
> > > > > > I assume hyp_panic() ends up being placed too far from
> > > > > > __guest_enter()
> > > > > > when the kernel is large enough. Possibly something to do with LLVM
> > > > > > always splitting functions into separate sections with LTO. I'm not
> > > > > > sure why the linker cannot shuffle things around to make everyone
> > > > > > happy in this case, but I confirmed that this patch also fixes the
> > > > > > build issue for me:
> > > > > >
> > > > > > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c
> > > > > > b/arch/arm64/kvm/hyp/vhe/switch.c
> > > > > > index af8e940d0f03..128197b7c794 100644
> > > > > > --- a/arch/arm64/kvm/hyp/vhe/switch.c
> > > > > > +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> > > > > > @@ -214,7 +214,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr,
> > > > > > u64 par)
> > > > > > }
> > > > > > NOKPROBE_SYMBOL(__hyp_call_panic);
> > > > > >
> > > > > > -void __noreturn hyp_panic(void)
> > > > > > +void __noreturn hyp_panic(void) __section(".text")
> > > > > > {
> > > > > > u64 spsr = read_sysreg_el2(SYS_SPSR);
> > > > > > u64 elr = read_sysreg_el2(SYS_ELR);
> > > > > >
> > > > >
> > > > > We're getting into black-magic territory here. Why wouldn't hyp_panic
> > > > > be in the .text section already?
> > > >
> > > > It's not quite black magic. LLVM essentially flips on
> > > > -ffunction-sections with LTO and therefore, hyp_panic() will be in
> > > > .text.hyp_panic in vmlinux.o, while __guest_enter() will be in .text.
> > > > Everything ends up in .text when we link vmlinux, of course.
> > > >
> > > > $ readelf --sections vmlinux.o | grep hyp_panic
> > > > [3936] .text.hyp_panic PROGBITS 004b56e4
> > >
> > > Note that disabling LTO here has essentially the same effect as using
> > > __section(".text"). It stops the compiler from splitting these
> > > functions into .text.* sections and makes it less likely that
> > > hyp_panic() ends up too far away from __guest_enter().
> > >
> > > If neither of these workarounds sound appealing, I suppose we could
> > > alternatively change hyp/entry.S to use adr_l for hyp_panic. Thoughts?
> >
> > That would be an actual fix instead of a workaround, as it would
> > remove existing assumptions about the relative locations of the two
> > objects. I guess you need to fix both instances with something such
> > as:
> >
> > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> > index b0afad7a99c6..a43e1f7ee354 100644
> > --- a/arch/arm64/kvm/hyp/entry.S
> > +++ b/arch/arm64/kvm/hyp/entry.S
> > @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL)
> >
> > // If the hyp context is loaded, go straight to hyp_panic
> > get_loaded_vcpu x0, x1
> > - cbz x0, hyp_panic
> > -
> > + cbnzx0, 1f
> > + adr_l x0, hyp_panic
> > + br x0
> > +1:
>
> Agree with replacing the conditional branches that refer to external
> symbols: the compiler never emits those, for the reason we are seeing
> here, i.e., the range is simply insufficient.
>
> But let's just use 'b hyp_panic' instead, no?
Alright, this seems to work for me:
diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index b0afad7a99c6..c62265951467 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL)
// If the hyp context is loaded, go straight to hyp_panic
get_loaded_vcpu x0, x1
- cbz x0, hyp_panic
+ cbnzx0, 1f
+ b hyp_panic
+1:
/
Re: [PATCH] KVM: arm64: Disable LTO in hyp
On Thu, Mar 4, 2021 at 11:15 AM Marc Zyngier wrote:
>
> On Thu, 04 Mar 2021 18:45:44 +0000,
> Sami Tolvanen wrote:
> >
> > allmodconfig + CONFIG_LTO_CLANG_THIN=y fails to build due to following
> > linker errors:
> >
> > ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21CC):
>
> I assume this message is only an oddity, right? Because
> __guest_enter() is as far as you can imagine from irqbypass.c...
I'm not sure what's up with the filename in the error message. Fangrui
or Nick probably have a better idea.
> > relocation R_AARCH64_CONDBR19 out of range: 2031220 is not in
> > [-1048576, 1048575]; references hyp_panic
> > >>> defined in vmlinux.o
> >
> > ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21E0):
> > relocation R_AARCH64_ADR_PREL_LO21 out of range: 2031200 is not in
> > [-1048576, 1048575]; references hyp_panic
> > >>> defined in vmlinux.o
> >
> > As LTO is not really necessary for the hypervisor code, disable it for
> > the hyp directory to fix the build.
>
> Can you shed some light on what the problem is exactly?
I assume hyp_panic() ends up being placed too far from __guest_enter()
when the kernel is large enough. Possibly something to do with LLVM
always splitting functions into separate sections with LTO. I'm not
sure why the linker cannot shuffle things around to make everyone
happy in this case, but I confirmed that this patch also fixes the
build issue for me:
diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index af8e940d0f03..128197b7c794 100644
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -214,7 +214,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64 par)
}
NOKPROBE_SYMBOL(__hyp_call_panic);
-void __noreturn hyp_panic(void)
+void __noreturn hyp_panic(void) __section(".text")
{
u64 spsr = read_sysreg_el2(SYS_SPSR);
u64 elr = read_sysreg_el2(SYS_ELR);
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1317
> > Reported-by: Nathan Chancellor
> > Tested-by: Nathan Chancellor
> > Signed-off-by: Sami Tolvanen
> > ---
> > arch/arm64/kvm/hyp/Makefile | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
> > index 687598e41b21..e8116016e6a8 100644
> > --- a/arch/arm64/kvm/hyp/Makefile
> > +++ b/arch/arm64/kvm/hyp/Makefile
> > @@ -11,3 +11,6 @@ subdir-ccflags-y := -I$(incdir)
> > \
> > $(DISABLE_STACKLEAK_PLUGIN)
> >
> > obj-$(CONFIG_KVM) += vhe/ nvhe/ pgtable.o
> > +
> > +# Disable LTO for the files in this directory
> > +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO), $(KBUILD_CFLAGS))
> >
> > base-commit: f69d02e37a85645aa90d18cacfff36dba370f797
>
> Can this be reduced to the nvhe part of the tree? The rest of the
> hypervisor should support being built with LTO, I'd expect. Or am I
> missing something more significant?
No, this error appears to be about hyp_panic() in the vhe code. While
I'm not sure how beneficial LTO is in hypervisor code, there shouldn't
be any other reason we can't use it there.
Sami
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH] KVM: arm64: Disable LTO in hyp
allmodconfig + CONFIG_LTO_CLANG_THIN=y fails to build due to following linker errors: ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21CC): relocation R_AARCH64_CONDBR19 out of range: 2031220 is not in [-1048576, 1048575]; references hyp_panic >>> defined in vmlinux.o ld.lld: error: irqbypass.c:(function __guest_enter: .text+0x21E0): relocation R_AARCH64_ADR_PREL_LO21 out of range: 2031200 is not in [-1048576, 1048575]; references hyp_panic >>> defined in vmlinux.o As LTO is not really necessary for the hypervisor code, disable it for the hyp directory to fix the build. Link: https://github.com/ClangBuiltLinux/linux/issues/1317 Reported-by: Nathan Chancellor Tested-by: Nathan Chancellor Signed-off-by: Sami Tolvanen --- arch/arm64/kvm/hyp/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile index 687598e41b21..e8116016e6a8 100644 --- a/arch/arm64/kvm/hyp/Makefile +++ b/arch/arm64/kvm/hyp/Makefile @@ -11,3 +11,6 @@ subdir-ccflags-y := -I$(incdir) \ $(DISABLE_STACKLEAK_PLUGIN) obj-$(CONFIG_KVM) += vhe/ nvhe/ pgtable.o + +# Disable LTO for the files in this directory +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_LTO), $(KBUILD_CFLAGS)) base-commit: f69d02e37a85645aa90d18cacfff36dba370f797 -- 2.30.1.766.gb4fecdf3b7-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH] arm64: kvm: use -fno-jump-tables with clang
Starting with LLVM r308050, clang generates a jump table with EL1 virtual addresses in __init_stage2_translation, which results in a kernel panic when booting at EL2: Kernel panic - not syncing: HYP panic: PS:83c9 PC:089e6fd8 ESR:8604 FAR:089e6fd8 HPFAR:09825000 PAR: VCPU:000804fc20001221 CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc7-dirty #3 Hardware name: ARM Juno development board (r1) (DT) Call trace: [] dump_backtrace+0x0/0x34c [] show_stack+0x18/0x20 [] dump_stack+0xc4/0xfc [] panic+0x138/0x2b4 [] panic+0x0/0x2b4 SMP: stopping secondary CPUs SMP: failed to stop secondary CPUs 0-3,5 Kernel Offset: disabled CPU features: 0x002086 Memory Limit: none ---[ end Kernel panic - not syncing: HYP panic: PS:83c9 PC:089e6fd8 ESR:8604 FAR:089e6fd8 HPFAR:09825000 PAR: VCPU:000804fc20001221 This change adds -fno-jump-tables to arm64/hyp to work around the bug. Suggested-by: AKASHI Takahiro Signed-off-by: Sami Tolvanen --- arch/arm64/kvm/hyp/Makefile | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile index 4313f74753332..745b3255e7832 100644 --- a/arch/arm64/kvm/hyp/Makefile +++ b/arch/arm64/kvm/hyp/Makefile @@ -5,6 +5,10 @@ ccflags-y += -fno-stack-protector -DDISABLE_BRANCH_PROFILING +ifeq ($(cc-name),clang) +ccflags-y += -fno-jump-tables +endif + KVM=../../../../virt/kvm obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o -- 2.17.0.441.gb46fe60e1d-goog ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
