linux-next: build failure after merge of the kvm-arm tree

linux-next: build failure after merge of the kvm-arm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]

Hi all,

After merging the kvm tree, today's linux-next build (arm64 defconfig)
failed like this:

In file included from <command-line>:
In function 'check_res_bits',
    inlined from 'kvm_sys_reg_table_init' at arch/arm64/kvm/sys_regs.c:4109:2:
include/linux/compiler_types.h:449:45: error: call to '__compiletime_assert_591' declared with attribute error: BUILD_BUG_ON failed: ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))
  449 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |                                             ^
include/linux/compiler_types.h:430:25: note: in definition of macro '__compiletime_assert'
  430 |                         prefix ## suffix();                             \
      |                         ^~~~~~
include/linux/compiler_types.h:449:9: note: in expansion of macro '_compiletime_assert'
  449 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |         ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
      |                                     ^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
   50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
      |         ^~~~~~~~~~~~~~~~
arch/arm64/kvm/check-res-bits.h:58:9: note: in expansion of macro 'BUILD_BUG_ON'
   58 |         BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0       != (GENMASK_ULL(63, 0)));
      |         ^~~~~~~~~~~~

I bisected this to the merge of the kvm-arm tree into linux-next but I
could not figure out why it fails :-(

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the kvm-arm tree

[Joey Gouly]

On Thu, Feb 22, 2024 at 10:03:49PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the kvm tree, today's linux-next build (arm64 defconfig)
> failed like this:
> 
> In file included from <command-line>:
> In function 'check_res_bits',
>     inlined from 'kvm_sys_reg_table_init' at arch/arm64/kvm/sys_regs.c:4109:2:
> include/linux/compiler_types.h:449:45: error: call to '__compiletime_assert_591' declared with attribute error: BUILD_BUG_ON failed: ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))
>   449 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>       |                                             ^
> include/linux/compiler_types.h:430:25: note: in definition of macro '__compiletime_assert'
>   430 |                         prefix ## suffix();                             \
>       |                         ^~~~~~
> include/linux/compiler_types.h:449:9: note: in expansion of macro '_compiletime_assert'
>   449 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>       |         ^~~~~~~~~~~~~~~~~~~
> include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
>    39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>       |                                     ^~~~~~~~~~~~~~~~~~
> include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
>    50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>       |         ^~~~~~~~~~~~~~~~
> arch/arm64/kvm/check-res-bits.h:58:9: note: in expansion of macro 'BUILD_BUG_ON'
>    58 |         BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0       != (GENMASK_ULL(63, 0)));
>       |         ^~~~~~~~~~~~
> 
> I bisected this to the merge of the kvm-arm tree into linux-next but I
> could not figure out why it fails :-(
> 
> -- 
> Cheers,
> Stephen Rothwell

This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
added new fields to that register (ID_AA64DFR1_EL1)

and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.

Not sure how to resolve it in the git branches though.

Thanks,
Joey

Re: linux-next: build failure after merge of the kvm-arm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3863 bytes --]

Hi Joey,

