All of lore.kernel.org
 help / color / mirror / Atom feed
* Bug with KVM: arm64: Avoid setting the upper 32 bits of TCR_EL2 and CPTR_EL2 to 1
@ 2021-12-15 18:40 H. Nikolaus Schaller
  2021-12-16  6:58 ` H. Nikolaus Schaller
  0 siblings, 1 reply; 5+ messages in thread
From: H. Nikolaus Schaller @ 2021-12-15 18:40 UTC (permalink / raw)
  To: Catalin Marinas, Chris January, stable, Will Deacon,
	Marc Zyngier, reg Kroah-Hartman
  Cc: Discussions about the Letux Kernel

this seems to break build of 5.10.y (and maybe earlier) for me:

  CALL    scripts/checksyscalls.sh - due to target missing
  CALL    scripts/atomic/check-atomics.sh - due to target missing
  CHK     include/generated/compile.h
  AS      arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o - due to target missing
arch/arm64/kvm/hyp/nvhe/hyp-init.S: Assembler messages:
arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: unexpected characters following instruction at operand 2 -- `mov x1,#((1U<<31)|(1<<23))'
arch/arm64/kvm/hyp/nvhe/Makefile:28: recipe for target 'arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o' failed
make[5]: *** [arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o] Error 1
scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm/hyp/nvhe' failed
make[4]: *** [arch/arm64/kvm/hyp/nvhe] Error 2
scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm/hyp' failed
make[3]: *** [arch/arm64/kvm/hyp] Error 2
scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm' failed
make[2]: *** [arch/arm64/kvm] Error 2
Makefile:1822: recipe for target 'arch/arm64' failed
make[1]: *** [arch/arm64] Error 2
Makefile:336: recipe for target '__build_one_by_one' failed
make: *** [__build_one_by_one] Error 2

Looking at the problematic line 87 of hyp-init.S shows that
there is a macro expansion:

      mov     x1, #TCR_EL2_RES1

This macro was modified by the $subject patch
(commit c71b5f37b5ff1a673b2e4a91d1b34ea027546e23 in v5.10.y)
and reverting the patch makes the compile succeed.

Now: why does it build for me for v5.15.y and v5.16-rc5?
I think it is because my build system switches to gcc 6.3
instead of gcc 4.9 depending on scripts/min-tool-version.sh.