On Thu, 22 Feb 2024 11:11:29 +0000 Joey Gouly <joey.gouly@arm.com> wrote:
>
> On Thu, Feb 22, 2024 at 10:03:49PM +1100, Stephen Rothwell wrote:
> > 
> > After merging the kvm tree, today's linux-next build (arm64 defconfig)
> > failed like this:
> > 
> > In file included from <command-line>:
> > In function 'check_res_bits',
> >     inlined from 'kvm_sys_reg_table_init' at arch/arm64/kvm/sys_regs.c:4109:2:
> > include/linux/compiler_types.h:449:45: error: call to '__compiletime_assert_591' declared with attribute error: BUILD_BUG_ON failed: ID_AA64DFR1_EL1_RES0 != (GENMASK_ULL(63, 0))
> >   449 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >       |                                             ^
> > include/linux/compiler_types.h:430:25: note: in definition of macro '__compiletime_assert'
> >   430 |                         prefix ## suffix();                             \
> >       |                         ^~~~~~
> > include/linux/compiler_types.h:449:9: note: in expansion of macro '_compiletime_assert'
> >   449 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >       |         ^~~~~~~~~~~~~~~~~~~
> > include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
> >    39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> >       |                                     ^~~~~~~~~~~~~~~~~~
> > include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> >    50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> >       |         ^~~~~~~~~~~~~~~~
> > arch/arm64/kvm/check-res-bits.h:58:9: note: in expansion of macro 'BUILD_BUG_ON'
> >    58 |         BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0       != (GENMASK_ULL(63, 0)));
> >       |         ^~~~~~~~~~~~
> > 
> > I bisected this to the merge of the kvm-arm tree into linux-next but I
> > could not figure out why it fails :-(
> > 
> > -- 
> > Cheers,
> > Stephen Rothwell  
> 
> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
> added new fields to that register (ID_AA64DFR1_EL1)
> 
> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
> 
> Not sure how to resolve it in the git branches though.

Thanks.  I will apply this patch to the merge of the kvm-arm tree from
tomorrow (and at the end of today's tree).

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 22 Feb 2024 22:31:22 +1100
Subject: [PATCH] fix up for "arm64/sysreg: Add register fields for ID_AA64DFR1_EL1"

interacting with "KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/arm64/kvm/check-res-bits.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
index 967b5d171d53..39f537875d17 100644
--- a/arch/arm64/kvm/check-res-bits.h
+++ b/arch/arm64/kvm/check-res-bits.h
@@ -55,7 +55,6 @@ static inline void check_res_bits(void)
 	BUILD_BUG_ON(ID_AA64SMFR0_EL1_RES0	!= (GENMASK_ULL(62, 61) | GENMASK_ULL(51, 49) | GENMASK_ULL(31, 31) | GENMASK_ULL(27, 0)));
 	BUILD_BUG_ON(ID_AA64FPFR0_EL1_RES0	!= (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 2)));
 	BUILD_BUG_ON(ID_AA64DFR0_EL1_RES0	!= (GENMASK_ULL(27, 24) | GENMASK_ULL(19, 16)));
-	BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0	!= (GENMASK_ULL(63, 0)));
 	BUILD_BUG_ON(ID_AA64AFR0_EL1_RES0	!= (GENMASK_ULL(63, 32)));
 	BUILD_BUG_ON(ID_AA64AFR1_EL1_RES0	!= (GENMASK_ULL(63, 0)));
 	BUILD_BUG_ON(ID_AA64ISAR0_EL1_RES0	!= (GENMASK_ULL(3, 0)));
-- 
2.43.0

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the kvm-arm tree

[Paolo Bonzini]

On 2/22/24 12:40, Stephen Rothwell wrote:
>> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
>> added new fields to that register (ID_AA64DFR1_EL1)
>>
>> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
>> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
>>
>> Not sure how to resolve it in the git branches though.
> 
> Thanks.  I will apply this patch to the merge of the kvm-arm tree from
> tomorrow (and at the end of today's tree).

Marc, Iliver, can you get a topic branch from Catalin and friends for 
this sysreg patch, and apply the fixup directly to the kvm-arm branch in 
the merge commit?

Not _necessary_, as I can always ask Linus to do the fixup, but 
generally he prefers to have this sorted out by the maintainers if it is 
detected by linux-next.

Paolo


> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 22 Feb 2024 22:31:22 +1100
> Subject: [PATCH] fix up for "arm64/sysreg: Add register fields for ID_AA64DFR1_EL1"
> 
> interacting with "KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>   arch/arm64/kvm/check-res-bits.h | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
> index 967b5d171d53..39f537875d17 100644
> --- a/arch/arm64/kvm/check-res-bits.h
> +++ b/arch/arm64/kvm/check-res-bits.h
> @@ -55,7 +55,6 @@ static inline void check_res_bits(void)
>   	BUILD_BUG_ON(ID_AA64SMFR0_EL1_RES0	!= (GENMASK_ULL(62, 61) | GENMASK_ULL(51, 49) | GENMASK_ULL(31, 31) | GENMASK_ULL(27, 0)));
>   	BUILD_BUG_ON(ID_AA64FPFR0_EL1_RES0	!= (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 2)));
>   	BUILD_BUG_ON(ID_AA64DFR0_EL1_RES0	!= (GENMASK_ULL(27, 24) | GENMASK_ULL(19, 16)));
> -	BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0	!= (GENMASK_ULL(63, 0)));
>   	BUILD_BUG_ON(ID_AA64AFR0_EL1_RES0	!= (GENMASK_ULL(63, 32)));
>   	BUILD_BUG_ON(ID_AA64AFR1_EL1_RES0	!= (GENMASK_ULL(63, 0)));
>   	BUILD_BUG_ON(ID_AA64ISAR0_EL1_RES0	!= (GENMASK_ULL(3, 0)));

Re: linux-next: build failure after merge of the kvm-arm tree

[Paolo Bonzini]