So I assume that the fix is not compatible with the minimum
requirement for 5.10.y of gcc 4.9 (or even less - I don't know exactly).
Earlier kernels may also be affected if $subject patch was also
backported there, but I have not tested.

This should somehow be fixed so that arch/arm64/include/asm/kvm_arm.h
can be included by older assemblers.

BR and thanks,
Nikolaus Schaller


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Bug with KVM: arm64: Avoid setting the upper 32 bits of TCR_EL2 and CPTR_EL2 to 1
  2021-12-15 18:40 Bug with KVM: arm64: Avoid setting the upper 32 bits of TCR_EL2 and CPTR_EL2 to 1 H. Nikolaus Schaller
@ 2021-12-16  6:58 ` H. Nikolaus Schaller
  2021-12-16  8:43   ` Marc Zyngier
  0 siblings, 1 reply; 5+ messages in thread
From: H. Nikolaus Schaller @ 2021-12-16  6:58 UTC (permalink / raw)
  To: Catalin Marinas, Chris January, stable, Will Deacon,
	Marc Zyngier, reg Kroah-Hartman
  Cc: Discussions about the Letux Kernel

Hi Catalin,

> Am 15.12.2021 um 19:40 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> this seems to break build of 5.10.y (and maybe earlier) for me:
> 
>  CALL    scripts/checksyscalls.sh - due to target missing
>  CALL    scripts/atomic/check-atomics.sh - due to target missing
>  CHK     include/generated/compile.h
>  AS      arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o - due to target missing
> arch/arm64/kvm/hyp/nvhe/hyp-init.S: Assembler messages:
> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: unexpected characters following instruction at operand 2 -- `mov x1,#((1U<<31)|(1<<23))'
> arch/arm64/kvm/hyp/nvhe/Makefile:28: recipe for target 'arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o' failed
> make[5]: *** [arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o] Error 1
> scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm/hyp/nvhe' failed
> make[4]: *** [arch/arm64/kvm/hyp/nvhe] Error 2
> scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm/hyp' failed
> make[3]: *** [arch/arm64/kvm/hyp] Error 2
> scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm' failed
> make[2]: *** [arch/arm64/kvm] Error 2
> Makefile:1822: recipe for target 'arch/arm64' failed
> make[1]: *** [arch/arm64] Error 2
> Makefile:336: recipe for target '__build_one_by_one' failed
> make: *** [__build_one_by_one] Error 2
> 
> Looking at the problematic line 87 of hyp-init.S shows that
> there is a macro expansion:
> 
>      mov     x1, #TCR_EL2_RES1
> 
> This macro was modified by the $subject patch
> (commit c71b5f37b5ff1a673b2e4a91d1b34ea027546e23 in v5.10.y)
> and reverting the patch makes the compile succeed.
> 
> Now: why does it build for me for v5.15.y and v5.16-rc5?
> I think it is because my build system switches to gcc 6.3
> instead of gcc 4.9 depending on scripts/min-tool-version.sh.

I have run the cross-check and it
- fails with gcc 4.9.2 + binutils 2.25 (compatible to jessie)
- works with gcc 6.3.0 + binutils 2.28.1 (compatible to stretch)

> 
> So I assume that the fix is not compatible with the minimum
> requirement for 5.10.y of gcc 4.9 (or even less - I don't know exactly).
> Earlier kernels may also be affected if $subject patch was also
> backported there, but I have not tested.
> 
> This should somehow be fixed so that arch/arm64/include/asm/kvm_arm.h
> can be included by older assemblers.

BR and thanks,
Nikolaus Schaller


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Bug with KVM: arm64: Avoid setting the upper 32 bits of TCR_EL2 and CPTR_EL2 to 1
  2021-12-16  6:58 ` H. Nikolaus Schaller
@ 2021-12-16  8:43   ` Marc Zyngier
  2021-12-16 14:30     ` H. Nikolaus Schaller
  0 siblings, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2021-12-16  8:43 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Catalin Marinas, Chris January, stable, Will Deacon,
	reg Kroah-Hartman, Discussions about the Letux Kernel

Hi Nikolaus,

On 2021-12-16 06:58, H. Nikolaus Schaller wrote:
> Hi Catalin,
> 
>> Am 15.12.2021 um 19:40 schrieb H. Nikolaus Schaller 
>> <hns@goldelico.com>:
>> 
>> this seems to break build of 5.10.y (and maybe earlier) for me:
>> 
>>  CALL    scripts/checksyscalls.sh - due to target missing
>>  CALL    scripts/atomic/check-atomics.sh - due to target missing
>>  CHK     include/generated/compile.h
>>  AS      arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o - due to target 
>> missing
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S: Assembler messages:
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: unexpected characters 
>> following instruction at operand 2 -- `mov x1,#((1U<<31)|(1<<23))'
>> arch/arm64/kvm/hyp/nvhe/Makefile:28: recipe for target 
>> 'arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o' failed
>> make[5]: *** [arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o] Error 1
>> scripts/Makefile.build:497: recipe for target 
>> 'arch/arm64/kvm/hyp/nvhe' failed
>> make[4]: *** [arch/arm64/kvm/hyp/nvhe] Error 2
>> scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm/hyp' 
>> failed
>> make[3]: *** [arch/arm64/kvm/hyp] Error 2
>> scripts/Makefile.build:497: recipe for target 'arch/arm64/kvm' failed
>> make[2]: *** [arch/arm64/kvm] Error 2
>> Makefile:1822: recipe for target 'arch/arm64' failed
>> make[1]: *** [arch/arm64] Error 2
>> Makefile:336: recipe for target '__build_one_by_one' failed
>> make: *** [__build_one_by_one] Error 2
>> 
>> Looking at the problematic line 87 of hyp-init.S shows that
>> there is a macro expansion:
>> 
>>      mov     x1, #TCR_EL2_RES1
>> 
>> This macro was modified by the $subject patch
>> (commit c71b5f37b5ff1a673b2e4a91d1b34ea027546e23 in v5.10.y)
>> and reverting the patch makes the compile succeed.
>> 
>> Now: why does it build for me for v5.15.y and v5.16-rc5?
>> I think it is because my build system switches to gcc 6.3
>> instead of gcc 4.9 depending on scripts/min-tool-version.sh.
> 
> I have run the cross-check and it
> - fails with gcc 4.9.2 + binutils 2.25 (compatible to jessie)
> - works with gcc 6.3.0 + binutils 2.28.1 (compatible to stretch)
> 
>> 
>> So I assume that the fix is not compatible with the minimum
>> requirement for 5.10.y of gcc 4.9 (or even less - I don't know 
>> exactly).
>> Earlier kernels may also be affected if $subject patch was also
>> backported there, but I have not tested.
>> 
>> This should somehow be fixed so that arch/arm64/include/asm/kvm_arm.h
>> can be included by older assemblers.

GCC versions prior to 5.1 are known to miscompile the kernel,
and the minimal GCC version was bumped in dca5244d2f5b.

I am surprised this requirement wasn't backported to 5.10-stable,
as this results in all sorts of terrible bugs that are hard to
diagnose (see the horror story in the commit message).

As for the issue you describe, does the following help?

Thanks,

         M.

diff --git a/arch/arm64/include/asm/kvm_arm.h 
b/arch/arm64/include/asm/kvm_arm.h
index 01d47c5886dc..d03087308ab5 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -91,7 +91,7 @@
  #define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)

  /* TCR_EL2 Registers bits */
-#define TCR_EL2_RES1		((1U << 31) | (1 << 23))
+#define TCR_EL2_RES1		((UL(1) << 31) | (UL(1) << 23))
  #define TCR_EL2_TBI		(1 << 20)
  #define TCR_EL2_PS_SHIFT	16
  #define TCR_EL2_PS_MASK		(7 << TCR_EL2_PS_SHIFT)

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: Bug with KVM: arm64: Avoid setting the upper 32 bits of TCR_EL2 and CPTR_EL2 to 1
  2021-12-16  8:43   ` Marc Zyngier
@ 2021-12-16 14:30     ` H. Nikolaus Schaller
  2021-12-16 17:25       ` Catalin Marinas
  0 siblings, 1 reply; 5+ messages in thread
From: H. Nikolaus Schaller @ 2021-12-16 14:30 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, Chris January, stable, Will Deacon,
	reg Kroah-Hartman, Discussions about the Letux Kernel

Hi Marc,

> Am 16.12.2021 um 09:43 schrieb Marc Zyngier <maz@kernel.org>:
> 
> Hi Nikolaus,
> 
> On 2021-12-16 06:58, H. Nikolaus Schaller wrote:
>> Hi Catalin,
>>> Am 15.12.2021 um 19:40 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>>> this seems to break build of 5.10.y (and maybe earlier) for me:
>>> CALL    scripts/checksyscalls.sh - due to target missing
>>> CALL    scripts/atomic/check-atomics.sh - due to target missing
>>> CHK     include/generated/compile.h
>>> AS      arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o - due to target missing
>>> arch/arm64/kvm/hyp/nvhe/hyp-init.S: Assembler messages:
>>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
>>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'This should somehow be fixed so that arch/arm64/include/asm/kvm_arm.h
>>> can be included by older assemblers.
> 
> GCC versions prior to 5.1 are known to miscompile the kernel,
> and the minimal GCC version was bumped in dca5244d2f5b.

> I am surprised this requirement wasn't backported to 5.10-stable,
> as this results in all sorts of terrible bugs that are hard to
> diagnose (see the horror story in the commit message).

Indeed.

My build system checks for existence of scripts/min-tool-version.sh
and if it exists it chooses the right gcc version. If it does not exist
it assumes that gcc 4.9 is still good enough...

> 
> As for the issue you describe, does the following help?

> 
> Thanks,
> 
>        M.
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 01d47c5886dc..d03087308ab5 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -91,7 +91,7 @@
> #define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)
> 
> /* TCR_EL2 Registers bits */
> -#define TCR_EL2_RES1		((1U << 31) | (1 << 23))
> +#define TCR_EL2_RES1		((UL(1) << 31) | (UL(1) << 23))
> #define TCR_EL2_TBI		(1 << 20)
> #define TCR_EL2_PS_SHIFT	16
> #define TCR_EL2_PS_MASK		(7 << TCR_EL2_PS_SHIFT)
> 
> -- 
> Jazz is not dead. It just smells funny...


Yes, it does! This can be compiled with gcc 4.9 (resp. binutils).

So IMHO there are 3 different ways to solve it:
a) your fix applied to 5.10.y
b) someone backports scripts/min-tool-version.sh
to allow for dependable automation...
c) we leave 5.10.y unfixed and I just add a special
rule for arm64 to choose a newer gcc (it is no problem to
use 4.9 for other architectures) in my build setup.

BR and thanks,
Nikolaus



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Bug with KVM: arm64: Avoid setting the upper 32 bits of TCR_EL2 and CPTR_EL2 to 1
  2021-12-16 14:30     ` H. Nikolaus Schaller
@ 2021-12-16 17:25       ` Catalin Marinas
  0 siblings, 0 replies; 5+ messages in thread
From: Catalin Marinas @ 2021-12-16 17:25 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Marc Zyngier, Chris January, stable, Will Deacon,
	Greg Kroah-Hartman, Discussions about the Letux Kernel

On Thu, Dec 16, 2021 at 03:30:40PM +0100, H. Nikolaus Schaller wrote:
> > Am 16.12.2021 um 09:43 schrieb Marc Zyngier <maz@kernel.org>:
> > On 2021-12-16 06:58, H. Nikolaus Schaller wrote:
> >>> Am 15.12.2021 um 19:40 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> >>> this seems to break build of 5.10.y (and maybe earlier) for me:
> >>> CALL    scripts/checksyscalls.sh - due to target missing
> >>> CALL    scripts/atomic/check-atomics.sh - due to target missing
> >>> CHK     include/generated/compile.h
> >>> AS      arch/arm64/kvm/hyp/nvhe/hyp-init.nvhe.o - due to target missing
> >>> arch/arm64/kvm/hyp/nvhe/hyp-init.S: Assembler messages:
> >>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> >>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'
> >>> arch/arm64/kvm/hyp/nvhe/hyp-init.S:87: Error: missing ')'This should somehow be fixed so that arch/arm64/include/asm/kvm_arm.h
> >>> can be included by older assemblers.
> > 
> > GCC versions prior to 5.1 are known to miscompile the kernel,
> > and the minimal GCC version was bumped in dca5244d2f5b.
> 
> > I am surprised this requirement wasn't backported to 5.10-stable,
> > as this results in all sorts of terrible bugs that are hard to
> > diagnose (see the horror story in the commit message).
> 
> Indeed.
> 
> My build system checks for existence of scripts/min-tool-version.sh
> and if it exists it chooses the right gcc version. If it does not exist
> it assumes that gcc 4.9 is still good enough...

I wonder whether the problem is binutils rather than gcc. We have a
minimum requirement of 2.23 but it looks like it failed to build for you
with 2.25. Unless the compiler got smarter and it drops the 'U' from 1U
when passing it to gas.

> Yes, it does! This can be compiled with gcc 4.9 (resp. binutils).
> 
> So IMHO there are 3 different ways to solve it:
> a) your fix applied to 5.10.y
> b) someone backports scripts/min-tool-version.sh
> to allow for dependable automation...
> c) we leave 5.10.y unfixed and I just add a special
> rule for arm64 to choose a newer gcc (it is no problem to
> use 4.9 for other architectures) in my build setup.

Another option is to merge Marc's fix in 5.16 (there are two more 1U in
the same file) with a Fixes tag and cc stable so that it gets backported
to 5.10.y.

-- 
Catalin

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-16 17:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-15 18:40 Bug with KVM: arm64: Avoid setting the upper 32 bits of TCR_EL2 and CPTR_EL2 to 1 H. Nikolaus Schaller
2021-12-16  6:58 ` H. Nikolaus Schaller
2021-12-16  8:43   ` Marc Zyngier
2021-12-16 14:30     ` H. Nikolaus Schaller
2021-12-16 17:25       ` Catalin Marinas

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.