On 2/22/24 12:40, Stephen Rothwell wrote:
>> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
>> added new fields to that register (ID_AA64DFR1_EL1)
>>
>> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
>> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
>>
>> Not sure how to resolve it in the git branches though.
> 
> Thanks.  I will apply this patch to the merge of the kvm-arm tree from
> tomorrow (and at the end of today's tree).

Marc, Oliver, can you get a topic branch from Catalin and friends for 
this sysreg patch, and apply the fixup directly to the kvm-arm branch in 
the merge commit?

Not _necessary_, as I can always ask Linus to do the fixup, but 
generally he prefers to have this sorted out by the maintainers if it is 
detected by linux-next.

Paolo


> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 22 Feb 2024 22:31:22 +1100
> Subject: [PATCH] fix up for "arm64/sysreg: Add register fields for ID_AA64DFR1_EL1"
> 
> interacting with "KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>   arch/arm64/kvm/check-res-bits.h | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
> index 967b5d171d53..39f537875d17 100644
> --- a/arch/arm64/kvm/check-res-bits.h
> +++ b/arch/arm64/kvm/check-res-bits.h
> @@ -55,7 +55,6 @@ static inline void check_res_bits(void)
>   	BUILD_BUG_ON(ID_AA64SMFR0_EL1_RES0	!= (GENMASK_ULL(62, 61) | GENMASK_ULL(51, 49) | GENMASK_ULL(31, 31) | GENMASK_ULL(27, 0)));
>   	BUILD_BUG_ON(ID_AA64FPFR0_EL1_RES0	!= (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 2)));
>   	BUILD_BUG_ON(ID_AA64DFR0_EL1_RES0	!= (GENMASK_ULL(27, 24) | GENMASK_ULL(19, 16)));
> -	BUILD_BUG_ON(ID_AA64DFR1_EL1_RES0	!= (GENMASK_ULL(63, 0)));
>   	BUILD_BUG_ON(ID_AA64AFR0_EL1_RES0	!= (GENMASK_ULL(63, 32)));
>   	BUILD_BUG_ON(ID_AA64AFR1_EL1_RES0	!= (GENMASK_ULL(63, 0)));
>   	BUILD_BUG_ON(ID_AA64ISAR0_EL1_RES0	!= (GENMASK_ULL(3, 0)));

Re: linux-next: build failure after merge of the kvm-arm tree

[Marc Zyngier]

On Thu, 22 Feb 2024 13:11:59 +0000,
Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 2/22/24 12:40, Stephen Rothwell wrote:
> >> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
> >> added new fields to that register (ID_AA64DFR1_EL1)
> >> 
> >> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
> >> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
> >> 
> >> Not sure how to resolve it in the git branches though.
> > 
> > Thanks.  I will apply this patch to the merge of the kvm-arm tree from
> > tomorrow (and at the end of today's tree).
> 
> Marc, Oliver, can you get a topic branch from Catalin and friends for
> this sysreg patch, and apply the fixup directly to the kvm-arm branch
> in the merge commit?
> 
> Not _necessary_, as I can always ask Linus to do the fixup, but
> generally he prefers to have this sorted out by the maintainers if it
> is detected by linux-next.

I think that's not the correct thing to do at this time. I should have
timed the introduction of these checks a bit later, after the merge
window.

But more to the point, the proposed patch is also not the best thing
to merge, because it hides that there is a discrepancy between what
the architecture describes, and what KVM knows. I really want to know
about it, or it will be yet another bug that we wont detect easily.
Specially for ID_AA64DFR*_EL1 which are a bloody mine-field.

So I'd rather we make the check optional, and we'll play catch up for
a bit longer. Something like the patch below.

Oliver, do you mind queuing this ASAP (also pushed out to my dev
branch)?

Thanks,

	M.

From 85d861a6ca055c7681c826c580e7c90d74c26ac5 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Thu, 22 Feb 2024 14:12:09 +0000
Subject: [PATCH] KVM: arm64: Make build-time check of RES0/RES1 bits optional

In order to ease the transition towards a state of absolute
paranoia where all RES0/RES1 bits gets checked against what
KVM know of them, make the checks optional and garded by a
config symbol (CONFIG_KVM_ARM64_RES_BITS_PARANOIA) default to n.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/Kconfig          | 11 +++++++++++
 arch/arm64/kvm/check-res-bits.h |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 5c2a672c06a8..fa9389270cfe 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -67,4 +67,15 @@ config PROTECTED_NVHE_STACKTRACE
 
 	  If unsure, or not using protected nVHE (pKVM), say N.
 
+config KVM_ARM64_RES_BITS_PARANOIA
+	bool "Build-time check of RES0/RES1 bits"
+	depends on KVM
+	default n
+	help
+	  Say Y here to validate that KVM's knowledge of most system
+	  registers' RES0/RES1 bits matches when the rest of the kernel
+	  defines. Expect the build to fail badly if you enable this.
+
+	  Just say N.
+
 endif # VIRTUALIZATION
diff --git a/arch/arm64/kvm/check-res-bits.h b/arch/arm64/kvm/check-res-bits.h
index 967b5d171d53..2d98e60efc3c 100644
--- a/arch/arm64/kvm/check-res-bits.h
+++ b/arch/arm64/kvm/check-res-bits.h
@@ -21,6 +21,8 @@
  */
 static inline void check_res_bits(void)
 {
+#ifdef CONFIG_KVM_ARM64_RES_BITS_PARANOIA
+
 	BUILD_BUG_ON(OSDTRRX_EL1_RES0		!= (GENMASK_ULL(63, 32)));
 	BUILD_BUG_ON(MDCCINT_EL1_RES0		!= (GENMASK_ULL(63, 31) | GENMASK_ULL(28, 0)));
 	BUILD_BUG_ON(MDSCR_EL1_RES0		!= (GENMASK_ULL(63, 36) | GENMASK_ULL(28, 28) | GENMASK_ULL(25, 24) | GENMASK_ULL(20, 20) | GENMASK_ULL(18, 16) | GENMASK_ULL(11, 7) | GENMASK_ULL(5, 1)));
@@ -118,4 +120,6 @@ static inline void check_res_bits(void)
 	BUILD_BUG_ON(TRBMAR_EL1_RES0		!= (GENMASK_ULL(63, 12)));
 	BUILD_BUG_ON(TRBTRG_EL1_RES0		!= (GENMASK_ULL(63, 32)));
 	BUILD_BUG_ON(TRBIDR_EL1_RES0		!= (GENMASK_ULL(63, 12) | GENMASK_ULL(7, 6)));
+
+#endif
 }
-- 
2.39.2


-- 
Without deviation from the norm, progress is not possible.

Re: linux-next: build failure after merge of the kvm-arm tree

[Oliver Upton]

On Thu, Feb 22, 2024 at 02:31:38PM +0000, Marc Zyngier wrote:
> On Thu, 22 Feb 2024 13:11:59 +0000,
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 
> > On 2/22/24 12:40, Stephen Rothwell wrote:
> > >> This fails because https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=fdd867fe9b32
> > >> added new fields to that register (ID_AA64DFR1_EL1)
> > >> 
> > >> and commit b80b701d5a6 ("KVM: arm64: Snapshot all non-zero RES0/RES1 sysreg fields for later checking")
> > >> took a snapshot of the fields, so the RES0 (reserved 0) bits don't match anymore.
> > >> 
> > >> Not sure how to resolve it in the git branches though.
> > > 
> > > Thanks.  I will apply this patch to the merge of the kvm-arm tree from
> > > tomorrow (and at the end of today's tree).
> > 
> > Marc, Oliver, can you get a topic branch from Catalin and friends for
> > this sysreg patch, and apply the fixup directly to the kvm-arm branch
> > in the merge commit?
> > 
> > Not _necessary_, as I can always ask Linus to do the fixup, but
> > generally he prefers to have this sorted out by the maintainers if it
> > is detected by linux-next.
> 
> I think that's not the correct thing to do at this time. I should have
> timed the introduction of these checks a bit later, after the merge
> window.
> 
> But more to the point, the proposed patch is also not the best thing
> to merge, because it hides that there is a discrepancy between what
> the architecture describes, and what KVM knows. I really want to know
> about it, or it will be yet another bug that we wont detect easily.
> Specially for ID_AA64DFR*_EL1 which are a bloody mine-field.
> 
> So I'd rather we make the check optional, and we'll play catch up for
> a bit longer. Something like the patch below.
> 
> Oliver, do you mind queuing this ASAP (also pushed out to my dev
> branch)?
> 
> Thanks,
> 
> 	M.
> 
> From 85d861a6ca055c7681c826c580e7c90d74c26ac5 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Thu, 22 Feb 2024 14:12:09 +0000
> Subject: [PATCH] KVM: arm64: Make build-time check of RES0/RES1 bits optional
> 
> In order to ease the transition towards a state of absolute
> paranoia where all RES0/RES1 bits gets checked against what
> KVM know of them, make the checks optional and garded by a
> config symbol (CONFIG_KVM_ARM64_RES_BITS_PARANOIA) default to n.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Applied as commit 99101dda29e3 ("KVM: arm64: Make build-time check of
RES0/RES1 bits optional") on the kvmarm/next branch.

-- 
Thanks,
Oliver